From: Emeel Hakim <[email protected]>
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 <[email protected]>
---
lib/dpif-netdev.c | 14 ++++++++++----
tests/dpif-netdev.at | 25 +++++++++++++++++++++++++
2 files changed, 35 insertions(+), 4 deletions(-)
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 \
--
2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
