| Message ID | 20260831151803.3756241-1-trozet@nvidia.com |
|---|---|
| State | New |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev] userspace datapath: Include UDP ports in symmetric L4 hash. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_FreeBSD_Build_and_Test | success | github build: passed |
On Mon, Aug 31, 2026 at 11:20 AM Tim Rozet via dev <ovs-dev@openvswitch.org> wrote: > The userspace implementation of OVS_HASH_ALG_SYM_L4 calls > flow_hash_symmetric_l3l4() with UDP port hashing disabled. As a > result, UDP packets with identical addresses but different ports > receive the same datapath hash. > > The Linux datapath implements this algorithm with > __skb_get_hash_symmetric(). Its symmetric flow dissector requests > FLOW_DISSECTOR_KEY_PORTS, and skb_flow_get_ports() extracts source > and destination ports for IPPROTO_UDP. Thus, the two datapaths use > different hash fields for the same OVS action. > > Include UDP ports in the userspace algorithm to align its field > selection with Linux. The exact hash value remains datapath-specific > and opaque to ovs-vswitchd. > > Add dummy-datapath coverage that verifies reversed UDP tuples remain > symmetric while changing only the UDP source port produces more than > one datapath hash. > > Tested with GitHub Actions and an HBN/OVN integration topology. The > userspace datapath distributed 128 VXLAN flows across two underlay > ECMP paths and moved all traffic to the remaining path after failover. > > Fixes: 6a0b0d3be857 ("userspace datapath: Add OVS_HASH_L4_SYMMETRIC > dp_hash algorithm") > Assisted-by: GPT-5, OpenAI Codex > Signed-off-by: Tim Rozet <trozet@nvidia.com> > --- Looks good to me. Acked-by: Mike Pattrick <mkp@redhat.com>
diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 618fb5ac8..a22d60295 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -1014,7 +1014,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal, flow_extract(packet, &flow); hash = flow_hash_symmetric_l3l4(&flow, hash_act->hash_basis, - false); + true); packet->md.dp_hash = hash; } break; diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index ee6ac873d..93fa97111 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -1201,6 +1201,52 @@ n_flows=ok n_buckets=ok OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - symmetric dp_hash includes UDP ports]) + +OVS_VSWITCHD_START +add_of_ports br0 1 10 11 + +AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 \ + 'group_id=1234,type=select,bucket=output:10,bucket=output:11']) +AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 \ + 'udp actions=group:1234']) + +dp_hash_flow_count () { + ovs-appctl dpctl/dump-flows \ + | sed -n '/recirc_id(0x[[0-9a-f]][[0-9a-f]]*),dp_hash/p' \ + | wc -l +} + +forward_flow="in_port(1),eth(src=50:54:00:00:00:01,dst=50:54:00:00:00:02),\ +eth_type(0x0800),ipv4(src=192.0.2.1,dst=198.51.100.1,proto=17,\ +tos=0,ttl=64,frag=no)" +reverse_flow="in_port(1),eth(src=50:54:00:00:00:02,dst=50:54:00:00:00:01),\ +eth_type(0x0800),ipv4(src=198.51.100.1,dst=192.0.2.1,proto=17,\ +tos=0,ttl=64,frag=no)" + +AT_CHECK([ovs-appctl netdev-dummy/receive p1 \ + "$forward_flow,udp(src=10000,dst=4789)"]) +OVS_WAIT_UNTIL_EQUAL([dp_hash_flow_count], [1]) + +dnl Reversing the complete L3/L4 tuple must produce the same hash. +AT_CHECK([ovs-appctl netdev-dummy/receive p1 \ + "$reverse_flow,udp(src=4789,dst=10000)"]) +OVS_WAIT_UNTIL([ovs-appctl dpctl/dump-flows \ + | grep 'recirc_id(0),' | grep -q 'packets:1']) +AT_CHECK([dp_hash_flow_count], [0], [1 +]) + +dnl Vary only the UDP source port. At least one packet must produce a +dnl different hash value from the first packet. +for port in `seq 10001 10016`; do + AT_CHECK([ovs-appctl netdev-dummy/receive p1 \ + "$forward_flow,udp(src=$port,dst=4789)"]) +done +OVS_WAIT_UNTIL([test `dp_hash_flow_count` -gt 1]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - select group with watch port]) OVS_VSWITCHD_START
The userspace implementation of OVS_HASH_ALG_SYM_L4 calls flow_hash_symmetric_l3l4() with UDP port hashing disabled. As a result, UDP packets with identical addresses but different ports receive the same datapath hash. The Linux datapath implements this algorithm with __skb_get_hash_symmetric(). Its symmetric flow dissector requests FLOW_DISSECTOR_KEY_PORTS, and skb_flow_get_ports() extracts source and destination ports for IPPROTO_UDP. Thus, the two datapaths use different hash fields for the same OVS action. Include UDP ports in the userspace algorithm to align its field selection with Linux. The exact hash value remains datapath-specific and opaque to ovs-vswitchd. Add dummy-datapath coverage that verifies reversed UDP tuples remain symmetric while changing only the UDP source port produces more than one datapath hash. Tested with GitHub Actions and an HBN/OVN integration topology. The userspace datapath distributed 128 VXLAN flows across two underlay ECMP paths and moved all traffic to the remaining path after failover. Fixes: 6a0b0d3be857 ("userspace datapath: Add OVS_HASH_L4_SYMMETRIC dp_hash algorithm") Assisted-by: GPT-5, OpenAI Codex Signed-off-by: Tim Rozet <trozet@nvidia.com> --- lib/odp-execute.c | 2 +- tests/ofproto-dpif.at | 46 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) base-commit: 1d4157643ca16ef8d84c8a27fe524aedb7b538d0