ofproto-dpif-rid: Always store tunnel metadata.

Tunnel metadata was only stored if the tunnel destination was set.  It's
possible, for example, that a flow could set the tunnel id field before
recirculation and then set the destination field afterwards.  The
previous behavior is that the tunnel id would be lost during
recirculation under such a circumstance.  This changes the behavior to
always copy the tunnel metadata, regardless of whether the tunnel
destination is set.  It also adds a unit test.

Signed-off-by: Justin Pettit <[email protected]>
Acked-by: Ben Pfaff <[email protected]>

ofproto/ofproto-dpif-rid.c

@@ -127,16 +127,9 @@ frozen_state_hash(const struct frozen_state *state)
 
     hash = uuid_hash(&state->ofproto_uuid);
     hash = hash_int(state->table_id, hash);
-    if (flow_tnl_dst_is_set(&state->metadata.tunnel)) {
-        /* We may leave remainder bytes unhashed, but that is unlikely as
-         * the tunnel is not in the datapath format. */
-        hash = hash_bytes64((const uint64_t *) &state->metadata.tunnel,
-                            flow_tnl_size(&state->metadata.tunnel), hash);
-    }
+    hash = hash_bytes64((const uint64_t *) &state->metadata,
+                        sizeof state->metadata, hash);
     hash = hash_boolean(state->conntracked, hash);
-    hash = hash_bytes64((const uint64_t *) &state->metadata.metadata,
-                        sizeof state->metadata - sizeof state->metadata.tunnel,
-                        hash);
     if (state->stack && state->stack_size) {
         hash = hash_bytes(state->stack, state->stack_size, hash);
     }
@@ -158,9 +151,7 @@ frozen_state_equal(const struct frozen_state *a, const struct frozen_state *b)
 {
     return (a->table_id == b->table_id
             && uuid_equals(&a->ofproto_uuid, &b->ofproto_uuid)
-            && flow_tnl_equal(&a->metadata.tunnel, &b->metadata.tunnel)
-            && !memcmp(&a->metadata.metadata, &b->metadata.metadata,
-                       sizeof a->metadata - sizeof a->metadata.tunnel)
+            && !memcmp(&a->metadata, &b->metadata, sizeof a->metadata)
             && a->stack_size == b->stack_size
             && !memcmp(a->stack, b->stack, a->stack_size)
             && a->mirrors == b->mirrors
@@ -204,8 +195,6 @@ static void
 frozen_state_clone(struct frozen_state *new, const struct frozen_state *old)
 {
     *new = *old;
-    flow_tnl_copy__(&new->metadata.tunnel, &old->metadata.tunnel);
-
     new->stack = (new->stack_size
                   ? xmemdup(new->stack, new->stack_size)
                   : NULL);

ofproto/ofproto-dpif-rid.h

@@ -124,11 +124,7 @@ static inline void
 frozen_metadata_to_flow(const struct frozen_metadata *md,
                         struct flow *flow)
 {
-    if (flow_tnl_dst_is_set(&md->tunnel)) {
-        flow->tunnel = md->tunnel;
-    } else {
-        memset(&flow->tunnel, 0, sizeof flow->tunnel);
-    }
+    flow->tunnel = md->tunnel;
     flow->metadata = md->metadata;
     memcpy(flow->regs, md->regs, sizeof flow->regs);
     flow->in_port.ofp_port = md->in_port;

tests/ofproto-dpif.at

@@ -4763,6 +4763,35 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath actions: 3
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+
+# This test verifies that tunnel metadata is preserved across
+# recirculation.  At the time of recirculation, fields such as "tun_id"
+# may be set before the tunnel is "valid" (ie, has a destination
+# address), but the field should still be available after recirculation.
+AT_SETUP([ofproto-dpif - resubmit with tun_id])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3
+
+AT_DATA([flows.txt], [dnl
+table=0 in_port=1  actions=2,load:6->NXM_NX_TUN_ID[[]],resubmit(,1)
+table=1 in_port=1  actions=debug_recirc,resubmit:55
+table=1 in_port=55 actions=3
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+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_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow" -generate], [0], [stdout])
+AT_CHECK([grep "Final flow:" stdout], [0], [Final flow: icmp,tun_id=0x6,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=128,icmp_type=8,icmp_code=0
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow,recirc_id(1)" -generate], [0], [stdout])
+AT_CHECK([grep "Final flow:" stdout], [0], [Final flow: recirc_id=0x1,icmp,tun_id=0x6,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=128,icmp_type=8,icmp_code=0
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
 # This test verifies that "resubmit", when it triggers recirculation
 # indirectly through the flow that it recursively invokes, is not
 # re-executed when execution continues later post-recirculation.