[patches-sonic]: Drop the bridge FDB protocol field patch - #619
Draft
tahmed-dev wants to merge 3 commits into
Draft
[patches-sonic]: Drop the bridge FDB protocol field patch#619tahmed-dev wants to merge 3 commits into
tahmed-dev wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
The PR direction looks right, but the commit text says fdbsyncd still spells "proto hw" and probes/falls back when the kernel does not accept it. The matching SWSS PR appears to remove the bridge FDB proto probe/path and simply omits proto on the legacy kernel path. Can we update the commit message / PR description to say the legacy kernel path intentionally runs without the SONiC FDB protocol extension, matching the pre-proto behavior? |
pbrisset
reviewed
Sep 2, 2026
pbrisset
left a comment
pbrisset
left a comment
There was a problem hiding this comment.
Can we confirm the remaining patch stack applies cleanly and boots with only 0001-vxlan-bridge-Add-NDA_FLAGS_EXT-support-with-NTF_EXT_... left under the EVPN MH section? The important validation is that NTF_EXT_MH_PEER_SYNC still works after removing the protocol and extern_valid patches.
Nothing consumes the flag. It exists for EVPN multihoming neighbour proxy advertisement, and FRR implements that in user space through its own ZEBRA_NEIGH_ES_PEER_PROXY state rather than asking the kernel, so neither FRR nor any SONiC daemon references NTF_EXT_VALIDATED or extern_valid. The series note claimed the flag was already upstream at 6.12, but it is absent from the 6.12.41 tree we build, so that was not the reason to drop it either way. The EVPN MH patches that remain are load bearing: NDA_FLAGS_EXT carries NTF_E_MH_PEER_SYNC, and the bridge FDB protocol field is what records whether a MAC was learned by the hardware or by zebra. Signed-off-by: Tamer Ahmed <tamerahmed@microsoft.com>
NDA_PROTOCOL on a bridge FDB entry is not upstream, and nothing needs it any more. Where a MAC came from now travels over the FPM channel, so zebra no longer has to read an origin tag back off the kernel entry, and it no longer writes one. fdbsyncd still spells "proto hw" on its kernel path, but it probes for support at startup and drops the tag when the kernel does not accept it, so removing this patch degrades that path rather than breaking it. The NDA_FLAGS_EXT patch stays: it carries NTF_EXT_MH_PEER_SYNC for EVPN multihoming, which is a separate feature and is not upstream either. Signed-off-by: Tamer Ahmed <tamerahmed@microsoft.com>
tahmed-dev
force-pushed
the
tahmed/fpm-mac-learning-master
branch
from
September 9, 2026 19:34
b988d6e to
a575e64
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
…patch" This reverts commit 2ff7429. The patch was not unused. The original assessment searched for the symbol NTF_EXT_VALIDATED, found no references in FRR, and concluded the patch was dead. These flags couple by bit value inside a netlink policy mask, not by symbol name. zebra sends NTF_E_MH_PEER_SYNC, which FRR defines as 0x04. In the kernel uapi that bit is NTF_EXT_EXT_VALIDATED (1 << 2); NTF_EXT_MH_PEER_SYNC is (1 << 3). Removing this patch narrowed NTF_EXT_MASK from 0x0D to 0x09, and nda_policy[NDA_FLAGS_EXT] is NLA_POLICY_MASK(NLA_U32, NTF_EXT_MASK), which rejects out-of-mask bits with -EINVAL. Every RTM_NEWNEIGH zebra sends for a peer-synced neighbour is therefore refused, the entry is never refreshed and ages to NUD_FAILED. The visible effect is that EVPN multi-homing failover breaks: when an Ethernet Segment link goes down, traffic does not move to the peer leaf. Reported by Cisco against test_portchannel_shut_noshut. Signed-off-by: Tamer Ahmed <tamerahmed@microsoft.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
tahmed-dev
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why I did it
0002-net-bridge-vxlan-Protocol-field-in-bridge-fdb.patchexists only to supportMAC learning through the Linux bridge FDB. With MAC synchronization moved onto the
FPM channel it is no longer needed, and carrying out-of-tree kernel patches has an
ongoing cost at every kernel rebase.
It adds an
NDA_PROTOCOLfield to the bridge/vxlan FDB and, more significantly,makes deletes protocol-scoped so control-plane and data-plane entries can coexist.
That arbitration belongs in SONiC, not in the kernel, and nothing issues those
protocol-scoped deletes once MAC state is carried over FPM.
Note on NTF_EXT_VALIDATED
An earlier revision of this PR also dropped
0003-neighbor-Add-NTF_EXT_VALIDATED-flag-for-externally-v.patchon the groundsthat it was unused. That was wrong and has been reverted in this PR.
The search was for the symbol name, which has no users in FRR. These flags couple
by bit value inside a netlink policy mask, not by symbol. zebra sends
NTF_E_MH_PEER_SYNC, which FRR defines as0x04; in the kernel uapi that bit isNTF_EXT_EXT_VALIDATED (1 << 2), whileNTF_EXT_MH_PEER_SYNCis(1 << 3).Dropping the patch narrowed
NTF_EXT_MASKfrom0x0Dto0x09, andnda_policy[NDA_FLAGS_EXT]isNLA_POLICY_MASK(NLA_U32, NTF_EXT_MASK), whichrejects out-of-mask bits with
-EINVAL. EveryRTM_NEWNEIGHzebra sends for apeer-synced neighbour was refused, so the entry aged to
NUD_FAILEDand EVPNmultihoming failover broke.
Measured on a two-leaf setup, before and after the revert:
The control message, sent without
NDA_FLAGS_EXT, returnederrno=0in bothcases, so only the flag bit was at issue.
How I did it
Remove
0002-net-bridge-vxlan-Protocol-field-in-bridge-fdb.patchand its entryfrom
patches-sonic/series.0001-vxlan-bridge-Add-NDA_FLAGS_EXT-support-with-NTF_EXT_.patchand0003-neighbor-Add-NTF_EXT_VALIDATED-flag-for-externally-v.patchare bothretained. The series comment above
0003now records why it must not bedropped while on 6.12.
How to verify it
The remaining series applies cleanly and the kernel builds. Verified on an Arista
7060X6-64PE-B pair, including EVPN-MH Ethernet Segment link failure.
Dependency / merge order
This PR must merge LAST. On master today,
sonic-swssfdbsyncd still containsthe kernel FDB protocol plumbing and
sonic-buildimagestill ships the iproute2protopatch that this kernel patch supports. Merging this first would removekernel support that shipping code still depends on.
Required to merge first:
sonic-swss: drops the dead kernel FDB protocol plumbingsonic-buildimage: drops the iproute2 bridge FDB protocol field patchWhich release branch to backport
None requested at this time.
Description for the changelog
Drop the bridge FDB protocol field kernel patch.