xlate_lookup_ofproto_() takes a shortcut when the frozen state has
in_port == OFPP_NONE: it returns the bridge's ofproto via uuid lookup
without further validation. The cited commit introduced this for
controller-originated packet-outs, where there is no real ingress port.
However, OVN also produces OFPP_NONE in the frozen state at logical
datapath crossings by explicitly clearing in_port mid-pipeline
(load:0xffff->in_port) before a ct() freeze. In this case the datapath
flow still has a real in_port.
When that port is deleted, the revalidator re-translates these flows.
The shortcut bypasses port validation, the actions look unchanged, and
the flows are kept. They then accumulate until max-idle fires, which is
problematic in high-churn environments like Kubernetes, and especially
in offload setups where matching flows are held in hardware.
Fix this by verifying that the datapath flow's in_port still maps to an
OF port via odp_port_to_ofport() in the shortcut path. Skip the check
when in_port.odp_port == ODPP_NONE so the original controller packet-out
scenarios continue to work.
A regression test reproducing the bug on the dummy datapath is added.
Fixes: 323ae1e808e6 ("ofproto-dpif-xlate: Fix recirculation when in_port is
OFPP_CONTROLLER.")
Assisted-by: Claude Opus 4.8, Cursor
Signed-off-by: Salem Sol <[email protected]>
---
ofproto/ofproto-dpif-xlate.c | 22 +++++++++++++++++++-
tests/ofproto-dpif.at | 39 ++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index c1ba447e93..d33b4afabf 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1689,7 +1689,9 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer,
* the packet originated from OFPP_CONTROLLER passed
* through a patch port.
*
- * OFPP_NONE can also indicate that a bond caused recirculation. */
+ * OFPP_NONE can also indicate that a bond caused recirculation,
+ * or that an action cleared in_port before a freeze (e.g. OVN at
+ * logical datapath crossings). */
struct uuid uuid = recirc_id_node->state.ofproto_uuid;
const struct xbridge *bridge = xbridge_lookup_by_uuid(xcfg, &uuid);
@@ -1698,6 +1700,24 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer,
!get_ofp_port(bridge, in_port)) {
goto xport_lookup;
}
+
+ /* The OF in_port is OFPP_NONE/OFPP_CONTROLLER, but the
+ * datapath flow still ingresses on a concrete odp_port.
+ * If that port is gone the flow is dead, so fall through
+ * to xport_lookup and fail. ODPP_NONE has no datapath
+ * ingress (packet-out); skip it. */
+ if (flow->in_port.odp_port != ODPP_NONE) {
+ const struct ofport_dpif *ingress;
+
+ ingress = tnl_port_should_receive(flow)
+ ? tnl_port_receive(flow)
+ : odp_port_to_ofport(backer,
+ flow->in_port.odp_port);
+ if (!ingress) {
+ goto xport_lookup;
+ }
+ }
+
if (errorp) {
*errorp = NULL;
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index dcab7bba45..cb4cc46a69 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -9010,6 +9010,45 @@ packets:2, bytes:68, used:0.001s, actions:drop
OVS_VSWITCHD_STOP(["/sending to collector failed/d"])
AT_CLEANUP
+dnl OVN clears the OF in_port (load:0xffff->NXM_OF_IN_PORT[]) before a ct()
+dnl freeze, so the frozen state has in_port == OFPP_NONE while the datapath
+dnl flow still keys on the real ingress port. Removing that port must evict
+dnl the recirculated flow instead of keeping it until max-idle.
+AT_SETUP([ofproto-dpif - evict recirc flows on in_port removal])
+AT_KEYWORDS([recirc revalidator])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2
+
+m4_define([PKT], [m4_join([,],
+ [in_port(1)],
+ [eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a)],
+ [eth_type(0x0800)],
+ [ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no)])])
+
+AT_DATA([flows.txt], [dnl
+table=0,in_port=1,tcp
actions=load:0xffff->NXM_OF_IN_PORT[[]],ct(zone=1,table=1)
+table=1,tcp actions=output:2
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+dnl A few packets so both datapath flows get hits.
+for i in 1 2 3; do
+ AT_CHECK([ovs-appctl netdev-dummy/receive p1 'PKT'])
+done
+
+AT_CHECK([ovs-appctl dpctl/dump-flows | strip_used | strip_stats | sort], [0],
[dnl
+flow-dump from the main thread:
+recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=no),
packets:0, bytes:0, used:0.0s, actions:ct(zone=1),recirc(0x1)
+recirc_id(0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=no),
packets:0, bytes:0, used:0.0s, actions:2
+])
+
+AT_CHECK([ovs-vsctl del-port br0 p1])
+AT_CHECK([ovs-appctl revalidator/wait])
+AT_CHECK([ovs-appctl dpctl/dump-flows | strip_used | strip_stats | sort], [0],
[])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
dnl Bridge IPFIX statistics check
AT_SETUP([ofproto-dpif - Bridge IPFIX statistics check])
OVS_VSWITCHD_START
--
2.43.7
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
