vxlan-gbp: Decap ignores reserved bits

Type: fix

from the draft:
3.  Backward Compatibility

   VXLAN [RFC7348] requires reserved fields to be set to zero on
   transmit and ignored on receive.

Change-Id: I98544907894f1a6eba9595a37c3c88322905630e
Signed-off-by: Neale Ranns <nranns@cisco.com>

4 files changed, 35 insertions, 22 deletions

diff --git a/MAINTAINERS b/MAINTAINERS
index 3fd5ef6878e..54e9a63a5bd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -209,6 +209,12 @@ I:	vxlan-gpe
 M:	Hongjun Ni <hongjun.ni@intel.com>
 F:	src/vnet/vxlan-gpe/
 
+VNET VXLAN-GBP
+I:	vxlan-gbp
+M:	Mohsin Kazmi <sykazmi@cisco.com>
+M:	Neale Ranns <nranns@cisco.com>
+F:	src/vnet/vxlan-gbp/
+
 VNET IPIP
 I:	ipip
 Y:	src/vnet/ipip/FEATURE.yaml
diff --git a/src/vnet/vxlan-gbp/decap.c b/src/vnet/vxlan-gbp/decap.c
index 23995c49fcb..927c778b211 100644
--- a/src/vnet/vxlan-gbp/decap.c
+++ b/src/vnet/vxlan-gbp/decap.c
@@ -76,9 +76,11 @@ vxlan4_gbp_find_tunnel (vxlan_gbp_main_t * vxm, last_tunnel_cache4 * cache,
   vxlan4_gbp_tunnel_key_t key4;
   int rv;
 
-  key4.key[1] = ((u64) fib_index << 32) | vxlan_gbp0->vni_reserved;
-  key4.key[0] = (((u64) ip4_0->dst_address.as_u32 << 32) |
-		 ip4_0->src_address.as_u32);
+  key4.key[1] = (((u64) fib_index << 32) |
+		 (vxlan_gbp0->vni_reserved &
+		  clib_host_to_net_u32 (0xffffff00)));
+  key4.key[0] =
+    (((u64) ip4_0->dst_address.as_u32 << 32) | ip4_0->src_address.as_u32);
 
   if (PREDICT_FALSE (key4.key[0] != cache->key[0] ||
 		     key4.key[1] != cache->key[1]))
@@ -122,7 +124,9 @@ vxlan6_gbp_find_tunnel (vxlan_gbp_main_t * vxm, last_tunnel_cache6 * cache,
     .key = {
 	    [0] = ip6_0->src_address.as_u64[0],
 	    [1] = ip6_0->src_address.as_u64[1],
-	    [2] = (((u64) fib_index) << 32) | vxlan_gbp0->vni_reserved,
+	    [2] = ((((u64) fib_index) << 32) |
+		   (vxlan_gbp0->vni_reserved &
+		    clib_host_to_net_u32 (0xffffff00))),
 	    }
   };
   int rv;
@@ -293,13 +297,13 @@ vxlan_gbp_input (vlib_main_t * vm,
 	  vlib_buffer_advance (b0, sizeof *vxlan_gbp0);
 	  vlib_buffer_advance (b1, sizeof *vxlan_gbp1);
 
+	  u8 i_and_g0 = ((flags0 & VXLAN_GBP_FLAGS_GI) == VXLAN_GBP_FLAGS_GI);
+	  u8 i_and_g1 = ((flags1 & VXLAN_GBP_FLAGS_GI) == VXLAN_GBP_FLAGS_GI);
+
 	  /* Validate VXLAN_GBP tunnel encap-fib index against packet */
-	  if (PREDICT_FALSE
-	      (t0 == NULL
-	       || flags0 != (VXLAN_GBP_FLAGS_I | VXLAN_GBP_FLAGS_G)))
+	  if (PREDICT_FALSE (t0 == NULL || !i_and_g0))
 	    {
-	      if (t0 != NULL
-		  && flags0 != (VXLAN_GBP_FLAGS_I | VXLAN_GBP_FLAGS_G))
+	      if (t0 != NULL && !i_and_g0)
 		{
 		  error0 = VXLAN_GBP_ERROR_BAD_FLAGS;
 		  vlib_increment_combined_counter
@@ -335,11 +339,9 @@ vxlan_gbp_input (vlib_main_t * vm,
 	  vnet_buffer2 (b0)->gbp.sclass = vxlan_gbp_get_sclass (vxlan_gbp0);
 
 
-	  if (PREDICT_FALSE
-	      (t1 == 0 || flags1 != (VXLAN_GBP_FLAGS_I | VXLAN_GBP_FLAGS_G)))
+	  if (PREDICT_FALSE (t1 == NULL || !i_and_g1))
 	    {
-	      if (t1 != 0
-		  && flags1 != (VXLAN_GBP_FLAGS_I | VXLAN_GBP_FLAGS_G))
+	      if (t1 != NULL && !i_and_g1)
 		{
 		  error1 = VXLAN_GBP_ERROR_BAD_FLAGS;
 		  vlib_increment_combined_counter
@@ -443,13 +445,13 @@ vxlan_gbp_input (vlib_main_t * vm,
 
 	  /* pop (ip, udp, vxlan_gbp) */
 	  vlib_buffer_advance (b0, sizeof (*vxlan_gbp0));
+
+	  u8 i_and_g0 = ((flags0 & VXLAN_GBP_FLAGS_GI) == VXLAN_GBP_FLAGS_GI);
+
 	  /* Validate VXLAN_GBP tunnel encap-fib index against packet */
-	  if (PREDICT_FALSE
-	      (t0 == NULL
-	       || flags0 != (VXLAN_GBP_FLAGS_I | VXLAN_GBP_FLAGS_G)))
+	  if (PREDICT_FALSE (t0 == NULL || !i_and_g0))
 	    {
-	      if (t0 != NULL
-		  && flags0 != (VXLAN_GBP_FLAGS_I | VXLAN_GBP_FLAGS_G))
+	      if (t0 != NULL && !i_and_g0)
 		{
 		  error0 = VXLAN_GBP_ERROR_BAD_FLAGS;
 		  vlib_increment_combined_counter
diff --git a/src/vnet/vxlan-gbp/vxlan_gbp_packet.h b/src/vnet/vxlan-gbp/vxlan_gbp_packet.h
index 58fe00323b0..e655b333b89 100644
--- a/src/vnet/vxlan-gbp/vxlan_gbp_packet.h
+++ b/src/vnet/vxlan-gbp/vxlan_gbp_packet.h
@@ -104,6 +104,8 @@ typedef enum
 #undef _
 } __attribute__ ((packed)) vxlan_gbp_flags_t;
 
+#define VXLAN_GBP_FLAGS_GI (VXLAN_GBP_FLAGS_G|VXLAN_GBP_FLAGS_I)
+
 #define foreach_vxlan_gbp_gpflags \
 _ (0x40, D)                       \
 _ (0x20, E)                       \
diff --git a/test/test_gbp.py b/test/test_gbp.py
index e4d4390f081..37d587f51b4 100644
--- a/test/test_gbp.py
+++ b/test/test_gbp.py
@@ -1719,14 +1719,15 @@ class TestGBP(VppTestCase):
         # Learn new EPs from GARP packets received on the BD's mcast tunnel
         #
         for ii, l in enumerate(learnt):
-            # a packet with an sclass from a known EPG
-            # arriving on an unknown TEP
+            # add some junk in the reserved field of the vxlan-header
+            # next to the VNI. we should accept since reserved bits are
+            # ignored on rx.
             p = (Ether(src=self.pg2.remote_mac,
                        dst=self.pg2.local_mac) /
                  IP(src=self.pg2.remote_hosts[1].ip4,
                     dst="239.1.1.1") /
                  UDP(sport=1234, dport=48879) /
-                 VXLAN(vni=88, gpid=112, flags=0x88) /
+                 VXLAN(vni=88, reserved2=0x80, gpid=112, flags=0x88) /
                  Ether(src=l['mac'], dst="ff:ff:ff:ff:ff:ff") /
                  ARP(op="who-has",
                      psrc=l['ip'], pdst=l['ip'],
@@ -1832,12 +1833,14 @@ class TestGBP(VppTestCase):
         #
         for l in learnt:
             # a packet with an sclass from a known EPG
+            # set a reserved bit in addition to the G and I
+            # reserved bits should not be checked on rx.
             p = (Ether(src=self.pg2.remote_mac,
                        dst=self.pg2.local_mac) /
                  IP(src=self.pg2.remote_hosts[1].ip4,
                     dst=self.pg2.local_ip4) /
                  UDP(sport=1234, dport=48879) /
-                 VXLAN(vni=99, gpid=112, flags=0x88) /
+                 VXLAN(vni=99, gpid=112, flags=0xc8) /
                  Ether(src=l['mac'], dst=ep.mac) /
                  IP(src=l['ip'], dst=ep.ip4.address) /
                  UDP(sport=1234, dport=1234) /