From d44bb65b455db60b6627522b6b77fcb8b2e81de7 Mon Sep 17 00:00:00 2001 From: Paolo Valerio Date: Tue, 5 Sep 2023 21:13:02 +0200 Subject: [PATCH] ofproto-dpif-xlate: Fix recirculation with patch port and controller. If a packet originating from the controller recirculates after going through a patch port, it gets dropped with the following message: ofproto_dpif_upcall(handler8)|INFO|received packet on unassociated datapath port 4294967295 This happens because there's no xport_uuid in the recirculation node and at the same type in_port refers to the patch port. The patch, in the case of zeroed uuid, checks that in_port belongs to the bridge and returns the related ofproto. Reported-at: https://bugzilla.redhat.com/2170920 Signed-off-by: Paolo Valerio Signed-off-by: Ilya Maximets --- ofproto/ofproto-dpif-xlate.c | 12 +++++++++++- tests/ofproto-dpif.at | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index a7423983940..f574886752a 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1604,7 +1604,8 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer, } ofp_port_t in_port = recirc_id_node->state.metadata.in_port; - if (in_port != OFPP_NONE && in_port != OFPP_CONTROLLER) { + if (in_port != OFPP_NONE && in_port != OFPP_CONTROLLER && + !uuid_is_zero(&recirc_id_node->state.xport_uuid)) { struct uuid xport_uuid = recirc_id_node->state.xport_uuid; xport = xport_lookup_by_uuid(xcfg, &xport_uuid); if (xport && xport->xbridge && xport->xbridge->ofproto) { @@ -1615,11 +1616,19 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer, * that the packet originated from the controller via an OpenFlow * "packet-out". The right thing to do is to find just the * ofproto. There is no xport, which is OK. + * Also a zeroed xport_uuid with a valid in_port, means that + * the packet originated from OFPP_CONTROLLER passed + * through a patch port. * * OFPP_NONE can also indicate that a bond caused recirculation. */ struct uuid uuid = recirc_id_node->state.ofproto_uuid; const struct xbridge *bridge = xbridge_lookup_by_uuid(xcfg, &uuid); + if (bridge && bridge->ofproto) { + if (in_port != OFPP_CONTROLLER && in_port != OFPP_NONE && + !get_ofp_port(bridge, in_port)) { + goto xport_lookup; + } if (errorp) { *errorp = NULL; } @@ -1632,6 +1641,7 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer, } } +xport_lookup: xport = xport_lookup(xcfg, tnl_port_should_receive(flow) ? tnl_port_receive(flow) : odp_port_to_ofport(backer, flow->in_port.odp_port)); diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 45b669d35cc..127a5b4ea0c 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -5854,6 +5854,40 @@ OVS_WAIT_UNTIL([check_flows], [ovs-ofctl dump-flows br0]) 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]) +OVS_VSWITCHD_START( + [add-port br0 patch-br1 -- \ + set interface patch-br1 type=patch options:peer=patch-br0 -- \ + add-br br1 -- set bridge br1 datapath-type=dummy fail-mode=secure -- \ + add-port br1 patch-br0 -- set interface patch-br0 type=patch options:peer=patch-br1 +]) + +add_of_ports --pcap br1 1 + +AT_DATA([flows-br0.txt], [dnl +table=0 icmp actions=output:patch-br1 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows-br0.txt]) + +AT_DATA([flows-br1.txt], [dnl +table=0, icmp actions=ct(table=1,zone=1) +table=1, ct_state=+trk, icmp actions=p1 +]) +AT_CHECK([ovs-ofctl add-flows br1 flows-br1.txt]) + +packet=50540000000750540000000508004500005c000000008001b94dc0a80001c0a80002080013fc00000000000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f +AT_CHECK([ovs-ofctl packet-out br0 "in_port=CONTROLLER packet=$packet actions=table"]) + +OVS_WAIT_UNTIL_EQUAL([ovs-ofctl dump-flows -m br1 | grep "ct_state" | ofctl_strip], [dnl + table=1, n_packets=1, n_bytes=106, ct_state=+trk,icmp actions=output:2]) + +OVS_WAIT_UNTIL([ovs-pcap p1-tx.pcap | grep -q "$packet"]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - debug_slow action]) OVS_VSWITCHD_START add_of_ports br0 1 2 3