ofproto-dpif: Maintain tun_id across action lists.

Previously, a tunnel ID received on input would be used by default
on output if the packet was later sent out a tunnel.  This was not
actually intentional behavior - tunnel information is not supposed
to carry over.  However, it is useful in the case where move actions
are used to load the original tunnel ID into registers for further
processing or additional lookups in resubmits since otherwise the
information is lost before handling actions.

When the initial components for flow based tunneling were introduced
this behavior for tunnel headers caused problems since it resulted
in packets being sent to the IP address they were originally received
on.  All of this behavior was "fixed" in commit
47d4a9db26329f9d93eb945c1fcc0e248cf2656a (ofproto-dpif: Initialize
tunnel metadata in both 'flow' and 'base_flow'.), breaking the use
cases the require tun_id during action processing.

For the time being, at least, this restores the original behavior
for tun_id, while keeping the new behavior for the rest of the
tunnel headers.  The latter are not exposed through OpenFlow and
therefore do not have similar complications.  If we do expose these
headers then we might have to revist this.

Thanks to Ben Pfaff for identifying the commit that introduced the
problem.

Bug #14625

Reported-by: Michael Hu <humichael@vmware.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index bc54122..55b4e48 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6094,11 +6094,35 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx,
                       ovs_be16 initial_tci, struct rule_dpif *rule,
                       uint8_t tcp_flags, const struct ofpbuf *packet)
 {
+    ovs_be64 initial_tun_id = flow->tunnel.tun_id;
+
+    /* Flow initialization rules:
+     * - 'base_flow' must match the kernel's view of the packet at the
+     *   time that action processing starts.  'flow' represents any
+     *   transformations we wish to make through actions.
+     * - By default 'base_flow' and 'flow' are the same since the input
+     *   packet matches the output before any actions are applied.
+     * - When using VLAN splinters, 'base_flow''s VLAN is set to the value
+     *   of the received packet as seen by the kernel.  If we later output
+     *   to another device without any modifications this will cause us to
+     *   insert a new tag since the original one was stripped off by the
+     *   VLAN device.
+     * - Tunnel 'flow' is largely cleared when transitioning between
+     *   the input and output stages since it does not make sense to output
+     *   a packet with the exact headers that it was received with (i.e.
+     *   the destination IP is us).  The one exception is the tun_id, which
+     *   is preserved to allow use in later resubmit lookups and loads into
+     *   registers.
+     * - Tunnel 'base_flow' is completely cleared since that is what the
+     *   kernel does.  If we wish to maintain the original values an action
+     *   needs to be generated. */
+
     ctx->ofproto = ofproto;
     ctx->flow = *flow;
     memset(&ctx->flow.tunnel, 0, sizeof ctx->flow.tunnel);
     ctx->base_flow = ctx->flow;
     ctx->base_flow.vlan_tci = initial_tci;
+    ctx->flow.tunnel.tun_id = initial_tun_id;
     ctx->rule = rule;
     ctx->packet = packet;
     ctx->may_learn = packet != NULL;