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 | 36 ++++++++++++++++++++++-
tests/ofproto-dpif.at | 56 ++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index c1ba447e93..ac267a49d2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1689,7 +1689,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);
@@ -1698,6 +1701,37 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer,
!get_ofp_port(bridge, in_port)) {
goto xport_lookup;
}
+
+ /* Even when the frozen state's OF in_port is OFPP_NONE or
+ * OFPP_CONTROLLER, the megaflow under revalidation still has
+ * a concrete datapath in_port in its key
+ * (flow->in_port.odp_port). If that ingress can no longer
+ * be resolved -- e.g. the underlying port has been deleted,
+ * or the tunnel removed -- then this megaflow is unreachable
+ * (no future packet can match a non-existent ingress) and
+ * must be evicted. Mirror the xport_lookup logic below so
+ * tunnel arrivals are handled the same way, and fall through
+ * to xport_lookup when the ingress is gone so we return
+ * NULL/ENODEV.
+ *
+ * ODPP_NONE means there is no datapath ingress at all
+ * (legitimate for controller-originated packet-outs); skip
+ * the check there. */
+ if (flow->in_port.odp_port != ODPP_NONE) {
+ const struct ofport_dpif *ingress;
+ bool tnl_receive;
+
+ tnl_receive = !(flow->tunnel.flags & FLOW_TNL_F_EXPLICIT)
+ && tnl_port_should_receive(flow);
+ ingress = tnl_receive
+ ? 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..b15457b06f 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -9010,6 +9010,62 @@ packets:2, bytes:68, used:0.001s, actions:drop
OVS_VSWITCHD_STOP(["/sending to collector failed/d"])
AT_CLEANUP
+dnl Reproducer for stale recirculated megaflows that survive port deletion
+dnl when the OpenFlow pipeline clears in_port (load:0xffff->NXM_OF_IN_PORT[])
+dnl before triggering a freeze with ct(table=N). This is the exact pattern
+dnl OVN uses at logical-datapath crossings, and the resulting frozen state
+dnl with metadata.in_port == OFPP_NONE makes xlate_lookup_ofproto_() bypass
+dnl any validation of the datapath in_port that the megaflow keys on, so the
+dnl megaflow is revalidated as UKEY_KEEP for max-idle even
+dnl though the underlying port is gone.
+AT_SETUP([ofproto-dpif - stale recirc megaflow after in_port deletion
(OFPP_NONE frozen in_port)])
+AT_KEYWORDS([megaflow revalidator recirc])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3
+
+dnl Pipeline mirrors OVN's logical-datapath-crossing pattern:
+dnl table=0: receive on p3, clear OF in_port, then ct(table=1) which freezes
+dnl table=1: post-recirc, output to p2
+AT_DATA([flows.txt], [dnl
+table=0,in_port=3,tcp
actions=load:0xffff->NXM_OF_IN_PORT[[]],ct(zone=1,table=1)
+table=1,tcp,ct_state=+trk actions=output:2
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+dnl Send a packet to install both the recirc_id=0 and the recirc_id=0x1 ukeys.
+AT_CHECK([ovs-appctl netdev-dummy/receive p3 \
+
'in_port(3),eth(src=50:54:00:00:00:08,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),tcp(src=1234,dst=80)'])
+
+dnl Sanity: both the pre-CT (recirc_id=0,in_port=3) and the post-CT
+dnl (recirc_id=0x1,in_port=3) megaflows must be installed. Use
+dnl OVS_WAIT_UNTIL so the test does not depend on revalidator exec speed.
+OVS_WAIT_UNTIL([test $(ovs-appctl dpctl/dump-flows | dnl
+ grep -c 'recirc_id(0),in_port(3)') -eq 1], [ovs-appctl dpctl/dump-flows])
+OVS_WAIT_UNTIL([test $(ovs-appctl dpctl/dump-flows | dnl
+ grep -c 'recirc_id(0x1),in_port(3)') -eq 1], [ovs-appctl dpctl/dump-flows])
+
+dnl Delete the ingress port. This triggers REV_RECONFIGURE, after which
+dnl the revalidator must drop any datapath megaflows that key on the
+dnl now-deleted port.
+
+AT_CHECK([ovs-vsctl del-port br0 p3])
+
+dnl The recirc_id=0 flow is correctly deleted (no frozen state involved).
+OVS_WAIT_UNTIL([test $(ovs-appctl dpctl/dump-flows | dnl
+ grep -c 'recirc_id(0),in_port(3)') -eq 0], [ovs-appctl dpctl/dump-flows])
+
+dnl The recirc_id>0 megaflow MUST also be deleted: its datapath key is
+dnl in_port(3) which no longer exists. Without the fix this wait times
+dnl out, because xlate_lookup_ofproto_() takes the frozen-in_port=OFPP_NONE
+dnl shortcut and returns SUCCESS without verifying the datapath in_port,
+dnl so revalidate_ukey__() decides UKEY_KEEP and the megaflow only goes
+dnl away when max-idle fires.
+OVS_WAIT_UNTIL([test $(ovs-appctl dpctl/dump-flows | dnl
+ grep -c 'recirc_id(0x1),in_port(3)') -eq 0], [ovs-appctl dpctl/dump-flows])
+
+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
