ip: fix arc start in ip46-local for local mfib entries 28/35928/2
authorAlexander Chernavin <achernavin@netgate.com>
Mon, 11 Apr 2022 13:02:11 +0000 (13:02 +0000)
committerMatthew Smith <mgsmith@netgate.com>
Tue, 12 Apr 2022 16:06:43 +0000 (16:06 +0000)
Type: fix

After changes made in f840880, VRRP IPv6 cannot reply for neighbor
solicitations requesting the link layer address of the configured
virtual address.

VRRP IPv6 enables the vrrp6-nd-input feature in the ip6-local feature
arc for an interface on which a virtual router is configured. When
neighbor solicitations arrive on that interface, ip6-local should start
feature arc walk for that interface and the messages should be processed
by vrrp6-nd-input. The problem is that currently, the feature arc is
started for the interface obtained from the receive DPO that has
interface unset (i.e. max u32) for local mfib entries. Thus, the feature
arc is started not on the interface the messages were received on and
vrrp6-nd-input is not traversed.

With this fix, if interface obtained from the receive DPO is unset, use
RX interface from the buffer to start the ip46-local feature arc.

Also, enable tests of this case for both IPv4 and IPv6 address families
that are currently tagged as extended and not run on every change. They
configure VRRP with priority 255 and are expected to be stable.

Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
Change-Id: I11ef3d5a7a986e04431e8613d1510b8666094bd7

src/vnet/ip/ip4_forward.c
src/vnet/ip/ip6_forward.c
test/test_vrrp.py

index c1bfc7d..33ab341 100644 (file)
@@ -1538,14 +1538,14 @@ ip4_local_check_src (vlib_buffer_t *b, ip4_header_t *ip0,
     vnet_buffer (b)->sw_if_index[VLIB_TX] != ~0 ?
     vnet_buffer (b)->sw_if_index[VLIB_TX] : vnet_buffer (b)->ip.fib_index;
 
+  vnet_buffer (b)->ip.rx_sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
   if (is_receive_dpo)
     {
       receive_dpo_t *rd;
       rd = receive_dpo_get (vnet_buffer (b)->ip.adj_index[VLIB_TX]);
-      vnet_buffer (b)->ip.rx_sw_if_index = rd->rd_sw_if_index;
+      if (rd->rd_sw_if_index != ~0)
+       vnet_buffer (b)->ip.rx_sw_if_index = rd->rd_sw_if_index;
     }
-  else
-    vnet_buffer (b)->ip.rx_sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
 
   /*
    * vnet_buffer()->ip.adj_index[VLIB_RX] will be set to the index of the
@@ -1628,20 +1628,19 @@ ip4_local_check_src_x2 (vlib_buffer_t **b, ip4_header_t **ip,
   not_last_hit |= vnet_buffer (b[0])->ip.fib_index ^ last_check->fib_index;
   not_last_hit |= vnet_buffer (b[1])->ip.fib_index ^ last_check->fib_index;
 
+  vnet_buffer (b[0])->ip.rx_sw_if_index =
+    vnet_buffer (b[0])->sw_if_index[VLIB_RX];
+  vnet_buffer (b[1])->ip.rx_sw_if_index =
+    vnet_buffer (b[1])->sw_if_index[VLIB_RX];
   if (is_receive_dpo)
     {
       const receive_dpo_t *rd0, *rd1;
       rd0 = receive_dpo_get (vnet_buffer (b[0])->ip.adj_index[VLIB_TX]);
       rd1 = receive_dpo_get (vnet_buffer (b[1])->ip.adj_index[VLIB_TX]);
-      vnet_buffer (b[0])->ip.rx_sw_if_index = rd0->rd_sw_if_index;
-      vnet_buffer (b[1])->ip.rx_sw_if_index = rd1->rd_sw_if_index;
-    }
-  else
-    {
-      vnet_buffer (b[0])->ip.rx_sw_if_index =
-       vnet_buffer (b[0])->sw_if_index[VLIB_RX];
-      vnet_buffer (b[1])->ip.rx_sw_if_index =
-       vnet_buffer (b[1])->sw_if_index[VLIB_RX];
+      if (rd0->rd_sw_if_index != ~0)
+       vnet_buffer (b[0])->ip.rx_sw_if_index = rd0->rd_sw_if_index;
+      if (rd1->rd_sw_if_index != ~0)
+       vnet_buffer (b[1])->ip.rx_sw_if_index = rd1->rd_sw_if_index;
     }
 
   /*
index 5951de4..44b0052 100644 (file)
@@ -1475,6 +1475,11 @@ ip6_local_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
            vnet_buffer (b[1])->sw_if_index[VLIB_TX] != ~0 ?
            vnet_buffer (b[1])->sw_if_index[VLIB_TX] :
            vnet_buffer (b[1])->ip.fib_index;
+
+         vnet_buffer (b[0])->ip.rx_sw_if_index =
+           vnet_buffer (b[0])->sw_if_index[VLIB_RX];
+         vnet_buffer (b[1])->ip.rx_sw_if_index =
+           vnet_buffer (b[1])->sw_if_index[VLIB_RX];
          if (is_receive_dpo)
            {
              const receive_dpo_t *rd0, *rd1;
@@ -1482,15 +1487,10 @@ ip6_local_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
                receive_dpo_get (vnet_buffer (b[0])->ip.adj_index[VLIB_TX]);
              rd1 =
                receive_dpo_get (vnet_buffer (b[1])->ip.adj_index[VLIB_TX]);
-             vnet_buffer (b[0])->ip.rx_sw_if_index = rd0->rd_sw_if_index;
-             vnet_buffer (b[1])->ip.rx_sw_if_index = rd1->rd_sw_if_index;
-           }
-         else
-           {
-             vnet_buffer (b[0])->ip.rx_sw_if_index =
-               vnet_buffer (b[0])->ip.adj_index[VLIB_RX];
-             vnet_buffer (b[1])->ip.rx_sw_if_index =
-               vnet_buffer (b[1])->ip.adj_index[VLIB_RX];
+             if (rd0->rd_sw_if_index != ~0)
+               vnet_buffer (b[0])->ip.rx_sw_if_index = rd0->rd_sw_if_index;
+             if (rd1->rd_sw_if_index != ~0)
+               vnet_buffer (b[1])->ip.rx_sw_if_index = rd1->rd_sw_if_index;
            }
        }                       /* head_of_feature_arc */
 
@@ -1619,15 +1619,16 @@ ip6_local_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
            vnet_buffer (b[0])->sw_if_index[VLIB_TX] != ~0 ?
            vnet_buffer (b[0])->sw_if_index[VLIB_TX] :
            vnet_buffer (b[0])->ip.fib_index;
+
+         vnet_buffer (b[0])->ip.rx_sw_if_index =
+           vnet_buffer (b[0])->sw_if_index[VLIB_RX];
          if (is_receive_dpo)
            {
              receive_dpo_t *rd;
              rd = receive_dpo_get (vnet_buffer (b[0])->ip.adj_index[VLIB_TX]);
-             vnet_buffer (b[0])->ip.rx_sw_if_index = rd->rd_sw_if_index;
+             if (rd->rd_sw_if_index != ~0)
+               vnet_buffer (b[0])->ip.rx_sw_if_index = rd->rd_sw_if_index;
            }
-         else
-           vnet_buffer (b[0])->ip.rx_sw_if_index =
-             vnet_buffer (b[0])->ip.adj_index[VLIB_RX];
        }                       /* head_of_feature_arc */
 
       next[0] = lm->local_next_by_ip_protocol[ip->protocol];
index f7a0662..6a62a88 100644 (file)
@@ -233,7 +233,6 @@ class VppVRRPVirtualRouter(VppObject):
         return pkt
 
 
-@unittest.skipUnless(config.extended, "part of extended tests")
 class TestVRRP4(VppTestCase):
     """ IPv4 VRRP Test Case """
 
@@ -329,6 +328,7 @@ class TestVRRP4(VppTestCase):
 
     # VR with priority 255 owns the virtual address and should
     # become master and start advertising immediately.
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_master_adv(self):
         """ IPv4 Master VR advertises """
         self.pg_enable_capture(self.pg_interfaces)
@@ -360,6 +360,7 @@ class TestVRRP4(VppTestCase):
 
     # Same as above but with the update API, and add a change
     # of parameters to test that too
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_master_adv_update(self):
         """ IPv4 Master VR adv + Update to Backup """
         self.pg_enable_capture(self.pg_interfaces)
@@ -410,6 +411,7 @@ class TestVRRP4(VppTestCase):
 
     # VR with priority < 255 enters backup state and does not advertise as
     # long as it receives higher priority advertisements
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_backup_noadv(self):
         """ IPv4 Backup VR does not advertise """
         self.pg_enable_capture(self.pg_interfaces)
@@ -480,6 +482,7 @@ class TestVRRP4(VppTestCase):
         vr.remove_vpp_config()
         self._vrs = []
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_backup_noarp(self):
         """ IPv4 Backup VR ignores ARP """
         # We need an address for a virtual IP that is not the IP that
@@ -515,6 +518,7 @@ class TestVRRP4(VppTestCase):
         vr.remove_vpp_config()
         self._vrs = []
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_election(self):
         """ IPv4 Backup VR becomes master if no advertisements received """
 
@@ -551,6 +555,7 @@ class TestVRRP4(VppTestCase):
         self.pg0.wait_for_packet(intvl_s, is_not_adv)
         vr.assert_state_equals(VRRP_VR_STATE_MASTER)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_backup_preempts(self):
         """ IPv4 Backup VR preempts lower priority master """
 
@@ -588,6 +593,7 @@ class TestVRRP4(VppTestCase):
         self.pg0.wait_for_packet(timeout=intvl_s, filter_out_fn=is_not_adv)
         vr.assert_state_equals(VRRP_VR_STATE_MASTER)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_master_preempted(self):
         """ IPv4 Master VR preempted by higher priority backup """
 
@@ -624,6 +630,7 @@ class TestVRRP4(VppTestCase):
         # VR should be in backup state again
         vr.assert_state_equals(VRRP_VR_STATE_BACKUP)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_accept_mode_disabled(self):
         """ IPv4 Master VR does not reply for VIP w/ accept mode off """
 
@@ -663,6 +670,7 @@ class TestVRRP4(VppTestCase):
         time.sleep(1)
         self.pg0.assert_nothing_captured(filter_out_fn=is_not_echo_reply)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_accept_mode_enabled(self):
         """ IPv4 Master VR replies for VIP w/ accept mode on """
 
@@ -710,6 +718,7 @@ class TestVRRP4(VppTestCase):
         self.assertEqual(rx_pkts[0][ICMP].seq, 1)
         self.assertEqual(rx_pkts[0][ICMP].id, self.pg0.sw_if_index)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_intf_tracking(self):
         """ IPv4 Master VR adjusts priority based on tracked interface """
 
@@ -774,6 +783,7 @@ class TestVRRP4(VppTestCase):
                                       filter_out_fn=is_not_adv)
         self.assertEqual(rx, adv_configured)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp4_master_adv_unicast(self):
         """ IPv4 Master VR advertises (unicast) """
 
@@ -816,7 +826,6 @@ class TestVRRP4(VppTestCase):
         self.assertEqual(rx[VRRPv3].addrlist, [vip])
 
 
-@unittest.skipUnless(config.extended, "part of extended tests")
 class TestVRRP6(VppTestCase):
     """ IPv6 VRRP Test Case """
 
@@ -911,6 +920,7 @@ class TestVRRP6(VppTestCase):
 
     # VR with priority 255 owns the virtual address and should
     # become master and start advertising immediately.
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_master_adv(self):
         """ IPv6 Master VR advertises """
         self.pg_enable_capture(self.pg_interfaces)
@@ -944,6 +954,7 @@ class TestVRRP6(VppTestCase):
 
     # Same as above but with the update API, and add a change
     # of parameters to test that too
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_master_adv_update(self):
         """ IPv6 Master VR adv + Update to Backup """
         self.pg_enable_capture(self.pg_interfaces)
@@ -997,6 +1008,7 @@ class TestVRRP6(VppTestCase):
 
     # VR with priority < 255 enters backup state and does not advertise as
     # long as it receives higher priority advertisements
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_backup_noadv(self):
         """ IPv6 Backup VR does not advertise """
         self.pg_enable_capture(self.pg_interfaces)
@@ -1068,6 +1080,7 @@ class TestVRRP6(VppTestCase):
         vr.remove_vpp_config()
         self._vrs = []
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_backup_nond(self):
         """ IPv6 Backup VR ignores NDP """
         # We need an address for a virtual IP that is not the IP that
@@ -1106,6 +1119,7 @@ class TestVRRP6(VppTestCase):
 
         vr.start_stop(is_start=0)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_election(self):
         """ IPv6 Backup VR becomes master if no advertisements received """
 
@@ -1142,6 +1156,7 @@ class TestVRRP6(VppTestCase):
         self.pg0.wait_for_packet(intvl_s, is_not_adv)
         vr.assert_state_equals(VRRP_VR_STATE_MASTER)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_backup_preempts(self):
         """ IPv6 Backup VR preempts lower priority master """
 
@@ -1179,6 +1194,7 @@ class TestVRRP6(VppTestCase):
         self.pg0.wait_for_packet(timeout=intvl_s, filter_out_fn=is_not_adv)
         vr.assert_state_equals(VRRP_VR_STATE_MASTER)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_master_preempted(self):
         """ IPv6 Master VR preempted by higher priority backup """
 
@@ -1215,6 +1231,7 @@ class TestVRRP6(VppTestCase):
         # VR should be in backup state again
         vr.assert_state_equals(VRRP_VR_STATE_BACKUP)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_accept_mode_disabled(self):
         """ IPv6 Master VR does not reply for VIP w/ accept mode off """
 
@@ -1254,6 +1271,7 @@ class TestVRRP6(VppTestCase):
         time.sleep(1)
         self.pg0.assert_nothing_captured(filter_out_fn=is_not_echo_reply)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_accept_mode_enabled(self):
         """ IPv6 Master VR replies for VIP w/ accept mode on """
 
@@ -1300,6 +1318,7 @@ class TestVRRP6(VppTestCase):
         self.assertEqual(rx_pkts[0][ICMPv6EchoReply].seq, 1)
         self.assertEqual(rx_pkts[0][ICMPv6EchoReply].id, self.pg0.sw_if_index)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_intf_tracking(self):
         """ IPv6 Master VR adjusts priority based on tracked interface """
 
@@ -1364,6 +1383,7 @@ class TestVRRP6(VppTestCase):
                                       filter_out_fn=is_not_adv)
         self.assertEqual(rx, adv_configured)
 
+    @unittest.skipUnless(config.extended, "part of extended tests")
     def test_vrrp6_master_adv_unicast(self):
         """ IPv6 Master VR advertises (unicast) """