ethernet: run callbacks for subifs too when mac changes

When MAC address changes for an interface, address change callbacks are
executed for it. In turn adjacencies register a callback for MAC address
changes to be able to update their rewrite strings accordingly.

Subinterfaces inherit MAC address from the parent interface. When MAC
address of the parent interface changes, it also implies MAC address
change for its subinterfaces. The problem is that this is currently not
considered when address change callbacks are executed. After MAC address
change on the parent interface, packets sent from subinterfaces might
have wrong source MAC address as the result of stale adjacencies. For
example, ARP messages might be sent with the wrong (previous) MAC
address and address resolution will fail.

With this fix, when address change callbacks are executed for an
interface, they will be also executed for its subinterfaces. And
adjacencies will be able to update accordingly.

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

2 files changed, 113 insertions, 4 deletions

diff --git a/src/vnet/ethernet/interface.c b/src/vnet/ethernet/interface.c
index bf2256630f0..849024238bd 100644
--- a/src/vnet/ethernet/interface.c
+++ b/src/vnet/ethernet/interface.c
@@ -303,8 +303,17 @@ ethernet_mac_change (vnet_hw_interface_t * hi,
 
   {
     ethernet_address_change_ctx_t *cb;
+    u32 id, sw_if_index;
     vec_foreach (cb, em->address_change_callbacks)
-      cb->function (em, hi->sw_if_index, cb->function_opaque);
+      {
+	cb->function (em, hi->sw_if_index, cb->function_opaque);
+	/* clang-format off */
+	hash_foreach (id, sw_if_index, hi->sub_interface_sw_if_index_by_id,
+	({
+	  cb->function (em, sw_if_index, cb->function_opaque);
+	}));
+	/* clang-format on */
+      }
   }
 
   return (NULL);
diff --git a/test/test_neighbor.py b/test/test_neighbor.py
index 24737da0eca..041d78245a5 100644
--- a/test/test_neighbor.py
+++ b/test/test_neighbor.py
@@ -16,8 +16,9 @@ from vpp_ip_route import (
     FibPathType,
     VppIpInterfaceAddress,
 )
-from vpp_papi import VppEnum
+from vpp_papi import VppEnum, MACAddress
 from vpp_ip import VppIpPuntRedirect
+from vpp_sub_interface import VppDot1ADSubint
 
 import scapy.compat
 from scapy.packet import Raw
@@ -86,11 +87,11 @@ class ARPTestCase(VppTestCase):
 
         super(ARPTestCase, self).tearDown()
 
-    def verify_arp_req(self, rx, smac, sip, dip):
+    def verify_arp_req(self, rx, smac, sip, dip, etype=0x0806):
         ether = rx[Ether]
         self.assertEqual(ether.dst, "ff:ff:ff:ff:ff:ff")
         self.assertEqual(ether.src, smac)
-        self.assertEqual(ether.type, 0x0806)
+        self.assertEqual(ether.type, etype)
 
         arp = rx[ARP]
         self.assertEqual(arp.hwtype, 1)
@@ -845,6 +846,105 @@ class ARPTestCase(VppTestCase):
         self.pg2.admin_down()
         self.pg1.admin_down()
 
+    def test_arp_after_mac_change(self):
+        """ARP (after MAC address change)"""
+
+        #
+        # Prepare a subinterface
+        #
+        subif0 = VppDot1ADSubint(self, self.pg1, 0, 300, 400)
+        subif0.admin_up()
+        subif0.config_ip4()
+
+        #
+        # Send a packet to cause ARP generation for the parent interface's remote host
+        #
+        p1 = (
+            Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac)
+            / IP(src=self.pg0.remote_ip4, dst=self.pg1.remote_ip4)
+            / UDP(sport=1234, dport=1234)
+            / Raw()
+        )
+
+        self.pg0.add_stream(p1)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx = self.pg1.get_capture(1)
+
+        self.verify_arp_req(
+            rx[0], self.pg1.local_mac, self.pg1.local_ip4, self.pg1.remote_ip4
+        )
+
+        #
+        # Send a packet to cause ARP generation for the subinterface's remote host
+        #
+        p2 = (
+            Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac)
+            / IP(src=self.pg0.remote_ip4, dst=subif0.remote_ip4)
+            / UDP(sport=1234, dport=1234)
+            / Raw()
+        )
+
+        self.pg0.add_stream(p2)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx = self.pg1.get_capture(1)
+
+        self.verify_arp_req(
+            rx[0],
+            self.pg1.local_mac,
+            subif0.local_ip4,
+            subif0.remote_ip4,
+            subif0.DOT1AD_TYPE,
+        )
+
+        #
+        # Change MAC address of the parent interface
+        #
+        pg1_mac_saved = self.pg1.local_mac
+        self.pg1.set_mac(MACAddress("00:00:00:11:22:33"))
+
+        #
+        # Send a packet to cause ARP generation for the parent interface's remote host
+        #   - expect new MAC address is used as the source
+        #
+        self.pg0.add_stream(p1)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx = self.pg1.get_capture(1)
+
+        self.verify_arp_req(
+            rx[0], self.pg1.local_mac, self.pg1.local_ip4, self.pg1.remote_ip4
+        )
+
+        #
+        # Send a packet to cause ARP generation for the subinterface's remote host
+        #   - expect new MAC address is used as the source
+        #
+
+        self.pg0.add_stream(p2)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+
+        rx = self.pg1.get_capture(1)
+
+        self.verify_arp_req(
+            rx[0],
+            self.pg1.local_mac,
+            subif0.local_ip4,
+            subif0.remote_ip4,
+            subif0.DOT1AD_TYPE,
+        )
+
+        #
+        # Cleanup
+        #
+        subif0.remove_vpp_config()
+        self.pg1.set_mac(MACAddress(pg1_mac_saved))
+
     def test_proxy_mirror_arp(self):
         """Interface Mirror Proxy ARP"""