T8426: FRR support for EVPN Anycast - #1199
Conversation
Add FRR patch for better MACVLAN support in FRR code.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis patch enhances FRR's zebra daemon to properly handle MACVLAN interfaces and VRR (Virtual Router Redundancy) in EVPN neighbor and MAC-IP programming. It preprocesses MACVLAN input to establish SVI context, extends EVPN neighbor dataplane programming to VRR interfaces, and conditions MAC-IP reading on VRR availability and advertise settings. ChangesMACVLAN and VRR EVPN Neighbor Support
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👍 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/package-build/frr/patches/frr/0024-zebra-enhance-MACVLAN-support-in-EVPN.patch (1)
135-138: 💤 Low valueMisleading debug message: "ignoring" is imprecise.
When linked interface lookup fails, the function still processes the neighbor locally via
zebra_neigh_add/zebra_neigh_delonifp. Only VXLAN operations are skipped. Consider:Suggested clarification
- zlog_debug(" Neighbor Entry received on MACVLAN %s, but linked interface not found, ignoring", + zlog_debug(" Neighbor Entry received on MACVLAN %s, but linked interface not found, skipping EVPN processing", ifp->name);🤖 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 `@scripts/package-build/frr/patches/frr/0024-zebra-enhance-MACVLAN-support-in-EVPN.patch` around lines 135 - 138, The debug message logged when linked-interface lookup fails is misleading because neighbor handling still continues locally via zebra_neigh_add/zebra_neigh_del on ifp while only VXLAN-related operations are skipped; update the zlog_debug call (guarded by IS_ZEBRA_DEBUG_KERNEL) to clearly state that the linked interface was not found and VXLAN handling will be skipped but the neighbor will still be processed on the MACVLAN (ifp) — e.g. change the message string to mention "linked interface not found; skipping VXLAN operations, processing neighbor on %s" so intent is explicit.
🤖 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.
Nitpick comments:
In
`@scripts/package-build/frr/patches/frr/0024-zebra-enhance-MACVLAN-support-in-EVPN.patch`:
- Around line 135-138: The debug message logged when linked-interface lookup
fails is misleading because neighbor handling still continues locally via
zebra_neigh_add/zebra_neigh_del on ifp while only VXLAN-related operations are
skipped; update the zlog_debug call (guarded by IS_ZEBRA_DEBUG_KERNEL) to
clearly state that the linked interface was not found and VXLAN handling will be
skipped but the neighbor will still be processed on the MACVLAN (ifp) — e.g.
change the message string to mention "linked interface not found; skipping VXLAN
operations, processing neighbor on %s" so intent is explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c2c8c4aa-87f2-4af1-947a-b97fd9dfec94
📒 Files selected for processing (1)
scripts/package-build/frr/patches/frr/0024-zebra-enhance-MACVLAN-support-in-EVPN.patch
📜 Review details
🧰 Additional context used
🔍 Remote MCP Context7
Summary of Relevant FRR/EVPN Context
Based on FRR documentation, here are the key architectural and operational context points relevant to reviewing this patch:
MACVLAN and VRRP Architecture in FRR
FRR's VRRP implementation relies on Linux macvlan devices to achieve the protocol's shared virtual MAC feature. These system interfaces must be configured externally before VRRP can be enabled on them, with each interface intended for VRRP requiring at least one macvlan device configured with the correct virtual MAC and set to bridge mode.
EVPN Anycast Gateway Configuration
L2VNI anycast gateways use a macvlan device created from the L2VNI with a shared MAC address across VTEPs, requiring a local FDB entry on the bridge for the anycast MAC to prevent it from traversing the overlay.
Neighbor Management and Kernel Interaction
The Zebra neighbor API includes functions for querying neighbor information from Zebra's cache via zclient_neigh_get() on a specific interface and address family, with Zebra responding asynchronously via ZEBRA_NEIGH_ADDED messages.
Key Technical Context for Review
The patch's modifications to zebra_evpn_read_mac_neigh, zebra_evpn_neigh.c, and zebra_neigh.c align with FRR's architecture where:
- MACVLAN devices act as SVIs linked to their parent interfaces
- Neighbor entries must be programmed through both the primary interface and any associated virtual interfaces (VRR/macvlan)
- Kernel neighbor operations (dplane) need to use the correct SVI interface context for proper programming
This context supports the patch's objective of treating MACVLAN-linked interfaces as SVIs and extending neighbor/MAC-IP programming to VRR interfaces, which is essential for EVPN Anycast support.
🔇 Additional comments (3)
scripts/package-build/frr/patches/frr/0024-zebra-enhance-MACVLAN-support-in-EVPN.patch (3)
29-46: LGTM!
48-96: LGTM!
147-204: LGTM!
|
I have checked this fix in lab and it works.
R2 has the same results. |
|
The implications of this PR are nontrivial and it has not been reviewed in FRR mainline yet. |
c-po
left a comment
c-po
left a comment
There was a problem hiding this comment.
Add patch for FRR which is pending upstream to add EVPN anycast gateway support to zebra
vyos-bot
Bot
added
mirror-initiated
5 participants
Change summary
Add FRR patch for better MACVLAN support in FRR code.
This version creates NOARP entry on both VLAN and MACVLAN, this is in line with other places in FRR + works.
Types of changes
Related Task(s)
Related PR(s)
How to test / Smoketest result
Test as described in related task.
Checklist: