ARP/ND use path_remove to complement path_add 86/6886/2
authorNeale Ranns <nranns@cisco.com>
Fri, 26 May 2017 09:59:16 +0000 (02:59 -0700)
committerDamjan Marion <dmarion.lists@gmail.com>
Fri, 26 May 2017 18:15:14 +0000 (18:15 +0000)
don't add duplicate extensions.

Change-Id: Icf72d6e1b004d0dda532bec2b51f6b74544925bb
Signed-off-by: Neale Ranns <nranns@cisco.com>
src/vnet/ethernet/arp.c
src/vnet/fib/fib_path_ext.c
src/vnet/fib/fib_path_ext.h
src/vnet/ip/ip6_neighbor.c
test/test_ip6.py
test/test_neighbor.py

index ad0c2c8..624c48c 100644 (file)
@@ -1673,7 +1673,23 @@ arp_entry_free (ethernet_arp_interface_t * eai, ethernet_arp_ip4_entry_t * e)
   ethernet_arp_main_t *am = &ethernet_arp_main;
 
   if (FIB_NODE_INDEX_INVALID != e->fib_entry_index)
-    fib_table_entry_delete_index (e->fib_entry_index, FIB_SOURCE_ADJ);
+    {
+      fib_prefix_t pfx = {
+       .fp_len = 32,
+       .fp_proto = FIB_PROTOCOL_IP4,
+       .fp_addr.ip4 = e->ip4_address,
+      };
+      u32 fib_index;
+
+      fib_index = ip4_fib_table_get_index_for_sw_if_index (e->sw_if_index);
+
+      fib_table_entry_path_remove (fib_index, &pfx,
+                                  FIB_SOURCE_ADJ,
+                                  FIB_PROTOCOL_IP4,
+                                  &pfx.fp_addr,
+                                  e->sw_if_index, ~0, 1,
+                                  FIB_ROUTE_PATH_FLAG_NONE);
+    }
   hash_unset (eai->arp_entries, e->ip4_address.as_u32);
   pool_put (am->ip4_entry_pool, e);
 }
index 45ab1f1..26f2b9b 100644 (file)
@@ -105,7 +105,7 @@ fib_path_ext_resolve (fib_path_ext_t *path_ext,
                       path_ext);
 }
 
-void
+static void
 fib_path_ext_init (fib_path_ext_t *path_ext,
                   fib_node_index_t path_list_index,
                    fib_path_ext_type_t ext_type,
@@ -338,7 +338,18 @@ fib_path_ext_list_insert (fib_path_ext_list_t *list,
 
     vec_foreach(path_ext, list->fpel_exts)
     {
-        if (fib_path_ext_cmp(path_ext, rpath) < 0)
+        int res = fib_path_ext_cmp(path_ext, rpath);
+
+        if (0 == res)
+        {
+            /*
+             * don't add duplicate extensions. modify instead
+             */
+            vec_free(path_ext->fpe_label_stack);
+            *path_ext = new_path_ext;
+            goto done;
+        }
+        else if (res < 0)
         {
             i++;
         }
@@ -348,7 +359,7 @@ fib_path_ext_list_insert (fib_path_ext_list_t *list,
         }
     }
     vec_insert_elts(list->fpel_exts, &new_path_ext, 1, i);
-
+done:
     return (&(list->fpel_exts[i]));
 }
 
index d1571a1..d07941c 100644 (file)
@@ -71,11 +71,6 @@ typedef enum fib_path_ext_adj_flags_t_
  */
 typedef struct fib_path_ext_t_
 {
-    /**
-     * The type of path extension
-     */
-    fib_path_ext_type_t fpe_type;
-
     /**
      * A description of the path that is being extended.
      * This description is used to match this extension with the [changing]
@@ -93,6 +88,11 @@ typedef struct fib_path_ext_t_
         fib_path_ext_adj_flags_t fpe_adj_flags;
     };
 
+    /**
+     * The type of path extension
+     */
+    fib_path_ext_type_t fpe_type;
+
     /**
      * The index of the path. This is the global index, not the path's
      * position in the path-list.
index e140c55..073b67f 100644 (file)
@@ -274,10 +274,22 @@ ip6_neighbor_sw_interface_up_down (vnet_main_t * vnm,
          n = pool_elt_at_index (nm->neighbor_pool, to_delete[i]);
          mhash_unset (&nm->neighbor_index_by_key, &n->key, 0);
          if (FIB_NODE_INDEX_INVALID != n->fib_entry_index)
-           fib_table_entry_delete_index (n->fib_entry_index, FIB_SOURCE_ADJ);
-         pool_put (nm->neighbor_pool, n);
+           {
+             fib_prefix_t pfx = {
+               .fp_len = 128,
+               .fp_proto = FIB_PROTOCOL_IP6,
+               .fp_addr.ip6 = n->key.ip6_address,
+             };
+             fib_table_entry_path_remove
+               (ip6_fib_table_get_index_for_sw_if_index (n->key.sw_if_index),
+                &pfx,
+                FIB_SOURCE_ADJ,
+                FIB_PROTOCOL_IP6,
+                &pfx.fp_addr,
+                n->key.sw_if_index, ~0, 1, FIB_ROUTE_PATH_FLAG_NONE);
+             pool_put (nm->neighbor_pool, n);
+           }
        }
-
       vec_free (to_delete);
     }
 
@@ -628,10 +640,10 @@ vnet_set_ip6_ethernet_neighbor (vlib_main_t * vm,
          };
          u32 fib_index;
 
-         fib_index = ip6_main.fib_index_by_sw_if_index[n->key.sw_if_index];
+         fib_index =
+           ip6_fib_table_get_index_for_sw_if_index (n->key.sw_if_index);
          n->fib_entry_index =
-           fib_table_entry_path_add (fib_index, &pfx,
-                                     FIB_SOURCE_ADJ,
+           fib_table_entry_path_add (fib_index, &pfx, FIB_SOURCE_ADJ,
                                      FIB_ENTRY_FLAG_ATTACHED,
                                      FIB_PROTOCOL_IP6, &pfx.fp_addr,
                                      n->key.sw_if_index, ~0, 1, NULL,
@@ -754,7 +766,19 @@ vnet_unset_ip6_ethernet_neighbor (vlib_main_t * vm,
 
 
   if (FIB_NODE_INDEX_INVALID != n->fib_entry_index)
-    fib_table_entry_delete_index (n->fib_entry_index, FIB_SOURCE_ADJ);
+    {
+      fib_prefix_t pfx = {
+       .fp_len = 128,
+       .fp_proto = FIB_PROTOCOL_IP6,
+       .fp_addr.ip6 = n->key.ip6_address,
+      };
+      fib_table_entry_path_remove
+       (ip6_fib_table_get_index_for_sw_if_index (n->key.sw_if_index),
+        &pfx,
+        FIB_SOURCE_ADJ,
+        FIB_PROTOCOL_IP6,
+        &pfx.fp_addr, n->key.sw_if_index, ~0, 1, FIB_ROUTE_PATH_FLAG_NONE);
+    }
   pool_put (nm->neighbor_pool, n);
 
 out:
index 700b334..1432858 100644 (file)
@@ -76,6 +76,31 @@ class TestIPv6ND(VppTestCase):
         dll = rx[ICMPv6NDOptDstLLAddr]
         self.assertEqual(dll.lladdr, intf.local_mac)
 
+    def validate_ns(self, intf, rx, tgt_ip):
+        nsma = in6_getnsma(inet_pton(AF_INET6, tgt_ip))
+        dst_ip = inet_ntop(AF_INET6, nsma)
+
+        # NS is broadcast
+        self.assertEqual(rx[Ether].dst, "ff:ff:ff:ff:ff:ff")
+
+        # and from the router's MAC
+        self.assertEqual(rx[Ether].src, intf.local_mac)
+
+        # the rx'd NS should be addressed to an mcast address
+        # derived from the target address
+        self.assertEqual(in6_ptop(rx[IPv6].dst), in6_ptop(dst_ip))
+
+        # expect the tgt IP in the NS header
+        ns = rx[ICMPv6ND_NS]
+        self.assertEqual(in6_ptop(ns.tgt), in6_ptop(tgt_ip))
+
+        # packet is from the router's local address
+        self.assertEqual(in6_ptop(rx[IPv6].src), intf.local_ip6)
+
+        # Src link-layer options should have the router's MAC
+        sll = rx[ICMPv6NDOptSrcLLAddr]
+        self.assertEqual(sll.lladdr, intf.local_mac)
+
     def send_and_expect_ra(self, intf, pkts, remark, dst_ip=None,
                            filter_out_fn=is_ipv6_misc):
         intf.add_stream(pkts)
@@ -99,6 +124,17 @@ class TestIPv6ND(VppTestCase):
         rx = rx[0]
         self.validate_na(intf, rx, dst_ip, tgt_ip)
 
+    def send_and_expect_ns(self, tx_intf, rx_intf, pkts, tgt_ip,
+                           filter_out_fn=is_ipv6_misc):
+        tx_intf.add_stream(pkts)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        rx = rx_intf.get_capture(1, filter_out_fn=filter_out_fn)
+
+        self.assertEqual(len(rx), 1)
+        rx = rx[0]
+        self.validate_ns(rx_intf, rx, tgt_ip)
+
     def send_and_assert_no_replies(self, intf, pkts, remark):
         intf.add_stream(pkts)
         self.pg_enable_capture(self.pg_interfaces)
@@ -107,6 +143,15 @@ class TestIPv6ND(VppTestCase):
             i.get_capture(0)
             i.assert_nothing_captured(remark=remark)
 
+    def verify_ip(self, rx, smac, dmac, sip, dip):
+        ether = rx[Ether]
+        self.assertEqual(ether.dst, dmac)
+        self.assertEqual(ether.src, smac)
+
+        ip = rx[IPv6]
+        self.assertEqual(ip.src, sip)
+        self.assertEqual(ip.dst, dip)
+
 
 class TestIPv6(TestIPv6ND):
     """ IPv6 Test Case """
@@ -444,6 +489,78 @@ class TestIPv6(TestIPv6ND):
                                     128,
                                     inet=AF_INET6))
 
+    def test_ns_duplicates(self):
+        """ ARP Duplicates"""
+
+        #
+        # Generate some hosts on the LAN
+        #
+        self.pg1.generate_remote_hosts(3)
+
+        #
+        # Add host 1 on pg1 and pg2
+        #
+        ns_pg1 = VppNeighbor(self,
+                             self.pg1.sw_if_index,
+                             self.pg1.remote_hosts[1].mac,
+                             self.pg1.remote_hosts[1].ip6,
+                             af=AF_INET6)
+        ns_pg1.add_vpp_config()
+        ns_pg2 = VppNeighbor(self,
+                             self.pg2.sw_if_index,
+                             self.pg2.remote_mac,
+                             self.pg1.remote_hosts[1].ip6,
+                             af=AF_INET6)
+        ns_pg2.add_vpp_config()
+
+        #
+        # IP packet destined for pg1 remote host arrives on pg1 again.
+        #
+        p = (Ether(dst=self.pg0.local_mac,
+                   src=self.pg0.remote_mac) /
+             IPv6(src=self.pg0.remote_ip6,
+                  dst=self.pg1.remote_hosts[1].ip6) /
+             UDP(sport=1234, dport=1234) /
+             Raw())
+
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx1 = self.pg1.get_capture(1)
+
+        self.verify_ip(rx1[0],
+                       self.pg1.local_mac,
+                       self.pg1.remote_hosts[1].mac,
+                       self.pg0.remote_ip6,
+                       self.pg1.remote_hosts[1].ip6)
+
+        #
+        # remove the duplicate on pg1
+        # packet stream shoud generate ARPs out of pg1
+        #
+        ns_pg1.remove_vpp_config()
+
+        self.send_and_expect_ns(self.pg0, self.pg1,
+                                p, self.pg1.remote_hosts[1].ip6)
+
+        #
+        # Add it back
+        #
+        ns_pg1.add_vpp_config()
+
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx1 = self.pg1.get_capture(1)
+
+        self.verify_ip(rx1[0],
+                       self.pg1.local_mac,
+                       self.pg1.remote_hosts[1].mac,
+                       self.pg0.remote_ip6,
+                       self.pg1.remote_hosts[1].ip6)
+
     def validate_ra(self, intf, rx, dst_ip=None, mtu=9000, pi_opt=None):
         if not dst_ip:
             dst_ip = intf.remote_ip6
index 67d64a2..d4f7729 100644 (file)
@@ -812,5 +812,84 @@ class ARPTestCase(VppTestCase):
         self.pg1.admin_down()
         self.pg1.admin_up()
 
+    def test_arp_duplicates(self):
+        """ ARP Duplicates"""
+
+        #
+        # Generate some hosts on the LAN
+        #
+        self.pg1.generate_remote_hosts(3)
+
+        #
+        # Add host 1 on pg1 and pg2
+        #
+        arp_pg1 = VppNeighbor(self,
+                              self.pg1.sw_if_index,
+                              self.pg1.remote_hosts[1].mac,
+                              self.pg1.remote_hosts[1].ip4)
+        arp_pg1.add_vpp_config()
+        arp_pg2 = VppNeighbor(self,
+                              self.pg2.sw_if_index,
+                              self.pg2.remote_mac,
+                              self.pg1.remote_hosts[1].ip4)
+        arp_pg2.add_vpp_config()
+
+        #
+        # IP packet destined for pg1 remote host arrives on pg1 again.
+        #
+        p = (Ether(dst=self.pg0.local_mac,
+                   src=self.pg0.remote_mac) /
+             IP(src=self.pg0.remote_ip4,
+                dst=self.pg1.remote_hosts[1].ip4) /
+             UDP(sport=1234, dport=1234) /
+             Raw())
+
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx1 = self.pg1.get_capture(1)
+
+        self.verify_ip(rx1[0],
+                       self.pg1.local_mac,
+                       self.pg1.remote_hosts[1].mac,
+                       self.pg0.remote_ip4,
+                       self.pg1.remote_hosts[1].ip4)
+
+        #
+        # remove the duplicate on pg1
+        # packet stream shoud generate ARPs out of pg1
+        #
+        arp_pg1.remove_vpp_config()
+
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx1 = self.pg1.get_capture(1)
+
+        self.verify_arp_req(rx1[0],
+                            self.pg1.local_mac,
+                            self.pg1.local_ip4,
+                            self.pg1.remote_hosts[1].ip4)
+
+        #
+        # Add it back
+        #
+        arp_pg1.add_vpp_config()
+
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx1 = self.pg1.get_capture(1)
+
+        self.verify_ip(rx1[0],
+                       self.pg1.local_mac,
+                       self.pg1.remote_hosts[1].mac,
+                       self.pg0.remote_ip4,
+                       self.pg1.remote_hosts[1].ip4)
+
+
 if __name__ == '__main__':
     unittest.main(testRunner=VppTestRunner)