From f6bfaf35c788a0425fc250dc40da3fe87a76825b Mon Sep 17 00:00:00 2001 From: Nazarii Hnydyn Date: Fri, 10 Dec 2021 17:49:54 +0000 Subject: [PATCH 1/2] [pbh]: Implement Edit Flows. Signed-off-by: Nazarii Hnydyn --- common/configdb.cpp | 57 +++++++++++++++++++++++++++++++++++++++++++++ common/configdb.h | 6 +++-- common/schema.h | 1 + 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index f9f5015bb..6d8747bf3 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -326,6 +326,63 @@ void ConfigDBPipeConnector_Native::_delete_table(DBConnector& client, RedisTrans } } +// Write a table entry to config db +// Remove extra fields in the db which are not in the data +// Args: +// table: Table name +// key: Key of table entry, or a tuple of keys if it is a multi-key table +// data: Table row data in a form of dictionary {'column_key': 'value', ...} +// Pass {} as data will delete the entry +void ConfigDBPipeConnector_Native::set_entry(string table, string key, const map& data) +{ + auto& client = get_redis_client(m_db_name); + DBConnector clientPipe(client); + RedisTransactioner pipe(&clientPipe); + + pipe.multi(); + _set_entry(pipe, table, key, data); + pipe.exec(); +} + +// Write a table entry to config db +// Remove extra fields in the db which are not in the data +// Args: +// pipe: Redis DB pipe +// table: Table name +// key: Key of table entry, or a tuple of keys if it is a multi-key table +// data: Table row data in a form of dictionary {'column_key': 'value', ...} +// Pass {} as data will delete the entry +void ConfigDBPipeConnector_Native::_set_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data) +{ + string _hash = to_upper(table) + m_table_name_separator + key; + if (data.empty()) + { + RedisCommand sdel; + sdel.format("DEL %s", _hash.c_str()); + pipe.enqueue(sdel, REDIS_REPLY_INTEGER); + } + else + { + auto original = get_entry(table, key); + + RedisCommand shmset; + shmset.formatHMSET(_hash, data.begin(), data.end()); + pipe.enqueue(shmset, REDIS_REPLY_STATUS); + + for (auto& it: original) + { + auto& k = it.first; + bool found = data.find(k) != data.end(); + if (!found) + { + RedisCommand shdel; + shdel.formatHDEL(_hash, k); + pipe.enqueue(shdel, REDIS_REPLY_INTEGER); + } + } + } +} + // Modify a table entry to config db. // Args: // table: Table name. diff --git a/common/configdb.h b/common/configdb.h index f8b77ee69..25cbcf5dd 100644 --- a/common/configdb.h +++ b/common/configdb.h @@ -17,8 +17,8 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native void db_connect(std::string db_name, bool wait_for_init = false, bool retry_on = false); void connect(bool wait_for_init = true, bool retry_on = false); - void set_entry(std::string table, std::string key, const std::map& data); - void mod_entry(std::string table, std::string key, const std::map& data); + virtual void set_entry(std::string table, std::string key, const std::map& data); + virtual void mod_entry(std::string table, std::string key, const std::map& data); std::map get_entry(std::string table, std::string key); std::vector get_keys(std::string table, bool split = true); std::map> get_table(std::string table); @@ -218,6 +218,7 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native public: ConfigDBPipeConnector_Native(bool use_unix_socket_path = false, const char *netns = ""); + void set_entry(std::string table, std::string key, const std::map& data) override; void mod_config(const std::map>>& data) override; std::map>> get_config() override; @@ -226,6 +227,7 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native int _delete_entries(DBConnector& client, RedisTransactioner& pipe, const char *pattern, int cursor); void _delete_table(DBConnector& client, RedisTransactioner& pipe, std::string table); + void _set_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); void _mod_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor); }; diff --git a/common/schema.h b/common/schema.h index 71d196136..edf41ed19 100644 --- a/common/schema.h +++ b/common/schema.h @@ -374,6 +374,7 @@ namespace swss { #define STATE_SWITCH_CAPABILITY_TABLE_NAME "SWITCH_CAPABILITY_TABLE" #define STATE_ACL_STAGE_CAPABILITY_TABLE_NAME "ACL_STAGE_CAPABILITY_TABLE" +#define STATE_PBH_CAPABILITIES_TABLE_NAME "PBH_CAPABILITIES" #define STATE_PORT_TABLE_NAME "PORT_TABLE" #define STATE_LAG_TABLE_NAME "LAG_TABLE" #define STATE_VLAN_TABLE_NAME "VLAN_TABLE" From ae8ac1a65e2182df4f688d2e7498dc0886c1638b Mon Sep 17 00:00:00 2001 From: Nazarii Hnydyn Date: Tue, 8 Mar 2022 14:46:07 +0000 Subject: [PATCH 2/2] [ConfigDBPipeConnector]: Added UTs to increase code coverage. Signed-off-by: Nazarii Hnydyn --- tests/test_redis_ut.py | 89 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 10 deletions(-) diff --git a/tests/test_redis_ut.py b/tests/test_redis_ut.py index 1bebd2e04..d9389204d 100644 --- a/tests/test_redis_ut.py +++ b/tests/test_redis_ut.py @@ -398,24 +398,93 @@ def test_ConfigDBPipeConnector(): config_db = ConfigDBPipeConnector() config_db.connect(wait_for_init=False) config_db.get_redis_client(config_db.CONFIG_DB).flushdb() - config_db.set_entry("TEST_PORT", "Ethernet112", {"alias": "etp1x"}) + + # + # set_entry + # + + # Verify entry set + config_db.set_entry("PORT_TABLE", "Ethernet1", {"alias": "etp1x"}) allconfig = config_db.get_config() - assert allconfig["TEST_PORT"]["Ethernet112"]["alias"] == "etp1x" + assert allconfig["PORT_TABLE"]["Ethernet1"]["alias"] == "etp1x" - config_db.set_entry("TEST_PORT", "Ethernet112", {"mtu": "12345"}) - allconfig = config_db.get_config() - assert "alias" not in allconfig["TEST_PORT"]["Ethernet112"] - assert allconfig["TEST_PORT"]["Ethernet112"]["mtu"] == "12345" + config_db.set_entry("ACL_TABLE", "EVERFLOW", {"ports": ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"]}) + allconfig = config_db.get_config() + assert allconfig["ACL_TABLE"]["EVERFLOW"]["ports"] == ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] + + # Verify entry update + config_db.set_entry("PORT_TABLE", "Ethernet1", {"mtu": "12345"}) + allconfig = config_db.get_config() + assert "alias" not in allconfig["PORT_TABLE"]["Ethernet1"] + assert allconfig["PORT_TABLE"]["Ethernet1"]["mtu"] == "12345" + + # Verify entry clear + config_db.set_entry("PORT_TABLE", "Ethernet1", {}) + allconfig = config_db.get_config() + assert len(allconfig["PORT_TABLE"]["Ethernet1"]) == 0 + + # Verify entry delete + config_db.set_entry("PORT_TABLE", "Ethernet1", None) + config_db.set_entry("ACL_TABLE", "EVERFLOW", None) + allconfig = config_db.get_config() + assert len(allconfig) == 0 + + # + # mod_config + # + + # Verify entry set + allconfig.setdefault("PORT_TABLE", {}).setdefault("Ethernet1", {}) + allconfig["PORT_TABLE"]["Ethernet1"]["alias"] = "etp1x" + config_db.mod_config(allconfig) + allconfig = config_db.get_config() + assert allconfig["PORT_TABLE"]["Ethernet1"]["alias"] == "etp1x" + allconfig.setdefault("VLAN_TABLE", {}) + allconfig["VLAN_TABLE"]["Vlan1"] = {} config_db.mod_config(allconfig) - allconfig["TEST_PORT"]["Ethernet113"] = None - allconfig["TEST_VLAN"] = None + allconfig = config_db.get_config() + assert len(allconfig["VLAN_TABLE"]["Vlan1"]) == 0 + + allconfig.setdefault("ACL_TABLE", {}).setdefault("EVERFLOW", {}) + allconfig["ACL_TABLE"]["EVERFLOW"]["ports"] = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] config_db.mod_config(allconfig) - allconfig.setdefault("ACL_TABLE", {}).setdefault("EVERFLOW", {})["ports"] = ["Ethernet0", "Ethernet4", "Ethernet8"] + allconfig = config_db.get_config() + assert allconfig["ACL_TABLE"]["EVERFLOW"]["ports"] == ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] + + # Verify entry delete + allconfig["PORT_TABLE"]["Ethernet1"] = None + allconfig["VLAN_TABLE"]["Vlan1"] = None + allconfig["ACL_TABLE"]["EVERFLOW"] = None config_db.mod_config(allconfig) allconfig = config_db.get_config() + assert len(allconfig) == 0 - config_db.delete_table("TEST_PORT") + # Verify table delete + for i in range(1, 1001, 1): + # Make sure we have enough entries to trigger REDIS_SCAN_BATCH_SIZE + allconfig.setdefault("PORT_TABLE", {}).setdefault("Ethernet{}".format(i), {}) + allconfig["PORT_TABLE"]["Ethernet{}".format(i)]["alias"] = "etp{}x".format(i) + + config_db.mod_config(allconfig) + allconfig = config_db.get_config() + assert len(allconfig["PORT_TABLE"]) == 1000 + + allconfig["PORT_TABLE"] = None + config_db.mod_config(allconfig) + allconfig = config_db.get_config() + assert len(allconfig) == 0 + + # + # delete_table + # + + # Verify direct table delete + allconfig.setdefault("PORT_TABLE", {}).setdefault("Ethernet1", {}) + allconfig["PORT_TABLE"]["Ethernet1"]["alias"] = "etp1x" + allconfig.setdefault("ACL_TABLE", {}).setdefault("EVERFLOW", {}) + allconfig["ACL_TABLE"]["EVERFLOW"]["ports"] = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] + config_db.delete_table("PORT_TABLE") config_db.delete_table("ACL_TABLE") allconfig = config_db.get_config() assert len(allconfig) == 0