Skip to content

[swss]: BFD hardware offload for BGP sessions - #3267

Open
baorliu wants to merge 393 commits into
sonic-net:masterfrom
baorliu:baorliu_bgp_bfd
Open

[swss]: BFD hardware offload for BGP sessions#3267
baorliu wants to merge 393 commits into
sonic-net:masterfrom
baorliu:baorliu_bgp_bfd

Conversation

@baorliu

@baorliu baorliu commented Aug 27, 2024

Copy link
Copy Markdown
Contributor

[Notes] some other commits and files were pushed into this branch by mistake when trying to fix a DCO error, this PR is under repair. for design changes, please refer to the following PR before this one is fixed;
#4253

What I did
using hardware offloaded BFD to replace frr/bfdd for BGP neighbor reachability detection.

Why I did it
Hardware offloaded BFD usually provide more bfd sessions support, and has shorter detection time comparing to software bfd. and also lower CPU load because that BFD sessions are offloaded to hardware/ASIC.

How I verified it
The design was built into a sonic image, and verified in a hardware testbed.

Details if related
This program is built under sonic-swss, but running in bgp container.

HLD in PR review:
sonic-net/SONiC#1599

[notes] some commit history are lost after a force push after fixing a DCO error.
git log before the force push:

commit 42c0b3ef874d3f58c10e03b29cdce29876cb497a
Author: Baorong Liu <96146196+baorliu@users.noreply.github.com>
Date:   Mon Feb 23 11:51:51 2026 -0800

    fix makefile
    
    Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
commit 84d4c1bac48e93e3c6fcf80ddda75d4325e61484
Author: Baorong Liu <96146196+baorliu@users.noreply.github.com>
Date:   Tue Apr 29 13:07:18 2025 -0700

    add ut for bfdsyncd
    
    Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
commit 32db91f472501b67f42b74dc16f3a5ece9c49c23
Author: Baorong Liu <96146196+baorliu@users.noreply.github.com>
Date:   Wed Feb 12 12:03:23 2025 -0800

    handle bgp restart case
    
    Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>

commit d9d71e8da3c2b3e7ae14557a01d3abed1d0eaa06
Author: Baorong Liu <96146196+baorliu@users.noreply.github.com>
Date:   Mon Nov 4 15:16:34 2024 -0800

    remove some debugging code
    
    Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>

commit 4e608ca1d163454dd0d70d1fa50e5d1a53839ad6
Author: Baorong Liu <baorliu@cisco.com>
Date:   Mon Aug 26 21:14:19 2024 -0700

    bgp bfd hardware offload initial commit
    
    Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>

@baorliu
baorliu requested a review from prsunny as a code owner August 27, 2024 04:23
@siqbal1986
siqbal1986 self-requested a review September 3, 2024 03:34
@siqbal1986

Copy link
Copy Markdown
Contributor

can you please make sure that the build is passing. Also can you add soem swss tests to verify functionality

@baorliu baorliu changed the title [swss]: bgp bfd hardware offload initial commit [swss]: BFD hardware offload for BGP sessions Sep 15, 2024
@prsunny
prsunny requested a review from kperumalbfn September 19, 2024 18:57
@baorliu
baorliu marked this pull request as draft November 4, 2024 23:19
@baorliu

baorliu commented Nov 4, 2024

Copy link
Copy Markdown
Contributor Author

convert it to draft due to some planned update

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@baorliu
baorliu marked this pull request as ready for review February 25, 2025 22:23
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@baorliu

baorliu commented Apr 29, 2025

Copy link
Copy Markdown
Contributor Author

can you please make sure that the build is passing. Also can you add soem swss tests to verify functionality

added mock test.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@baorliu

baorliu commented Feb 23, 2026

Copy link
Copy Markdown
Contributor Author

can you please make sure that the build is passing. Also can you add soem swss tests to verify functionality

mock test was added. The feature is in some production image and no build issue. will monitor pipeline build issue here.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

…et#3440)

What I did:
- Updated stpmgr.h to replace packed structs with aligned structs
- Replaced the old processStpPortAttr in stpmgr.cpp with a version
  that uses properly aligned STP_PORT_CONFIG_MSG allocations

How I did it:
- Applied the same alignment fixes from 202211 branch to master
- Removed #pragma GCC diagnostic usage and introduced ALIGNED(4)
  as well as padding fields in the relevant STP_* structures

Why I did it:
- To resolve address-of-packed-member warnings that break builds
  with certain compiler/platform configurations (e.g., Broadcom)
- Aligning data structures properly avoids potential misalignment errors

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
mint570 and others added 9 commits February 23, 2026 12:32
…ries.

Signed-off-by: mint570 <runmingwu@google.com>
Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
What I did
Remove deprecated parameters.

Why I did it
The parameter, print_to_console, has been removed, but the CI will not run any bench program, so this error was ignored.

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
…n table entries.

Signed-off-by: mint570 <runmingwu@google.com>
Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>
Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
What I did
Added support for polling PORT PHY attributes using the flex counter infrastructure.
Following changes are added in swss to support this feature

define a new counter type PORT_PHY_ATTR
support added for polling following attributes,
SAI_PORT_ATTR_RX_SIGNAL_DETECT,
SAI_PORT_ATTR_FEC_ALIGNMENT_LOCK,
SAI_PORT_ATTR_RX_SNR
Add port_phy_attr_manager to set counter id list in FLEX_COUNTERS_DB
Added unit tests to validate the enable/disable code flow, generatePortAttrCounterMap and QueryPortAttrCapabilitiesWithMockedSAI.

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
…s is Inband port (sonic-net#4157)

What I did
Fixed the bug reported in sonic-net/sonic-buildimage#25211
Fixed the bug introduced in sonic-net#4054 which was preventing the ever flow mirror sessions becoming active in remote Asics. When the nexthop is added in remote asics in Voq systems, it is always added against the Inband port and not on the remote system port. So when the nexthop is resolved for the mirror session in the remote asic, the mirror code calls getNeighborEntry to find the nexthop entry by passing the nexthop of the route entry for the mirror destination. Since the nexthop was added on Inband port and getNeighborEntry was checking for RemoteSystemPort, finding nexthop was failing and hence the mirror session never gets activated in remote asics. So modified the code to include the isInbandPort check in addition to isRemoteSystemPortIntf . The isRemoteSystemPortIntf might be needed since the neighbor entries are added against the remote system port.
Why I did it
Fixed the bug introduced in sonic-net#4054 which prevents the ever flow mirror sessions becoming active in remote Asics. When the nexthop is added in remote asics in Voq systems, it is always added against the Inband port and not on the remote system port. So when the nexthop is resolved for the mirror session in the remote asic, the mirror code calls getNeighborEntry to find the nexthop entry by passing the nexthop of the route entry for the mirror destination. Since the nexthop was added on Inband port and getNeighborEntry was checking for RemoteSystemPort, finding nexthop was failing and hence the mirror session never gets activated in remote asics. So modified the code to include the isInbandPort check in addition to isRemoteSystemPortIntf . The isRemoteSystemPortIntf might be needed since the neighbor entries are added against the remote system port.

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
…cast.

Signed-off-by: mint570 <runmingwu@google.com>
Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
What I did
Support route update from vnet tunnel route config change for custom_bfd monitoring. In DPU repairing scenario (to form HA sets), VNetOrch needs to support live update of endpoints, monitoring endpoints, primary and secondary groups.

sign-off: Jing Zhang zhangjing@microsoft.com

Why I did it
This change is required for DPU repair in HA.

How I did it
When there is a route config change (i.e. primary endpoint changes), pass a BFD state update with Init state to trigger the monitoring session creation & deletion, and route update. Notice that monitoring orch will pass Down and Up state to vnet orch.

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
* [ci] Install pytest and redis from Debian

For the CI runner, install the python3 pytest and redis packages from
the Debian repos instead of via pip. This has the advantage of the
version being fixed for the Debian version the test runner is using.

This works around the regression for Unix sockets introduced in redis
7.2.0 (see also redis/redis-py#3957 and redis/redis-py#3957).

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@linux-foundation-easycla

linux-foundation-easycla Bot commented Feb 23, 2026

Copy link
Copy Markdown

CLA Not Signed

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@baorliu

baorliu commented Feb 23, 2026

Copy link
Copy Markdown
Contributor Author

messed up the PR by trying to solve DCO error ... trying to recover this PR now

followed this instruction to solve DCO error, and the branch get messed up:
https://github.com/sonic-net/sonic-swss/pull/3267/checks?check_run_id=64583499903

There are 2 commits incorrectly signed off. This means that the author(s) of these commits failed to include a Signed-off-by line in their commit message.

To avoid having PRs blocked in the future, always include Signed-off-by: Author Name <authoremail@example.com> in every commit message. You can also do this automatically by using the -s flag (i.e., git commit -s).

Here is how to fix the problem so that this code can be merged.

Rebase the branch
If you have a local git environment and meet the criteria below, one option is to rebase the branch and add your Signed-off-by lines in the new commits. Please note that if others have already begun work based upon the commits in this branch, this solution will rewrite history and may cause serious issues for collaborators ([described in the git documentation](https://git-scm.com/book/en/v2/Git-Branching-Rebasing) under "The Perils of Rebasing").

You should only do this if:

You are the only author of the commits in this branch
You are absolutely certain nobody else is doing any work based upon this branch
There are no empty commits in the branch (for example, a DCO Remediation Commit which was added using --allow-empty)
To add your Signed-off-by line to every commit in this branch:

Ensure you have a local copy of your branch by [checking out the pull request locally via command line](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally).
In your local branch, run: git rebase HEAD~10 --signoff
Force push your changes to overwrite the branch: git push --force-with-lease origin baorliu_bgp_bfd

@baorliu

baorliu commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

can you please make sure that the build is passing. Also can you add soem swss tests to verify functionality

mock tests added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.