Skip to content

Added Change to Skip Route Programming if NH is link/oper down#3520

Merged
prsunny merged 6 commits intosonic-net:masterfrom
abdosi:nhop_link
Feb 24, 2025
Merged

Added Change to Skip Route Programming if NH is link/oper down#3520
prsunny merged 6 commits intosonic-net:masterfrom
abdosi:nhop_link

Conversation

@abdosi
Copy link
Contributor

@abdosi abdosi commented Feb 18, 2025

What I did:
Added Change to Skip Route Programming if NH is link/oper down. With Scale Route testing of 60K+ routes when we toggle all the interfaces[14+ interface back to back] as done here: https://github.com/sonic-net/sonic-mgmt/blob/master/tests/snappi_tests/multidut/bgp/test_bgp_outbound_uplink_multi_po_flap.py we see because of slowness of FRR Route APP_DB processing compare to Link Notification Handling where we have updated the Nexthop Group as part of Link Notification handling to point to default route via #3389 [if eligible] FRR slowness can reprogram the Route back to Nexthop which is link down.

This change is similar to #3394 which was done for Nexthop Group.

How I verify:
Manual Verification and Ixia tetsing
Also UT has been updated to make sure it passes after this change.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi abdosi requested a review from prsunny as a code owner February 18, 2025 21:36
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

arlakshm
arlakshm previously approved these changes Feb 20, 2025
Copy link
Contributor

@arlakshm arlakshm left a comment

Choose a reason for hiding this comment

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

lgtm

return false;
}
}
else if (m_neighOrch->isNextHopFlagSet(nexthop, NHFLAGS_IFDOWN))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just thinking if it impact tunnel route in dualtor case if NHFLAG is down? @Ndancejic , can you check this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think tunnel routes should be safe, I believe line 1839 would catch interface nexthops. also I think routes to tunnel nh get skipped in doTask:

/* skip direct routes to tun0 */
else if (alsv[0] == "tun0")
{
    it = consumer.m_toSync.erase(it);
}

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This reverts commit ac88093.
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit 82632ea into sonic-net:master Feb 24, 2025
15 checks passed
@abdosi abdosi deleted the nhop_link branch February 24, 2025 21:01
@abdosi
Copy link
Contributor Author

abdosi commented Feb 24, 2025

@arlakshm added label for msft/202405

@arlakshm
Copy link
Contributor

@abdosi, looks like there is a cherry-pick conflict

@kperumalbfn
Copy link
Contributor

@abdosi could you resolve the conflicts for 202411

@r12f
Copy link

r12f commented Feb 27, 2025

Hi @abdosi , I wonder if you could help with the cherry pick to 202411? Once it lands there, it will be automatically merged into 202412.

@r12f
Copy link

r12f commented Feb 27, 2025

Creating change directly to 202412 will create a merge conflict in later merges and break the automation. : D

arlakshm added a commit to Azure/sonic-swss.msft that referenced this pull request Feb 28, 2025
Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025
…-net#3520)

*What I did:
Added Change to Skip Route Programming if NH is link/oper down. With Scale Route testing of 60K+ routes when we toggle all the interfaces[14+ interface back to back] as done here: https://github.com/sonic-net/sonic-mgmt/blob/master/tests/snappi_tests/multidut/bgp/test_bgp_outbound_uplink_multi_po_flap.py we see because of slowness of FRR Route APP_DB processing compare to Link Notification Handling where we have updated the Nexthop Group as part of Link Notification handling to point to default route via sonic-net#3389 [if eligible] FRR slowness can reprogram the Route back to Nexthop which is link down.

This change is similar to sonic-net#3394 which was done for Nexthop Group.
baorliu pushed a commit to baorliu/sonic-swss that referenced this pull request Feb 23, 2026
…-net#3520)

*What I did:
Added Change to Skip Route Programming if NH is link/oper down. With Scale Route testing of 60K+ routes when we toggle all the interfaces[14+ interface back to back] as done here: https://github.com/sonic-net/sonic-mgmt/blob/master/tests/snappi_tests/multidut/bgp/test_bgp_outbound_uplink_multi_po_flap.py we see because of slowness of FRR Route APP_DB processing compare to Link Notification Handling where we have updated the Nexthop Group as part of Link Notification handling to point to default route via sonic-net#3389 [if eligible] FRR slowness can reprogram the Route back to Nexthop which is link down.

This change is similar to sonic-net#3394 which was done for Nexthop Group.

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants