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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+118 −10
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
087ab08
zebra: V6 RA not sent anymore after interface up-down-up
soumyar-roy Mar 21, 2025
3a711fc
test: Add test case to verify RA adv
soumyar-roy Apr 16, 2025
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
97 changes: 97 additions & 0 deletions tests/topotests/high_ecmp/test_high_ecmp_unnumbered.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,103 @@ def teardown_module(_mod):
tgen.stop_topology()


def test_v6_rtadv_():
failures = 0
tgen = get_topogen()
print("Shutdown r1-eth200")
tgen.gears["r1"].vtysh_cmd(
"""
configure terminal
interface r1-eth200
shutdown
"""
)

# Take two snap shots for RA status, it should not change
# Give enough time, RA adv timer to expire
sleep(2)
rtadv_output1 = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
sleep(2)
rtadv_output2 = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
if rtadv_output1 != rtadv_output2:
sys.stderr.write(
f"RA state should not have changed: got {rtadv_output1}, expected {rtadv_output2}\n"
)
failures += 1

logger.info("Do no shutdown for r1-eth200")
tgen.gears["r1"].vtysh_cmd(
"""
configure terminal
interface r1-eth200
no shutdown
"""
)

sleep(2)
rtadv_output1 = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
sleep(2)
rtadv_output2 = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
if rtadv_output1 == rtadv_output2:
sys.stderr.write(
f"RA state didn't change: got {rtadv_output1}, previous {rtadv_output2}\n"
)
failures += 1

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
"""
)

sleep(2)
rtadv_output1 = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
pattern1 = '"administrativeStatus":"down"'

if pattern1 not in rtadv_output1:
sys.stderr.write(f"Interface state not down yet:: got {rtadv_output1}\n")
failures += 1

# Parse the JSON string into a Python dictionary
rtadv_output1 = json.loads(rtadv_output1)
sent_count1 = rtadv_output1.get("r1-eth200", {}).get("ndRouterAdvertisementsSent")

if sent_count1 is not None:
# Wait 2 seconds
sleep(2)

# Get second output
rtadv_output2 = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")

rtadv_output2 = json.loads(rtadv_output2)
sent_count2 = rtadv_output2.get("r1-eth200", {}).get(
"ndRouterAdvertisementsSent"
)

# Verify counts are the same
if sent_count1 is not None and sent_count2 is not None:
if sent_count1 != sent_count2:
sys.stderr.write(
f"RA count changed from {sent_count1} to {sent_count2} within 2 seconds\n"
)
failures += 1

tgen.gears["r1"].cmd(
"""
sudo ip link set dev r1-eth200 up
"""
)

pattern1 = '"administrativeStatus":"up"'
rtadv_output1 = tgen.gears["r1"].vtysh_cmd("show interface r1-eth200 json")
if pattern1 not in rtadv_output1:
sys.stderr.write(f"Interface state not up yet:: got {rtadv_output1}\n")
failures += 1
assert failures == 0, f"Test failed with {failures} failures"


def test_bgp_route_cleanup():
failures = 0
net = get_topogen().net
Expand Down
3 changes: 2 additions & 1 deletion zebra/interface.c
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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