sonic-swss: bfdorch changes to support software bfd sessions #3406

abdbaig · 2024-12-01T22:04:16Z

What I did

Added logic in bfdorch to check for switch_type value of dpu and if it's dpu, program BFD sessions in a new software BFD session table in STATE_DB, instead of programming sessions in the HW through ASIC_DB. This table will be monitored by bgpcfgd which will program BFD sessions in FRR accordingly.
Added pytest testcases

Why I did it

As part of the Smartswitch project, BFD sessions need to be run between DPU and NPU but DPU doesn't currently support BFD hardware offload.
HLD: https://github.com/kperumalbfn/SONiC/blob/master/doc/smart-switch/BFD/SmartSwitchDpuLivenessUsingBfd.md

HA manager (hamgrd) will be responsible for programming BFD sessions in the DPU by programming BFD_SESSION_TABLE in APP_DB (just like it's currently done for NPU), but bfdorch needs to now program software BFD sessions if it's running on the DPU.

How I verified it

Added a new test_soft_bfd.py test file and verified the tests passed.

Details if related

siqbal1986

overall looks good. jsut a few minor comments

siqbal1986 · 2024-12-10T18:15:03Z

orchagent/bfdorch.cpp

 extern sai_bfd_api_t*       sai_bfd_api;
 extern sai_object_id_t      gSwitchId;
 extern sai_object_id_t      gVirtualRouterId;
 extern PortsOrch*           gPortsOrch;
 extern sai_switch_api_t*    sai_switch_api;
 extern Directory<Orch*>     gDirectory;
+extern string               gMySwitchType;
+


why extra space

orchagent/bfdorch.cpp

siqbal1986 · 2024-12-10T19:44:17Z

tests/test_soft_bfd.py

+
+        # Check BFD session 3 in STATE_DB
+        createdSessions = self.get_exist_bfd_session() - bfdSessions
+        bfdSessions = self.get_exist_bfd_session()


can you please add a test in test_vnet.py where a vnet route with BFD session is created and added, that way we can be sure that it works fine with VNET routes as well.

@prsunny is software BFD on DPU over VNET a requirement? Currently not supported

siqbal1986 · 2024-12-10T19:46:37Z

orchagent/bfdorch.cpp

@@ -81,6 +100,21 @@ BfdOrch::~BfdOrch(void)
    SWSS_LOG_ENTER();
 }

+std::string BfdOrch::replaceFirstTwoColons(const std::string &input) {


can you please rename to something like createStateDBKey.

tests/test_soft_bfd.py

mssonicbld · 2024-12-16T22:34:13Z

/azp run

azure-pipelines · 2024-12-16T22:34:24Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2024-12-16T22:36:32Z

/azp run

azure-pipelines · 2024-12-16T22:36:43Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2024-12-18T19:43:37Z

/azp run

azure-pipelines · 2024-12-18T19:43:47Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-02T15:00:04Z

/azp run

azure-pipelines · 2025-01-02T15:00:20Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-10T15:43:32Z

/azp run

azure-pipelines · 2025-01-10T15:43:42Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-13T21:40:21Z

/azp run

azure-pipelines · 2025-01-13T21:40:31Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-15T23:25:16Z

/azp run

azure-pipelines · 2025-01-15T23:25:27Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-06T22:36:02Z

/azp run

azure-pipelines · 2025-02-06T22:36:13Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-07T15:00:33Z

/azp run

azure-pipelines · 2025-02-07T15:00:45Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-07T17:53:40Z

/azp run

azure-pipelines · 2025-02-07T17:53:51Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-07T23:21:31Z

/azp run

azure-pipelines · 2025-02-07T23:21:41Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-10T15:03:10Z

/azp run

azure-pipelines · 2025-02-10T15:03:21Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2025-02-10T18:34:00Z

orchagent/bfdorch.cpp

-
+    // Clean up state database software BFD entries
+    if (gMySwitchType == "dpu") {
+        m_stateSoftBfdSessionTable->getKeys(keys);


We don't have to check for 'dpu' here. Clean it anyways based on getKeys function

prsunny · 2025-02-10T18:36:13Z

orchagent/bfdorch.cpp

@@ -101,6 +130,13 @@ void BfdOrch::doTask(Consumer &consumer)

        if (op == SET_COMMAND)
        {
+            if (gMySwitchType == "dpu") {


IMO, this should be based on capability query and not based on 'dpu'. This check lacks flexibility if we've to enable software bfd for any future use case. Suggest doing a capability check via SAI api for offload and if it returns NOT Supported or implemented, then enable softbfd.

The current design checks for switch type DPU in bgpcfgd and bfdmon as well: sonic-net/sonic-buildimage#20981. Additionally, the SAI call in VS reports that offload is supported; the pytests run on VS so there would be no way to cover and test the new code.

It should've been based on feature flag. bfdmon to run if software bfd feature is enabled. Since its already merged, we may have to take it as a different PR. @kperumalbfn for viz

prsunny · 2025-02-10T18:37:15Z

orchagent/bfdorch.h

    swss::NotificationConsumer* m_bfdStateNotificationConsumer;
    bool register_state_change_notif;
    std::map<std::string, vector<FieldValueTuple>> bfd_session_cache;
-


Please revert changes not related to PR

mssonicbld · 2025-02-10T23:17:57Z

/azp run

azure-pipelines · 2025-02-10T23:18:07Z

Azure Pipelines successfully started running 1 pipeline(s).

dypet · 2025-02-12T19:36:40Z

@kperumalbfn thanks for the approval; can this be merged?

kperumalbfn · 2025-02-12T19:45:25Z

@kperumalbfn thanks for the approval; can this be merged?

@dypet Please check and resolve @prsunny comments.

dypet · 2025-02-12T19:54:31Z

@kperumalbfn thanks for the approval; can this be merged?

@dypet Please check and resolve @prsunny comments.

@kperumalbfn @prsunny Responded to comments.

mssonicbld · 2025-02-13T15:03:36Z

/azp run

azure-pipelines · 2025-02-13T15:03:49Z

Azure Pipelines successfully started running 1 pipeline(s).

bfdorch changes to support software bfd sessions

fd1292c

abdbaig requested a review from prsunny as a code owner December 1, 2024 22:04

fixed syntax errors in the test script

63a2fa6

prsunny requested review from siqbal1986 and kperumalbfn December 10, 2024 18:06

siqbal1986 reviewed Dec 10, 2024

View reviewed changes

Merge branch 'master' into soft_bfd

2b61a9a

github-advanced-security bot found potential problems Dec 11, 2024

View reviewed changes

tests/test_soft_bfd.py Fixed Show resolved Hide resolved

tests/test_soft_bfd.py Fixed Show resolved Hide resolved

tests/test_soft_bfd.py Fixed Show resolved Hide resolved

Update replaceFirstTwoColons to createStateDBKey.

c19ce96

Merge branch 'master' into soft_bfd

b19b3e8

Merge branch 'master' into soft_bfd

9d2ee05

Merge branch 'master' into soft_bfd

8737875

Merge branch 'master' into soft_bfd

52a3838

Merge branch 'master' into soft_bfd

8204965

Update name to BFD_SOFTWARE_SESSION_TABLE

f0e54f4

kperumalbfn previously approved these changes Feb 6, 2025

View reviewed changes

Merge branch 'master' into soft_bfd

8b3c845

Trigger CI/CD pipeline.

6be6631

Merge branch 'master' into soft_bfd

e6c469c

Trigger CI/CD pipeline.

f2b4cb1

prsunny reviewed Feb 10, 2025

View reviewed changes

Remove with getKeys without checking for dpu.

3a89882

dypet dismissed kperumalbfn’s stale review via 3a89882 February 10, 2025 23:17

kperumalbfn approved these changes Feb 12, 2025

View reviewed changes

Merge branch 'master' into soft_bfd

5eaf72e

prsunny approved these changes Feb 13, 2025

View reviewed changes

prsunny merged commit 29b7db3 into sonic-net:master Feb 13, 2025
15 checks passed

sonic-swss: bfdorch changes to support software bfd sessions #3406

sonic-swss: bfdorch changes to support software bfd sessions #3406

Conversation

abdbaig commented Dec 1, 2024 • edited by prsunny Loading

siqbal1986 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssonicbld commented Dec 16, 2024

azure-pipelines bot commented Dec 16, 2024

mssonicbld commented Dec 16, 2024

azure-pipelines bot commented Dec 16, 2024

mssonicbld commented Dec 18, 2024

azure-pipelines bot commented Dec 18, 2024

mssonicbld commented Jan 2, 2025

azure-pipelines bot commented Jan 2, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 13, 2025

azure-pipelines bot commented Jan 13, 2025

mssonicbld commented Jan 15, 2025

azure-pipelines bot commented Jan 15, 2025

mssonicbld commented Feb 6, 2025

azure-pipelines bot commented Feb 6, 2025

mssonicbld commented Feb 7, 2025

azure-pipelines bot commented Feb 7, 2025

mssonicbld commented Feb 7, 2025

azure-pipelines bot commented Feb 7, 2025

mssonicbld commented Feb 7, 2025

azure-pipelines bot commented Feb 7, 2025

mssonicbld commented Feb 10, 2025

azure-pipelines bot commented Feb 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssonicbld commented Feb 10, 2025

azure-pipelines bot commented Feb 10, 2025

dypet commented Feb 12, 2025

kperumalbfn commented Feb 12, 2025

dypet commented Feb 12, 2025

mssonicbld commented Feb 13, 2025

azure-pipelines bot commented Feb 13, 2025

abdbaig commented Dec 1, 2024 •

edited by prsunny

Loading