ip: Router ID included in flow hash

Type: feature

A device/router needs to have a unique ID which is included in the flow
has so that flows are not polarised through the network, i.e. each deice
in the network chooses the same nth link for the same flow.

Signed-off-by: Neale Ranns <neale@graphiant.com>
Change-Id: I963e03674adbb085902b4084fdc4886b88f5734c

12 files changed, 122 insertions, 29 deletions

diff --git a/src/plugins/gbp/test/test_gbp.py b/src/plugins/gbp/test/test_gbp.py
index 7e0d5c18799..df3c3ad54f0 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, 446)
-        self.assertEqual(rxs[1][VXLAN].vni, 445)
+        self.assertEqual(rxs[0][VXLAN].vni, 445)
+        self.assertEqual(rxs[1][VXLAN].vni, 446)
 
         #
         # 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, 100)
-        self.assertEqual(rxs[1][Dot1Q].vlan, 101)
+        self.assertEqual(rxs[0][Dot1Q].vlan, 101)
+        self.assertEqual(rxs[1][Dot1Q].vlan, 100)
 
         # 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 f201ffbd8a6..3072e3e7c63 100644
--- a/src/vnet/ip/ip.api
+++ b/src/vnet/ip/ip.api
@@ -20,7 +20,7 @@
     called through a shared memory interface.
 */
 
-option version = "3.0.2";
+option version = "3.0.3";
 
 import "vnet/interface_types.api";
 import "vnet/fib/fib_types.api";
@@ -277,6 +277,20 @@ autoreply define set_ip_flow_hash_v2
   vl_api_ip_flow_hash_config_t flow_hash_config;
 };
 
+/** \brief Set the ip flow hash router ID
+    @param client_index - opaque cookie to identify the sender
+    @param context - sender context, to match reply w/ request
+    @param router_id - The ID of the router. Mixed into the hash.
+                       Used to prevent polarisation across a network,
+                       since each router is assumed to have a different ID
+*/
+autoreply define set_ip_flow_hash_router_id
+{
+  u32 client_index;
+  u32 context;
+  u32 router_id;
+};
+
 /** \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 f2475335463..5d0c7707dd3 100644
--- a/src/vnet/ip/ip.c
+++ b/src/vnet/ip/ip.c
@@ -16,6 +16,8 @@
 #include <vnet/ip/ip.h>
 #include <vnet/fib/fib_table.h>
 
+u32 ip_flow_hash_router_id;
+
 u8
 ip_is_zero (ip46_address_t * ip46_address, u8 is_ip4)
 {
@@ -203,6 +205,12 @@ ip_flow_hash_set (ip_address_family_t af, u32 table_id, u32 flow_hash_config)
   return 0;
 }
 
+void
+ip_flow_hash_router_id_set (u32 router_id)
+{
+  ip_flow_hash_router_id = router_id;
+}
+
 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 1789fa1a659..6d822d29dbe 100644
--- a/src/vnet/ip/ip.h
+++ b/src/vnet/ip/ip.h
@@ -278,8 +278,6 @@ 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_inlines.h b/src/vnet/ip/ip4_inlines.h
index bdb82af0034..3075fbf42a1 100644
--- a/src/vnet/ip/ip4_inlines.h
+++ b/src/vnet/ip/ip4_inlines.h
@@ -89,6 +89,7 @@ ip4_compute_flow_hash (const ip4_header_t * ip,
   b ^= (flow_hash_config & IP_FLOW_HASH_PROTO) ? ip->protocol : 0;
   c = (flow_hash_config & IP_FLOW_HASH_REVERSE_SRC_DST) ?
     (t1 << 16) | t2 : (t2 << 16) | t1;
+  a ^= ip_flow_hash_router_id;
 
   hash_v3_mix32 (a, b, c);
   hash_v3_finalize32 (a, b, c);
diff --git a/src/vnet/ip/ip6_inlines.h b/src/vnet/ip/ip6_inlines.h
index 8376377600a..2a4bb70573b 100644
--- a/src/vnet/ip/ip6_inlines.h
+++ b/src/vnet/ip/ip6_inlines.h
@@ -106,10 +106,13 @@ 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;
+  c = ((flow_hash_config & IP_FLOW_HASH_REVERSE_SRC_DST) ? ((t1 << 16) | t2) :
+							   ((t2 << 16) | t1));
+  t1 = ((u64) ip_flow_hash_router_id << 32);
+  t1 |=
+    ((flow_hash_config & IP_FLOW_HASH_FL) ? ip6_flow_label_network_order (ip) :
+					    0);
+  c ^= t1;
 
   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 03aac1bd4d4..1be2ceae5a1 100644
--- a/src/vnet/ip/ip6_packet.h
+++ b/src/vnet/ip/ip6_packet.h
@@ -343,6 +343,13 @@ ip6_ecn_network_order (const ip6_header_t * ip6)
 	  & IP6_PACKET_ECN_MASK) >> 20;
 }
 
+static_always_inline u32
+ip6_flow_label_network_order (const ip6_header_t *ip6)
+{
+  return (clib_net_to_host_u32 (ip6->ip_version_traffic_class_and_flow_label) &
+	  IP6_PACKET_FL_MASK);
+}
+
 static_always_inline void
 ip6_set_traffic_class_network_order (ip6_header_t * ip6, ip_dscp_t dscp)
 {
diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c
index 37656f3232f..3bf404baadf 100644
--- a/src/vnet/ip/ip_api.c
+++ b/src/vnet/ip/ip_api.c
@@ -88,6 +88,7 @@
   _ (IP_PUNT_REDIRECT, ip_punt_redirect)                                      \
   _ (SET_IP_FLOW_HASH, set_ip_flow_hash)                                      \
   _ (SET_IP_FLOW_HASH_V2, set_ip_flow_hash_v2)                                \
+  _ (SET_IP_FLOW_HASH_ROUTER_ID, set_ip_flow_hash_router_id)                  \
   _ (IP_CONTAINER_PROXY_ADD_DEL, ip_container_proxy_add_del)                  \
   _ (IP_CONTAINER_PROXY_DUMP, ip_container_proxy_dump)                        \
   _ (IOAM_ENABLE, ioam_enable)                                                \
@@ -1046,6 +1047,18 @@ vl_api_set_ip_flow_hash_v2_t_handler (vl_api_set_ip_flow_hash_v2_t *mp)
   REPLY_MACRO (VL_API_SET_IP_FLOW_HASH_V2_REPLY);
 }
 
+static void
+vl_api_set_ip_flow_hash_router_id_t_handler (
+  vl_api_set_ip_flow_hash_router_id_t *mp)
+{
+  vl_api_set_ip_flow_hash_router_id_reply_t *rmp;
+  int rv = 0;
+
+  ip_flow_hash_router_id_set (ntohl (mp->router_id));
+
+  REPLY_MACRO (VL_API_SET_IP_FLOW_HASH_ROUTER_ID_REPLY);
+}
+
 void
 vl_mfib_signal_send_one (vl_api_registration_t * reg,
 			 u32 context, const mfib_signal_t * mfs)
diff --git a/src/vnet/ip/ip_flow_hash.h b/src/vnet/ip/ip_flow_hash.h
index 82e0efb0d08..bd37ef7307b 100644
--- a/src/vnet/ip/ip_flow_hash.h
+++ b/src/vnet/ip/ip_flow_hash.h
@@ -16,6 +16,8 @@
 #ifndef __IP_FLOW_HASH_H__
 #define __IP_FLOW_HASH_H__
 
+#include <vnet/ip/ip_types.h>
+
 /** Default: 5-tuple + flowlabel without the "reverse" bit */
 #define IP_FLOW_HASH_DEFAULT (0x9F)
 
@@ -48,6 +50,13 @@ typedef enum flow_hash_config_t_
 #undef _
 } flow_hash_config_t;
 
+/* Router ID mixed into the flow hash to prevent network polarisation */
+extern u32 ip_flow_hash_router_id;
+
+int ip_flow_hash_set (ip_address_family_t af, u32 table_id,
+		      flow_hash_config_t flow_hash_config);
+void ip_flow_hash_router_id_set (u32 router_id);
+
 #endif /* __IP_TYPES_H__ */
 
 /*
diff --git a/src/vnet/mpls/mpls_lookup.h b/src/vnet/mpls/mpls_lookup.h
index 5b88be17d8f..81c67cef8ee 100644
--- a/src/vnet/mpls/mpls_lookup.h
+++ b/src/vnet/mpls/mpls_lookup.h
@@ -53,6 +53,7 @@ mpls_compute_flow_hash (const mpls_unicast_header_t * hdr,
 
     ho_label = clib_net_to_host_u32(hdr->label_exp_s_ttl);
     hash = vnet_mpls_uc_get_label(ho_label);
+    hash ^= ip_flow_hash_router_id;
     next_label_is_entropy = 0;
 
     while (MPLS_EOS != vnet_mpls_uc_get_s(ho_label))
diff --git a/test/test_ip4.py b/test/test_ip4.py
index a428db339f4..589c668a456 100644
--- a/test/test_ip4.py
+++ b/test/test_ip4.py
@@ -1118,6 +1118,7 @@ class TestIPLoadBalance(VppTestCase):
         super(TestIPLoadBalance, self).tearDown()
 
     def send_and_expect_load_balancing(self, input, pkts, outputs):
+        self.vapi.cli("clear trace")
         input.add_stream(pkts)
         self.pg_enable_capture(self.pg_interfaces)
         self.pg_start()
@@ -1125,8 +1126,7 @@ class TestIPLoadBalance(VppTestCase):
         for oo in outputs:
             rx = oo._get_capture(1)
             self.assertNotEqual(0, len(rx))
-            for r in rx:
-                rxs.append(r)
+            rxs.append(rx)
         return rxs
 
     def send_and_expect_one_itf(self, input, pkts, itf):
@@ -1135,6 +1135,12 @@ class TestIPLoadBalance(VppTestCase):
         self.pg_start()
         rx = itf.get_capture(len(pkts))
 
+    def total_len(self, rxs):
+        n = 0
+        for rx in rxs:
+            n += len(rx)
+        return n
+
     def test_ip_load_balance(self):
         """ IP Load-Balancing """
 
@@ -1195,14 +1201,29 @@ class TestIPLoadBalance(VppTestCase):
         # be guaranteed. But with 64 different packets we do expect some
         # balancing. So instead just ensure there is traffic on each link.
         #
-        self.send_and_expect_load_balancing(self.pg0, port_ip_pkts,
-                                            [self.pg1, self.pg2])
+        rx = self.send_and_expect_load_balancing(self.pg0, port_ip_pkts,
+                                                 [self.pg1, self.pg2])
+        n_ip_pg0 = len(rx[0])
         self.send_and_expect_load_balancing(self.pg0, src_ip_pkts,
                                             [self.pg1, self.pg2])
         self.send_and_expect_load_balancing(self.pg0, port_mpls_pkts,
                                             [self.pg1, self.pg2])
-        self.send_and_expect_load_balancing(self.pg0, src_mpls_pkts,
-                                            [self.pg1, self.pg2])
+        rx = self.send_and_expect_load_balancing(self.pg0, src_mpls_pkts,
+                                                 [self.pg1, self.pg2])
+        n_mpls_pg0 = len(rx[0])
+
+        #
+        # change the router ID and expect the distribution changes
+        #
+        self.vapi.set_ip_flow_hash_router_id(router_id=0x11111111)
+
+        rx = self.send_and_expect_load_balancing(self.pg0, port_ip_pkts,
+                                                 [self.pg1, self.pg2])
+        self.assertNotEqual(n_ip_pg0, len(rx[0]))
+
+        rx = self.send_and_expect_load_balancing(self.pg0, src_mpls_pkts,
+                                                 [self.pg1, self.pg2])
+        self.assertNotEqual(n_mpls_pg0, len(rx[0]))
 
         #
         # change the flow hash config so it's only IP src,dst
@@ -1273,23 +1294,23 @@ class TestIPLoadBalance(VppTestCase):
                                              self.pg3, self.pg4])
 
         #
-        # bring down pg1 expect LB to adjust to use only those that are pu
+        # bring down pg1 expect LB to adjust to use only those that are up
         #
         self.pg1.link_down()
 
         rx = self.send_and_expect_load_balancing(self.pg0, src_pkts,
                                                  [self.pg2, self.pg3,
                                                   self.pg4])
-        self.assertEqual(len(src_pkts), len(rx))
+        self.assertEqual(len(src_pkts), self.total_len(rx))
 
         #
-        # bring down pg2 expect LB to adjust to use only those that are pu
+        # bring down pg2 expect LB to adjust to use only those that are up
         #
         self.pg2.link_down()
 
         rx = self.send_and_expect_load_balancing(self.pg0, src_pkts,
                                                  [self.pg3, self.pg4])
-        self.assertEqual(len(src_pkts), len(rx))
+        self.assertEqual(len(src_pkts), self.total_len(rx))
 
         #
         # bring the links back up - expect LB over all again
@@ -1300,7 +1321,7 @@ class TestIPLoadBalance(VppTestCase):
         rx = self.send_and_expect_load_balancing(self.pg0, src_pkts,
                                                  [self.pg1, self.pg2,
                                                   self.pg3, self.pg4])
-        self.assertEqual(len(src_pkts), len(rx))
+        self.assertEqual(len(src_pkts), self.total_len(rx))
 
         #
         # The same link-up/down but this time admin state
@@ -1309,7 +1330,7 @@ class TestIPLoadBalance(VppTestCase):
         self.pg2.admin_down()
         rx = self.send_and_expect_load_balancing(self.pg0, src_pkts,
                                                  [self.pg3, self.pg4])
-        self.assertEqual(len(src_pkts), len(rx))
+        self.assertEqual(len(src_pkts), self.total_len(rx))
         self.pg1.admin_up()
         self.pg2.admin_up()
         self.pg1.resolve_arp()
@@ -1317,7 +1338,7 @@ class TestIPLoadBalance(VppTestCase):
         rx = self.send_and_expect_load_balancing(self.pg0, src_pkts,
                                                  [self.pg1, self.pg2,
                                                   self.pg3, self.pg4])
-        self.assertEqual(len(src_pkts), len(rx))
+        self.assertEqual(len(src_pkts), self.total_len(rx))
 
         #
         # Recursive prefixes
@@ -1375,7 +1396,7 @@ class TestIPLoadBalance(VppTestCase):
 
         rx = self.send_and_expect_load_balancing(self.pg0, port_pkts,
                                                  [self.pg3, self.pg4])
-        self.assertEqual(len(src_pkts), len(rx))
+        self.assertEqual(len(src_pkts), self.total_len(rx))
 
 
 class TestIPVlan0(VppTestCase):
diff --git a/test/test_ip6.py b/test/test_ip6.py
index 0ef2dd1d2bd..1b3b9d0a1e6 100644
--- a/test/test_ip6.py
+++ b/test/test_ip6.py
@@ -1943,9 +1943,12 @@ class TestIP6LoadBalance(VppTestCase):
 
     def send_and_expect_load_balancing(self, input, pkts, outputs):
         self.pg_send(input, pkts)
+        rxs = []
         for oo in outputs:
             rx = oo._get_capture(1)
             self.assertNotEqual(0, len(rx))
+            rxs.append(rx)
+        return rxs
 
     def send_and_expect_one_itf(self, input, pkts, itf):
         self.pg_send(input, pkts)
@@ -2044,16 +2047,31 @@ class TestIP6LoadBalance(VppTestCase):
         # be guaranteed. But with 64 different packets we do expect some
         # balancing. So instead just ensure there is traffic on each link.
         #
-        self.send_and_expect_load_balancing(self.pg0, port_ip_pkts,
-                                            [self.pg1, self.pg2])
+        rx = self.send_and_expect_load_balancing(self.pg0, port_ip_pkts,
+                                                 [self.pg1, self.pg2])
+        n_ip_pg0 = len(rx[0])
         self.send_and_expect_load_balancing(self.pg0, src_ip_pkts,
                                             [self.pg1, self.pg2])
         self.send_and_expect_load_balancing(self.pg0, port_mpls_pkts,
                                             [self.pg1, self.pg2])
         self.send_and_expect_load_balancing(self.pg0, src_mpls_pkts,
                                             [self.pg1, self.pg2])
-        self.send_and_expect_load_balancing(self.pg0, port_mpls_neos_pkts,
-                                            [self.pg1, self.pg2])
+        rx = self.send_and_expect_load_balancing(self.pg0, port_mpls_neos_pkts,
+                                                 [self.pg1, self.pg2])
+        n_mpls_pg0 = len(rx[0])
+
+        #
+        # change the router ID and expect the distribution changes
+        #
+        self.vapi.set_ip_flow_hash_router_id(router_id=0x11111111)
+
+        rx = self.send_and_expect_load_balancing(self.pg0, port_ip_pkts,
+                                                 [self.pg1, self.pg2])
+        self.assertNotEqual(n_ip_pg0, len(rx[0]))
+
+        rx = self.send_and_expect_load_balancing(self.pg0, src_mpls_pkts,
+                                                 [self.pg1, self.pg2])
+        self.assertNotEqual(n_mpls_pg0, len(rx[0]))
 
         #
         # The packets with Entropy label in should not load-balance,