From 971dfc1a29daa5109672254d5ea48a62cf901aa7 Mon Sep 17 00:00:00 2001 From: Devesh Pathak <54966909+devpatha@users.noreply.github.com> Date: Mon, 23 Sep 2024 09:42:25 -0700 Subject: [PATCH 1/6] Add support for PACKET_ACTION_COPY (#3288) What I did Added COPY packet action, that uses SAI_PACKET_ACTION_COPY attribute. Why I did it This allows ACL to copy packets to CPU. It can be used to dump and debug transit packets. --- orchagent/aclorch.cpp | 1 + orchagent/aclorch.h | 1 + tests/mock_tests/aclorch_ut.cpp | 7 +++++++ 3 files changed, 9 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index f8bf775868..b20b382009 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -122,6 +122,7 @@ static acl_packet_action_lookup_t aclPacketActionLookup = { { PACKET_ACTION_FORWARD, SAI_PACKET_ACTION_FORWARD }, { PACKET_ACTION_DROP, SAI_PACKET_ACTION_DROP }, + { PACKET_ACTION_COPY, SAI_PACKET_ACTION_COPY }, }; static acl_dtel_flow_op_type_lookup_t aclDTelFlowOpTypeLookup = diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 5458e970be..be6c1f2af5 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -72,6 +72,7 @@ #define PACKET_ACTION_FORWARD "FORWARD" #define PACKET_ACTION_DROP "DROP" +#define PACKET_ACTION_COPY "COPY" #define PACKET_ACTION_REDIRECT "REDIRECT" #define PACKET_ACTION_DO_NOT_NAT "DO_NOT_NAT" diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 4a92d65c80..351d523219 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -889,6 +889,13 @@ namespace aclorch_test return false; } } + else if (attr_value == PACKET_ACTION_COPY) + { + if (it->second.getSaiAttr().value.aclaction.parameter.s32 != SAI_PACKET_ACTION_COPY) + { + return false; + } + } else { // unknown attr_value From 008f2865b57cff7f06fc176d57b83d4cfe4093d4 Mon Sep 17 00:00:00 2001 From: Oleksandr Ivantsiv Date: Tue, 24 Sep 2024 09:07:54 -0700 Subject: [PATCH 2/6] [crm][dash] Do not probe DASH resources on devices other than the DPU. (#3297) --- orchagent/crmorch.cpp | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/orchagent/crmorch.cpp b/orchagent/crmorch.cpp index b5844bbea3..4479a9a71d 100644 --- a/orchagent/crmorch.cpp +++ b/orchagent/crmorch.cpp @@ -16,14 +16,14 @@ #define CRM_EXCEEDED_MSG_MAX 10 #define CRM_ACL_RESOURCE_COUNT 256 +using namespace std; +using namespace swss; + extern sai_object_id_t gSwitchId; extern sai_switch_api_t *sai_switch_api; extern sai_acl_api_t *sai_acl_api; extern event_handle_t g_events_handle; - -using namespace std; -using namespace swss; - +extern string gMySwitchType; const map crmResTypeNameMap = { @@ -808,6 +808,12 @@ bool CrmOrch::getResAvailability(CrmResourceType type, CrmResourceEntry &res) bool CrmOrch::getDashAclGroupResAvailability(CrmResourceType type, CrmResourceEntry &res) { + if (gMySwitchType != "dpu") + { + res.resStatus = CrmResourceStatus::CRM_RES_NOT_SUPPORTED; + return false; + } + sai_object_type_t objType = crmResSaiObjAttrMap.at(type); for (auto &cnt : res.countersMap) @@ -872,6 +878,12 @@ void CrmOrch::getResAvailableCounters() case CrmResourceType::CRM_SRV6_MY_SID_ENTRY: case CrmResourceType::CRM_MPLS_NEXTHOP: case CrmResourceType::CRM_SRV6_NEXTHOP: + case CrmResourceType::CRM_TWAMP_ENTRY: + { + getResAvailability(res.first, res.second); + break; + } + case CrmResourceType::CRM_DASH_VNET: case CrmResourceType::CRM_DASH_ENI: case CrmResourceType::CRM_DASH_ENI_ETHER_ADDRESS_MAP: @@ -885,8 +897,13 @@ void CrmOrch::getResAvailableCounters() case CrmResourceType::CRM_DASH_IPV6_OUTBOUND_CA_TO_PA: case CrmResourceType::CRM_DASH_IPV4_ACL_GROUP: case CrmResourceType::CRM_DASH_IPV6_ACL_GROUP: - case CrmResourceType::CRM_TWAMP_ENTRY: { + if (gMySwitchType != "dpu") + { + res.second.resStatus = CrmResourceStatus::CRM_RES_NOT_SUPPORTED; + break; + } + getResAvailability(res.first, res.second); break; } From 69cf0872ce65dbb765d84920fd2361c4d8d19db0 Mon Sep 17 00:00:00 2001 From: fountzou <169114916+fountzou@users.noreply.github.com> Date: Tue, 24 Sep 2024 19:37:03 +0300 Subject: [PATCH 3/6] [orchagent]: Skip installing ACL counter when ACL mirror rule is inactive (#3223) * [orchagent]: Resolving issue #18844 * Don't install counters when session rule is inactive. Mirror session is not activated and thus, the ACL rule is not created in sairedis/SAI (there is no next hop for the mirror destination IP). However, orchagent creates and registers the ACL counter with the flexcounter (FC) and the counter is being polled every polling interval. Since the ACL counter is not attached to any ACL rule, the BCM SAI introduces print errors in syslog. --- orchagent/aclorch.cpp | 23 ++++++++++++++++++++++- orchagent/aclorch.h | 1 + 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index b20b382009..8e43a66db3 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2024,6 +2024,23 @@ bool AclRuleMirror::validate() return true; } +bool AclRuleMirror::createCounter() +{ + SWSS_LOG_ENTER(); + + bool state = false; + + m_pMirrorOrch->getSessionStatus(m_sessionName, state); + + // If the mirror session is active, create the ACL counter + if(state) + { + return AclRule::createCounter(); + } + + return true; +} + bool AclRuleMirror::createRule() { SWSS_LOG_ENTER(); @@ -2153,7 +2170,11 @@ void AclRuleMirror::onUpdate(SubjectType type, void *cntx) if (update->active) { SWSS_LOG_INFO("Activating mirroring ACL %s for session %s", m_id.c_str(), m_sessionName.c_str()); - activate(); + // During mirror session activation, the newly created counter needs to be registered to the FC. + if(activate() && hasCounter()) + { + m_pAclOrch->registerFlexCounter(*this); + } } else { diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index be6c1f2af5..6c0246ce4a 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -344,6 +344,7 @@ class AclRuleMirror: public AclRule AclRuleMirror(AclOrch *m_pAclOrch, MirrorOrch *m_pMirrorOrch, string rule, string table); bool validateAddAction(string attr_name, string attr_value); bool validate(); + bool createCounter(); bool createRule(); bool removeRule(); void onUpdate(SubjectType, void *) override; From 002cd256ca251c3dafad0a73bf8900e14c0e8e1a Mon Sep 17 00:00:00 2001 From: mint570 <70396898+mint570@users.noreply.github.com> Date: Tue, 24 Sep 2024 10:38:01 -0700 Subject: [PATCH 4/6] Close socket descriptor in checkPortIffUp. (#3263) What I did Close socket descriptor in checkPortIffUp. Why I did it Currently the file descriptor is not closed. It will cause descriptor leak. --- cfgmgr/teammgr.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cfgmgr/teammgr.cpp b/cfgmgr/teammgr.cpp index 36c9d134e1..d72b522047 100644 --- a/cfgmgr/teammgr.cpp +++ b/cfgmgr/teammgr.cpp @@ -396,11 +396,15 @@ bool TeamMgr::checkPortIffUp(const string &port) if (fd == -1 || ioctl(fd, SIOCGIFFLAGS, &ifr) == -1) { SWSS_LOG_ERROR("Failed to get port %s flags", port.c_str()); + if (fd != -1) + { + close(fd); + } return false; } SWSS_LOG_INFO("Get port %s flags %i", port.c_str(), ifr.ifr_flags); - + close(fd); return ifr.ifr_flags & IFF_UP; } From be3a15f62847ff1edabd3060cdf699ccf7a45205 Mon Sep 17 00:00:00 2001 From: siqbal1986 Date: Tue, 24 Sep 2024 10:38:51 -0700 Subject: [PATCH 5/6] Update CODEOWNERS VNET and ACL Orch (#3296) *Update CODEOWNERS (siqbal1986) to VNET and ACL Orch --- .github/CODEOWNERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 46732aa050..fd7c2fabb9 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -28,3 +28,10 @@ # Chassis /orchagent/fabricportsorch* @abdosi @judyjoseph /tests/test_virtual_chassis.py @abdosi @judyjoseph + +# Vnet Orch +/orchagent/vnet* @siqbal1986 + +# Acl Orch +/orchagent/acl* @siqbal1986 + From 3c230d2b51ebf2ffc7163b2641ffab7ef358bfd4 Mon Sep 17 00:00:00 2001 From: Pavan Naregundi <92989231+pavannaregundi@users.noreply.github.com> Date: Wed, 25 Sep 2024 06:07:36 +0530 Subject: [PATCH 6/6] [Orchagent] Add optional create_switch timeout parameter (#3258) What I did Change adds optional create_switch timeout as command line parameter to orchagent. Why I did it Older platforms are seeing increase in time required in bookworm based branches. --- orchagent/main.cpp | 20 +++++++++++++++----- tests/test_zmq.py | 3 ++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 0a804eb38c..556bb70892 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -69,10 +69,11 @@ uint32_t gCfgSystemPorts = 0; string gMyHostName = ""; string gMyAsicName = ""; bool gTraditionalFlexCounter = false; +uint32_t create_switch_timeout = 0; void usage() { - cout << "usage: orchagent [-h] [-r record_type] [-d record_location] [-f swss_rec_filename] [-j sairedis_rec_filename] [-b batch_size] [-m MAC] [-i INST_ID] [-s] [-z mode] [-k bulk_size] [-q zmq_server_address] [-c mode]" << endl; + cout << "usage: orchagent [-h] [-r record_type] [-d record_location] [-f swss_rec_filename] [-j sairedis_rec_filename] [-b batch_size] [-m MAC] [-i INST_ID] [-s] [-z mode] [-k bulk_size] [-q zmq_server_address] [-c mode] [-t create_switch_timeout]" << endl; cout << " -h: display this message" << endl; cout << " -r record_type: record orchagent logs with type (default 3)" << endl; cout << " Bit 0: sairedis.rec, Bit 1: swss.rec, Bit 2: responsepublisher.rec. For example:" << endl; @@ -92,6 +93,7 @@ void usage() cout << " -k max bulk size in bulk mode (default 1000)" << endl; cout << " -q zmq_server_address: ZMQ server address (default disable ZMQ)" << endl; cout << " -c counter mode (traditional|asic_db), default: asic_db" << endl; + cout << " -t Override create switch timeout, in sec" << endl; } void sighup_handler(int signo) @@ -346,7 +348,7 @@ int main(int argc, char **argv) string responsepublisher_rec_filename = Recorder::RESPPUB_FNAME; int record_type = 3; // Only swss and sairedis recordings enabled by default. - while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:")) != -1) + while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:")) != -1) { switch (opt) { @@ -437,6 +439,9 @@ int main(int argc, char **argv) enable_zmq = true; } break; + case 't': + create_switch_timeout = atoi(optarg); + break; default: /* '?' */ exit(EXIT_FAILURE); } @@ -629,7 +634,7 @@ int main(int argc, char **argv) delay_factor = 2; } - if (gMySwitchType == "voq" || gMySwitchType == "fabric" || gMySwitchType == "chassis-packet" || gMySwitchType == "dpu" || asan_enabled) + if (gMySwitchType == "voq" || gMySwitchType == "fabric" || gMySwitchType == "chassis-packet" || gMySwitchType == "dpu" || asan_enabled || create_switch_timeout) { /* We set this long timeout in order for orchagent to wait enough time for * response from syncd. It is needed since switch create takes more time @@ -637,7 +642,12 @@ int main(int argc, char **argv) * and systems ports to initialize */ - if (gMySwitchType == "voq" || gMySwitchType == "chassis-packet" || gMySwitchType == "dpu") + if (create_switch_timeout) + { + /* Convert timeout to milliseconds from seconds */ + attr.value.u64 = (create_switch_timeout * 1000); + } + else if (gMySwitchType == "voq" || gMySwitchType == "chassis-packet" || gMySwitchType == "dpu") { attr.value.u64 = (5 * SAI_REDIS_DEFAULT_SYNC_OPERATION_RESPONSE_TIMEOUT); } @@ -672,7 +682,7 @@ int main(int argc, char **argv) } SWSS_LOG_NOTICE("Create a switch, id:%" PRIu64, gSwitchId); - if (gMySwitchType == "voq" || gMySwitchType == "fabric" || gMySwitchType == "chassis-packet" || gMySwitchType == "dpu") + if (gMySwitchType == "voq" || gMySwitchType == "fabric" || gMySwitchType == "chassis-packet" || gMySwitchType == "dpu" || create_switch_timeout) { /* Set syncd response timeout back to the default value */ attr.id = SAI_REDIS_SWITCH_ATTR_SYNC_OPERATION_RESPONSE_TIMEOUT; diff --git a/tests/test_zmq.py b/tests/test_zmq.py index 8a3dc49894..4894df0751 100644 --- a/tests/test_zmq.py +++ b/tests/test_zmq.py @@ -57,8 +57,9 @@ class TestZmqDash(object): @pytest.fixture(scope="class") def enable_orchagent_zmq(self, dvs): # change orchagent to use ZMQ + # change orchagent to use custom create_switch_timeout dvs.runcmd("cp /usr/bin/orchagent.sh /usr/bin/orchagent.sh_zmq_ut_backup") - dvs.runcmd("sed -i.bak 's/\/usr\/bin\/orchagent /\/usr\/bin\/orchagent -q tcp:\/\/127.0.0.1:8100 /g' /usr/bin/orchagent.sh") + dvs.runcmd("sed -i.bak 's/\/usr\/bin\/orchagent /\/usr\/bin\/orchagent -q tcp:\/\/127.0.0.1:8100 -t 60 /g' /usr/bin/orchagent.sh") dvs.stop_swss() dvs.start_swss()