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. This was introduced by [1] 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
megaflow still has a real datapath in_port.
When that port is deleted, the revalidator re-translates these
megaflows. The shortcut bypasses port validation, the actions look
unchanged, and the megaflow is kept.
Stale megaflows then accumulate until max-idle fires,
which is problematic in high-churn environments like Kubernetes and
especially in offload setups where matching datapath flows are held
in hardware.
Fix this by verifying that the megaflow's datapath 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.
[1]: 323ae1e808e6 ("ofproto-dpif-xlate: Fix recirculation when in_port is
OFPP_CONTROLLER.")
Fixes: 323ae1e808e6 ("ofproto-dpif-xlate: Fix recirculation when in_port is
OFPP_CONTROLLER.")
Signed-off-by: Salem Sol <[email protected]>
---
ofproto/ofproto-dpif-xlate.c | 23 ++++++++++++++++++++++-
tests/ofproto-dpif.at | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 764dbd6627..3a64fc9ec6 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1690,7 +1690,10 @@ 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 OpenFlow action (e.g. load:0xffff->NXM_OF_IN_PORT[])
+ * cleared the OF in_port mid-pipeline before a freeze (as OVN
+ * does at logical-datapath crossings). */
struct uuid uuid = recirc_id_node->state.ofproto_uuid;
const struct xbridge *bridge = xbridge_lookup_by_uuid(xcfg, &uuid);
@@ -1699,6 +1702,24 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer,
!get_ofp_port(bridge, in_port)) {
goto xport_lookup;
}
+
+ /* If there is a datapath port associated with this flow and
+ * it no longer exists, the flow is stale and must be
+ * evicted, so fall through to xport_lookup. ODPP_NONE means
+ * there is no datapath port at all (e.g. a controller
+ * packet-out), so skip the check in that case. */
+ 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 ee6ac873d8..2bcefbc93b 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -6646,6 +6646,41 @@ OVS_WAIT_UNTIL([check_flows], [ovs-ofctl dump-flows br0])
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - evict recirculated OFPP_NONE flows on input 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])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
dnl Checks for regression against a bug in which OVS dropped packets
dnl originating from a controller passing through a patch port.
AT_SETUP([ofproto-dpif - packet-out recirculation OFPP_CONTROLLER and patch
port])
--
2.43.7
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
