infra,l2,ip: fix flow hashing for ports without RSS

[hcaldicott]

Several datapaths read m->hash.rss directly for ECMP and load-balancing
decisions (fib4/fib6 lookups, vxlan underlay routing and source port
selection). On ports without RSS — TAP and other virtual devices — that
field holds stale or zero data, so every flow shares one value: ECMP
collapses onto a single nexthop and all vxlan flows share one UDP source
port. After SRv6 decapsulation, the outer flow label is copied over the
hash while a hardware valid flag survives; with a zero label (as grout's
own encap emits) every inner flow shares hash 0.

The series first extracts the bond Toeplitz hashing into a shared helper
and fixes two fragment-handling bugs in it: packets with the DF flag set
were treated as fragments and hashed L3-only, and first fragments (offset
zero, MF set) were hashed with their L4 ports unlike the rest of their
datagram. It then caches one canonical per-packet hash by storing the
software result in m->hash.rss and marking it valid with
RTE_MBUF_F_RX_RSS_HASH, exactly as a hardware driver would — no new
mbuf metadata. The bond, vxlan and L3 consumers read it through one
helper. vxlan decapsulation leaves the outer hash in place: a VTEP
following RFC 7348 derives the outer UDP source port from the inner
flow, so an outer RSS value is already a function of the inner flow and
stays valid after decapsulation. SRv6 decapsulation keeps using a
non-zero outer flow label as the flow entropy, now stored through the
shared helper so the valid flag is coherent, and invalidates the hash
when the label is zero so the first consumer hashes the inner packet.

For packets whose driver does provide RSS, the datapath is unchanged:
the same flag test and the same m->hash.rss read as before the series,
with no recomputation after encap or decap.

Found while prototyping EVPN all-active multihoming (#698), where bridged
flows must keep per-flow path affinity across bond and vxlan ECMP, but
everything above is reproducible on plain upstream.

Testing

• New flow_hash unit tests: per-flow stability, DF/MF fragment handling,
  single computation and caching, hardware RSS precedence, hash
  invalidation.
• New srv6 decap unit test: a non-zero outer flow label becomes the flow
  hash with the valid flag set; a zero label drops a stale outer RSS
  value.
• Extended smoke/vxlan_test.sh: a second bridged port injects 32
  distinct UDP flows and the test asserts the encapsulated packets use
  several distinct source ports.
• Extended smoke/ip_loadbalance_test.sh: 64 distinct UDP flows through
  the ECMP route, asserting both group members carry traffic.
• Without the fixes (tests-only applied to main): the vxlan test fails
  with "32 flows shared 1 vxlan source ports", the load-balance test
  fails with "member p1 carried 2 of 64 distinct flows", the MF fragment
  unit test fails, and the srv6 decap test fails with the stale outer
  hash still marked valid.
• With the series: full unit suite and the vxlan, vxlan6 and
  ip_loadbalance smoke tests pass (AlmaLinux 9 container, arm64).

Related: #698

[rjarry]

There is a bit too much code churn in the successive commits. Could you try and make this more incremental?

Did you test if this series introduces any performance change? I am concerned about the re-computation of RSS after decap/encap even with hardware drivers that support L4 UDP ports.

Also, please drop the Assisted-by: <LLM> trailers.

[hcaldicott]

Thanks for the review - I will re-work and clean this up. This was cherry-picked from other development work on our MH implementation, so it could use some more scrutiny - sit tight.

[hcaldicott]

There is a bit too much code churn in the successive commits. Could you try and make this more incremental?

Could you confirm if the commit history is acceptable now? I updated Patch 1 so that it uses the final shape from the beginning to remove most of the churn. This was due to cherry-picking from my upstream MH codebase - apologies.

Did you test if this series introduces any performance change? I am concerned about the re-computation of RSS after decap/encap even with hardware drivers that support L4 UDP ports.

Fair call - I have reworked so that decap no longer recomputes anything. vxlan_input just clears the RSS flag; a software hash for the inner frame is computed lazily at the first consumer that needs one, then cached. Packets with genuine NIC RSS follow the exact upstream code path (same flag test, same field read), so hardware fast paths are untouched. I am going to see how I can benchmark this.

Also, please drop the Assisted-by: <LLM> trailers.

Done.

[hcaldicott]

Okay, I have thrown together some AI tests and sanity checked them, seems good to me.

I benchmarked the series against v0.17.1 on a pair of Xeon D-2143IT boxes (X722 10GbE SFP+, back to back), using the per-node cycle counters from grcli stats software under ~226 kpps of 8-flow UDP.

Two topologies: plain routed forwarding (kernel TAP in, 10G wire, TAP out) and a bridged VXLAN overlay mirroring a production EVPN setup. 30 s measured runs after warmup, 3 reps per build per scenario, medians below. X722 RSS was confirmed active (flows spread across both RX queues).

path	node	v0.17.1	this series	delta
routed ingress, NIC RSS present	ip_input	67.9 c/pkt	~68 c/pkt	none — one flag test
routed ingress, no RSS (TAP)	ip_input	70.3	206.6	+136 c/pkt (~62 ns): the software Toeplitz
vxlan encap from bridged no-RSS port	vxlan_output	84.6	245.7	+161 c/pkt: same hash over the inner frame
vxlan decap	vxlan_input	47.6	51.5	+4 c/pkt: clearing the flag
all other nodes				within noise (±5%)

Notes:

• c/pkt is CPU cycles per packet - lower being better.
• Where the driver provides RSS, the datapath is unchanged: same flag test, same m->hash.rss read, nothing recomputed after encap. An earlier revision of this series showed +5 c/pkt here from the getters being extern functions (call overhead on a test-and-return path); they are now static inline and that is gone.
• The +136/+161 only appear on paths that previously fed stale or zero data into the fib lookups and the source port — the bug being fixed. The hash is computed once and cached, so fib lookup, vxlan source port and bond selection on the same packet share one computation.
• After decap nothing is recomputed unless a consumer actually needs a hash for the inner frame.

Throughput was generator-limited and identical on both builds. The fixed build showed lower loss in the vxlan scenario (<=0.01% vs up to 0.77%): per-flow source ports let the receiving NIC spread flows across both RSS queues instead of piling them onto one.

Incidental finding from the same rig: the X722 PF firmware rejects rte_eth_allmulticast_enable() with -ENOTSUP, so interface add port on these NICs fails hard on current main — I had to apply #702 to both builds to run the benchmark, so statistics here might be slightly skewed by that change also.

Crux of this: I see about a 62ns (at 2.2ghz) performance decrease with the corrected hashing function in place. However I feel like it could be worthwhile for the correctness aspect of this fix?

[christophefontaine]

With the introduction of gr_mbuf_flow_hash_get to get the mbuf->hash.rss, we should also update process_behav_decap in modules/srv6/datapath/srv6_local.c to set the hash.
WDYT ?

And to update modules/ip/datapath/icmp_local_send.c as well as icmp6_local_send.c for consistency.

[christophefontaine]

vlan = PAYLOAD(eth);

[hcaldicott]

Done in v4 (with gr_macro.h included directly — nothing in the datapath headers re-exports it).

[christophefontaine]

Outside of this commit, and eligible for a future discussion / PR.
As we only look at ip6->proto, we do not take into account the extension headers which could be before a valid IPPROTO_*.
So, as part of the hash computation, what should we do ?

[hcaldicott]

Agreed, follow-up material. Right now anything that is not immediately TCP/UDP falls through to the default arm and gets a L3-only hash: less entropy, but stable, so flows with extension headers degrade the same way fragments do instead of risking reordering. For a future PR I see two options: a bounded walk over the extension chain (stopping at a fragment header and staying L3-only in that case, same rule this series applies to IPv4 fragments), or using the flow label per RFC 6438 when it is non-zero, which trusts the sender to have derived it from the inner flow. The walk is probably the safer default. Happy to open that discussion separately.

[christophefontaine]

I'm not comfortable in always invalidating the hash, as it may have been computed by the hardware on the inner packet.
Also, relying on the PTYPE may not be good either: as you said, even if the hash isn't computed on the inner, the remote VTEP may have spread the source UDP port.

  // keep a hardware-provided inner-flow hash; only invalidate an outer one
  if (!(m->packet_type & RTE_PTYPE_INNER_L4_MASK))
      gr_mbuf_flow_hash_invalidate(m);

And I know that @david-marchand is not fan of using packet_type as well :)

[david-marchand]

@hcaldicott

Hum, I did not look too much in detail at the full PR sorry.
I am trying to go back to the actual problem you are facing.

The issue seems to be that the rss hash is not a function of the inner packet.
This should be the case for routers implementing the recommendation in RFC7348.

   Outer UDP Header:  This is the outer UDP header with a source port
      provided by the VTEP and the destination port being a well-known
      UDP port.
...
-  Source Port:  It is recommended that the UDP source port number
         be calculated using a hash of fields from the inner packet --
         one example being a hash of the inner Ethernet frame's headers.
         This is to enable a level of entropy for the ECMP/load-
         balancing of the VM-to-VM traffic across the VXLAN overlay.
         When calculating the UDP source port number in this manner, it
         is RECOMMENDED that the value be in the dynamic/private port
         range 49152-65535 [[RFC6335](https://datatracker.ietf.org/doc/html/rfc6335)].

If you are receiving traffic from another grout instance, the rss hash should have a source port that depends on the inner packet RSS (https://github.com/DPDK/grout/blob/main/modules/l2/datapath/vxlan_output.c#L90).

Is it that in your usecase the inner (meaning, before encapsulation on the transmitter side) packet rss 0 because it is received from a net/virtio (including virtio-user) port?

If this is the case, you may want to compute a RSS when it is not set at the RX side (see rx_virtio_process/rx_bond_virtio_process).

[hcaldicott]

@david-marchand yes, that is exactly the use case: the access ports here are virtio/vhost and TAP, so pre-encap packets carry no RSS and vxlan_output was reading hash.rss == 0 for every flow - which is why all encapsulated flows shared one source port (the red test in the PR description).

On where to fix it: rather than computing eagerly in rx_virtio_process/rx_bond_virtio_process, the series computes the same software hash lazily at the first consumer (vxlan_output, bond_output, the fib lookups) through gr_mbuf_flow_hash_get(), which stores it in m->hash.rss and sets RTE_MBUF_F_RX_RSS_HASH exactly as a driver would. The end state is the same as computing it at RX, but packets that never reach a hash consumer do not pay for it. I measured the software hash at roughly 136 cycles/packet on Xeon D / X722; charged at RX it would tax every packet on virtio ports whether or not anything downstream needs a hash. If you would still rather see it in the RX path for uniformity I can move it, but the lazy placement was deliberate.

On the invalidation: fair point about RFC 7348. When the remote VTEP derives the source port from the inner flow (kernel vxlan, grout with this series), the outer RSS is already a function of the inner flow, so keeping it preserves both per-flow affinity and distribution after decap, and invalidating just forces a pointless software rehash - in our deployment that is every decapped packet that egresses over a bond. It also discards a hardware inner hash on NICs that parse the tunnel, per @christophefontaine's comment, without needing any packet_type check. The one thing the invalidation protected against is a remote VTEP using a fixed source port (which includes any pre-fix grout feeding from ports without RSS), but that traffic already polarizes across every underlay ECMP hop, so I agree it is not worth making the common case pay. The invalidation is dropped in v4; vxlan_input is now untouched by the series.

SRv6 decap is a different story (see Christophe's review comment on process_behav_decap): there the outer header carries no inner entropy at all, so that path does need to drop the stale hash. Handled in v4.

[david-marchand]

@david-marchand yes, that is exactly the use case: the access ports here are virtio/vhost and TAP, so pre-encap packets carry no RSS and vxlan_output was reading hash.rss == 0 for every flow - which is why all encapsulated flows shared one source port (the red test in the PR description).

Ok, thanks for confirming.

On where to fix it: rather than computing eagerly in rx_virtio_process/rx_bond_virtio_process, the series computes the same software hash lazily at the first consumer (vxlan_output, bond_output, the fib lookups) through gr_mbuf_flow_hash_get(), which stores it in m->hash.rss and sets RTE_MBUF_F_RX_RSS_HASH exactly as a driver would. The end state is the same as computing it at RX, but packets that never reach a hash consumer do not pay for it. I measured the software hash at roughly 136 cycles/packet on Xeon D / X722; charged at RX it would tax every packet on virtio ports whether or not anything downstream needs a hash. If you would still rather see it in the RX path for uniformity I can move it, but the lazy placement was deliberate.

There is always a tradeoff, unfortunately.
I am a bit scared at touching the stack to deal with a driver issue.

Do you have an idea of the performance impact of adding the branches in the RSS hash users?

On the invalidation: fair point about RFC 7348. When the remote VTEP derives the source port from the inner flow (kernel vxlan, grout with this series), the outer RSS is already a function of the inner flow, so keeping it preserves both per-flow affinity and distribution after decap, and invalidating just forces a pointless software rehash - in our deployment that is every decapped packet that egresses over a bond. It also discards a hardware inner hash on NICs that parse the tunnel, per @christophefontaine's comment, without needing any packet_type check. The one thing the invalidation protected against is a remote VTEP using a fixed source port (which includes any pre-fix grout feeding from ports without RSS), but that traffic already polarizes across every underlay ECMP hop, so I agree it is not worth making the common case pay. The invalidation is dropped in v4; vxlan_input is now untouched by the series.

Ack.

SRv6 decap is a different story (see Christophe's review comment on process_behav_decap): there the outer header carries no inner entropy at all, so that path does need to drop the stale hash. Handled in v4.

I had not considered SRv6 so far.
That seems problematic on the SRv6 receiving node.

https://learn.microsoft.com/en-us/windows-hardware/drivers/network/rss-hashing-types#ndis-hash-ipv6
https://learn.microsoft.com/en-us/windows-hardware/drivers/network/rss-hashing-types#ndis_hash_ipv6_ex

Nothing seems to look at the ipv6 flow label (that's me assuming grout sets it based on rss hash, @christophefontaine @rjarry ?).
On the other hand, I am not sure grout sets the described IPv6 options by Microsoft specification.

On DPDK side, it seems really few drivers support such rss on IPv6 extensions, and that seems to be for old hardware.

net/bnxt caught my eyes, as it seems to do something unspecified.. hashing ipv6 flow label (oh oh).

@christophefontaine had some funny idea, but this is something for another thread.

[christophefontaine]

(not introduced by this commit, but to open the discussion for a future PR)
The fib4_lookup gets the nh based on the hash. Yet, this hash is still the one computed by the hardware, pre-nat.
Shouldn't we update the RSS field to reflect the modified dst_ip & dsp_port?

[hcaldicott]

I think keeping the pre-NAT hash is actually correct: the hash is a stable per-flow identity for member selection, not a digest of the current header bytes. A given flow always produces the same pre-NAT tuple and distinct flows have distinct pre-NAT tuples, so both affinity and distribution hold; recomputing after the rewrite would spend cycles without changing either.

One inconsistency worth noting for that future discussion: the software fallback in this series computes after the rewrite (the lookup sits below the NAT code), so RSS ports hash pre-NAT and non-RSS ports hash post-NAT. Each flow still sees one stable value since a port either has RSS or it does not - but if we ever want the two paths to agree, the cheap fix is seeding the software hash before the rewrite, not recomputing after it.

[christophefontaine]

same comment as dnat44_dynamic

[hcaldicott]

Same reasoning as on the dnat44_dynamic thread: the pre-NAT hash is still a stable per-flow identity, so I would leave both as they are and take the wider question to a follow-up discussion.

[hcaldicott]

With the introduction of gr_mbuf_flow_hash_get to get the mbuf->hash.rss, we should also update process_behav_decap in modules/srv6/datapath/srv6_local.c to set the hash. WDYT ?

@christophefontaine agreed, and it is a bit worse than an inconsistency. hash.usr aliases hash.rss, so the decap overwrites the value with the outer flow label while a hardware RTE_MBUF_F_RX_RSS_HASH flag (if any) stays set - and since our own encap sets the outer flow label to zero (ip6_set_fields()), every decapped flow ends up with a "valid" hash of 0. Unlike vxlan there is no inner entropy in the outer header either: no UDP source port, and RSS cannot parse past the routing header.

I kept your flow-label approach for the case where the encapsulating node filled it in: a non-zero label is now stored through gr_mbuf_flow_hash_set() so the valid flag is coherent, and a zero label invalidates the hash so the first consumer computes one from the inner packet. New patch in v4, with a unit test covering both cases (it fails on the previous code with the stale outer hash still marked valid). Setting the outer flow label from the flow hash on encap (RFC 6438) would be a nice follow-up - it would also give the underlay ECMP something to work with.

And to update modules/ip/datapath/icmp_local_send.c as well as icmp6_local_send.c for consistency.

Sure — the "fake RSS" writes there are exactly what gr_mbuf_flow_hash_set() does; converted in v4.

---

v4 is pushed (7 patches now). Summary of the changes since v3:

• Dropped the vxlan decap invalidation entirely; vxlan_input is untouched by the series and the patch 5 commit message carries the RFC 7348 rationale (see the resolved inline thread).
• New patch "srv6: fix flow hash validity on decap" as described above, with a Fixes: tag and a unit test.
• icmp_local_send/icmp6_local_send converted to gr_mbuf_flow_hash_set().
• PAYLOAD(eth) in the VLAN parsing.

Revalidated in the AlmaLinux 9 container: full unit suite, vxlan, vxlan6 and ip_loadbalance smoke tests pass; the new srv6 test fails with only the fix reverted. PR description updated to match.

[rjarry]

I think it makes sense to move this function as static inline in flow_hash.h to benefit from the shortcut when the RSS hash has been computed by hardware. flow_hash_l3 can remain in flow_hash.c.

[rjarry]

	if (likely(mode == GR_MBUF_FLOW_HASH_RSS && (m->ol_flags & RTE_MBUF_F_RX_RSS_HASH)))
		return m->hash.rss;

[rjarry]

Sometimes clang-format makes me sad :(

Do you think we could use rte_ipv4_frag_pkt_is_fragmented instead?

			if (!rte_ipv4_frag_pkt_is_fragmented(l3.ip4)) {

It checks for the MF flag too which looks correct.

[rjarry]

Ditto.

[rjarry]

Reverse x-mas tree please.

	ip4_addr_t src, dst;
	rte_edge_t edge;
+	uint32_t hash;
	unsigned len;

[rjarry]

Reverse xmas tree too

[rjarry]

Oh dear, this function is already looking bad. Could you add the new variables roughly in the correct spot for reverse xmas tree?

	struct rte_ipv6_routing_ext *srh;
	struct rte_ipv6_hdr *outer_ip6;
	const struct nexthop *nh;
	uint32_t optlen, plen;
+	rte_be16_t eth_type;
	struct rte_mbuf *m;
	uint8_t proto, n_omitted_segs;
	rte_edge_t edge;
+	uint32_t hash;