[dualtor][intfsorch] Preserve port MAC when updating router interface MAC - #4816
Open
lolyu wants to merge 1 commit into
Open
[dualtor][intfsorch] Preserve port MAC when updating router interface MAC#4816lolyu wants to merge 1 commit into
lolyu wants to merge 1 commit into
Conversation
IntfsOrch::doTask() reconciles a router interface's SRC_MAC on every update: if the mac parsed from INTF_TABLE is empty (the zero sentinel written by intfmgr for interfaces without a per-interface MAC), it falls back to gMacAddress and, when that differs from the tracked MAC, issues a set_router_interface_attribute(SRC_MAC). The create path in setIntf() derives the tracked MAC from port.m_mac (falling back to gMacAddress only when the port has none), but the update path fell back straight to gMacAddress. For a VLAN SVI whose gateway MAC is configured on the VLAN (e.g. a dual-ToR shared MAC populated into port.m_mac from VLAN_TABLE), the two never agree: the RIF is created with the VLAN MAC, then the update immediately overwrites it with the switch MAC. On dual-ToR this replaces the shared gateway MAC with a per-device MAC and breaks seamless mux failover. Mirror the create path by preferring port.m_mac and only falling back to gMacAddress when the port has no MAC. This keeps the update symmetric with create so no spurious SRC_MAC set is generated. Behavior is unchanged for routed PHY/LAG interfaces (port.m_mac empty -> gMacAddress) and for SAG interfaces (intfmgr writes a non-zero anycast MAC, so the fallback is not taken). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates orchagent’s interface reconciliation logic to ensure IntfsOrch::doTask() preserves a port/VLAN’s configured MAC when updating a router interface’s SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS, avoiding unintended overwrites with the switch MAC (notably impacting dual-ToR VLAN SVI gateway MAC behavior).
Changes:
- Update
IntfsOrch::doTask()to preferport.m_macwhenmac_addrfromINTF_TABLEis unset/zero, falling back togMacAddressonly when the port has no MAC. - Align the interface update path behavior with the existing create path logic in
setIntf().
Comment on lines
+1081
to
+1084
| // Prefer the port's own MAC (e.g. a VLAN SVI's gateway MAC populated | ||
| // from VLAN_TABLE) and only fall back to the switch MAC when the port | ||
| // has none. | ||
| mac = port.m_mac ? port.m_mac : gMacAddress; |
tahmed-dev
approved these changes
Aug 5, 2026
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
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.
What I did
Made
IntfsOrch::doTask()prefer the port's own MAC (port.m_mac) when reconciling a router interface'sSAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS, only falling back to the switch MAC (gMacAddress) when the port has no MAC — mirroring what the create path insetIntf()already does.Why I did it
#4615 ([EVPN-MH] Integrate EVPN-MH with existing modules, f0c53b9) added an "update mac if it is changed" block to IntfsOrch::doTask() that reconciles the RIF's SRC_MAC on every interface update. When the MAC parsed from INTF_TABLE is empty (the 00:00:00:00:00:00 sentinel that intfmgr writes for interfaces without a per-interface MAC), it falls back to gMacAddress and, if that differs from the tracked MAC, issues a set_router_interface_attribute(SRC_MAC).
The problem is an asymmetry between the create and update paths:
For a VLAN SVI whose gateway MAC is configured on the VLAN (populated into port.m_mac from VLAN_TABLE), the two never agree:
On dual-ToR, replacing the shared VLAN gateway MAC with a per-device MAC breaks seamless mux failover (each ToR would answer the server's gateway ARP with a different MAC, so a switchover stales the server's ARP cache). This is a regression introduced in #4615; releases predating it never issued the set (the old code was guarded by if (mac), so the zero sentinel simply meant "don't touch the MAC").
The fix makes the update path fall back exactly like the create path, so the reconciliation MAC matches what the RIF was created with and no spurious set is generated.
How I verified it
Build and validate on dualtor device.
Details if related