Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 133 additions & 0 deletions tests/topotests/high_ecmp/test_high_ecmp_unnumbered.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion zebra/interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
24 changes: 17 additions & 7 deletions zebra/rtadv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still concerned about this a bit. I only see the "wheel_remove" in the path where the frr-configured "shut" is processed, but not in other interface "down" processing. are you sure this is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added wheel_remove in interface_down()

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
Expand All @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
4 changes: 2 additions & 2 deletions zebra/rtadv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
Loading