From f2984bbb013acb0d6872e8fbb2f9d57d3e4f49b9 Mon Sep 17 00:00:00 2001 From: Ahmed Abdelsalam Date: Fri, 20 Nov 2020 18:56:09 +0000 Subject: [PATCH] ip: use IPv6 flowlabel in flow hash computation extends ip6_compute_flow_hash() to include IPv6 flowlabel in flowhash computation Type: improvement Signed-off-by: Ahmed Abdelsalam Signed-off-by: Neale Ranns Change-Id: Id1aaa20c9dac729c22b714eea1cdd6e9e4d1f75e --- src/plugins/gbp/test/test_gbp.py | 8 ++++---- src/vnet/ip/ip.api | 33 ++++++++++++++++++++++++++++++ src/vnet/ip/ip.c | 15 ++++++++++++++ src/vnet/ip/ip.h | 2 ++ src/vnet/ip/ip4.h | 3 --- src/vnet/ip/ip4_forward.c | 28 +++++++------------------ src/vnet/ip/ip6.h | 3 --- src/vnet/ip/ip6_forward.c | 35 +++++++++----------------------- src/vnet/ip/ip6_inlines.h | 2 ++ src/vnet/ip/ip6_packet.h | 1 + src/vnet/ip/ip_api.c | 37 ++++++++++++--------------------- src/vnet/ip/ip_flow_hash.h | 44 +++++++++++++++++++++++----------------- src/vnet/ip/lookup.c | 4 +++- test/test_ip4.py | 13 ++++++++++-- test/test_ip6.py | 6 +++--- test/vpp_papi_provider.py | 2 -- 16 files changed, 129 insertions(+), 107 deletions(-) diff --git a/src/plugins/gbp/test/test_gbp.py b/src/plugins/gbp/test/test_gbp.py index df3c3ad54f0..7e0d5c18799 100644 --- a/src/plugins/gbp/test/test_gbp.py +++ b/src/plugins/gbp/test/test_gbp.py @@ -5234,8 +5234,8 @@ class TestGBP(VppTestCase): self.logger.info(self.vapi.cli("sh ip6 fib 10:222::1")) rxs = self.send_and_expect(self.pg0, p, self.pg7) - self.assertEqual(rxs[0][VXLAN].vni, 445) - self.assertEqual(rxs[1][VXLAN].vni, 446) + self.assertEqual(rxs[0][VXLAN].vni, 446) + self.assertEqual(rxs[1][VXLAN].vni, 445) # # ping from host in remote to local external subnets @@ -5368,8 +5368,8 @@ class TestGBP(VppTestCase): rxs = self.send_and_expect(self.pg0, p, self.pg0, 2) - self.assertEqual(rxs[0][Dot1Q].vlan, 101) - self.assertEqual(rxs[1][Dot1Q].vlan, 100) + self.assertEqual(rxs[0][Dot1Q].vlan, 100) + self.assertEqual(rxs[1][Dot1Q].vlan, 101) # two ip4 packets whose port are chosen so they load-balance p = [(Ether(src=lep1.mac, dst=str(self.router_mac)) / diff --git a/src/vnet/ip/ip.api b/src/vnet/ip/ip.api index 0dec7266b42..f201ffbd8a6 100644 --- a/src/vnet/ip/ip.api +++ b/src/vnet/ip/ip.api @@ -231,6 +231,7 @@ define ip_route_lookup_reply */ autoreply define set_ip_flow_hash { + option deprecated; u32 client_index; u32 context; u32 vrf_id; @@ -244,6 +245,38 @@ autoreply define set_ip_flow_hash bool symmetric; }; +/** + @brief flow hash settings for an IP table + @param src - include src in flow hash + @param dst - include dst in flow hash + @param sport - include sport in flow hash + @param dport - include dport in flow hash + @param proto - include proto in flow hash + @param reverse - include reverse in flow hash + @param symmetric - include symmetry in flow hash + @param flowlabel - include flowlabel in flow hash +*/ +enumflag ip_flow_hash_config +{ + IP_API_FLOW_HASH_SRC_IP = 0x01, + IP_API_FLOW_HASH_DST_IP = 0x02, + IP_API_FLOW_HASH_SRC_PORT = 0x04, + IP_API_FLOW_HASH_DST_PORT = 0x08, + IP_API_FLOW_HASH_PROTO = 0x10, + IP_API_FLOW_HASH_REVERSE = 0x20, + IP_API_FLOW_HASH_SYMETRIC = 0x40, + IP_API_FLOW_HASH_FLOW_LABEL = 0x80, +}; + +autoreply define set_ip_flow_hash_v2 +{ + u32 client_index; + u32 context; + u32 table_id; + vl_api_address_family_t af; + vl_api_ip_flow_hash_config_t flow_hash_config; +}; + /** \brief IPv6 interface enable / disable request @param client_index - opaque cookie to identify the sender @param context - sender context, to match reply w/ request diff --git a/src/vnet/ip/ip.c b/src/vnet/ip/ip.c index f76d51989e6..f2475335463 100644 --- a/src/vnet/ip/ip.c +++ b/src/vnet/ip/ip.c @@ -186,7 +186,22 @@ ip_feature_enable_disable (ip_address_family_t af, n_feature_config_bytes); } +int +ip_flow_hash_set (ip_address_family_t af, u32 table_id, u32 flow_hash_config) +{ + fib_protocol_t fproto; + u32 fib_index; + + fproto = ip_address_family_to_fib_proto (af); + fib_index = fib_table_find (fproto, table_id); + + if (~0 == fib_index) + return VNET_API_ERROR_NO_SUCH_FIB; + fib_table_set_flow_hash_config (fib_index, fproto, flow_hash_config); + + return 0; +} u8 * format_ip_address_family (u8 * s, va_list * args) diff --git a/src/vnet/ip/ip.h b/src/vnet/ip/ip.h index 6d822d29dbe..1789fa1a659 100644 --- a/src/vnet/ip/ip.h +++ b/src/vnet/ip/ip.h @@ -278,6 +278,8 @@ u8 ip_is_local (u32 fib_index, ip46_address_t * ip46_address, u8 is_ip4); void ip_copy (ip46_address_t * dst, ip46_address_t * src, u8 is_ip4); void ip_set (ip46_address_t * dst, void *src, u8 is_ip4); +int ip_flow_hash_set (ip_address_family_t af, u32 table_id, + flow_hash_config_t flow_hash_config); void ip_feature_enable_disable (ip_address_family_t af, ip_sub_address_family_t safi, ip_feature_location_t loc, diff --git a/src/vnet/ip/ip4.h b/src/vnet/ip/ip4.h index 3be2f7f1a7e..32552eafbff 100644 --- a/src/vnet/ip/ip4.h +++ b/src/vnet/ip/ip4.h @@ -257,9 +257,6 @@ void ip4_unregister_protocol (u32 protocolx); serialize_function_t serialize_vnet_ip4_main, unserialize_vnet_ip4_main; -int vnet_set_ip4_flow_hash (u32 table_id, - flow_hash_config_t flow_hash_config); - int vnet_set_ip4_classify_intfc (vlib_main_t * vm, u32 sw_if_index, u32 table_index); diff --git a/src/vnet/ip/ip4_forward.c b/src/vnet/ip/ip4_forward.c index bb70805b4e6..332c483aa9d 100644 --- a/src/vnet/ip/ip4_forward.c +++ b/src/vnet/ip/ip4_forward.c @@ -2771,24 +2771,6 @@ VLIB_CLI_COMMAND (lookup_test_command, static) = }; /* *INDENT-ON* */ -#ifndef CLIB_MARCH_VARIANT -int -vnet_set_ip4_flow_hash (u32 table_id, u32 flow_hash_config) -{ - u32 fib_index; - - fib_index = fib_table_find (FIB_PROTOCOL_IP4, table_id); - - if (~0 == fib_index) - return VNET_API_ERROR_NO_SUCH_FIB; - - fib_table_set_flow_hash_config (fib_index, FIB_PROTOCOL_IP4, - flow_hash_config); - - return 0; -} -#endif - static clib_error_t * set_ip_flow_hash_command_fn (vlib_main_t * vm, unformat_input_t * input, @@ -2803,8 +2785,12 @@ set_ip_flow_hash_command_fn (vlib_main_t * vm, { if (unformat (input, "table %d", &table_id)) matched = 1; -#define _(a,v) \ - else if (unformat (input, #a)) { flow_hash_config |= v; matched=1;} +#define _(a, b, v) \ + else if (unformat (input, #a)) \ + { \ + flow_hash_config |= v; \ + matched = 1; \ + } foreach_flow_hash_bit #undef _ else @@ -2815,7 +2801,7 @@ set_ip_flow_hash_command_fn (vlib_main_t * vm, return clib_error_return (0, "unknown input `%U'", format_unformat_error, input); - rv = vnet_set_ip4_flow_hash (table_id, flow_hash_config); + rv = ip_flow_hash_set (AF_IP4, table_id, flow_hash_config); switch (rv) { case 0: diff --git a/src/vnet/ip/ip6.h b/src/vnet/ip/ip6.h index a0fa3b49280..4d94d8c1230 100644 --- a/src/vnet/ip/ip6.h +++ b/src/vnet/ip/ip6.h @@ -282,9 +282,6 @@ void ip6_local_hop_by_hop_register_protocol (u32 protocol, u32 node_index); serialize_function_t serialize_vnet_ip6_main, unserialize_vnet_ip6_main; -int vnet_set_ip6_flow_hash (u32 table_id, - flow_hash_config_t flow_hash_config); - u8 *format_ip6_forward_next_trace (u8 * s, va_list * args); u32 ip6_tcp_udp_icmp_validate_checksum (vlib_main_t * vm, vlib_buffer_t * p0); diff --git a/src/vnet/ip/ip6_forward.c b/src/vnet/ip/ip6_forward.c index aba4067174a..0ed20eea4ee 100644 --- a/src/vnet/ip/ip6_forward.c +++ b/src/vnet/ip/ip6_forward.c @@ -2832,24 +2832,6 @@ ip6_lookup_init (vlib_main_t * vm) VLIB_INIT_FUNCTION (ip6_lookup_init); -#ifndef CLIB_MARCH_VARIANT -int -vnet_set_ip6_flow_hash (u32 table_id, u32 flow_hash_config) -{ - u32 fib_index; - - fib_index = fib_table_find (FIB_PROTOCOL_IP6, table_id); - - if (~0 == fib_index) - return VNET_API_ERROR_NO_SUCH_FIB; - - fib_table_set_flow_hash_config (fib_index, FIB_PROTOCOL_IP6, - flow_hash_config); - - return 0; -} -#endif - static clib_error_t * set_ip6_flow_hash_command_fn (vlib_main_t * vm, unformat_input_t * input, @@ -2864,8 +2846,12 @@ set_ip6_flow_hash_command_fn (vlib_main_t * vm, { if (unformat (input, "table %d", &table_id)) matched = 1; -#define _(a,v) \ - else if (unformat (input, #a)) { flow_hash_config |= v; matched=1;} +#define _(a, b, v) \ + else if (unformat (input, #a)) \ + { \ + flow_hash_config |= v; \ + matched = 1; \ + } foreach_flow_hash_bit #undef _ else @@ -2876,7 +2862,7 @@ set_ip6_flow_hash_command_fn (vlib_main_t * vm, return clib_error_return (0, "unknown input `%U'", format_unformat_error, input); - rv = vnet_set_ip6_flow_hash (table_id, flow_hash_config); + rv = ip_flow_hash_set (AF_IP6, table_id, flow_hash_config); switch (rv) { case 0: @@ -2969,11 +2955,10 @@ set_ip6_flow_hash_command_fn (vlib_main_t * vm, * @endparblock ?*/ /* *INDENT-OFF* */ -VLIB_CLI_COMMAND (set_ip6_flow_hash_command, static) = -{ +VLIB_CLI_COMMAND (set_ip6_flow_hash_command, static) = { .path = "set ip6 flow-hash", - .short_help = - "set ip6 flow-hash table [src] [dst] [sport] [dport] [proto] [reverse]", + .short_help = "set ip6 flow-hash table [src] [dst] [sport] " + "[dport] [proto] [reverse] [flowlabel]", .function = set_ip6_flow_hash_command_fn, }; /* *INDENT-ON* */ diff --git a/src/vnet/ip/ip6_inlines.h b/src/vnet/ip/ip6_inlines.h index ae7b7a1761b..8376377600a 100644 --- a/src/vnet/ip/ip6_inlines.h +++ b/src/vnet/ip/ip6_inlines.h @@ -108,6 +108,8 @@ ip6_compute_flow_hash (const ip6_header_t * ip, b ^= (flow_hash_config & IP_FLOW_HASH_PROTO) ? protocol : 0; c = (flow_hash_config & IP_FLOW_HASH_REVERSE_SRC_DST) ? ((t1 << 16) | t2) : ((t2 << 16) | t1); + t1 = ip->ip_version_traffic_class_and_flow_label & IP6_PACKET_FL_MASK; + c ^= (flow_hash_config & IP_FLOW_HASH_FL) ? (t1 << 32) : 0; hash_mix64 (a, b, c); return (u32) c; diff --git a/src/vnet/ip/ip6_packet.h b/src/vnet/ip/ip6_packet.h index 34bc7a8ec1a..03aac1bd4d4 100644 --- a/src/vnet/ip/ip6_packet.h +++ b/src/vnet/ip/ip6_packet.h @@ -313,6 +313,7 @@ typedef struct #define IP6_PACKET_TC_MASK 0x0FF00000 #define IP6_PACKET_DSCP_MASK 0x0FC00000 #define IP6_PACKET_ECN_MASK 0x00300000 +#define IP6_PACKET_FL_MASK 0x000FFFFF always_inline ip_dscp_t ip6_traffic_class (const ip6_header_t * i) diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c index 228d252534c..37656f3232f 100644 --- a/src/vnet/ip/ip_api.c +++ b/src/vnet/ip/ip_api.c @@ -87,6 +87,7 @@ _ (IP_PUNT_POLICE, ip_punt_police) \ _ (IP_PUNT_REDIRECT, ip_punt_redirect) \ _ (SET_IP_FLOW_HASH, set_ip_flow_hash) \ + _ (SET_IP_FLOW_HASH_V2, set_ip_flow_hash_v2) \ _ (IP_CONTAINER_PROXY_ADD_DEL, ip_container_proxy_add_del) \ _ (IP_CONTAINER_PROXY_DUMP, ip_container_proxy_dump) \ _ (IOAM_ENABLE, ioam_enable) \ @@ -1010,7 +1011,7 @@ vl_api_ip_dump_t_handler (vl_api_ip_dump_t * mp) } static void -set_ip6_flow_hash (vl_api_set_ip_flow_hash_t * mp) +vl_api_set_ip_flow_hash_t_handler (vl_api_set_ip_flow_hash_t *mp) { vl_api_set_ip_flow_hash_reply_t *rmp; int rv; @@ -1020,41 +1021,29 @@ set_ip6_flow_hash (vl_api_set_ip_flow_hash_t * mp) table_id = ntohl (mp->vrf_id); #define _(a,b) if (mp->a) flow_hash_config |= b; - foreach_flow_hash_bit; + foreach_flow_hash_bit_v1; #undef _ - rv = vnet_set_ip6_flow_hash (table_id, flow_hash_config); + rv = ip_flow_hash_set ((mp->is_ipv6 ? AF_IP6 : AF_IP4), table_id, + flow_hash_config); REPLY_MACRO (VL_API_SET_IP_FLOW_HASH_REPLY); } static void -set_ip4_flow_hash (vl_api_set_ip_flow_hash_t * mp) +vl_api_set_ip_flow_hash_v2_t_handler (vl_api_set_ip_flow_hash_v2_t *mp) { - vl_api_set_ip_flow_hash_reply_t *rmp; + vl_api_set_ip_flow_hash_v2_reply_t *rmp; + ip_address_family_t af; int rv; - u32 table_id; - flow_hash_config_t flow_hash_config = 0; - - table_id = ntohl (mp->vrf_id); - -#define _(a,b) if (mp->a) flow_hash_config |= b; - foreach_flow_hash_bit; -#undef _ - - rv = vnet_set_ip4_flow_hash (table_id, flow_hash_config); - REPLY_MACRO (VL_API_SET_IP_FLOW_HASH_REPLY); -} + rv = ip_address_family_decode (mp->af, &af); + if (!rv) + rv = ip_flow_hash_set (af, htonl (mp->table_id), + htonl (mp->flow_hash_config)); -static void -vl_api_set_ip_flow_hash_t_handler (vl_api_set_ip_flow_hash_t * mp) -{ - if (mp->is_ipv6 == 0) - set_ip4_flow_hash (mp); - else - set_ip6_flow_hash (mp); + REPLY_MACRO (VL_API_SET_IP_FLOW_HASH_V2_REPLY); } void diff --git a/src/vnet/ip/ip_flow_hash.h b/src/vnet/ip/ip_flow_hash.h index 04777320f38..82e0efb0d08 100644 --- a/src/vnet/ip/ip_flow_hash.h +++ b/src/vnet/ip/ip_flow_hash.h @@ -16,31 +16,37 @@ #ifndef __IP_FLOW_HASH_H__ #define __IP_FLOW_HASH_H__ -/** Flow hash configuration */ -#define IP_FLOW_HASH_SRC_ADDR (1<<0) -#define IP_FLOW_HASH_DST_ADDR (1<<1) -#define IP_FLOW_HASH_PROTO (1<<2) -#define IP_FLOW_HASH_SRC_PORT (1<<3) -#define IP_FLOW_HASH_DST_PORT (1<<4) -#define IP_FLOW_HASH_REVERSE_SRC_DST (1<<5) -#define IP_FLOW_HASH_SYMMETRIC (1<<6) +/** Default: 5-tuple + flowlabel without the "reverse" bit */ +#define IP_FLOW_HASH_DEFAULT (0x9F) -/** Default: 5-tuple without the "reverse" bit */ -#define IP_FLOW_HASH_DEFAULT (0x1F) +#define foreach_flow_hash_bit_v1 \ + _ (src, IP_FLOW_HASH_SRC_ADDR) \ + _ (dst, IP_FLOW_HASH_DST_ADDR) \ + _ (sport, IP_FLOW_HASH_SRC_PORT) \ + _ (dport, IP_FLOW_HASH_DST_PORT) \ + _ (proto, IP_FLOW_HASH_PROTO) \ + _ (reverse, IP_FLOW_HASH_REVERSE_SRC_DST) \ + _ (symmetric, IP_FLOW_HASH_SYMMETRIC) -#define foreach_flow_hash_bit \ -_(src, IP_FLOW_HASH_SRC_ADDR) \ -_(dst, IP_FLOW_HASH_DST_ADDR) \ -_(sport, IP_FLOW_HASH_SRC_PORT) \ -_(dport, IP_FLOW_HASH_DST_PORT) \ -_(proto, IP_FLOW_HASH_PROTO) \ -_(reverse, IP_FLOW_HASH_REVERSE_SRC_DST) \ -_(symmetric, IP_FLOW_HASH_SYMMETRIC) +#define foreach_flow_hash_bit \ + _ (src, 0, IP_FLOW_HASH_SRC_ADDR) \ + _ (dst, 1, IP_FLOW_HASH_DST_ADDR) \ + _ (sport, 2, IP_FLOW_HASH_SRC_PORT) \ + _ (dport, 3, IP_FLOW_HASH_DST_PORT) \ + _ (proto, 4, IP_FLOW_HASH_PROTO) \ + _ (reverse, 5, IP_FLOW_HASH_REVERSE_SRC_DST) \ + _ (symmetric, 6, IP_FLOW_HASH_SYMMETRIC) \ + _ (flowlabel, 7, IP_FLOW_HASH_FL) /** * A flow hash configuration is a mask of the flow hash options */ -typedef u32 flow_hash_config_t; +typedef enum flow_hash_config_t_ +{ +#define _(a, b, c) c = (1 << b), + foreach_flow_hash_bit +#undef _ +} flow_hash_config_t; #endif /* __IP_TYPES_H__ */ diff --git a/src/vnet/ip/lookup.c b/src/vnet/ip/lookup.c index b356b278e01..f674fec4823 100644 --- a/src/vnet/ip/lookup.c +++ b/src/vnet/ip/lookup.c @@ -119,7 +119,9 @@ format_ip_flow_hash_config (u8 * s, va_list * args) { flow_hash_config_t flow_hash_config = va_arg (*args, u32); -#define _(n,v) if (flow_hash_config & v) s = format (s, "%s ", #n); +#define _(n, b, v) \ + if (flow_hash_config & v) \ + s = format (s, "%s ", #n); foreach_flow_hash_bit; #undef _ diff --git a/test/test_ip4.py b/test/test_ip4.py index b93241b652d..a428db339f4 100644 --- a/test/test_ip4.py +++ b/test/test_ip4.py @@ -1138,6 +1138,9 @@ class TestIPLoadBalance(VppTestCase): def test_ip_load_balance(self): """ IP Load-Balancing """ + fhc = VppEnum.vl_api_ip_flow_hash_config_t + af = VppEnum.vl_api_address_family_t + # # An array of packets that differ only in the destination port # @@ -1206,7 +1209,12 @@ class TestIPLoadBalance(VppTestCase): # - now only the stream with differing source address will # load-balance # - self.vapi.set_ip_flow_hash(vrf_id=0, src=1, dst=1, sport=0, dport=0) + self.vapi.set_ip_flow_hash_v2( + af=af.ADDRESS_IP4, + table_id=0, + flow_hash_config=(fhc.IP_API_FLOW_HASH_SRC_IP | + fhc.IP_API_FLOW_HASH_DST_IP | + fhc.IP_API_FLOW_HASH_PROTO)) self.send_and_expect_load_balancing(self.pg0, src_ip_pkts, [self.pg1, self.pg2]) @@ -1218,7 +1226,8 @@ class TestIPLoadBalance(VppTestCase): # # change the flow hash config back to defaults # - self.vapi.set_ip_flow_hash(vrf_id=0, src=1, dst=1, sport=1, dport=1) + self.vapi.set_ip_flow_hash(vrf_id=0, src=1, dst=1, + proto=1, sport=1, dport=1) # # Recursive prefixes diff --git a/test/test_ip6.py b/test/test_ip6.py index 99c63873afb..5dd2bbcff64 100644 --- a/test/test_ip6.py +++ b/test/test_ip6.py @@ -2069,8 +2069,8 @@ class TestIP6LoadBalance(VppTestCase): # - now only the stream with differing source address will # load-balance # - self.vapi.set_ip_flow_hash(vrf_id=0, src=1, dst=1, sport=0, dport=0, - is_ipv6=1) + self.vapi.set_ip_flow_hash(vrf_id=0, src=1, dst=1, proto=1, + sport=0, dport=0, is_ipv6=1) self.send_and_expect_load_balancing(self.pg0, src_ip_pkts, [self.pg1, self.pg2]) @@ -2082,7 +2082,7 @@ class TestIP6LoadBalance(VppTestCase): # change the flow hash config back to defaults # self.vapi.set_ip_flow_hash(vrf_id=0, src=1, dst=1, sport=1, dport=1, - is_ipv6=1) + proto=1, is_ipv6=1) # # Recursive prefixes diff --git a/test/vpp_papi_provider.py b/test/vpp_papi_provider.py index c5e8ff0e068..81e25ecc621 100644 --- a/test/vpp_papi_provider.py +++ b/test/vpp_papi_provider.py @@ -82,8 +82,6 @@ defaultmapping = { 'l2_table_index': 4294967295, }, 'pppoe_add_del_session': {'is_add': 1, }, 'policer_add_del': {'is_add': 1, 'conform_action': {'type': 1}, }, - 'set_ip_flow_hash': {'src': 1, 'dst': 1, 'sport': 1, 'dport': 1, - 'proto': 1, }, 'set_ipfix_exporter': {'collector_port': 4739, }, 'sr_policy_add': {'weight': 1, 'is_encap': 1, }, 'sw_interface_add_del_address': {'is_add': 1, }, -- 2.16.6