Merge pull request FRRouting#18451 from soumyar-roy/soumya/fastra

zebra: V6 RA not sent anymore after interface up-down-up

Author: ton31337

tests/topotests/high_ecmp/test_high_ecmp_unnumbered.py

@@ -105,6 +105,139 @@ def teardown_module(_mod):
     tgen.stop_topology()
 
 
+def test_v6_rtadv():
+    tgen = get_topogen()
+
+    # Get the initial interface state and RA interval before making any changes
+    interface_output = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
+    interface_data = json.loads(interface_output)
+
+    # Check if the interface exists and get its current state
+    interface_section = interface_data.get("r1-eth200", {})
+    if not interface_section:
+        logger.error("Interface r1-eth200 not found. Test cannot continue.")
+        return
+
+    # Get the RA interval
+    ra_interval = interface_section.get("ndRouterAdvertisementsIntervalSecs")
+    if ra_interval is None:
+        # If RA interval isn't found, log a message and exit the test
+        logger.error(
+            "Could not find RA interval (ndRouterAdvertisementsIntervalSecs) in interface configuration. Test cannot continue."
+        )
+        return
+
+    logger.info(f"Using RA interval of {ra_interval} seconds")
+
+    # Function to get current RA state - returns the router advertisement sent count
+    def _get_ra_state():
+        output = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
+        data = json.loads(output)
+        return data.get("r1-eth200", {}).get("ndRouterAdvertisementsSent")
+
+    logger.info("Shutdown r1-eth200")
+    tgen.gears["r1"].vtysh_cmd(
+        """
+    configure terminal
+      interface r1-eth200
+        shutdown
+    """
+    )
+
+    # Verify interface is down before proceeding
+    def _check_interface_down():
+        output = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
+        return True if '"administrativeStatus":"down"' in output else False
+
+    _, result = topotest.run_and_expect(_check_interface_down, True, count=10, wait=15)
+    assert result is True, "Interface r1-eth200 did not go down after shutdown command"
+
+    # Take snapshots for RA status when interface is down
+    ra_sent_count1 = _get_ra_state()
+
+    if ra_sent_count1 is None:
+        logger.error("Could not get RA sent count. Test cannot continue.")
+        return
+
+    # We need to wait for at least ra_interval + buffer time(1 sec) to avoid
+    # situation where any event was delayed or due to any processing delay
+    logger.info(f"Waiting another {ra_interval + 1} seconds for RA timer...")
+    sleep(ra_interval + 1)
+    ra_sent_count2 = _get_ra_state()
+
+    # Verify RA sent count didn't change when interface is down
+    assert (
+        ra_sent_count1 == ra_sent_count2
+    ), f"RA sent count should not have changed when interface is down: was {ra_sent_count1}, now {ra_sent_count2}"
+
+    logger.info("Do no shutdown for r1-eth200")
+    tgen.gears["r1"].vtysh_cmd(
+        """
+    configure terminal
+      interface r1-eth200
+        no shutdown
+    """
+    )
+
+    # Verify interface is up before proceeding
+    def _check_interface_up():
+        output = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
+        return True if '"administrativeStatus":"up"' in output else False
+
+    _, result = topotest.run_and_expect(_check_interface_up, True, count=10, wait=15)
+    assert result is True, "Interface r1-eth200 did not go up after no shutdown command"
+
+    # Take snapshots for RA status when interface is up
+    ra_sent_count1 = _get_ra_state()
+
+    # We need to wait for at least ra_interval + buffer time(1 sec)
+    logger.info(f"Waiting another {ra_interval + 1} seconds for RA timer...")
+    sleep(ra_interval + 1)
+    ra_sent_count2 = _get_ra_state()
+
+    # Verify RA sent count changed when interface is up (RAs should be sent)
+    assert (
+        ra_sent_count1 != ra_sent_count2
+    ), f"RA sent count should have changed when interface is up: was {ra_sent_count1}, still {ra_sent_count2}"
+
+    logger.info("Remove r1-eth200")
+    existing_config = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200")
+    tgen.gears["r1"].cmd(
+        """
+    sudo ip link set dev r1-eth200 down
+    """
+    )
+
+    # Verify interface is down after ip link set down
+    _, result = topotest.run_and_expect(_check_interface_down, True, count=10, wait=15)
+    assert result is True, "Interface r1-eth200 did not go down after ip link set down"
+
+    # Get current RA sent count
+    ra_sent_count1 = _get_ra_state()
+
+    # Wait for the RA interval
+    logger.info(f"Waiting {ra_interval + 1} seconds for RA timer...")
+    sleep(ra_interval + 1)
+
+    # Get second RA sent count
+    ra_sent_count2 = _get_ra_state()
+
+    # Verify counts are the same when interface is down
+    assert (
+        ra_sent_count1 == ra_sent_count2
+    ), f"RA sent count changed from {ra_sent_count1} to {ra_sent_count2} within {ra_interval + 1} seconds while interface is down"
+
+    tgen.gears["r1"].cmd(
+        """
+    sudo ip link set dev r1-eth200 up
+    """
+    )
+
+    # Verify interface is up after ip link set up
+    _, result = topotest.run_and_expect(_check_interface_up, True, count=10, wait=15)
+    assert result is True, "Interface r1-eth200 did not go up after ip link set up"
+
+
 def test_bgp_route_cleanup():
     failures = 0
     net = get_topogen().net

zebra/interface.c

@@ -990,6 +990,7 @@ void if_down(struct interface *ifp)
 	zif->down_count++;
 	frr_timestamp(2, zif->down_last, sizeof(zif->down_last));
 
+	rtadv_stop_ra(ifp, true);
 	if_down_nhg_dependents(ifp);
 
 	/* Handle interface down for specific types for EVPN. Non-VxLAN
@@ -3706,7 +3707,7 @@ int if_shutdown(struct interface *ifp)
 
 	if (ifp->ifindex != IFINDEX_INTERNAL) {
 		/* send RA lifetime of 0 before stopping. rfc4861/6.2.5 */
-		rtadv_stop_ra(ifp);
+		rtadv_stop_ra(ifp, false);
 		if (if_unset_flags(ifp, IFF_UP) < 0) {
 			zlog_debug("Can't shutdown interface %s", ifp->name);
 			return -1;

zebra/rtadv.c

@@ -1400,8 +1400,11 @@ static void rtadv_start_interface_events(struct zebra_vrf *zvrf,
 	}
 
 	adv_if = adv_if_add(zvrf, zif->ifp->name);
-	if (adv_if != NULL)
+	if (adv_if != NULL) {
+		rtadv_send_packet(zvrf->rtadv.sock, zif->ifp, RA_ENABLE);
+		wheel_add_item(zrouter.ra_wheel, zif->ifp);
 		return; /* Already added */
+	}
 
 	if (if_join_all_router(zvrf->rtadv.sock, zif->ifp)) {
 		/*Failed to join on 1st attempt, wait random amount of time between 1 ms 
@@ -1413,6 +1416,9 @@ static void rtadv_start_interface_events(struct zebra_vrf *zvrf,
 
 	if (adv_if_list_count(&zvrf->rtadv.adv_if) == 1)
 		rtadv_event(zvrf, RTADV_START, 0);
+
+	rtadv_send_packet(zvrf->rtadv.sock, zif->ifp, RA_ENABLE);
+	wheel_add_item(zrouter.ra_wheel, zif->ifp);
 }
 
 void ipv6_nd_suppress_ra_set(struct interface *ifp,
@@ -1461,7 +1467,6 @@ void ipv6_nd_suppress_ra_set(struct interface *ifp,
 					RTADV_NUM_FAST_REXMITS;
 			}
 
-			wheel_add_item(zrouter.ra_wheel, ifp);
 			rtadv_start_interface_events(zvrf, zif);
 		}
 	}
@@ -1581,20 +1586,25 @@ static void zebra_interface_radv_set(ZAPI_HANDLER_ARGS, int enable)
  * ceasing to advertise and want to let our neighbors know.
  * RFC 4861 secion 6.2.5
  */
-void rtadv_stop_ra(struct interface *ifp)
+void rtadv_stop_ra(struct interface *ifp, bool if_down_event)
 {
 	struct zebra_if *zif;
 	struct zebra_vrf *zvrf;
 
-	zif = ifp->info;
-	zvrf = rtadv_interface_get_zvrf(ifp);
-
 	/*Try to delete from ra wheels */
 	wheel_remove_item(zrouter.ra_wheel, ifp);
 
+	zif = ifp->info;
+	zvrf = rtadv_interface_get_zvrf(ifp);
+
 	/*Turn off event for ICMPv6 join*/
 	event_cancel(&zif->icmpv6_join_timer);
 
+	if (if_down_event) {
+		/* Nothing to do more, return */
+		return;
+	}
+
 	if (zif->rtadv.AdvSendAdvertisements)
 		rtadv_send_packet(zvrf->rtadv.sock, ifp, RA_SUPPRESS);
 }
@@ -1622,7 +1632,7 @@ void rtadv_stop_ra_all(void)
 				       rprefix)
 				rtadv_prefix_reset(zif, rprefix, rprefix);
 
-			rtadv_stop_ra(ifp);
+			rtadv_stop_ra(ifp, false);
 		}
 }
 

zebra/rtadv.h

@@ -404,7 +404,7 @@ enum ipv6_nd_suppress_ra_status {
 
 extern void rtadv_vrf_init(struct zebra_vrf *zvrf);
 extern void rtadv_vrf_terminate(struct zebra_vrf *zvrf);
-extern void rtadv_stop_ra(struct interface *ifp);
+extern void rtadv_stop_ra(struct interface *ifp, bool if_down_event);
 extern void rtadv_stop_ra_all(void);
 extern void rtadv_cmd_init(void);
 extern void rtadv_if_init(struct zebra_if *zif);
@@ -494,7 +494,7 @@ static inline void rtadv_delete_prefix(struct zebra_if *zif,
 				       const struct prefix *p)
 {
 }
-static inline void rtadv_stop_ra(struct interface *ifp)
+static inline void rtadv_stop_ra(struct interface *ifp, bool if_down_event)
 {
 }
 static inline void rtadv_stop_ra_all(void)