Merge branch 'master' into master

anilkpan · Dec 10, 2024 · 124c419 · 124c419
2 parents a49fa2f + fa2b530
commit 124c419
Show file tree

Hide file tree

Showing 9 changed files with 129 additions and 25 deletions.
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -127,20 +127,21 @@ jobs:
         git config --local --unset user.name
         git config --local --unset user.email
         ln -s ../debian debian
-        dpkg-buildpackage -rfakeroot -us -uc -b -j$(nproc)
+        DPKG_GENSYMBOLS_CHECK_LEVEL=0 dpkg-buildpackage -rfakeroot -us -uc -b -j$(nproc)
         popd
-        dpkg-deb -x libnl-3-200_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
-        dpkg-deb -x libnl-3-dev_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
-        dpkg-deb -x libnl-genl-3-200_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
-        dpkg-deb -x libnl-genl-3-dev_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
-        dpkg-deb -x libnl-route-3-200_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
-        dpkg-deb -x libnl-route-3-dev_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
-        dpkg-deb -x libnl-nf-3-200_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
-        dpkg-deb -x libnl-nf-3-dev_${LIBNL3_VER}-${LIBNL3_REV}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-3-200_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-3-dev_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-genl-3-200_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-genl-3-dev_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-route-3-200_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-route-3-dev_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-nf-3-200_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
+        dpkg-deb -x libnl-nf-3-dev_${LIBNL3_VER}-${LIBNL3_REV_SONIC}_amd64.deb $(dirname $GITHUB_WORKSPACE)
         popd
       env:
         LIBNL3_VER: "3.7.0"
         LIBNL3_REV: "0.2"
+        LIBNL3_REV_SONIC: "0.2+b1sonic1"
 
     - if: matrix.language == 'cpp'
       name: Build repository

diff --git a/fpmsyncd/fpm/fpm.h b/fpmsyncd/fpm/fpm.h
@@ -92,7 +92,7 @@
 /*
  * Largest message that can be sent to or received from the FPM.
  */
-#define FPM_MAX_MSG_LEN 4096
+#define FPM_MAX_MSG_LEN 16384
 
 /*
  * Header that precedes each fpm message to/from the FPM.

diff --git a/fpmsyncd/fpmsyncd.cpp b/fpmsyncd/fpmsyncd.cpp
@@ -98,6 +98,7 @@ int main(int argc, char **argv)
     NetDispatcher::getInstance().registerMessageHandler(RTM_DELLINK, &sync);
 
     rtnl_route_read_protocol_names(DefaultRtProtoPath);
+    nlmsg_set_default_size(FPM_MAX_MSG_LEN);
 
     std::string suppressionEnabledStr;
     deviceMetadataTable.hget("localhost", "suppress-fib-pending", suppressionEnabledStr);

diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp
@@ -2235,12 +2235,23 @@ bool RouteSync::sendOffloadReply(struct nlmsghdr* hdr)
 bool RouteSync::sendOffloadReply(struct rtnl_route* route_obj)
 {
     SWSS_LOG_ENTER();
+    int ret = 0;
 
     nl_msg* msg{};
-    rtnl_route_build_add_request(route_obj, NLM_F_CREATE, &msg);
+    ret = rtnl_route_build_add_request(route_obj, NLM_F_CREATE, &msg);
 
+    if (ret !=0)
+    {
+        SWSS_LOG_ERROR("Route build add returned %d", ret);
+        return false;
+    }
     auto nlMsg = makeUniqueWithDestructor(msg, nlmsg_free);
 
+    if (nlMsg.get() == NULL)
+    {
+        SWSS_LOG_ERROR("Error in allocation for sending offload reply");
+        return false;
+    }
     return sendOffloadReply(nlmsg_hdr(nlMsg.get()));
 }
 

diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp
@@ -360,6 +360,25 @@ const object_reference_map &BufferOrch::getBufferPoolNameOidMap(void)
     return *m_buffer_type_maps[APP_BUFFER_POOL_TABLE_NAME];
 }
 
+void BufferOrch::getBufferObjectsWithNonZeroProfile(vector<string> &nonZeroQueues, const string &table)
+{
+    for (auto &&queueRef: (*m_buffer_type_maps[table]))
+    {
+        for (auto &&profileRef: queueRef.second.m_objsReferencingByMe)
+        {
+            if (profileRef.second.find("_zero_") == std::string::npos)
+            {
+                SWSS_LOG_INFO("Selected key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str());
+                nonZeroQueues.push_back(queueRef.first);
+            }
+            else
+            {
+                SWSS_LOG_INFO("Skipped key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str());
+            }
+        }
+    }
+}
+
 task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple)
 {
     SWSS_LOG_ENTER();
@@ -797,6 +816,9 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple)
     sai_uint32_t range_low, range_high;
     bool need_update_sai = true;
     bool local_port = false;
+    bool counter_was_added = false;
+    bool counter_needs_to_add = false;
+    string old_buffer_profile_name;
     string local_port_name;
 
     SWSS_LOG_DEBUG("Processing:%s", key.c_str());
@@ -856,7 +878,6 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple)
             return task_process_status::task_failed;
         }
 
-        string old_buffer_profile_name;
         if (doesObjectExist(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name)
             && (old_buffer_profile_name == buffer_profile_name))
         {
@@ -874,11 +895,14 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple)
         SWSS_LOG_NOTICE("Set buffer queue %s to %s", key.c_str(), buffer_profile_name.c_str());
 
         setObjectReference(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key, buffer_profile_field_name, buffer_profile_name);
+
+        // Counter operation
+        counter_needs_to_add = buffer_profile_name.find("_zero_") == std::string::npos;
+        SWSS_LOG_INFO("%s to create counter for %s with new profile %s", counter_needs_to_add ? "Need" : "No need", key.c_str(), buffer_profile_name.c_str());
     }
     else if (op == DEL_COMMAND)
     {
-        auto &typemap = (*m_buffer_type_maps[APP_BUFFER_QUEUE_TABLE_NAME]);
-        if (typemap.find(key) == typemap.end())
+        if (!doesObjectExist(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name))
         {
             SWSS_LOG_INFO("%s doesn't not exist, don't need to notfiy SAI", key.c_str());
             need_update_sai = false;
@@ -887,13 +911,17 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple)
         SWSS_LOG_NOTICE("Remove buffer queue %s", key.c_str());
         removeObject(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key);
         m_partiallyAppliedQueues.erase(key);
+        counter_needs_to_add = false;
     }
     else
     {
         SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
         return task_process_status::task_invalid_entry;
     }
 
+    counter_was_added = !old_buffer_profile_name.empty() && old_buffer_profile_name.find("_zero_") == std::string::npos;
+    SWSS_LOG_INFO("%s to remove counter for %s with old profile %s", counter_was_added ? "Need" : "No need", key.c_str(), old_buffer_profile_name.c_str());
+
     sai_attribute_t attr;
     attr.id = SAI_QUEUE_ATTR_BUFFER_PROFILE_ID;
     attr.value.oid = sai_buffer_profile;
@@ -963,14 +991,16 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple)
                 {
                     auto flexCounterOrch = gDirectory.get<FlexCounterOrch*>();
                     auto queues = tokens[1];
-                    if (op == SET_COMMAND &&
+                    if (!counter_was_added && counter_needs_to_add &&
                         (flexCounterOrch->getQueueCountersState() || flexCounterOrch->getQueueWatermarkCountersState()))
                     {
+                        SWSS_LOG_INFO("Creating counters for %s %zd", port_name.c_str(), ind);
                         gPortsOrch->createPortBufferQueueCounters(port, queues);
                     }
-                    else if (op == DEL_COMMAND &&
+                    else if (counter_was_added && !counter_needs_to_add &&
                              (flexCounterOrch->getQueueCountersState() || flexCounterOrch->getQueueWatermarkCountersState()))
                     {
+                        SWSS_LOG_INFO("Removing counters for %s %zd", port_name.c_str(), ind);
                         gPortsOrch->removePortBufferQueueCounters(port, queues);
                     }
                 }
@@ -1057,6 +1087,9 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
     vector<string> tokens;
     sai_uint32_t range_low, range_high;
     bool need_update_sai = true;
+    bool counter_was_added = false;
+    bool counter_needs_to_add = false;
+    string old_buffer_profile_name;
 
     SWSS_LOG_DEBUG("processing:%s", key.c_str());
     tokens = tokenize(key, delimiter);
@@ -1089,7 +1122,6 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
             return task_process_status::task_failed;
         }
 
-        string old_buffer_profile_name;
         if (doesObjectExist(m_buffer_type_maps, APP_BUFFER_PG_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name)
             && (old_buffer_profile_name == buffer_profile_name))
         {
@@ -1100,11 +1132,14 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
         SWSS_LOG_NOTICE("Set buffer PG %s to %s", key.c_str(), buffer_profile_name.c_str());
 
         setObjectReference(m_buffer_type_maps, APP_BUFFER_PG_TABLE_NAME, key, buffer_profile_field_name, buffer_profile_name);
+
+        // Counter operation
+        counter_needs_to_add = buffer_profile_name.find("_zero_") == std::string::npos;
+        SWSS_LOG_INFO("%s to create counter for priority group %s with new profile %s", counter_needs_to_add ? "Need" : "No need", key.c_str(), buffer_profile_name.c_str());
     }
     else if (op == DEL_COMMAND)
     {
-        auto &typemap = (*m_buffer_type_maps[APP_BUFFER_PG_TABLE_NAME]);
-        if (typemap.find(key) == typemap.end())
+        if (!doesObjectExist(m_buffer_type_maps, APP_BUFFER_PG_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name))
         {
             SWSS_LOG_INFO("%s doesn't not exist, don't need to notfiy SAI", key.c_str());
             need_update_sai = false;
@@ -1119,6 +1154,9 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
         return task_process_status::task_invalid_entry;
     }
 
+    counter_was_added = !old_buffer_profile_name.empty() && old_buffer_profile_name.find("_zero_") == std::string::npos;
+    SWSS_LOG_INFO("%s to remove counter for priority group %s with old profile %s", counter_was_added ? "Need" : "No need", key.c_str(), old_buffer_profile_name.c_str());
+
     sai_attribute_t attr;
     attr.id = SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE;
     attr.value.oid = sai_buffer_profile;
@@ -1161,14 +1199,16 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
                     {
                         auto flexCounterOrch = gDirectory.get<FlexCounterOrch*>();
                         auto pgs = tokens[1];
-                        if (op == SET_COMMAND &&
+                        if (!counter_was_added && counter_needs_to_add &&
                             (flexCounterOrch->getPgCountersState() || flexCounterOrch->getPgWatermarkCountersState()))
                         {
+                            SWSS_LOG_INFO("Creating counters for priority group %s %zd", port_name.c_str(), ind);
                             gPortsOrch->createPortBufferPgCounters(port, pgs);
                         }
-                        else if (op == DEL_COMMAND &&
+                        else if (counter_was_added && !counter_needs_to_add &&
                                  (flexCounterOrch->getPgCountersState() || flexCounterOrch->getPgWatermarkCountersState()))
                         {
+                            SWSS_LOG_INFO("Removing counters for priority group %s %zd", port_name.c_str(), ind);
                             gPortsOrch->removePortBufferPgCounters(port, pgs);
                         }
                     }

diff --git a/orchagent/bufferorch.h b/orchagent/bufferorch.h
@@ -38,6 +38,7 @@ class BufferOrch : public Orch
     static type_map m_buffer_type_maps;
     void generateBufferPoolWatermarkCounterIdList(void);
     const object_reference_map &getBufferPoolNameOidMap(void);
+    void getBufferObjectsWithNonZeroProfile(vector<string> &nonZeroQueues, const string &table);
 
 private:
     typedef task_process_status (BufferOrch::*buffer_table_handler)(KeyOpFieldsValuesTuple &tuple);

diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp
@@ -370,11 +370,11 @@ map<string, FlexCounterQueueStates> FlexCounterOrch::getQueueConfigurations()
     }
 
     std::vector<std::string> portQueueKeys;
-    m_bufferQueueConfigTable.getKeys(portQueueKeys);
+    gBufferOrch->getBufferObjectsWithNonZeroProfile(portQueueKeys, APP_BUFFER_QUEUE_TABLE_NAME);
 
     for (const auto& portQueueKey : portQueueKeys)
     {
-        auto toks = tokenize(portQueueKey, '|');
+        auto toks = tokenize(portQueueKey, ':');
         if (toks.size() != 2)
         {
             SWSS_LOG_ERROR("Invalid BUFFER_QUEUE key: [%s]", portQueueKey.c_str());
@@ -439,11 +439,11 @@ map<string, FlexCounterPgStates> FlexCounterOrch::getPgConfigurations()
     }
 
     std::vector<std::string> portPgKeys;
-    m_bufferPgConfigTable.getKeys(portPgKeys);
+    gBufferOrch->getBufferObjectsWithNonZeroProfile(portPgKeys, APP_BUFFER_PG_TABLE_NAME);
 
     for (const auto& portPgKey : portPgKeys)
     {
-        auto toks = tokenize(portPgKey, '|');
+        auto toks = tokenize(portPgKey, ':');
         if (toks.size() != 2)
         {
             SWSS_LOG_ERROR("Invalid BUFFER_PG key: [%s]", portPgKey.c_str());

diff --git a/tests/mock_tests/fake_netlink.cpp b/tests/mock_tests/fake_netlink.cpp
@@ -1,12 +1,15 @@
 #include <swss/linkcache.h>
 #include <swss/logger.h>
+#include <netlink/route/route.h>
 
 static rtnl_link* g_fakeLink = [](){
     auto fakeLink = rtnl_link_alloc();
     rtnl_link_set_ifindex(fakeLink, 42);
     return fakeLink;
 }();
 
+extern int rt_build_ret;
+extern bool nlmsg_alloc_ret;
 extern "C"
 {
 
@@ -15,4 +18,34 @@ struct rtnl_link* rtnl_link_get_by_name(struct nl_cache *cache, const char *name
     return g_fakeLink;
 }
 
+static int build_route_msg(struct rtnl_route *tmpl, int cmd, int flags,
+			   struct nl_msg **result)
+{
+	struct nl_msg *msg;
+	int err;
+	if (!(msg = nlmsg_alloc_simple(cmd, flags)))
+		return -NLE_NOMEM;
+	if ((err = rtnl_route_build_msg(msg, tmpl)) < 0) {
+		nlmsg_free(msg);
+		return err;
+	}
+	*result = msg;
+	return 0;
+}
+
+int rtnl_route_build_add_request(struct rtnl_route *tmpl, int flags,
+				 struct nl_msg **result)
+{
+    if (rt_build_ret != 0)
+    {
+        return rt_build_ret;
+    }
+    else if (!nlmsg_alloc_ret)
+    {
+	*result = NULL;
+        return 0;
+    }
+    return build_route_msg(tmpl, RTM_NEWROUTE, NLM_F_CREATE | flags,
+                           result);
+}
 }
diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp
@@ -12,6 +12,8 @@ using namespace swss;
 
 using ::testing::_;
 
+int rt_build_ret = 0;
+bool nlmsg_alloc_ret = true;
 class MockRouteSync : public RouteSync
 {
 public:
@@ -232,3 +234,18 @@ TEST_F(FpmSyncdResponseTest, testEvpn)
     ASSERT_EQ(value.get(), "0xc8");
 
 }
+
+TEST_F(FpmSyncdResponseTest, testSendOffloadReply)
+{
+
+    rt_build_ret = 1;
+    rtnl_route* routeObject{};
+
+
+    ASSERT_EQ(m_routeSync.sendOffloadReply(routeObject), false);
+    rt_build_ret = 0;
+    nlmsg_alloc_ret = false;
+    ASSERT_EQ(m_routeSync.sendOffloadReply(routeObject), false);
+    nlmsg_alloc_ret = true;
+
+}