Skip to content

Revert "Avoid nhgroup update when mux state changes"#4030

Merged
prsunny merged 1 commit intomasterfrom
revert-3822-mux_avoid_nhgroup_update
Dec 4, 2025
Merged

Revert "Avoid nhgroup update when mux state changes"#4030
prsunny merged 1 commit intomasterfrom
revert-3822-mux_avoid_nhgroup_update

Conversation

@yxieca
Copy link
Copy Markdown
Contributor

@yxieca yxieca commented Dec 3, 2025

Reverts #3822

This change is blocking sonic-swss submodule advance for 2 weeks.

Copilot AI review requested due to automatic review settings December 3, 2025 02:01
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@yxieca yxieca requested review from Ndancejic and prsunny December 3, 2025 02:02
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@yxieca
Copy link
Copy Markdown
Contributor Author

yxieca commented Dec 3, 2025

@manamand2020 Please chime in.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reverts PR #3822, restoring nexthop group update operations during mux state changes. The revert is being performed to unblock sonic-swss submodule advancement that has been blocked for 2 weeks.

  • Restores nexthop group invalidation and validation calls in MuxNbrHandler::enable()
  • Restores nexthop group invalidation and validation calls in MuxNbrHandler::disable()
  • Re-adds ECMP nexthop member reference counting during state transitions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@prsunny
Copy link
Copy Markdown
Collaborator

prsunny commented Dec 3, 2025

@Ndancejic, @zjswhhh , dualtor mock tests are failing . Can you check?

@prsunny
Copy link
Copy Markdown
Collaborator

prsunny commented Dec 3, 2025

@manamand2020
Copy link
Copy Markdown
Contributor

@manamand2020 Please chime in.
https://elastictest.org/scheduler/publictestplan/6920093d05c827bcee33e11a?testcase=dualtor%2Ftest_orchagent_active_tor_downstream.py&type=console

I think this seems to be happening because the neighbor remains in use inside the ECMP group due to the change in the original PR while we try to remove the entry from disableneighbors() which throws this error. We should have some DVS test to cover this. This change is fine though when we do not disable neighbors as part of no-host-route based mux neighbors.

@prsunny
Copy link
Copy Markdown
Collaborator

prsunny commented Dec 3, 2025

@manamand2020 Please chime in.
https://elastictest.org/scheduler/publictestplan/6920093d05c827bcee33e11a?testcase=dualtor%2Ftest_orchagent_active_tor_downstream.py&type=console

I think this seems to be happening because the neighbor remains in use inside the ECMP group due to the change in the original PR while we try to remove the entry from disableneighbors() which throws this error. We should have some DVS test to cover this. This change is fine though when we do not disable neighbors as part of no-host-route based mux neighbors.

@manamand2020 , while the change is good with no-host-route, its failing currently on disableneighbors flow. May be we can introduce this as part of no-host-route change. agree?

@manamand2020
Copy link
Copy Markdown
Contributor

@manamand2020 Please chime in.
https://elastictest.org/scheduler/publictestplan/6920093d05c827bcee33e11a?testcase=dualtor%2Ftest_orchagent_active_tor_downstream.py&type=console

I think this seems to be happening because the neighbor remains in use inside the ECMP group due to the change in the original PR while we try to remove the entry from disableneighbors() which throws this error. We should have some DVS test to cover this. This change is fine though when we do not disable neighbors as part of no-host-route based mux neighbors.

@manamand2020 , while the change is good with no-host-route, its failing currently on disableneighbors flow. May be we can introduce this as part of no-host-route change. agree?

Agreed.

@prsunny prsunny merged commit 83adbd9 into master Dec 4, 2025
19 of 22 checks passed
kalash-nexthop pushed a commit to kalash-nexthop/sonic-swss that referenced this pull request Dec 16, 2025
…sonic-net#4030)

Reverts sonic-net#3822

This change is blocking sonic-swss submodule advance for 2 weeks.

Signed-off-by: Kalash Nainwal <[email protected]>
Pterosaur pushed a commit to Janetxxx/sonic-swss that referenced this pull request Jan 6, 2026
…sonic-net#4030)

Reverts sonic-net#3822

This change is blocking sonic-swss submodule advance for 2 weeks.
Pterosaur pushed a commit to Janetxxx/sonic-swss that referenced this pull request Jan 6, 2026
…sonic-net#4030)

Reverts sonic-net#3822

This change is blocking sonic-swss submodule advance for 2 weeks.
baorliu pushed a commit to baorliu/sonic-swss that referenced this pull request Feb 23, 2026
…sonic-net#4030)

Reverts sonic-net#3822

This change is blocking sonic-swss submodule advance for 2 weeks.

Signed-off-by: Baorong Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants