Adding `HaOrch` for smartswitch dpu #3550

zjswhhh · 2025-03-10T06:50:55Z

HLD:
https://github.com/sonic-net/DASH/blob/main/documentation/high-avail/ha-api-hld.md
https://github.com/sonic-net/SONiC/blob/master/doc/smart-switch/high-availability/smart-switch-ha-detailed-design.md

Requires:
sonic-net/sonic-swss-common#985
sonic-net/sonic-dash-api#33

How did I verify?
UTs:

[----------] 8 tests from DashHaOrchTest
[ RUN      ] DashHaOrchTest.AddRemoveHaSet
[       OK ] DashHaOrchTest.AddRemoveHaSet (10 ms)
[ RUN      ] DashHaOrchTest.HaSetAlreadyExists
[       OK ] DashHaOrchTest.HaSetAlreadyExists (10 ms)
[ RUN      ] DashHaOrchTest.AddRemoveHaScope
[       OK ] DashHaOrchTest.AddRemoveHaScope (10 ms)
[ RUN      ] DashHaOrchTest.NoHaSetFound
[       OK ] DashHaOrchTest.NoHaSetFound (10 ms)
[ RUN      ] DashHaOrchTest.SetHaScopeHaRole
[       OK ] DashHaOrchTest.SetHaScopeHaRole (11 ms)
[ RUN      ] DashHaOrchTest.SetHaScopeActivateRoleRequest
[       OK ] DashHaOrchTest.SetHaScopeActivateRoleRequest (10 ms)
[ RUN      ] DashHaOrchTest.SetHaScopeFlowReconcileRequest
[       OK ] DashHaOrchTest.SetHaScopeFlowReconcileRequest (10 ms)
[ RUN      ] DashHaOrchTest.InvalidInput
[       OK ] DashHaOrchTest.InvalidInput (10 ms)
[----------] 8 tests from DashHaOrchTest (86 ms total)

TODO:
2. ~~add support to set ha_scope attribute ha_role~~
3. event notifications handling (new PR)
4. Bulk create/remove (new PR)
5. Ha stats (new PR)

sign-off: Jing Zhang [email protected]

mssonicbld · 2025-03-10T06:50:58Z

/azp run

azure-pipelines · 2025-03-10T06:51:04Z

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 · 2025-03-17T03:54:27Z

/azp run

azure-pipelines · 2025-03-17T03:54:36Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-03-17T19:10:50Z

/azp run

azure-pipelines · 2025-03-17T19:11:00Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-03-17T19:14:51Z

/azp run

azure-pipelines · 2025-03-17T19:15:00Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-03-21T20:36:00Z

/azp run

azure-pipelines · 2025-03-21T20:36:12Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-03-30T00:14:18Z

/azp run

azure-pipelines · 2025-03-30T00:14:28Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-03-30T07:45:36Z

/azp run

azure-pipelines · 2025-03-30T07:45:45Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-03-30T08:15:01Z

/azp run

azure-pipelines · 2025-03-30T08:15:11Z

Azure Pipelines successfully started running 1 pipeline(s).

theasianpianist · 2025-04-30T23:53:57Z

orchagent/dash/dashhaorch.cpp

+extern sai_dash_eni_api_t* sai_dash_eni_api;
+extern sai_object_id_t gSwitchId;
+
+static std::map<dash::types::HaRole, std::string> ha_role_to_str_map = {


Is it necessary to explicitly define these maps? Might be simpler to use dash::types::HaRole_Name and dash::types::HaRole_Parse

theasianpianist · 2025-04-30T23:54:14Z

orchagent/dash/dashhaorch.cpp

+    {dash::types::HA_SCOPE_ROLE_SWITCHING_TO_ACTIVE, "HA_SCOPE_ROLE_SWITCHING_TO_ACTIVE"}
+};
+
+static std::map<dash::types::HaScope, std::string> ha_set_scope_to_str_map = {


same as above

theasianpianist · 2025-04-30T23:56:29Z

orchagent/dash/dashhaorch.cpp

+    }
+}
+
+sai_ip_address_t DashHaOrch::covertPbIpaddrToSaiIpaddr(const dash::types::IpAddress &ipaddr)


Please use pbutils.cpp::to_sai instead

theasianpianist · 2025-04-30T23:58:39Z

orchagent/dash/dashhaorch.cpp

+            return parseHandleSaiStatusFailure(handle_status);
+        }
+    }
+    m_ha_set_entries[key] = HaSetEntry {sai_ha_set_oid, entry};


What's the expected scale/how many entries do we expect per DPU?

1 ha set per DPU.

mssonicbld · 2025-05-01T01:24:47Z

/azp run

azure-pipelines · 2025-05-01T01:24:57Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-05-01T15:58:07Z

/azp run

azure-pipelines · 2025-05-01T15:58:17Z

Azure Pipelines successfully started running 1 pipeline(s).

theasianpianist · 2025-05-01T20:52:09Z

orchagent/dash/dashhaorch.cpp

+        if (ha_scope_it->second.metadata.ha_role() != entry.ha_role())
+        {
+            return setHaScopeHaRole(key, entry);
+        }
+
+        if (entry.flow_reconcile_requested() == true)
+        {
+            return setHaScopeFlowReconcileRequest(key);
+        }
+
+        if (entry.activate_role_requested() == true)
+        {
+            return setHaScopeActivateRoleRequest(key);
+        }


Are these three attributes mutually exclusive, or is it possible for more than one to be changed in a single update?

theasianpianist · 2025-05-01T20:57:32Z

orchagent/dash/dashhaorch.cpp

+
+    // TODO: add ha_role to attribute value enum
+    ha_scope_attrs[1].id = SAI_HA_SCOPE_ATTR_DASH_HA_ROLE;
+    ha_scope_attrs[1].value.u16 = static_cast<sai_uint16_t>(entry.ha_role());


Do we need to do entry.ha_role() + 1 here? In the SAI enum, value 0 for sai_dash_ha_role_t is SAI_DASH_HA_ROLE_DEAD, but in the protobuf enum value 0 is HA_SCOPE_ROLE_UNSPECIFIED

theasianpianist · 2025-05-01T21:01:10Z

orchagent/dash/dashhaorch.cpp

+
+    sai_attribute_t ha_scope_attr;
+    ha_scope_attr.id = SAI_HA_SCOPE_ATTR_DASH_HA_ROLE;
+    ha_scope_attr.value.u32 = entry.ha_role();


same concern as above about SAI/protobuf enum off-by-one

theasianpianist · 2025-05-01T21:08:42Z

tests/mock_tests/mock_sai_api.h

+
+
+
+#define DEFINE_SAI_GENERIC_APIS_MOCK(sai_api_name, ...) \


What's the difference between this macro and DEFINE_SAI_GENERIC_API_MOCK, is it just allow mocking multiple APIs with one macro call?

yes, we have ha_set and ha_scope in the same sai_dash_ha_api, that's why I need this macro.

Ah I see. This should also still work for mocking just a single object type right? Can you add a comment above the old DEFINE_SAI_GENERIC_API_MOCK macro to deprecate it and use your new macro instead?

theasianpianist · 2025-05-01T21:15:34Z

tests/mock_tests/dashhaorch_ut.cpp

+
+        void CreateHaSet()
+        {
+            auto consumer = unique_ptr<Consumer>(new Consumer(


Just curious, why not use swss::Table and m_dashHaOrch->addExistingData like existing tests?

No particular reason. Is addExistingData preferred?

I don't think it makes a difference functionally but it might be better to keep thing consistent across UTs, plus it needs less boilerplate

I noticed this consumer and addToSync are also commonly used in UTs, I think the benefit is we can explicitly set the operation?

Table has del which should allow us to choose the op. But I didn't realize that we used addToSync so much, I think it's fine to leave this as-is then.

prabhataravind · 2025-05-01T21:27:34Z

orchagent/dash/dashhaorch.cpp

+            {
+                it++;
+            }
+        }


Could you also add changes to write result of operations back to DPU_APPL_STATE_DB? Refer sonic-net/SONiC#1759 and #3490

… ha_orch

…enum to sai enum

mssonicbld · 2025-05-02T17:37:56Z

/azp run

azure-pipelines · 2025-05-02T17:38:08Z

Azure Pipelines successfully started running 1 pipeline(s).

… ha_orch

mssonicbld · 2025-05-02T22:53:32Z

/azp run

azure-pipelines · 2025-05-02T22:53:41Z

Azure Pipelines successfully started running 1 pipeline(s).

zjswhhh added 5 commits March 10, 2025 05:51

create/remove haset and hascope

04811ce

makefile

d0122de

haorch init

ba172ef

update comment

81674c9

init sai api

4d4e76f

set scope id to eni

dacff67

use DPU_APPL_DB

d4228b9

use dpu_appl_db

f6cdfe2

add set ha scope attributes

b8da370

zjswhhh added 2 commits March 23, 2025 22:38

add zmq producer tables

3b293c7

use a specific build of swss-common cause dependency is not merged yet

e916546

use specific build of dash api

492f7bb

typo

6a4c882

typos

ccfe0c9

theasianpianist reviewed Apr 30, 2025

View reviewed changes

add more coverage

1f7ce48

zjswhhh added 2 commits May 1, 2025 06:53

use pbutils

0a4b863

even more coverage

ffc0104

zjswhhh requested a review from theasianpianist May 1, 2025 17:10

theasianpianist reviewed May 1, 2025

View reviewed changes

prabhataravind reviewed May 1, 2025

View reviewed changes

zjswhhh added 4 commits May 1, 2025 21:50

Merge branch 'master' of https://github.com/sonic-net/sonic-swss into…

2535b4e

… ha_orch

update set attribute logic, deprecate old macro, add mapping from pb …

bbf086f

…enum to sai enum

more UTs

1b1d8d6

add gnmi feedback

01d9544

zjswhhh requested review from theasianpianist and prabhataravind May 2, 2025 18:32

zjswhhh added 2 commits May 2, 2025 21:02

Merge branch 'master' of https://github.com/sonic-net/sonic-swss into…

473638a

… ha_orch

last try on the coverage

f977fb2

Adding HaOrch for smartswitch dpu #3550

Are you sure you want to change the base?

Adding HaOrch for smartswitch dpu #3550

Conversation

zjswhhh commented Mar 10, 2025 • edited Loading

mssonicbld commented Mar 10, 2025

azure-pipelines bot commented Mar 10, 2025

mssonicbld commented Mar 17, 2025

azure-pipelines bot commented Mar 17, 2025

mssonicbld commented Mar 17, 2025

azure-pipelines bot commented Mar 17, 2025

mssonicbld commented Mar 17, 2025

azure-pipelines bot commented Mar 17, 2025

mssonicbld commented Mar 21, 2025

azure-pipelines bot commented Mar 21, 2025

mssonicbld commented Mar 30, 2025

azure-pipelines bot commented Mar 30, 2025

mssonicbld commented Mar 30, 2025

azure-pipelines bot commented Mar 30, 2025

mssonicbld commented Mar 30, 2025

azure-pipelines bot commented Mar 30, 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

Choose a reason for hiding this comment

mssonicbld commented May 1, 2025

azure-pipelines bot commented May 1, 2025

mssonicbld commented May 1, 2025

azure-pipelines bot commented May 1, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssonicbld commented May 2, 2025

azure-pipelines bot commented May 2, 2025

mssonicbld commented May 2, 2025

azure-pipelines bot commented May 2, 2025

Adding `HaOrch` for smartswitch dpu #3550

Adding `HaOrch` for smartswitch dpu #3550

zjswhhh commented Mar 10, 2025 •

edited

Loading