From 855c9c4ee9fcd09770ac59cc6cf242afac7d0486 Mon Sep 17 00:00:00 2001 From: Akhilesh Samineni <47657796+AkhileshSamineni@users.noreply.github.com> Date: Fri, 9 Apr 2021 12:10:04 +0530 Subject: [PATCH] NAT : Update the CRM used counters for SNAT and DNAT entries (#1655) Issue : CRM used counters are not getting updated for SNAT and DNAT entries Steps to recreate: Add a static NAT entry and verify the CRM counters root@sonic:/home/admin# config nat feature enable root@sonic:/home/admin# config interface ip add Ethernet9 12.12.0.1/24 root@sonic:/home/admin# config interface ip add Ethernet11 125.56.90.12/24 root@sonic:/home/admin# config nat add interface Ethernet11 -nat_zone 1 root@sonic:/home/admin# root@sonic:/home/admin# config nat add static basic 125.56.90.8 12.12.0.2 root@sonic:/home/admin# show nat translations Static NAT Entries ..................... 2 Static NAPT Entries ..................... 0 Dynamic NAT Entries ..................... 0 Dynamic NAPT Entries ..................... 0 Static Twice NAT Entries ..................... 0 Static Twice NAPT Entries ..................... 0 Dynamic Twice NAT Entries ..................... 0 Dynamic Twice NAPT Entries ..................... 0 Total SNAT/SNAPT Entries ..................... 1 Total DNAT/DNAPT Entries ..................... 1 Total Entries ..................... 2 Protocol Source Destination Translated Source Translated Destination ---------- --------- ------------- ------------------- ------------------------ all 12.12.0.2 --- 125.56.90.8 --- all --- 125.56.90.8 --- 12.12.0.2 root@sonic:/home/admin# =============After polling interval of 300 seconds ======== root@sonic:/home/admin# crm show resources snat Resource Name Used Count Available Count --------------- ------------ ----------------- snat_entry 0 1024 root@sonic:/home/admin# root@sonic:/home/admin# crm show resources dnat Resource Name Used Count Available Count --------------- ------------ ----------------- dnat_entry 0 1024 root@sonic:/home/admin# Fix: Increment/Decrement the crm used counters for snat/dnat entries when entry is created/deleted. Repeated the same steps to add static nat entry like above and verified the crm counters. root@sonic:/home/admin# crm show resources dnat Resource Name Used Count Available Count --------------- ------------ ----------------- dnat_entry 1 1023 root@sonic:/home/admin# crm show resources snat Resource Name Used Count Available Count --------------- ------------ ----------------- snat_entry 1 1023 root@sonic:/home/admin# Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com --- orchagent/natorch.cpp | 10 ++++++ tests/test_nat.py | 78 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/orchagent/natorch.cpp b/orchagent/natorch.cpp index 0eefbad6a2e..283110efd11 100644 --- a/orchagent/natorch.cpp +++ b/orchagent/natorch.cpp @@ -26,7 +26,9 @@ #include "natorch.h" #include "notifier.h" #include "sai_serialize.h" +#include "crmorch.h" +extern CrmOrch *gCrmOrch; extern PortsOrch *gPortsOrch; extern sai_object_id_t gSwitchId; extern sai_switch_api_t *sai_switch_api; @@ -790,6 +792,7 @@ bool NatOrch::addHwDnatEntry(const IpAddress &ip_address) updateNatCounters(ip_address, 0, 0); m_natEntries[ip_address].addedToHw = true; + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_DNAT_ENTRY); if (entry.entry_type == "static") { @@ -876,6 +879,7 @@ bool NatOrch::addHwDnaptEntry(const NaptEntryKey &key) m_naptEntries[key].addedToHw = true; updateNaptCounters(key.prototype.c_str(), key.ip_address, key.l4_port, 0, 0); + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_DNAT_ENTRY); if (entry.entry_type == "static") { @@ -947,6 +951,7 @@ bool NatOrch::removeHwDnatEntry(const IpAddress &dstIp) entry.entry_type.c_str(), dstIp.to_string().c_str(), entry.translated_ip.to_string().c_str()); deleteNatCounters(dstIp); + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_DNAT_ENTRY); if (entry.entry_type == "static") { @@ -1138,6 +1143,7 @@ bool NatOrch::removeHwDnaptEntry(const NaptEntryKey &key) entry.translated_ip.to_string().c_str(), entry.translated_l4_port); deleteNaptCounters(key.prototype.c_str(), key.ip_address, key.l4_port); + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_DNAT_ENTRY); if (entry.entry_type == "static") { @@ -1336,6 +1342,7 @@ bool NatOrch::addHwSnatEntry(const IpAddress &ip_address) updateNatCounters(ip_address, 0, 0); m_natEntries[ip_address].addedToHw = true; m_natEntries[ip_address].activeTime = time_now.tv_sec; + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_SNAT_ENTRY); if (entry.entry_type == "static") { @@ -1514,6 +1521,7 @@ bool NatOrch::addHwSnaptEntry(const NaptEntryKey &keyEntry) m_naptEntries[keyEntry].activeTime = time_now.tv_sec; updateNaptCounters(keyEntry.prototype.c_str(), keyEntry.ip_address, keyEntry.l4_port, 0, 0); + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_SNAT_ENTRY); if (entry.entry_type == "static") { @@ -1670,6 +1678,7 @@ bool NatOrch::removeHwSnatEntry(const IpAddress &ip_address) } deleteNatCounters(ip_address); m_natEntries.erase(ip_address); + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_SNAT_ENTRY); if (entry.entry_type == "static") { @@ -1760,6 +1769,7 @@ bool NatOrch::removeHwSnaptEntry(const NaptEntryKey &keyEntry) } deleteNaptCounters(keyEntry.prototype.c_str(), keyEntry.ip_address, keyEntry.l4_port); m_naptEntries.erase(keyEntry); + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_SNAT_ENTRY); if (entry.entry_type == "static") { diff --git a/tests/test_nat.py b/tests/test_nat.py index 2c76e153c5d..3c4a5ddce33 100644 --- a/tests/test_nat.py +++ b/tests/test_nat.py @@ -287,9 +287,6 @@ def test_DelTwiceNaPtStaticEntry(self, dvs, testlog): #check the entry is not there in asic db self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NAT_ENTRY", 0) - # clear interfaces - self.clear_interfaces(dvs) - def test_VerifyConntrackTimeoutForNatEntry(self, dvs, testlog): # get neighbor and arp entry dvs.servers[0].runcmd("ping -c 1 18.18.18.2") @@ -353,6 +350,81 @@ def test_DoNotNatAclAction(self, dvs_acl, testlog): dvs_acl.remove_acl_table(L3_TABLE_NAME) dvs_acl.verify_acl_table_count(0) + def test_CrmSnatAndDnatEntryUsedCount(self, dvs, testlog): + # initialize + self.setup_db(dvs) + + # get neighbor and arp entry + dvs.servers[0].runcmd("ping -c 1 18.18.18.2") + + # set pooling interval to 1 + dvs.runcmd("crm config polling interval 1") + + dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_SNAT_ENTRY', '1000') + dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_DNAT_ENTRY', '1000') + + time.sleep(2) + + # get snat counters + used_snat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_snat_entry_used') + avail_snat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_snat_entry_available') + + # get dnat counters + used_dnat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_dnat_entry_used') + avail_dnat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_dnat_entry_available') + + # add a static nat entry + dvs.runcmd("config nat add static basic 67.66.65.1 18.18.18.2") + + #check the entry in asic db, 3 keys = SNAT, DNAT and DNAT_Pool + keys = self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NAT_ENTRY", 3) + for key in keys: + if (key.find("dst_ip:67.66.65.1")) or (key.find("src_ip:18.18.18.2")): + assert True + else: + assert False + + dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_SNAT_ENTRY', '999') + dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_DNAT_ENTRY', '999') + + time.sleep(2) + + # get snat counters + new_used_snat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_snat_entry_used') + new_avail_snat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_snat_entry_available') + + # get dnat counters + new_used_dnat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_dnat_entry_used') + new_avail_dnat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_dnat_entry_available') + + assert new_used_snat_counter - used_snat_counter == 1 + assert avail_snat_counter - new_avail_snat_counter == 1 + assert new_used_dnat_counter - used_dnat_counter == 1 + assert avail_dnat_counter - new_avail_dnat_counter == 1 + + # delete a static nat entry + dvs.runcmd("config nat remove static basic 67.66.65.1 18.18.18.2") + + dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_SNAT_ENTRY', '1000') + dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_DNAT_ENTRY', '1000') + + time.sleep(2) + + # get snat counters + new_used_snat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_snat_entry_used') + new_avail_snat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_snat_entry_available') + + # get dnat counters + new_used_dnat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_dnat_entry_used') + new_avail_dnat_counter = dvs.getCrmCounterValue('STATS', 'crm_stats_dnat_entry_available') + + assert new_used_snat_counter == used_snat_counter + assert new_avail_snat_counter == avail_snat_counter + assert new_used_dnat_counter == used_dnat_counter + assert new_avail_dnat_counter == avail_dnat_counter + + # clear interfaces + self.clear_interfaces(dvs) # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying