diff mbox series

[ovs-dev,v1,1/1] dpif-netdev: Fix mega flow ufid collisions for different wild cards.

Message ID 20260325101001.1642724-1-elibr@nvidia.com
State New
Delegated to: aaron conole
Headers show
Series [ovs-dev,v1,1/1] dpif-netdev: Fix mega flow ufid collisions for different wild cards. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/cirrus-robot success cirrus build: passed
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Eli Britstein March 25, 2026, 10:10 a.m. UTC 
From: Emeel Hakim <ehakim@nvidia.com>

dp_netdev_get_mega_ufid() computes a mega flow ufid by hashing the
masked flow (flow & wildcard mask).  The mega ufid is used as a key
to the offload objects. If they are not unique, instead of 2 different
offload rules, they will override each other.

For example, consider two flows that share the same 5-tuple but
differ only in TOS handling:
- Flow A: tos=0x20, wildcard mask tos=0x00 -> masked tos=0x00
- Flow B: tos=0x00, wildcard mask tos=0xff -> masked tos=0x00
Both produce identical masked flows, so they get the same mega ufid,
even though they belong to different megaflows (different wildcard
masks).

This can be triggered by an OpenFlow rule change while traffic is active:
1. Install a high-priority rule matching nw_tos=0 and a lower-priority
   catch-all ip, tcp rule.
2. Start TOS=0 traffic — flows match the nw_tos=0 rule and get an
   exact TOS mask (0xff).
3. Remove the nw_tos=0 rule — new flows now match the catch-all rule
   with TOS wild-carded (mask=0x00).
4. Start TOS=0x20 traffic — the masked TOS is 0x00 in both cases,
   producing the same mega ufid despite different wildcard masks.

Fix this by taking into account not only the masked values but also the masks.

Fixes: 241bad15d99a ("dpif-netdev: associate flow with a mark id")
Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
 lib/dpif-netdev.c    | 14 ++++++++++----
 tests/dpif-netdev.at | 25 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9df05c4c2..49f4fa2ac 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3288,14 +3288,20 @@  out:
 static void
 dp_netdev_get_mega_ufid(const struct match *match, ovs_u128 *mega_ufid)
 {
-    struct flow masked_flow;
+    struct {
+        struct flow masked_flow;
+        struct flow wc;
+    } key;
     size_t i;
 
+    memset(&key, 0, sizeof key);
     for (i = 0; i < sizeof(struct flow); i++) {
-        ((uint8_t *)&masked_flow)[i] = ((uint8_t *)&match->flow)[i] &
-                                       ((uint8_t *)&match->wc)[i];
+        ((uint8_t *)&key.masked_flow)[i] = ((uint8_t *)&match->flow)[i] &
+                                           ((uint8_t *)&match->wc)[i];
+        ((uint8_t *)&key.wc)[i] = ((uint8_t *)&match->wc)[i];
     }
-    odp_flow_key_hash(&masked_flow, sizeof masked_flow, mega_ufid);
+
+    odp_flow_key_hash(&key, sizeof key, mega_ufid);
 }
 
 uint64_t
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 231197970..b0eb9ea63 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -668,6 +668,31 @@  AT_CHECK([ovs-appctl revalidator/resume])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([dpif-netdev - mega flow ufid uniqueness for different wildcards])
+OVS_VSWITCHD_START(
+  [add-port br0 p1 \
+   -- set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p0.sock \
+   -- set bridge br0 datapath-type=dummy])
+AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg])
+AT_CHECK([ovs-appctl vlog/disable-rate-limit dpif_netdev])
+
+AT_CHECK([ovs-appctl revalidator/pause])
+# Two flows with different wildcard masks but identical masked flow.
+# Flow A: tos=0x20 with mask 0 (wildcarded) -> masked tos = 0
+# Flow B: tos=0x00 with exact match         -> masked tos = 0
+AT_CHECK([ovs-appctl dpctl/add-flow "in_port(1),eth(),eth_type(0x0800),ipv4(src=10.0.0.1,dst=10.0.0.2,proto=6,tos=0x20/0,ttl=64,frag=no),tcp(src=80,dst=8080)" "3"])
+AT_CHECK([ovs-appctl dpctl/add-flow "in_port(1),eth(),eth_type(0x0800),ipv4(src=10.0.0.1,dst=10.0.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=80,dst=8080)" "3"])
+
+AT_CHECK([ovs-appctl revalidator/resume])
+
+# Verify both flows have different mega_ufids by checking flow_add log.
+AT_CHECK([grep 'flow_add.*tcp(src=80,dst=8080)' ovs-vswitchd.log | sed -n 's/.*mega_ufid:\([[0-9a-f-]]*\).*/\1/p' | sort -u | wc -l | tr -d ' '], [0], [dnl
+2
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([dpif-netdev - check tx packet checksum offloading])
 OVS_VSWITCHD_START(
   [add-port br0 p1 \