ovn: Implement the ability to send a packet back out its input port.

Otherwise logical router ARP replies won't work as implemented.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>

4 files changed, 18 insertions, 38 deletions

diff --git a/ovn/TODO b/ovn/TODO
index 10c3adff3..7f695085d 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -12,41 +12,6 @@ one router to another, this doesn't seem to matter (just put more than
 one connection between them), but for connections between a router and
 a switch it might matter because a switch has only one router port.
 
-** OVN_SB schema
-
-*** Allow output to ingress port
-
-Sometimes when a packet ingresses into a router, it has to egress the
-same port.  One example is a "one-armed" router that has multiple
-routes on a single port (or in which a host is (mis)configured to send
-every IP packet to the router, e.g. due to a bad netmask).  Another is
-when a router needs to send an ICMP reply to an ingressing packet.
-
-To some degree this problem is layered, because there are two
-different notions of "ingress port".  The first is the OpenFlow
-ingress port, essentially a physical port identifier.  This is
-implemented as part of ovs-vswitchd's OpenFlow implementation.  It
-prevents a reply from being sent across the tunnel on which it
-arrived.  It is questionable whether this OpenFlow feature is useful
-to OVN.  (OVN already has to override it to allow a packet from one
-nested container to be forwarded to a different nested container.)
-OVS make it possible to disable this feature of OpenFlow by setting
-the OpenFlow input port field to 0.  (If one does this too early, of
-course, it means that there's no way to actually match on the input
-port in the OpenFlow flow tables, but one can work around that by
-instead setting the input port just before the output action, possibly
-wrapping these actions in push/pop pairs to preserve the input port
-for later.)
-
-The second is the OVN logical ingress port, which is implemented in
-ovn-controller as part of the logical abstraction, using an OVS
-register.  Dropping packets directed to the logical ingress port is
-implemented through an OpenFlow table not directly visible to the
-logical flow table.  Currently this behavior can't be disabled, but
-various ways to ensure it could be implemented, e.g. the same as for
-OpenFlow by allowing the logical inport to be zeroed, or by
-introducing a new action that ignores the inport.
-
 ** New OVN logical actions
 
 *** arp
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 8a69e3e6a..f30500e89 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -19,6 +19,7 @@
 #include "dynamic-string.h"
 #include "json.h"
 #include "lex.h"
+#include "logical-fields.h"
 #include "match.h"
 #include "ofp-actions.h"
 #include "shash.h"
@@ -2810,8 +2811,17 @@ parse_assignment(struct expr_context *ctx, const struct simap *ports,
             uint32_t port = simap_get(ports, c->string);
             bitwise_put(port, &sf->value,
                         sf->field->n_bytes, 0, sf->field->n_bits);
-            bitwise_put(UINT64_MAX, &sf->mask,
-                        sf->field->n_bytes, 0, sf->field->n_bits);
+            bitwise_one(&sf->mask, sf->field->n_bytes, 0, sf->field->n_bits);
+
+            /* If the logical input port is being zeroed, clear the OpenFlow
+             * ingress port also, to allow a packet to be sent back to its
+             * origin. */
+            if (!port && sf->field->id == MFF_LOG_INPORT) {
+                sf = ofpact_put_SET_FIELD(ofpacts);
+                sf->field = mf_from_id(MFF_IN_PORT);
+                bitwise_one(&sf->mask,
+                            sf->field->n_bytes, 0, sf->field->n_bits);
+            }
         }
 
     exit_destroy_cs:
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 1d9104e02..9c2d41154 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -782,7 +782,11 @@
           <p>
             Output to the input port is implicitly dropped, that is,
             <code>output</code> becomes a no-op if <code>outport</code> ==
-            <code>inport</code>.
+            <code>inport</code>.  Occasionally it may be useful to override
+            this behavior, e.g. to send an ARP reply to an ARP request; to do
+            so, use <code>inport = "";</code> to set the logical input port to
+            an empty string (which should not be used as the name of any
+            logical port).
           </p>
         </dd>
 
diff --git a/tests/ovn.at b/tests/ovn.at
index c76b5dc28..a17d87029 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -454,6 +454,7 @@ reg0 = reg1; => actions=move:OXM_OF_PKT_REG0[0..31]->OXM_OF_PKT_REG0[32..63], pr
 vlan.pcp = reg0[0..2]; => actions=move:OXM_OF_PKT_REG0[32..34]->NXM_OF_VLAN_TCI[13..15], prereqs=vlan.tci[12]
 reg0[10] = vlan.pcp[1]; => actions=move:NXM_OF_VLAN_TCI[14]->OXM_OF_PKT_REG0[42], prereqs=vlan.tci[12]
 outport = inport; => actions=move:NXM_NX_REG6[]->NXM_NX_REG7[], prereqs=1
+inport = ""; => actions=set_field:0->reg6,set_field:0->in_port, prereqs=1
 reg0 <-> reg1; => actions=push:OXM_OF_PKT_REG0[0..31],push:OXM_OF_PKT_REG0[32..63],pop:OXM_OF_PKT_REG0[0..31],pop:OXM_OF_PKT_REG0[32..63], prereqs=1
 vlan.pcp <-> reg0[0..2]; => actions=push:OXM_OF_PKT_REG0[32..34],push:NXM_OF_VLAN_TCI[13..15],pop:OXM_OF_PKT_REG0[32..34],pop:NXM_OF_VLAN_TCI[13..15], prereqs=vlan.tci[12]
 reg0[10] <-> vlan.pcp[1]; => actions=push:NXM_OF_VLAN_TCI[14],push:OXM_OF_PKT_REG0[42],pop:NXM_OF_VLAN_TCI[14],pop:OXM_OF_PKT_REG0[42], prereqs=vlan.tci[12]