Skip to content

Commit

Permalink
Modify coppmgr mergeConfig to support preserving copp tables through …
Browse files Browse the repository at this point in the history
…reboot. (sonic-net#2548)

This PR should be merged together with sonic-net/sonic-utilities#2524 and is required to 202205 and 202211.
This PR implements [fastboot] Preserve CoPP table HLD to improve fastboot flow (sonic-net/SONiC#1107).

- What I did
Modified coppmgr mergeConfig logic to support preserving copp tables contents through reboot. Handle duplicates, overwrites, unsupported keys preserved and new keys. According to sonic-net/SONiC#1107

- Why I did it
To shorten dataplane downtime on fast-reboot.

- How I verified it
Manual tests (community fast-reboot test)
  • Loading branch information
arfeigin authored Jan 1, 2023
1 parent 7891e78 commit bdedf69
Show file tree
Hide file tree
Showing 5 changed files with 246 additions and 0 deletions.
57 changes: 57 additions & 0 deletions cfgmgr/coppmgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "shellcmd.h"
#include "warm_restart.h"
#include "json.hpp"
#include <unordered_map>
#include <unordered_set>

using json = nlohmann::json;

Expand Down Expand Up @@ -255,6 +257,42 @@ void CoppMgr::mergeConfig(CoppCfg &init_cfg, CoppCfg &m_cfg, std::vector<std::st
}
}

bool CoppMgr::isDupEntry(const std::string &key, std::vector<FieldValueTuple> &fvs)
{
/* Compare with the existing contents of copp tables, in case for a key K preserved fvs are the same
* as the fvs in trap_group_fvs it will be ignored as a duplicate continue to next key.
* In case one of the fvs differs the preserved entry will be deleted and new entry will be set instead.
*/
std::vector<FieldValueTuple> preserved_fvs;
bool key_found = m_coppTable.get(key, preserved_fvs);
if (!key_found)
{
return false;
}
else
{
unordered_map<string, string> preserved_copp_entry;
for (auto prev_fv : preserved_fvs)
{
preserved_copp_entry[fvField(prev_fv)] = fvValue(prev_fv);
}
for (auto fv: fvs)
{
string field = fvField(fv);
string value = fvValue(fv);
auto preserved_copp_it = preserved_copp_entry.find(field);
bool field_found = (preserved_copp_it != preserved_copp_entry.end());
if ((!field_found) || (field_found && preserved_copp_it->second.compare(value)))
{
// overwrite -> delete preserved entry from copp table and set a new entry instead
m_coppTable.del(key);
return false;
}
}
}
return true;
}

CoppMgr::CoppMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector<string> &tableNames) :
Orch(cfgDb, tableNames),
m_cfgCoppTrapTable(cfgDb, CFG_COPP_TRAP_TABLE_NAME),
Expand All @@ -270,16 +308,19 @@ CoppMgr::CoppMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
std::vector<string> group_keys;
std::vector<string> trap_keys;
std::vector<string> feature_keys;
std::vector<string> preserved_copp_keys;

std::vector<string> group_cfg_keys;
std::vector<string> trap_cfg_keys;
unordered_set<string> supported_copp_keys;

CoppCfg group_cfg;
CoppCfg trap_cfg;

m_cfgCoppGroupTable.getKeys(group_cfg_keys);
m_cfgCoppTrapTable.getKeys(trap_cfg_keys);
m_cfgFeatureTable.getKeys(feature_keys);
m_coppTable.getKeys(preserved_copp_keys);


for (auto i: feature_keys)
Expand Down Expand Up @@ -352,15 +393,31 @@ CoppMgr::CoppMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c

if (!trap_group_fvs.empty())
{
supported_copp_keys.emplace(i.first);
if (isDupEntry(i.first, trap_group_fvs))
{
continue;
}
m_appCoppTable.set(i.first, trap_group_fvs);
}

setCoppGroupStateOk(i.first);
auto g_cfg = std::find(group_cfg_keys.begin(), group_cfg_keys.end(), i.first);
if (g_cfg != group_cfg_keys.end())
{
g_copp_init_set.insert(i.first);
}
}

// Delete unsupported keys from preserved copp tables
for (auto it : preserved_copp_keys)
{
auto copp_it = supported_copp_keys.find(it);
if (copp_it == supported_copp_keys.end())
{
m_coppTable.del(it);
}
}
}

void CoppMgr::setCoppGroupStateOk(string alias)
Expand Down
1 change: 1 addition & 0 deletions cfgmgr/coppmgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class CoppMgr : public Orch
bool isTrapGroupInstalled(std::string key);
bool isFeatureEnabled(std::string feature);
void mergeConfig(CoppCfg &init_cfg, CoppCfg &m_cfg, std::vector<std::string> &cfg_keys, Table &cfgTable);
bool isDupEntry(const std::string &key, std::vector<FieldValueTuple> &fvs);

void removeTrap(std::string key);
void addTrap(std::string trap_ids, std::string trap_group);
Expand Down
1 change: 1 addition & 0 deletions tests/mock_tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ tests_SOURCES = aclorch_ut.cpp \
bufferorch_ut.cpp \
buffermgrdyn_ut.cpp \
fdborch/flush_syncd_notif_ut.cpp \
copp_ut.cpp \
copporch_ut.cpp \
saispy_ut.cpp \
consumer_ut.cpp \
Expand Down
111 changes: 111 additions & 0 deletions tests/mock_tests/copp_cfg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
{
"COPP_GROUP": {
"default": {
"queue": "0",
"meter_type":"packets",
"mode":"sr_tcm",
"cir":"600",
"cbs":"600",
"red_action":"drop"
},
"queue4_group1": {
"trap_action":"trap",
"trap_priority":"4",
"queue": "4"
},
"queue4_group2": {
"trap_action":"copy",
"trap_priority":"4",
"queue": "4",
"meter_type":"packets",
"mode":"sr_tcm",
"cir":"600",
"cbs":"600",
"red_action":"drop"
},
"queue4_group3": {
"trap_action":"trap",
"trap_priority":"4",
"queue": "4"
},
"queue1_group1": {
"trap_action":"trap",
"trap_priority":"1",
"queue": "1",
"meter_type":"packets",
"mode":"sr_tcm",
"cir":"6000",
"cbs":"6000",
"red_action":"drop"
},
"queue1_group2": {
"trap_action":"trap",
"trap_priority":"1",
"queue": "1",
"meter_type":"packets",
"mode":"sr_tcm",
"cir":"600",
"cbs":"600",
"red_action":"drop"
},
"queue2_group1": {
"cbs": "1000",
"cir": "1000",
"genetlink_mcgrp_name": "packets",
"genetlink_name": "psample",
"meter_type": "packets",
"mode": "sr_tcm",
"queue": "2",
"red_action": "drop",
"trap_action": "trap",
"trap_priority": "1"

}
},
"COPP_TRAP": {
"bgp": {
"trap_ids": "bgp,bgpv6",
"trap_group": "queue4_group1"
},
"lacp": {
"trap_ids": "lacp",
"trap_group": "queue4_group1",
"always_enabled": "true"
},
"arp": {
"trap_ids": "arp_req,arp_resp,neigh_discovery",
"trap_group": "queue4_group2",
"always_enabled": "true"
},
"lldp": {
"trap_ids": "lldp",
"trap_group": "queue4_group3"
},
"dhcp_relay": {
"trap_ids": "dhcp,dhcpv6",
"trap_group": "queue4_group3"
},
"udld": {
"trap_ids": "udld",
"trap_group": "queue4_group3",
"always_enabled": "true"
},
"ip2me": {
"trap_ids": "ip2me",
"trap_group": "queue1_group1",
"always_enabled": "true"
},
"macsec": {
"trap_ids": "eapol",
"trap_group": "queue4_group3"
},
"nat": {
"trap_ids": "src_nat_miss,dest_nat_miss",
"trap_group": "queue1_group2"
},
"sflow": {
"trap_group": "queue2_group1",
"trap_ids": "sample_packet"
}
}
}
76 changes: 76 additions & 0 deletions tests/mock_tests/copp_ut.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#include "gtest/gtest.h"
#include <string>
#include "schema.h"
#include "warm_restart.h"
#include "ut_helper.h"
#include "coppmgr.h"
#include "coppmgr.cpp"
#include <fstream>
#include <streambuf>
using namespace std;
using namespace swss;

void create_init_file()
{
int status = system("sudo mkdir /etc/sonic/");
ASSERT_EQ(status, 0);

status = system("sudo chmod 777 /etc/sonic/");
ASSERT_EQ(status, 0);

status = system("sudo cp copp_cfg.json /etc/sonic/");
ASSERT_EQ(status, 0);
}

void cleanup()
{
int status = system("sudo rm -rf /etc/sonic/");
ASSERT_EQ(status, 0);
}

TEST(CoppMgrTest, CoppTest)
{
create_init_file();

const vector<string> cfg_copp_tables = {
CFG_COPP_TRAP_TABLE_NAME,
CFG_COPP_GROUP_TABLE_NAME,
CFG_FEATURE_TABLE_NAME,
};

WarmStart::initialize("coppmgrd", "swss");
WarmStart::checkWarmStart("coppmgrd", "swss");

DBConnector cfgDb("CONFIG_DB", 0);
DBConnector appDb("APPL_DB", 0);
DBConnector stateDb("STATE_DB", 0);

/* The test will set an entry with queue1_group1|cbs value which differs from the init value
* found in the copp_cfg.json file. Then coppmgr constructor will be called and it will detect
* that there is already an entry for queue1_group1|cbs with different value and it should be
* overwritten with the init value.
* hget will verify that this indeed happened.
*/
Table coppTable = Table(&appDb, APP_COPP_TABLE_NAME);
coppTable.set("queue1_group1",
{
{"cbs", "6100"},
{"cir", "6000"},
{"meter_type", "packets"},
{"mode", "sr_tcm"},
{"queue", "1"},
{"red_action", "drop"},
{"trap_action", "trap"},
{"trap_priority", "1"},
{"trap_ids", "ip2me"}
});

CoppMgr coppmgr(&cfgDb, &appDb, &stateDb, cfg_copp_tables);

string overide_val;
coppTable.hget("queue1_group1", "cbs",overide_val);
EXPECT_EQ( overide_val, "6000");

cleanup();
}

0 comments on commit bdedf69

Please sign in to comment.