ofproto-dpif: Fix continuation with patch port

This patch fixes the ofp_port to odp_port translation issue on patch
port with nxt_resume.  When OVS resumes processing a packet from
nxt_resume, OVS does not translate the ofp in_port to odp in_port
correctly if the packet is originally received from a patch port.
Currently,OVS sets the odp in_port for this resume pakcet as ODPP_NONE
and push the resume packet back to the datapath. Later on, if the packet
goes through a recirc, OVS will generate the following message since it
can not translate odp in_port (ODPP_NONE) back to ofp in_port during upcall,
and push down a datapath rule to drop the packet.

    ofproto_dpif_upcall(handler16)|INFO|received packet on unassociated
        datapath port 4294967295

When OVS revalidates the drop datapath flow with ODPP_NONE in_port, we
will see the following warning.
    ofproto_dpif_upcall(revalidator18)|WARN|Failed to acquire udpif_key
        corresponding to unexpected flow (Invalid argument): ufid:....

This patch resolves this issue by storing the odp in_port in the
continuation messages, and restores the odp in_port before push the
packet back to the datapath.

VMWare-BZ: 2364696
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

diff --git a/include/openvswitch/ofp-packet.h b/include/openvswitch/ofp-packet.h
index 67001cb3f5d0024b0a6b8671b3aaf9e74730c314..77128d829b6de5983652cc5393d29f778e5aecd2 100644 (file)
--- a/include/openvswitch/ofp-packet.h
+++ b/include/openvswitch/ofp-packet.h
@@ -140,6 +140,9 @@ struct ofputil_packet_in_private {
     /* NXCPT_ACTION_SET. */
     struct ofpact *action_set;
     size_t action_set_len;
+
+    /* NXCPT_ODP_PORT. */
+    odp_port_t odp_port;
 };
 
 struct ofpbuf *ofputil_encode_packet_in_private(

diff --git a/lib/ofp-packet.c b/lib/ofp-packet.c
index a1ffe17de5f615dacd266ebd338ea33d689bfb5a..4579548ee174e61da5b3f2dbcd0c98f060d094d7 100644 (file)
--- a/lib/ofp-packet.c
+++ b/lib/ofp-packet.c
@@ -420,6 +420,7 @@ enum nx_continuation_prop_type {
     NXCPT_COOKIE,
     NXCPT_ACTIONS,
     NXCPT_ACTION_SET,
+    NXCPT_ODP_PORT,
 };
 
 /* Only NXT_PACKET_IN2 (not NXT_RESUME) should include NXCPT_USERDATA, so this
@@ -506,6 +507,10 @@ ofputil_put_packet_in_private(const struct ofputil_packet_in_private *pin,
         ofpprop_end(msg, start);
     }
 
+    if (pin->odp_port) {
+        ofpprop_put_u32(msg, NXCPT_ODP_PORT, odp_to_u32(pin->odp_port));
+    }
+
     if (msg->size > inner_ofs) {
         ofpprop_end(msg, continuation_ofs);
     } else {
@@ -875,6 +880,13 @@ ofputil_decode_packet_in_private(const struct ofp_header *oh, bool loose,
             error = parse_actions_property(&payload, oh->version, &action_set);
             break;
 
+        case NXCPT_ODP_PORT: {
+            uint32_t value;
+            error = ofpprop_parse_u32(&payload, &value);
+            pin->odp_port = u32_to_odp(value);
+            break;
+         }
+
         default:
             error = OFPPROP_UNKNOWN(loose, "continuation", type);
             break;
@@ -1010,6 +1022,11 @@ ofputil_packet_in_private_format(struct ds *s,
         ds_put_char(s, '\n');
     }
 
+    if (pin->odp_port) {
+        ds_put_format(s, " continuation.odp_port=%"PRIu32, pin->odp_port);
+        ds_put_char(s, '\n');
+    }
+
     if (verbosity > 0) {
         char *packet = ofp_packet_to_string(
             public->packet, public->packet_len,

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index a19aa9576b5c2c50ca13d7d7aced791ee97b14b3..02d7afb2ef8ffd55b643fee8d6ccb4a4e81c2e59 100644 (file)
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1526,6 +1526,7 @@ process_upcall(struct udpif *udpif, struct upcall *upcall,
                                : NULL),
                 am->pin.up.action_set_len = state->action_set_len,
                 am->pin.up.bridge = upcall->ofproto->uuid;
+                am->pin.up.odp_port = upcall->packet->md.in_port.odp_port;
             }
 
             /* We don't want to use the upcall 'flow', since it may be

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index eb9c5a5f5534d05cf3995bd66ca32c23d78f489e..cbeb6776f99d258c80e7add26c3ace9eb43c85b0 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5122,9 +5122,7 @@ nxt_resume(struct ofproto *ofproto_,
     pkt_metadata_from_flow(&packet.md, &pin->base.flow_metadata.flow);
 
     /* Fix up in_port. */
-    ofproto_dpif_set_packet_odp_port(ofproto,
-                                     pin->base.flow_metadata.flow.in_port.ofp_port,
-                                     &packet);
+    packet.md.in_port.odp_port = pin->odp_port;
 
     struct flow headers;
     flow_extract(&packet, &headers);

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 9640d1cb9f1356d47659feef0f48f04c8a5ed76a..dc21c8889d70c3226089079f4cb57cb96cc403d8 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5434,6 +5434,47 @@ AT_CHECK([test 1 = `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap | wc
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - continuation with patch port])
+AT_KEYWORDS([continuations pause resume])
+OVS_VSWITCHD_START(
+    [add-port br0 p0 -- set Interface p0 type=dummy -- \
+     add-port br0 patch- -- \
+     set interface patch- type=patch options:peer=patch+ -- \
+     add-br br1 -- set bridge br1 datapath-type=dummy fail-mode=secure -- \
+     add-port br1 patch+ -- set interface patch+ type=patch options:peer=patch-
+])
+add_of_ports --pcap br1 1
+
+flow="in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)"
+
+AT_DATA([flows.txt], [dnl
+table=0, in_port=patch+ icmp action=controller(pause), resubmit(,1)
+table=1, in_port=patch+ icmp action=ct(table=2)
+table=2, in_port=patch+ icmp ct_state=+trk+new action=ct(commit, table=3)
+table=3, in_port=patch+ icmp action=p1
+])
+
+AT_CHECK([ovs-ofctl add-flow br0 action=normal])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br1 flows.txt])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+ovs-ofctl monitor br1 resume --detach --no-chdir --pidfile=ovs-ofctl.pid 2> ofctl_monitor.log
+
+# Run a packet through the switch.
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 "$flow"], [0], [stdout])
+
+# Check flow stats
+AT_CHECK([ovs-ofctl dump-flows br1], [0], [stdout])
+AT_CHECK([strip_xids < stdout | sed -n 's/duration=[[0-9]]*\.[[0-9]]*s/duration=0.0s/p' | sed -n 's/idle_age=[[0-9]]*/idle_age=0/p' | grep 'table=3' | grep -v 'commit'], [0], [dnl
+ cookie=0x0, duration=0.0s, table=3, n_packets=1, n_bytes=106, idle_age=0, icmp,in_port=1 actions=output:2
+])
+
+# The packet should be received by port 1
+AT_CHECK([test 1 = `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" p1-tx.pcap | wc -l`])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 # Check that pause works after the packet is cloned.
 AT_SETUP([ofproto-dpif - continuation after clone])
 AT_KEYWORDS([continuations clone pause resume])