Neigh suppression

[rjarry]

Depends on #566

Summary

• Added L2 bridge neighbor suppression for ARP and IPv6 Neighbor Solicitation: when a eligible remote, reachable L3 nexthop with a non-zero MAC exists, the datapath rewrites packets into ARP Reply/Neighbor Advertisement and forwards them instead of flooding (driven by the new NEIGH_SUPPRESS datapath edge and bridge_neigh_suppress graph node).
• Added neigh_suppress bridge configuration and CLI support, including enabling/disabling FLOOD/LEARN via positive flag semantics and validating the new GR_BRIDGE_F_NEIGH_SUPPRESS bit.
• Added neigh_snoop interface support so ARP/NDP processing is routed to control-plane neighbor-origin nexthop creation even when no existing remote nexthop is found (used to avoid dropping NA/solicitation paths on missing nexthops and to enable creating “neigh” origin nexthops).
• Extended nexthop API and lifecycle for neighbor-origin nexthops:
  • Introduced GR_NH_ORIGIN_NEIGH, changed GR_NH_T_L3 value, and updated nexthop deletion to use an embedded struct gr_nexthop (plus missing_ok) instead of nh_id.
  • Added GR_NH_FLUSH with struct gr_nh_flush_req { gr_nh_origin_t origin; }; implemented flush to iterate nexthops, filter by origin, clean up L3 routes, and decref until refs reach zero.
  • Updated nexthop lookup failure reporting to encode ENOENT when lookups fail.
• Implemented FRR/zebra neighbor-driven dplane updates for L3 nexthops:
  • Added grout_neigh_notify/grout_neigh_update_ctx() to emit zebra dplane neighbor install/delete operations for L3 nexthops carrying IP4/IP6 addresses with non-zero MACs, skipping local/link/multicast/flagged/zero-MAC cases.
  • For remote L3 nexthops, added helpers to send grout L3 nexthop add/delete requests based on neighbor events (including setting/unsetting GR_NH_F_REMOTE and using neighbor MAC/IP as appropriate).
  • Added EVPN RMAC caching tied to L3VNI via l3vni_rmac_set()/l3vni_rmac_del(), and added L3VNI-aware handling to redirect req->nh.iface_id to a VXLAN interface and apply cached RMAC into L3 nexthop info.
• Added generic flag iteration and updated macro usage across the codebase:
  • Introduced gr_flags_foreach(f, flags) and removed gr_nh_flags_foreach, updating callers to use the new helper.
  • Refactored nexthop type-op iteration using nexthop_type_ops_foreach.
• Updated API compatibility and tooling:
  • Bumped GR_API_VERSION to 5.
  • Added/updated CLI commands and argument parsing (neigh_snoop, nexthop flush, and corrected nexthop del ID parsing).
• Updated smoke tests and dependencies to validate suppression behavior and remote RMAC/next-hop state transitions:
  • Added bridge_neigh_suppress ARP suppression smoke test and added EVPN neighbor-suppression smoke test.
  • Updated FRR EVPN RMAC/next-hop event waits from nh new to nh update, and added arping to build dependencies.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

frr/rt_grout.c (1)

1148-1153: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Carry vrf_id in the key-based remote neighbor install/delete.

Both helpers leave req->nh.vrf_id unset, defaulting it to GR_VRF_ID_UNDEF. Because bridge neighbor suppression performs VRF-scoped lookups, these entries won't be matched correctly within the bridge's actual VRF.

• frr/rt_grout.c#L1148-L1153: Assign req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)); when building the add request.
• frr/rt_grout.c#L1191-L1194: Assign req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)); when building the delete request.

🐛 Proposed fixes

For neigh_install_nexthop:

 	req->exist_ok = true;
 	req->nh.type = GR_NH_T_L3;
 	req->nh.origin = zebra2origin(dplane_ctx_get_type(ctx));
 	req->nh.iface_id = iface_id;
+	req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx));
 	l3 = (struct gr_nexthop_info_l3 *)req->nh.info;

For neigh_delete_nexthop:

 	req->missing_ok = true;
 	req->nh.type = GR_NH_T_L3;
 	req->nh.iface_id = iface_id;
+	req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx));
 	l3 = (struct gr_nexthop_info_l3 *)req->nh.info;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frr/rt_grout.c` around lines 1148 - 1153, Set req->nh.vrf_id using
vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)) in both neigh_install_nexthop’s add
request and neigh_delete_nexthop’s delete request, alongside the existing
nexthop fields, so both key-based remote neighbor operations carry the active
VRF.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modules/l2/datapath/bridge_input.c`:
- Around line 95-102: Update the edge normalization at the next label in the
bridge input flow so both FLOOD and NEIGH_SUPPRESS outcomes are converted to
FLOOD_DISABLED when GR_BRIDGE_F_FLOOD is unset. Preserve the existing edge
selection and allow either edge to reach its normal handler when flooding is
enabled.

In `@smoke/evpn_l2l3vni_frr_test.sh`:
- Around line 222-225: Update each failure branch invoking fail in
smoke/evpn_l2l3vni_frr_test.sh at lines 222-225, 232-235, 243-246, and 256, and
smoke/evpn_l3vpn_vxlan6_frr_test.sh at lines 206-209, 216-219, 231, and 237.
Explicitly handle the nonzero result from fail within the surrounding loop, such
as breaking the loop or returning an appropriate status, so set -e does not
terminate the script before controlled cleanup or flow completion.

---

Duplicate comments:
In `@frr/rt_grout.c`:
- Around line 1148-1153: Set req->nh.vrf_id using
vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)) in both neigh_install_nexthop’s add
request and neigh_delete_nexthop’s delete request, alongside the existing
nexthop fields, so both key-based remote neighbor operations carry the active
VRF.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 877954ff-d2a3-4ed5-86c7-6155afd6e8f8

📥 Commits

Reviewing files that changed from the base of the PR and between fab7eb3 and becd129.

⛔ Files ignored due to path filters (1)

• docs/graph.svg is excluded by !**/*.svg

📒 Files selected for processing (29)

• .clang-format
• .github/workflows/check.yml
• README.md
• api/gr_api.h
• api/gr_macro.h
• frr/rt_grout.c
• modules/infra/api/gr_infra.h
• modules/infra/api/gr_nexthop.h
• modules/infra/api/nexthop.c
• modules/infra/cli/cli_iface.h
• modules/infra/cli/iface.c
• modules/infra/cli/nexthop.c
• modules/infra/control/nexthop.c
• modules/ip/control/nexthop.c
• modules/ip/datapath/arp_input_reply.c
• modules/ip6/control/nexthop.c
• modules/ip6/datapath/ndp_na_input.c
• modules/l2/api/gr_l2.h
• modules/l2/cli/bridge.c
• modules/l2/cli/fdb.c
• modules/l2/control/bridge.c
• modules/l2/datapath/bridge_input.c
• modules/l2/datapath/bridge_neigh_suppress.c
• modules/l2/datapath/meson.build
• smoke/bridge_neigh_suppress_test.sh
• smoke/evpn_l2l3vni_frr_test.sh
• smoke/evpn_l3vpn_frr_test.sh
• smoke/evpn_l3vpn_vxlan6_frr_test.sh
• smoke/evpn_neigh_suppress_frr_test.sh

🚧 Files skipped from review as they are similar to previous changes (13)

• .clang-format
• modules/l2/datapath/meson.build
• modules/l2/cli/fdb.c
• modules/infra/cli/iface.c
• api/gr_macro.h
• modules/l2/api/gr_l2.h
• modules/ip/datapath/arp_input_reply.c
• modules/ip6/datapath/ndp_na_input.c
• modules/l2/datapath/bridge_neigh_suppress.c
• modules/infra/api/nexthop.c
• modules/infra/cli/cli_iface.h
• modules/infra/control/nexthop.c
• modules/infra/cli/nexthop.c

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

♻️ Duplicate comments (1)

frr/rt_grout.c (1)

1148-1153: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Carry vrf_id in the key-based remote neighbor install/delete.

Both helpers leave req->nh.vrf_id unset, defaulting it to GR_VRF_ID_UNDEF. Because bridge neighbor suppression performs VRF-scoped lookups, these entries won't be matched correctly within the bridge's actual VRF.

• frr/rt_grout.c#L1148-L1153: Assign req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)); when building the add request.
• frr/rt_grout.c#L1191-L1194: Assign req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)); when building the delete request.

🐛 Proposed fixes

For neigh_install_nexthop:

 	req->exist_ok = true;
 	req->nh.type = GR_NH_T_L3;
 	req->nh.origin = zebra2origin(dplane_ctx_get_type(ctx));
 	req->nh.iface_id = iface_id;
+	req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx));
 	l3 = (struct gr_nexthop_info_l3 *)req->nh.info;

For neigh_delete_nexthop:

 	req->missing_ok = true;
 	req->nh.type = GR_NH_T_L3;
 	req->nh.iface_id = iface_id;
+	req->nh.vrf_id = vrf_frr_to_grout(dplane_ctx_get_vrf(ctx));
 	l3 = (struct gr_nexthop_info_l3 *)req->nh.info;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frr/rt_grout.c` around lines 1148 - 1153, Set req->nh.vrf_id using
vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)) in both neigh_install_nexthop’s add
request and neigh_delete_nexthop’s delete request, alongside the existing
nexthop fields, so both key-based remote neighbor operations carry the active
VRF.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modules/l2/datapath/bridge_input.c`:
- Around line 95-102: Update the edge normalization at the next label in the
bridge input flow so both FLOOD and NEIGH_SUPPRESS outcomes are converted to
FLOOD_DISABLED when GR_BRIDGE_F_FLOOD is unset. Preserve the existing edge
selection and allow either edge to reach its normal handler when flooding is
enabled.

In `@smoke/evpn_l2l3vni_frr_test.sh`:
- Around line 222-225: Update each failure branch invoking fail in
smoke/evpn_l2l3vni_frr_test.sh at lines 222-225, 232-235, 243-246, and 256, and
smoke/evpn_l3vpn_vxlan6_frr_test.sh at lines 206-209, 216-219, 231, and 237.
Explicitly handle the nonzero result from fail within the surrounding loop, such
as breaking the loop or returning an appropriate status, so set -e does not
terminate the script before controlled cleanup or flow completion.

---

Duplicate comments:
In `@frr/rt_grout.c`:
- Around line 1148-1153: Set req->nh.vrf_id using
vrf_frr_to_grout(dplane_ctx_get_vrf(ctx)) in both neigh_install_nexthop’s add
request and neigh_delete_nexthop’s delete request, alongside the existing
nexthop fields, so both key-based remote neighbor operations carry the active
VRF.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 877954ff-d2a3-4ed5-86c7-6155afd6e8f8

📥 Commits

Reviewing files that changed from the base of the PR and between fab7eb3 and becd129.

⛔ Files ignored due to path filters (1)

• docs/graph.svg is excluded by !**/*.svg

📒 Files selected for processing (29)

• .clang-format
• .github/workflows/check.yml
• README.md
• api/gr_api.h
• api/gr_macro.h
• frr/rt_grout.c
• modules/infra/api/gr_infra.h
• modules/infra/api/gr_nexthop.h
• modules/infra/api/nexthop.c
• modules/infra/cli/cli_iface.h
• modules/infra/cli/iface.c
• modules/infra/cli/nexthop.c
• modules/infra/control/nexthop.c
• modules/ip/control/nexthop.c
• modules/ip/datapath/arp_input_reply.c
• modules/ip6/control/nexthop.c
• modules/ip6/datapath/ndp_na_input.c
• modules/l2/api/gr_l2.h
• modules/l2/cli/bridge.c
• modules/l2/cli/fdb.c
• modules/l2/control/bridge.c
• modules/l2/datapath/bridge_input.c
• modules/l2/datapath/bridge_neigh_suppress.c
• modules/l2/datapath/meson.build
• smoke/bridge_neigh_suppress_test.sh
• smoke/evpn_l2l3vni_frr_test.sh
• smoke/evpn_l3vpn_frr_test.sh
• smoke/evpn_l3vpn_vxlan6_frr_test.sh
• smoke/evpn_neigh_suppress_frr_test.sh

🚧 Files skipped from review as they are similar to previous changes (13)

• .clang-format
• modules/l2/datapath/meson.build
• modules/l2/cli/fdb.c
• modules/infra/cli/iface.c
• api/gr_macro.h
• modules/l2/api/gr_l2.h
• modules/ip/datapath/arp_input_reply.c
• modules/ip6/datapath/ndp_na_input.c
• modules/l2/datapath/bridge_neigh_suppress.c
• modules/infra/api/nexthop.c
• modules/infra/cli/cli_iface.h
• modules/infra/control/nexthop.c
• modules/infra/cli/nexthop.c

🛑 Comments failed to post (2)

modules/l2/datapath/bridge_input.c (1)

95-102: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify if bridge_flood.c enforces GR_BRIDGE_F_FLOOD

rg "GR_BRIDGE_F_FLOOD" modules/l2/datapath/bridge_flood.c || echo "GR_BRIDGE_F_FLOOD check missing in bridge_flood.c"

Repository: DPDK/grout

Length of output: 198

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant files and inspect the control flow around the edges.
git ls-files modules/l2/datapath/bridge_input.c modules/l2/datapath/bridge_flood.c modules/l2/datapath/bridge_neigh_suppress.c

echo
echo "== bridge_input.c =="
ast-grep outline modules/l2/datapath/bridge_input.c --view expanded || true
echo
sed -n '1,180p' modules/l2/datapath/bridge_input.c

echo
echo "== bridge_neigh_suppress.c =="
ast-grep outline modules/l2/datapath/bridge_neigh_suppress.c --view expanded || true
echo
sed -n '1,220p' modules/l2/datapath/bridge_neigh_suppress.c

echo
echo "== bridge_flood.c =="
ast-grep outline modules/l2/datapath/bridge_flood.c --view expanded || true
echo
sed -n '1,220p' modules/l2/datapath/bridge_flood.c

Repository: DPDK/grout

Length of output: 13104

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check whether the flood edge is used anywhere else and whether the disable flag is enforced centrally.
rg -n "FLOOD_DISABLED|GR_BRIDGE_F_FLOOD|NEIGH_SUPPRESS|bridge_flood|bridge_neigh_suppress" modules/l2/datapath

Repository: DPDK/grout

Length of output: 1823

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read only the relevant line ranges if the files are large.
for f in modules/l2/datapath/bridge_input.c modules/l2/datapath/bridge_neigh_suppress.c modules/l2/datapath/bridge_flood.c; do
  echo "===== $f ====="
  wc -l "$f"
done

Repository: DPDK/grout

Length of output: 424

---

NEIGH_SUPPRESS still bypasses GR_BRIDGE_F_FLOOD modules/l2/datapath/bridge_input.c:95-102

When GR_BRIDGE_F_NEIGH_SUPPRESS is enabled, any frame bridge_neigh_suppress sends to its FLOOD edge reaches bridge_flood directly, and that path never applies the flood-disable flag. Flood-disabled bridges can still forward those frames; route that fallback to FLOOD_DISABLED instead.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/l2/datapath/bridge_input.c` around lines 95 - 102, Update the edge
normalization at the next label in the bridge input flow so both FLOOD and
NEIGH_SUPPRESS outcomes are converted to FLOOD_DISABLED when GR_BRIDGE_F_FLOOD
is unset. Preserve the existing edge selection and allow either edge to reach
its normal handler when flooding is enabled.

smoke/evpn_l2l3vni_frr_test.sh (1)

222-225: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Avoid if ... fail patterns under set -e to ensure controlled flow.

Based on learnings, calling fail (which returns 1) inside an if block or && chain under set -e causes the shell to exit immediately at the statement itself, terminating the script instead of allowing for controlled flow or cleanup.
Refactor these calls to explicitly handle the failure by breaking out of the loop or returning a status (e.g., fail "..." || break or fail "..." || exit 1) to prevent the implicit shell termination trap.

• smoke/evpn_l2l3vni_frr_test.sh#L222-L225: Refactor to explicitly handle the failure (e.g., fail "..." || break).
• smoke/evpn_l2l3vni_frr_test.sh#L232-L235: Refactor to explicitly handle the failure (e.g., fail "..." || break).
• smoke/evpn_l2l3vni_frr_test.sh#L243-L246: Refactor to explicitly handle the failure (e.g., fail "..." || break).
• smoke/evpn_l2l3vni_frr_test.sh#L256-L256: Refactor to explicitly handle the failure (e.g., fail "..." || break).
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L206-L209: Refactor to explicitly handle the failure (e.g., fail "..." || break).
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L216-L219: Refactor to explicitly handle the failure (e.g., fail "..." || break).
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L231-L231: Refactor to explicitly handle the failure (e.g., fail "..." || break).
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L237-L237: Refactor to explicitly handle the failure (e.g., fail "..." || break).

📍 Affects 2 files

• smoke/evpn_l2l3vni_frr_test.sh#L222-L225 (this comment)
• smoke/evpn_l2l3vni_frr_test.sh#L232-L235
• smoke/evpn_l2l3vni_frr_test.sh#L243-L246
• smoke/evpn_l2l3vni_frr_test.sh#L256-L256
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L206-L209
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L216-L219
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L231-L231
• smoke/evpn_l3vpn_vxlan6_frr_test.sh#L237-L237

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@smoke/evpn_l2l3vni_frr_test.sh` around lines 222 - 225, Update each failure
branch invoking fail in smoke/evpn_l2l3vni_frr_test.sh at lines 222-225,
232-235, 243-246, and 256, and smoke/evpn_l3vpn_vxlan6_frr_test.sh at lines
206-209, 216-219, 231, and 237. Explicitly handle the nonzero result from fail
within the surrounding loop, such as breaking the loop or returning an
appropriate status, so set -e does not terminate the script before controlled
cleanup or flow completion.

Source: Learnings

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modules/infra/api/gr_nexthop.h`:
- Around line 347-353: Update nh_flush and any equivalent nexthop iteration
loops to cache the result of nexthop_next(nh) before calling nexthop_decref(nh),
then continue from the cached pointer. Preserve filtering and
reference-decrement behavior while ensuring traversal remains complete after the
current nexthop is removed or freed.

In `@smoke/bridge_neigh_suppress_test.sh`:
- Around line 58-61: Prevent fail() from terminating the scripts unexpectedly
under set -e by updating both affected if blocks in
smoke/bridge_neigh_suppress_test.sh (lines 58-61 and 64-67) and all three
affected if blocks in smoke/evpn_neigh_suppress_frr_test.sh (lines 115-118,
129-132, and 172-175); disable errexit for each block and restore it afterward,
or use equivalent explicit error handling while preserving the existing failure
checks and cleanup flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: af05154d-f43a-4f61-869d-fd14dd3928d8

📥 Commits

Reviewing files that changed from the base of the PR and between becd129 and cdec80f.

⛔ Files ignored due to path filters (1)

• docs/graph.svg is excluded by !**/*.svg

📒 Files selected for processing (33)

• .clang-format
• .github/workflows/check.yml
• README.md
• api/gr_api.h
• api/gr_macro.h
• cli/complete.c
• cli/exec.c
• frr/rt_grout.c
• frr/zebra_dplane_grout.c
• modules/infra/api/gr_infra.h
• modules/infra/api/gr_nexthop.h
• modules/infra/api/nexthop.c
• modules/infra/cli/cli_iface.h
• modules/infra/cli/iface.c
• modules/infra/cli/nexthop.c
• modules/infra/control/nexthop.c
• modules/infra/control/port.c
• modules/ip/control/nexthop.c
• modules/ip/datapath/arp_input_reply.c
• modules/ip6/control/nexthop.c
• modules/ip6/datapath/ndp_na_input.c
• modules/l2/api/gr_l2.h
• modules/l2/cli/bridge.c
• modules/l2/cli/fdb.c
• modules/l2/control/bridge.c
• modules/l2/datapath/bridge_input.c
• modules/l2/datapath/bridge_neigh_suppress.c
• modules/l2/datapath/meson.build
• smoke/bridge_neigh_suppress_test.sh
• smoke/evpn_l2l3vni_frr_test.sh
• smoke/evpn_l3vpn_frr_test.sh
• smoke/evpn_l3vpn_vxlan6_frr_test.sh
• smoke/evpn_neigh_suppress_frr_test.sh

🚧 Files skipped from review as they are similar to previous changes (22)

• modules/l2/datapath/meson.build
• smoke/evpn_l3vpn_frr_test.sh
• .github/workflows/check.yml
• modules/infra/api/gr_infra.h
• api/gr_api.h
• api/gr_macro.h
• smoke/evpn_l3vpn_vxlan6_frr_test.sh
• smoke/evpn_l2l3vni_frr_test.sh
• modules/ip/control/nexthop.c
• modules/l2/api/gr_l2.h
• modules/l2/cli/fdb.c
• modules/infra/cli/cli_iface.h
• modules/l2/control/bridge.c
• modules/infra/cli/iface.c
• modules/ip6/control/nexthop.c
• README.md
• modules/l2/datapath/bridge_input.c
• modules/ip6/datapath/ndp_na_input.c
• modules/l2/datapath/bridge_neigh_suppress.c
• modules/l2/cli/bridge.c
• modules/infra/api/nexthop.c
• frr/rt_grout.c

[coderabbitai]

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Hazard: Iterator invalidation and use-after-free in downstream nh_flush handler.

The downstream API handler for GR_NH_FLUSH (nh_flush in modules/infra/api/nexthop.c) iterates using nh = nexthop_next(nh) and destroys matching nexthops via nexthop_decref(nh) within the loop.

If nexthop_decref destroys and frees the nexthop, passing it to nexthop_next on the next iteration leads to a use-after-free. Additionally, if nexthop_next relies on list/hash membership or nh_id (which is safely set to 0 for GR_NH_ORIGIN_NEIGH), the iteration may abort prematurely or skip elements once the current nh is removed from those structures.

Consider modifying nh_flush (and any similar iteration loops) to cache the next pointer before decrementing references to ensure safe traversal.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/infra/api/gr_nexthop.h` around lines 347 - 353, Update nh_flush and
any equivalent nexthop iteration loops to cache the result of nexthop_next(nh)
before calling nexthop_decref(nh), then continue from the cached pointer.
Preserve filtering and reference-decrement behavior while ensuring traversal
remains complete after the current nexthop is removed or freed.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Avoid fail() inside if blocks without disabling set -e.

Based on learnings, in scripts using set -e, a function returning non-zero inside an if block will cause the shell to exit immediately, abruptly terminating the script instead of allowing further execution or graceful cleanup. To fix this, wrap these blocks with set +e and set -e, or refactor to handle errors using explicit checks/controlled flow.

• smoke/bridge_neigh_suppress_test.sh#L58-L61: wrap the if block with set +e and set -e or refactor.
• smoke/bridge_neigh_suppress_test.sh#L64-L67: wrap the if block with set +e and set -e or refactor.
• smoke/evpn_neigh_suppress_frr_test.sh#L115-L118: wrap the if block with set +e and set -e or refactor.
• smoke/evpn_neigh_suppress_frr_test.sh#L129-L132: wrap the if block with set +e and set -e or refactor.
• smoke/evpn_neigh_suppress_frr_test.sh#L172-L175: wrap the if block with set +e and set -e or refactor.

🧰 Tools

🪛 Shellcheck (0.11.0)

[info] 58-58: Double quote to prevent globbing and word splitting.

(SC2086)

---

[info] 59-59: Double quote to prevent globbing and word splitting.

(SC2086)

📍 Affects 2 files

• smoke/bridge_neigh_suppress_test.sh#L58-L61 (this comment)
• smoke/bridge_neigh_suppress_test.sh#L64-L67
• smoke/evpn_neigh_suppress_frr_test.sh#L115-L118
• smoke/evpn_neigh_suppress_frr_test.sh#L129-L132
• smoke/evpn_neigh_suppress_frr_test.sh#L172-L175

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@smoke/bridge_neigh_suppress_test.sh` around lines 58 - 61, Prevent fail()
from terminating the scripts unexpectedly under set -e by updating both affected
if blocks in smoke/bridge_neigh_suppress_test.sh (lines 58-61 and 64-67) and all
three affected if blocks in smoke/evpn_neigh_suppress_frr_test.sh (lines
115-118, 129-132, and 172-175); disable errexit for each block and restore it
afterward, or use equivalent explicit error handling while preserving the
existing failure checks and cleanup flow.

Source: Learnings