From 8c6efc06104ebccf4915d0d10c399292bac55874 Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Fri, 3 Apr 2020 07:46:28 +0000 Subject: geneve: Fix the byte swapping for the VNI Type: fix - swipe away the vomit indent left last time. - add tests for VNIs > 16bit Change-Id: I2d1f591bfb9d7a18996c38969365a509168d4193 Signed-off-by: Neale Ranns (cherry picked from commit 91fd910d7d7611a28d1f85482ed5d5c3ee6a8853) --- src/plugins/gtpu/test/test_gtpu.py | 5 ++- src/vnet/geneve/decap.c | 76 ++++++++++++++++---------------------- src/vnet/geneve/geneve.c | 23 ++++++------ src/vnet/geneve/geneve_packet.h | 13 ++----- test/template_bd.py | 4 +- test/test_geneve.py | 3 +- test/test_vxlan.py | 5 ++- test/test_vxlan6.py | 3 +- test/test_vxlan_gbp.py | 9 +++-- test/test_vxlan_gpe.py | 3 +- 10 files changed, 64 insertions(+), 80 deletions(-) diff --git a/src/plugins/gtpu/test/test_gtpu.py b/src/plugins/gtpu/test/test_gtpu.py index 667d61c0143..00f641c0088 100644 --- a/src/plugins/gtpu/test/test_gtpu.py +++ b/src/plugins/gtpu/test/test_gtpu.py @@ -187,7 +187,7 @@ class TestGtpu(BridgeDomain, VppTestCase): # Pick first received frame and check if it's correctly encapsulated. out = self.pg0.get_capture(1) pkt = out[0] - self.check_encapsulation(pkt, self.single_tunnel_bd) + self.check_encapsulation(pkt, self.single_tunnel_vni) # payload = self.decapsulate(pkt) # self.assert_eq_pkts(payload, self.frame_reply) @@ -343,13 +343,14 @@ class TestGtpu(BridgeDomain, VppTestCase): # Create GTPU VTEP on VPP pg0, and put gtpu_tunnel0 and pg1 # into BD. cls.single_tunnel_bd = 11 + cls.single_tunnel_vni = 11 r = cls.vapi.gtpu_add_del_tunnel( is_add=True, mcast_sw_if_index=0xFFFFFFFF, decap_next_index=0xFFFFFFFF, src_address=cls.pg0.local_ip4, dst_address=cls.pg0.remote_ip4, - teid=cls.single_tunnel_bd) + teid=cls.single_tunnel_vni) cls.vapi.sw_interface_set_l2_bridge(rx_sw_if_index=r.sw_if_index, bd_id=cls.single_tunnel_bd) cls.vapi.sw_interface_set_l2_bridge( diff --git a/src/vnet/geneve/decap.c b/src/vnet/geneve/decap.c index e30a56c0327..7914ef66005 100644 --- a/src/vnet/geneve/decap.c +++ b/src/vnet/geneve/decap.c @@ -213,7 +213,7 @@ geneve_input (vlib_main_t * vm, if (is_ip4) { key4_0.remote = ip4_0->src_address.as_u32; - key4_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key4_0.vni = vnet_get_geneve_vni_network_order (geneve0); /* Make sure GENEVE tunnel exist according to packet SIP and VNI */ if (PREDICT_FALSE (key4_0.as_u64 != last_key4.as_u64)) @@ -248,7 +248,7 @@ geneve_input (vlib_main_t * vm, (ip4_address_is_multicast (&ip4_0->dst_address))) { key4_0.remote = ip4_0->dst_address.as_u32; - key4_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key4_0.vni = vnet_get_geneve_vni_network_order (geneve0); /* Make sure mcast GENEVE tunnel exist by packet DIP and VNI */ p0 = hash_get (vxm->geneve4_tunnel_by_key, key4_0.as_u64); if (PREDICT_TRUE (p0 != NULL)) @@ -266,7 +266,7 @@ geneve_input (vlib_main_t * vm, { key6_0.remote.as_u64[0] = ip6_0->src_address.as_u64[0]; key6_0.remote.as_u64[1] = ip6_0->src_address.as_u64[1]; - key6_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key6_0.vni = vnet_get_geneve_vni_network_order (geneve0); /* Make sure GENEVE tunnel exist according to packet SIP and VNI */ if (PREDICT_FALSE @@ -303,7 +303,7 @@ geneve_input (vlib_main_t * vm, { key6_0.remote.as_u64[0] = ip6_0->dst_address.as_u64[0]; key6_0.remote.as_u64[1] = ip6_0->dst_address.as_u64[1]; - key6_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key6_0.vni = vnet_get_geneve_vni_network_order (geneve0); p0 = hash_get_mem (vxm->geneve6_tunnel_by_key, &key6_0); if (PREDICT_TRUE (p0 != NULL)) { @@ -380,7 +380,7 @@ geneve_input (vlib_main_t * vm, if (is_ip4) { key4_1.remote = ip4_1->src_address.as_u32; - key4_1.vni = vnet_get_geneve_vni_bigendian (geneve1); + key4_1.vni = vnet_get_geneve_vni_network_order (geneve1); /* Make sure unicast GENEVE tunnel exist by packet SIP and VNI */ if (PREDICT_FALSE (key4_1.as_u64 != last_key4.as_u64)) @@ -415,7 +415,7 @@ geneve_input (vlib_main_t * vm, (ip4_address_is_multicast (&ip4_1->dst_address))) { key4_1.remote = ip4_1->dst_address.as_u32; - key4_1.vni = vnet_get_geneve_vni_bigendian (geneve1); + key4_1.vni = vnet_get_geneve_vni_network_order (geneve1); /* Make sure mcast GENEVE tunnel exist by packet DIP and VNI */ p1 = hash_get (vxm->geneve4_tunnel_by_key, key4_1.as_u64); if (PREDICT_TRUE (p1 != NULL)) @@ -433,7 +433,7 @@ geneve_input (vlib_main_t * vm, { key6_1.remote.as_u64[0] = ip6_1->src_address.as_u64[0]; key6_1.remote.as_u64[1] = ip6_1->src_address.as_u64[1]; - key6_1.vni = vnet_get_geneve_vni_bigendian (geneve1); + key6_1.vni = vnet_get_geneve_vni_network_order (geneve1); /* Make sure GENEVE tunnel exist according to packet SIP and VNI */ if (PREDICT_FALSE @@ -472,7 +472,7 @@ geneve_input (vlib_main_t * vm, { key6_1.remote.as_u64[0] = ip6_1->dst_address.as_u64[0]; key6_1.remote.as_u64[1] = ip6_1->dst_address.as_u64[1]; - key6_1.vni = vnet_get_geneve_vni_bigendian (geneve1); + key6_1.vni = vnet_get_geneve_vni_network_order (geneve1); p1 = hash_get_mem (vxm->geneve6_tunnel_by_key, &key6_1); if (PREDICT_TRUE (p1 != NULL)) { @@ -618,7 +618,7 @@ geneve_input (vlib_main_t * vm, if (is_ip4) { key4_0.remote = ip4_0->src_address.as_u32; - key4_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key4_0.vni = vnet_get_geneve_vni_network_order (geneve0); /* Make sure unicast GENEVE tunnel exist by packet SIP and VNI */ if (PREDICT_FALSE (key4_0.as_u64 != last_key4.as_u64)) @@ -653,7 +653,7 @@ geneve_input (vlib_main_t * vm, (ip4_address_is_multicast (&ip4_0->dst_address))) { key4_0.remote = ip4_0->dst_address.as_u32; - key4_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key4_0.vni = vnet_get_geneve_vni_network_order (geneve0); /* Make sure mcast GENEVE tunnel exist by packet DIP and VNI */ p0 = hash_get (vxm->geneve4_tunnel_by_key, key4_0.as_u64); if (PREDICT_TRUE (p0 != NULL)) @@ -671,7 +671,7 @@ geneve_input (vlib_main_t * vm, { key6_0.remote.as_u64[0] = ip6_0->src_address.as_u64[0]; key6_0.remote.as_u64[1] = ip6_0->src_address.as_u64[1]; - key6_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key6_0.vni = vnet_get_geneve_vni_network_order (geneve0); /* Make sure GENEVE tunnel exist according to packet SIP and VNI */ if (PREDICT_FALSE @@ -708,7 +708,7 @@ geneve_input (vlib_main_t * vm, { key6_0.remote.as_u64[0] = ip6_0->dst_address.as_u64[0]; key6_0.remote.as_u64[1] = ip6_0->dst_address.as_u64[1]; - key6_0.vni = vnet_get_geneve_vni_bigendian (geneve0); + key6_0.vni = vnet_get_geneve_vni_network_order (geneve0); p0 = hash_get_mem (vxm->geneve6_tunnel_by_key, &key6_0); if (PREDICT_TRUE (p0 != NULL)) { @@ -1262,24 +1262,17 @@ VLIB_NODE_FN (ip4_geneve_bypass_node) (vlib_main_t * vm, /* *INDENT-OFF* */ VLIB_REGISTER_NODE (ip4_geneve_bypass_node) = { - .name = "ip4-geneve-bypass",.vector_size = - sizeof (u32),.n_next_nodes = IP_GENEVE_BYPASS_N_NEXT,.next_nodes = + .name = "ip4-geneve-bypass", + .vector_size = sizeof (u32), + .n_next_nodes = IP_GENEVE_BYPASS_N_NEXT,.next_nodes = { - [IP_GENEVE_BYPASS_NEXT_DROP] = "error-drop", - [IP_GENEVE_BYPASS_NEXT_GENEVE] = "geneve4-input",} -,.format_buffer = format_ip4_header,.format_trace = - format_ip4_forward_next_trace,}; - -#ifndef CLIB_MARCH_VARIANT -/* Dummy init function to get us linked in. */ - clib_error_t *ip4_geneve_bypass_init (vlib_main_t * vm) -{ - return 0; -} - -VLIB_INIT_FUNCTION (ip4_geneve_bypass_init); + [IP_GENEVE_BYPASS_NEXT_DROP] = "error-drop", + [IP_GENEVE_BYPASS_NEXT_GENEVE] = "geneve4-input", + }, + .format_buffer = format_ip4_header, + .format_trace = format_ip4_forward_next_trace, +}; /* *INDENT-ON* */ -#endif /* CLIB_MARCH_VARIANT */ VLIB_NODE_FN (ip6_geneve_bypass_node) (vlib_main_t * vm, vlib_node_runtime_t * node, @@ -1291,26 +1284,19 @@ VLIB_NODE_FN (ip6_geneve_bypass_node) (vlib_main_t * vm, /* *INDENT-OFF* */ VLIB_REGISTER_NODE (ip6_geneve_bypass_node) = { - .name = "ip6-geneve-bypass",.vector_size = - sizeof (u32),.n_next_nodes = IP_GENEVE_BYPASS_N_NEXT,.next_nodes = + .name = "ip6-geneve-bypass", + .vector_size = sizeof (u32), + .n_next_nodes = IP_GENEVE_BYPASS_N_NEXT, + .next_nodes = { - [IP_GENEVE_BYPASS_NEXT_DROP] = "error-drop", - [IP_GENEVE_BYPASS_NEXT_GENEVE] = "geneve6-input",} -,.format_buffer = format_ip6_header,.format_trace = - format_ip6_forward_next_trace,}; + [IP_GENEVE_BYPASS_NEXT_DROP] = "error-drop", + [IP_GENEVE_BYPASS_NEXT_GENEVE] = "geneve6-input", + }, + .format_buffer = format_ip6_header, + .format_trace = format_ip6_forward_next_trace, +}; /* *INDENT-ON* */ -#ifndef CLIB_MARCH_VARIANT -/* Dummy init function to get us linked in. */ -clib_error_t * -ip6_geneve_bypass_init (vlib_main_t * vm) -{ - return 0; -} - -VLIB_INIT_FUNCTION (ip6_geneve_bypass_init); -#endif /* CLIB_MARCH_VARIANT */ - /* * fd.io coding-style-patch-verification: ON * diff --git a/src/vnet/geneve/geneve.c b/src/vnet/geneve/geneve.c index 0c551a0545e..f04917a71a4 100644 --- a/src/vnet/geneve/geneve.c +++ b/src/vnet/geneve/geneve.c @@ -321,14 +321,15 @@ vtep_addr_unref (ip46_address_t * ip) return 0; } -typedef CLIB_PACKED (union - { - struct - { - fib_node_index_t mfib_entry_index; - adj_index_t mcast_adj_index; - }; u64 as_u64; - }) mcast_shared_t; +typedef union +{ + struct + { + fib_node_index_t mfib_entry_index; + adj_index_t mcast_adj_index; + }; + u64 as_u64; +} __clib_packed mcast_shared_t; static inline mcast_shared_t mcast_shared_get (ip46_address_t * ip) @@ -381,15 +382,13 @@ int vnet_geneve_add_del_tunnel if (!is_ip6) { key4.remote = a->remote.ip4.as_u32; - key4.vni = - clib_host_to_net_u32 ((a->vni << GENEVE_VNI_SHIFT) & GENEVE_VNI_MASK); + key4.vni = clib_host_to_net_u32 (a->vni << GENEVE_VNI_SHIFT); p = hash_get (vxm->geneve4_tunnel_by_key, key4.as_u64); } else { key6.remote = a->remote.ip6; - key6.vni = - clib_host_to_net_u32 ((a->vni << GENEVE_VNI_SHIFT) & GENEVE_VNI_MASK); + key6.vni = clib_host_to_net_u32 (a->vni << GENEVE_VNI_SHIFT); p = hash_get_mem (vxm->geneve6_tunnel_by_key, &key6); } diff --git a/src/vnet/geneve/geneve_packet.h b/src/vnet/geneve/geneve_packet.h index ab37f5378df..a0d0687434c 100644 --- a/src/vnet/geneve/geneve_packet.h +++ b/src/vnet/geneve/geneve_packet.h @@ -137,21 +137,14 @@ typedef struct static inline u32 vnet_get_geneve_vni (geneve_header_t * h) { - return (clib_net_to_host_u32 (h->vni_rsvd & GENEVE_VNI_MASK) >> + return ((clib_net_to_host_u32 (h->vni_rsvd) & GENEVE_VNI_MASK) >> GENEVE_VNI_SHIFT); } -/* - * Return the VNI in network-byte order - * - * To be used in the DECAP phase to create the lookup key (IP + VNI) - */ static inline u32 -vnet_get_geneve_vni_bigendian (geneve_header_t * h) +vnet_get_geneve_vni_network_order (geneve_header_t * h) { - u32 vni_host = vnet_get_geneve_vni (h); - return clib_host_to_net_u32 ((vni_host << GENEVE_VNI_SHIFT) & - GENEVE_VNI_MASK); + return (h->vni_rsvd & clib_net_to_host_u32 (GENEVE_VNI_MASK)); } static inline void diff --git a/test/template_bd.py b/test/template_bd.py index bd55b9b6890..198cc2258b8 100644 --- a/test/template_bd.py +++ b/test/template_bd.py @@ -74,7 +74,7 @@ class BridgeDomain(object): """ encapsulated_pkt = self.encapsulate(self.frame_request, - self.single_tunnel_bd) + self.single_tunnel_vni) self.pg0.add_stream([encapsulated_pkt, ]) @@ -102,7 +102,7 @@ class BridgeDomain(object): # Pick first received frame and check if it's correctly encapsulated. out = self.pg0.get_capture(1) pkt = out[0] - self.check_encapsulation(pkt, self.single_tunnel_bd) + self.check_encapsulation(pkt, self.single_tunnel_vni) payload = self.decapsulate(pkt) self.assert_eq_pkts(payload, self.frame_reply) diff --git a/test/test_geneve.py b/test/test_geneve.py index 24019e7316b..7eb23f27f9b 100644 --- a/test/test_geneve.py +++ b/test/test_geneve.py @@ -182,10 +182,11 @@ class TestGeneve(BridgeDomain, VppTestCase): # Create GENEVE VTEP on VPP pg0, and put geneve_tunnel0 and pg1 # into BD. + cls.single_tunnel_vni = 0xabcde cls.single_tunnel_bd = 1 r = cls.vapi.geneve_add_del_tunnel( local_address=cls.pg0.local_ip4, - remote_address=cls.pg0.remote_ip4, vni=cls.single_tunnel_bd) + remote_address=cls.pg0.remote_ip4, vni=cls.single_tunnel_vni) cls.vapi.sw_interface_set_l2_bridge(rx_sw_if_index=r.sw_if_index, bd_id=cls.single_tunnel_bd) cls.vapi.sw_interface_set_l2_bridge( diff --git a/test/test_vxlan.py b/test/test_vxlan.py index 7613054882a..fffebcf9237 100644 --- a/test/test_vxlan.py +++ b/test/test_vxlan.py @@ -185,9 +185,10 @@ class TestVxlan(BridgeDomain, VppTestCase): # Create VXLAN VTEP on VPP pg0, and put vxlan_tunnel0 and pg1 # into BD. cls.single_tunnel_bd = 1 + cls.single_tunnel_vni = 0xabcde r = cls.vapi.vxlan_add_del_tunnel(src_address=cls.pg0.local_ip4n, dst_address=cls.pg0.remote_ip4n, - vni=cls.single_tunnel_bd) + vni=cls.single_tunnel_vni) cls.vapi.sw_interface_set_l2_bridge(rx_sw_if_index=r.sw_if_index, bd_id=cls.single_tunnel_bd) cls.vapi.sw_interface_set_l2_bridge( @@ -250,7 +251,7 @@ class TestVxlan(BridgeDomain, VppTestCase): ether = out[0] pkt = reassemble4(out) pkt = ether / pkt - self.check_encapsulation(pkt, self.single_tunnel_bd) + self.check_encapsulation(pkt, self.single_tunnel_vni) payload = self.decapsulate(pkt) # TODO: Scapy bug? diff --git a/test/test_vxlan6.py b/test/test_vxlan6.py index b1fda508394..c0745b199d1 100644 --- a/test/test_vxlan6.py +++ b/test/test_vxlan6.py @@ -138,10 +138,11 @@ class TestVxlan6(BridgeDomain, VppTestCase): # Create VXLAN VTEP on VPP pg0, and put vxlan_tunnel0 and pg1 # into BD. cls.single_tunnel_bd = 1 + cls.single_tunnel_vni = 0xabcde r = cls.vapi.vxlan_add_del_tunnel(src_address=cls.pg0.local_ip6n, dst_address=cls.pg0.remote_ip6n, is_ipv6=1, - vni=cls.single_tunnel_bd) + vni=cls.single_tunnel_vni) cls.vapi.sw_interface_set_l2_bridge(rx_sw_if_index=r.sw_if_index, bd_id=cls.single_tunnel_bd) cls.vapi.sw_interface_set_l2_bridge( diff --git a/test/test_vxlan_gbp.py b/test/test_vxlan_gbp.py index 19790df1f76..d3cd7aa6fdc 100644 --- a/test/test_vxlan_gbp.py +++ b/test/test_vxlan_gbp.py @@ -145,11 +145,12 @@ class TestVxlanGbp(VppTestCase): # Create VXLAN GBP VTEP on VPP pg0, and put vxlan_gbp_tunnel0 and # pg1 into BD. cls.single_tunnel_bd = 1 + cls.single_tunnel_vni = 0xabcde r = cls.vapi.vxlan_gbp_tunnel_add_del( tunnel={ 'src': cls.pg0.local_ip4, 'dst': cls.pg0.remote_ip4, - 'vni': cls.single_tunnel_bd, + 'vni': cls.single_tunnel_vni, 'instance': INVALID_INDEX, 'mcast_sw_if_index': INVALID_INDEX, 'mode': 1, @@ -197,7 +198,7 @@ class TestVxlanGbp(VppTestCase): Verify receipt of decapsulated frames on pg1 """ encapsulated_pkt = self.encapsulate(self.frame_request, - self.single_tunnel_bd) + self.single_tunnel_vni) self.pg0.add_stream([encapsulated_pkt, ]) @@ -225,7 +226,7 @@ class TestVxlanGbp(VppTestCase): # Pick first received frame and check if it's correctly encapsulated. out = self.pg0.get_capture(1) pkt = out[0] - self.check_encapsulation(pkt, self.single_tunnel_bd) + self.check_encapsulation(pkt, self.single_tunnel_vni) payload = self.decapsulate(pkt) self.assert_eq_pkts(payload, self.frame_reply) @@ -269,7 +270,7 @@ class TestVxlanGbp(VppTestCase): # Pick first received frame and check if it's correctly encapsulated. out = self.pg0.get_capture(2) pkt = reassemble4_ether(out) - self.check_encapsulation(pkt, self.single_tunnel_bd) + self.check_encapsulation(pkt, self.single_tunnel_vni) payload = self.decapsulate(pkt) self.assert_eq_pkts(payload, frame) diff --git a/test/test_vxlan_gpe.py b/test/test_vxlan_gpe.py index ca1ad9c60bf..3d6e26024c9 100644 --- a/test/test_vxlan_gpe.py +++ b/test/test_vxlan_gpe.py @@ -190,11 +190,12 @@ class TestVxlanGpe(BridgeDomain, VppTestCase): # Create VXLAN-GPE VTEP on VPP pg0, and put vxlan_gpe_tunnel0 # and pg1 into BD. + cls.single_tunnel_vni = 0xabcde cls.single_tunnel_bd = 11 r = cls.vapi.vxlan_gpe_add_del_tunnel( src_addr=cls.pg0.local_ip4n, dst_addr=cls.pg0.remote_ip4n, - vni=cls.single_tunnel_bd) + vni=cls.single_tunnel_vni) cls.vapi.sw_interface_set_l2_bridge(rx_sw_if_index=r.sw_if_index, bd_id=cls.single_tunnel_bd) cls.vapi.sw_interface_set_l2_bridge( -- cgit 1.2.3-korg