[bulker] Add support for bulk set object attributes (sonic-net#3703)

Ndancejic · web-flow · commit d644d2e40598 · 2025-12-02T15:06:25.000-08:00
Adds bulker API to allow bulk set attribute operations in orchagent.
Adds unit test for bulker to test adding nexthops, setting nexthop attributes, and removing nexthops
What I did
Added support for bulk setting object attributes in orchagent

Why I did it
bulk nexthop attribute setting is supported in SAI and will help with efforts in imporoving dualtor switchover times.
diff --git a/orchagent/bulker.h b/orchagent/bulker.h
@@ -368,8 +368,7 @@ struct SaiBulkerTraits<sai_next_hop_group_api_t>
     using set_entry_attribute_fn = sai_set_next_hop_group_member_attribute_fn;
     using bulk_create_entry_fn = sai_bulk_object_create_fn;
     using bulk_remove_entry_fn = sai_bulk_object_remove_fn;
-    // TODO: wait until available in SAI
-    //using bulk_set_entry_attribute_fn = sai_bulk_object_set_attribute_fn;
+    using bulk_set_entry_attribute_fn = sai_bulk_object_set_attribute_fn;
 };
 
 template<>
@@ -382,8 +381,7 @@ struct SaiBulkerTraits<sai_next_hop_api_t>
     using set_entry_attribute_fn = sai_set_next_hop_attribute_fn;
     using bulk_create_entry_fn = sai_bulk_object_create_fn;
     using bulk_remove_entry_fn = sai_bulk_object_remove_fn;
-    // TODO: wait until available in SAI
-    //using bulk_set_entry_attribute_fn = sai_bulk_object_set_attribute_fn;
+    using bulk_set_entry_attribute_fn = sai_bulk_object_set_attribute_fn;
 };
 
 template<>
@@ -422,6 +420,7 @@ struct SaiBulkerTraits<sai_dash_meter_api_t>
     using set_entry_attribute_fn = sai_set_meter_rule_attribute_fn;
     using bulk_create_entry_fn = sai_bulk_object_create_fn;
     using bulk_remove_entry_fn = sai_bulk_object_remove_fn;
+    using bulk_set_entry_attribute_fn = sai_bulk_object_set_attribute_fn;
 };
 
 template<>
@@ -434,6 +433,7 @@ struct SaiBulkerTraits<sai_dash_vnet_api_t>
     using set_entry_attribute_fn = sai_set_vnet_attribute_fn;
     using bulk_create_entry_fn = sai_bulk_object_create_fn;
     using bulk_remove_entry_fn = sai_bulk_object_remove_fn;
+    using bulk_set_entry_attribute_fn = sai_bulk_object_set_attribute_fn;
 };
 
 template<>
@@ -495,6 +495,7 @@ struct SaiBulkerTraits<sai_dash_tunnel_api_t>
     using api_t = sai_dash_tunnel_api_t;
     using bulk_create_entry_fn = sai_bulk_object_create_fn;
     using bulk_remove_entry_fn = sai_bulk_object_remove_fn;
+    using bulk_set_entry_attribute_fn = sai_bulk_object_set_attribute_fn;
 };
 
 template<>
@@ -1071,8 +1072,6 @@ class ObjectBulker
         return *object_status;
     }
 
-    // TODO: wait until available in SAI
-    /*
     sai_status_t set_entry_attribute(
         _In_ sai_object_id_t object_id,
         _In_ const sai_attribute_t *attr)
@@ -1086,14 +1085,15 @@ class ObjectBulker
         else
         {
             // Create a new key if not exists in the map
-            setting_entries.emplace(std::piecewise_construct,
+            auto& attrs = setting_entries.emplace(std::piecewise_construct,
                 std::forward_as_tuple(object_id),
-                std::forward_as_tuple(1, *attr));
+                std::forward_as_tuple()).first->second;
+
+            attrs.emplace_back(*attr);
         }
 
         return SAI_STATUS_SUCCESS;
     }
-    */
 
     void flush()
     {
@@ -1149,9 +1149,6 @@ class ObjectBulker
             creating_entries.clear();
         }
 
-        // Setting
-        // TODO: wait until available in SAI
-        /*
         if (!setting_entries.empty())
         {
             std::vector<sai_object_id_t> rs;
@@ -1176,7 +1173,6 @@ class ObjectBulker
 
             setting_entries.clear();
         }
-        */
     }
 
     void clear()
@@ -1228,11 +1224,8 @@ class ObjectBulker
     >>                                                      creating_entries;
 
     std::unordered_map<                                     // A map of
-            sai_object_id_t,                                // object_id -> (OUT object_status, attributes)
-            std::pair<
-                    sai_status_t *,
-                    std::vector<sai_attribute_t>
-            >
+            sai_object_id_t,                                // object_id -> attrs
+            std::vector<sai_attribute_t>
     >                                                       setting_entries;
 
                                                             // A map of
@@ -1241,8 +1234,7 @@ class ObjectBulker
 
     sai_bulk_object_create_fn                               create_entries;
     sai_bulk_object_remove_fn                               remove_entries;
-    // TODO: wait until available in SAI
-    //typename Ts::bulk_set_entry_attribute_fn                set_entries_attribute;
+    sai_bulk_object_set_attribute_fn                        set_entries_attribute;
 
     std::unordered_map<sai_object_id_t, sai_status_t>       create_statuses;
 
@@ -1316,8 +1308,6 @@ class ObjectBulker
         return status;
     }
 
-    // TODO: wait until available in SAI
-    /*
     sai_status_t flush_setting_entries(
         _Inout_ std::vector<sai_object_id_t> &rs,
         _Inout_ std::vector<sai_attribute_t> &ts)
@@ -1328,8 +1318,8 @@ class ObjectBulker
         }
         size_t count = rs.size();
         std::vector<sai_status_t> statuses(count);
-        sai_status_t status = (*set_entries_attribute)((uint32_t)count, rs.data(), ts.data()
-            , SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, statuses.data());
+        sai_status_t status = (*set_entries_attribute)((uint32_t)count, rs.data(), ts.data(),
+                               SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, statuses.data());
         if (status == SAI_STATUS_SUCCESS)
         {
             SWSS_LOG_INFO("ObjectBulker.flush setting_entries %zu\n", count);
@@ -1345,7 +1335,6 @@ class ObjectBulker
 
         return status;
     }
-     */
 };
 
 template <>
@@ -1355,8 +1344,7 @@ inline ObjectBulker<sai_next_hop_group_api_t>::ObjectBulker(SaiBulkerTraits<sai_
 {
     create_entries = api->create_next_hop_group_members;
     remove_entries = api->remove_next_hop_group_members;
-    // TODO: wait until available in SAI
-    //set_entries_attribute = ;
+    set_entries_attribute = api->set_next_hop_group_members_attribute;
 }
 
 template <>
@@ -1366,8 +1354,7 @@ inline ObjectBulker<sai_next_hop_api_t>::ObjectBulker(SaiBulkerTraits<sai_next_h
 {
     create_entries = api->create_next_hops;
     remove_entries = api->remove_next_hops;
-    // TODO: wait until available in SAI
-    //set_entries_attribute = ;
+    set_entries_attribute = api->set_next_hops_attribute;
 }
 
 template <>
@@ -1377,6 +1364,7 @@ inline ObjectBulker<sai_dash_vnet_api_t>::ObjectBulker(SaiBulkerTraits<sai_dash_
 {
     create_entries = api->create_vnets;
     remove_entries = api->remove_vnets;
+    set_entries_attribute = nullptr;
 }
 
 template <>
@@ -1386,6 +1374,7 @@ inline ObjectBulker<sai_dash_meter_api_t>::ObjectBulker(SaiBulkerTraits<sai_dash
 {
     create_entries = api->create_meter_rules;
     remove_entries = api->remove_meter_rules;
+    set_entries_attribute = nullptr;
 }
 
 template <>
@@ -1398,14 +1387,17 @@ inline ObjectBulker<sai_dash_tunnel_api_t>::ObjectBulker(SaiBulkerTraits<sai_das
         case SAI_OBJECT_TYPE_DASH_TUNNEL:
             create_entries = api->create_dash_tunnels;
             remove_entries = api->remove_dash_tunnels;
+            set_entries_attribute = nullptr;
             break;
         case SAI_OBJECT_TYPE_DASH_TUNNEL_MEMBER:
             create_entries = api->create_dash_tunnel_members;
             remove_entries = api->remove_dash_tunnel_members;
+            set_entries_attribute = nullptr;
             break;
         case SAI_OBJECT_TYPE_DASH_TUNNEL_NEXT_HOP:
             create_entries = api->create_dash_tunnel_next_hops;
             remove_entries = api->remove_dash_tunnel_next_hops;
+            set_entries_attribute = nullptr;
             break;
         default:
             std::string type_str = sai_serialize_object_type((sai_object_type_t) object_type);
diff --git a/orchagent/p4orch/tests/mock_sai_next_hop.h b/orchagent/p4orch/tests/mock_sai_next_hop.h
@@ -29,6 +29,9 @@ class MockSaiNextHop
 
     MOCK_METHOD4(remove_next_hops, sai_status_t(_In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, _In_ sai_bulk_op_error_mode_t mode,
                                                 _Out_ sai_status_t *object_statuses));
+
+    MOCK_METHOD5(set_next_hops_attribute, sai_status_t(_In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, _In_ const sai_attribute_t *attr_list,
+                                                       _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses));
 };
 
 // Note that before mock functions below are used, mock_sai_next_hop must be
@@ -51,3 +54,6 @@ sai_status_t mock_create_next_hops(_In_ sai_object_id_t switch_id, _In_ uint32_t
 
 sai_status_t mock_remove_next_hops(_In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, _In_ sai_bulk_op_error_mode_t mode,
                                    _Out_ sai_status_t *object_statuses);
+
+sai_status_t mock_set_next_hops_attribute(_In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, _In_ const sai_attribute_t *attr_list,
+                                          _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses);
diff --git a/tests/mock_tests/bulker_ut.cpp b/tests/mock_tests/bulker_ut.cpp
@@ -1,12 +1,25 @@
 #include "ut_helper.h"
 #include "bulker.h"
+#include "mock_sai_api.h"
 
 extern sai_route_api_t *sai_route_api;
 extern sai_neighbor_api_t *sai_neighbor_api;
+extern sai_next_hop_api_t *sai_next_hop_api;
+
+EXTERN_MOCK_FNS
 
 namespace bulker_test
 {
     using namespace std;
+    using ::testing::SetArrayArgument;
+    using ::testing::Return;
+    using ::testing::DoAll;
+
+    DEFINE_SAI_GENERIC_API_OBJECT_BULK_MOCK_WITH_SET(next_hop, next_hop);
+
+    sai_bulk_object_create_fn old_object_create;
+    sai_bulk_object_remove_fn old_object_remove;
+    sai_bulk_object_set_attribute_fn old_object_set_attribute;
 
     struct BulkerTest : public ::testing::Test
     {
@@ -21,15 +34,36 @@ namespace bulker_test
 
             ASSERT_EQ(sai_neighbor_api, nullptr);
             sai_neighbor_api = new sai_neighbor_api_t();
+
+            ASSERT_EQ(sai_next_hop_api, nullptr);
+            sai_next_hop_api = new sai_next_hop_api_t();
+
+            INIT_SAI_API_MOCK(next_hop);
+            MockSaiApis();
+            old_object_create = sai_next_hop_api->create_next_hops;
+            old_object_remove = sai_next_hop_api->remove_next_hops;
+            old_object_set_attribute = sai_next_hop_api->set_next_hops_attribute;
+            sai_next_hop_api->create_next_hops = mock_create_next_hops;
+            sai_next_hop_api->remove_next_hops = mock_remove_next_hops;
+            sai_next_hop_api->set_next_hops_attribute = mock_set_next_hops_attribute;
         }
 
         void TearDown() override
         {
+            RestoreSaiApis();
+            DEINIT_SAI_API_MOCK(next_hop);
+            sai_next_hop_api->create_next_hops = old_object_create;
+            sai_next_hop_api->remove_next_hops = old_object_remove;
+            sai_next_hop_api->set_next_hops_attribute = old_object_set_attribute;
+
             delete sai_route_api;
             sai_route_api = nullptr;
 
             delete sai_neighbor_api;
             sai_neighbor_api = nullptr;
+
+            delete sai_next_hop_api;
+            sai_next_hop_api = nullptr;
         }
     };
 
@@ -174,6 +208,87 @@ namespace bulker_test
         ASSERT_TRUE(gNeighBulker.bulk_entry_pending_removal(neighbor_entry_remove));
     }
 
+    TEST_F(BulkerTest, ObjectBulkSet)
+    {
+        // Create bulker
+        ObjectBulker<sai_next_hop_api_t> gNextHopBulker(sai_next_hop_api, 0x0, 1000);
+        vector<sai_attribute_t> next_hop_attrs;
+        vector<sai_object_id_t> next_hop_ids = {0x101, 0x102};
+        vector<sai_status_t> statuses;
+        sai_attribute_t next_hop_attr;
+        sai_object_id_t next_hop_id_0;
+        sai_object_id_t next_hop_id_1;
+        std::vector<sai_status_t> exp_status{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS};
+
+        // Create 2 next hops
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_TYPE;
+        next_hop_attr.value.s32 = SAI_NEXT_HOP_TYPE_IP;
+        next_hop_attrs.push_back(next_hop_attr);
+
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_IP;
+        next_hop_attr.value.ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV4;
+        next_hop_attr.value.ipaddr.addr.ip4 = 0x10000001;
+        next_hop_attrs.push_back(next_hop_attr);
+
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID;
+        next_hop_attr.value.oid = 0x0;
+        next_hop_attrs.push_back(next_hop_attr);
+
+        gNextHopBulker.create_entry(&next_hop_id_0, (uint32_t)next_hop_attrs.size(), next_hop_attrs.data());
+        next_hop_attrs.clear();
+
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_TYPE;
+        next_hop_attr.value.s32 = SAI_NEXT_HOP_TYPE_IP;
+        next_hop_attrs.push_back(next_hop_attr);
+
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_IP;
+        next_hop_attr.value.ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV4;
+        next_hop_attr.value.ipaddr.addr.ip4 = 0x10000002;
+        next_hop_attrs.push_back(next_hop_attr);
+
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID;
+        next_hop_attr.value.oid = 0x0;
+        next_hop_attrs.push_back(next_hop_attr);
+
+        gNextHopBulker.create_entry(&next_hop_id_1, (uint32_t)next_hop_attrs.size(), next_hop_attrs.data());
+        next_hop_attrs.clear();
+
+        EXPECT_CALL(*mock_sai_next_hop_api, create_next_hops)
+            .WillOnce(DoAll(
+                SetArrayArgument<5>(next_hop_ids.begin(), next_hop_ids.end()),
+                SetArrayArgument<6>(exp_status.begin(), exp_status.end()),
+                Return(SAI_STATUS_SUCCESS)));
+        gNextHopBulker.flush();
+
+        // Update the nexthops
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_IP;
+        next_hop_attr.value.ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV4;
+        next_hop_attr.value.ipaddr.addr.ip4 = 0x10000003;
+
+        gNextHopBulker.set_entry_attribute(next_hop_id_0, &next_hop_attr);
+
+        next_hop_attr.id = SAI_NEXT_HOP_ATTR_IP;
+        next_hop_attr.value.ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV4;
+        next_hop_attr.value.ipaddr.addr.ip4 = 0x10000004;
+        next_hop_attrs.push_back(next_hop_attr);
+
+        gNextHopBulker.set_entry_attribute(next_hop_id_1, &next_hop_attr);
+
+        EXPECT_CALL(*mock_sai_next_hop_api, set_next_hops_attribute)
+            .WillOnce(DoAll(SetArrayArgument<4>(exp_status.begin(), exp_status.end()), Return(SAI_STATUS_SUCCESS)));
+        gNextHopBulker.flush();
+
+        // Delete the nexthops
+        statuses.emplace_back();
+        gNextHopBulker.remove_entry(&statuses.back(), next_hop_id_0);
+        statuses.emplace_back();
+        gNextHopBulker.remove_entry(&statuses.back(), next_hop_id_1);
+
+        EXPECT_CALL(*mock_sai_next_hop_api, remove_next_hops)
+            .WillOnce(DoAll(SetArrayArgument<3>(exp_status.begin(), exp_status.end()), Return(SAI_STATUS_SUCCESS)));
+        gNextHopBulker.flush();
+    }
+
     TEST_F(BulkerTest, BulkerPendingRemovalOrSet_OnlyRemoval)
     {
         // Create bulker
diff --git a/tests/mock_tests/mock_sai_api.h b/tests/mock_tests/mock_sai_api.h
