diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 24957fede5..2bdb3e9933 100644 --- 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 index 8af9b30ae9..11f13b168a 100644 --- 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 index c3479efc84..f01bebe203 100644 --- 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 index 4d6aa708ab..cea63dc42f 100644 --- 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 index ccbc8e9b25..fb38cfe447 100644 --- 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 &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,6 +911,7 @@ 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 { @@ -894,6 +919,9 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) 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(); 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 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(); 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 index f78fe05318..3d255b87dd 100644 --- 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 &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 index b8918d0ac9..2832f0bd12 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -370,11 +370,11 @@ map FlexCounterOrch::getQueueConfigurations() } std::vector 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 FlexCounterOrch::getPgConfigurations() } std::vector 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 index 2370e13129..54eb197493 100644 --- a/tests/mock_tests/fake_netlink.cpp +++ b/tests/mock_tests/fake_netlink.cpp @@ -1,5 +1,6 @@ #include #include +#include static rtnl_link* g_fakeLink = [](){ auto fakeLink = rtnl_link_alloc(); @@ -7,6 +8,8 @@ static rtnl_link* g_fakeLink = [](){ 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 index a8de78859f..b1c23aca85 100644 --- 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; + +}