From d257cb1a3b4857a2973221e8ede0e9ce8c9823fe Mon Sep 17 00:00:00 2001 From: vedganes Date: Tue, 15 Dec 2020 16:40:57 -0500 Subject: [PATCH 1/3] [lagidallocator] LAG allocator class Signed-off-by: vedganes Class for lag id allocation in atomic fashion. The LAG ID is allocated from central chassis app db. A lua script loaded in the redis at the time of lag allocator instantiaion ensures allocation unqiue lag id when multiple clients requests for lag id simultaneously. System wide unique lag id is used for system lag in VOQ based switches. --- orchagent/lagid.cpp | 105 +++++++++++++++++++++++++++++++++++++++++++ orchagent/lagid.h | 44 ++++++++++++++++++ orchagent/lagids.lua | 90 +++++++++++++++++++++++++++++++++++++ 3 files changed, 239 insertions(+) create mode 100644 orchagent/lagid.cpp create mode 100644 orchagent/lagid.h create mode 100644 orchagent/lagids.lua diff --git a/orchagent/lagid.cpp b/orchagent/lagid.cpp new file mode 100644 index 0000000000..0fac772e6d --- /dev/null +++ b/orchagent/lagid.cpp @@ -0,0 +1,105 @@ +#include "lagid.h" + +LagIdAllocator::LagIdAllocator( + _In_ DBConnector* chassis_app_db) +{ + SWSS_LOG_ENTER(); + + m_dbConnector = chassis_app_db; + + // Load lua script to allocate system lag id. This lua script ensures allocation + // of unique system lag id from global chassis app db in atomic fashion when allocation + // is requested by different asic instances simultaneously + + string luaScript = loadLuaScript("lagids.lua"); + m_shaLagId = loadRedisScript(m_dbConnector, luaScript); +} + +int32_t LagIdAllocator::lagIdAdd( + _In_ const string &pcname, + _In_ int32_t lag_id) +{ + SWSS_LOG_ENTER(); + + vector keys; + keys.push_back(CHASSIS_APP_LAG_ID_START_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_END_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_TABLE_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_SET_NAME); + + vector args; + args.push_back("add"); + args.push_back(pcname); + args.push_back(to_string(lag_id)); + + set ret = runRedisScript(*m_dbConnector, m_shaLagId, keys, args); + + if (!ret.empty()) + { + // We expect only one value in the set returned + + auto rv_lag_id = ret.begin(); + + return (stoi(*rv_lag_id)); + } + + return LAG_ID_ALLOCATOR_ERROR_DB_ERROR; +} + +int32_t LagIdAllocator::lagIdDel( + _In_ const string &pcname) +{ + SWSS_LOG_ENTER(); + + vector keys; + keys.push_back(CHASSIS_APP_LAG_ID_START_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_END_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_TABLE_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_SET_NAME); + + vector args; + args.push_back("del"); + args.push_back(pcname); + + set ret = runRedisScript(*m_dbConnector, m_shaLagId, keys, args); + + if (!ret.empty()) + { + // We expect only one value in the set returned + + auto rv_lag_id = ret.begin(); + + return (stoi(*rv_lag_id)); + } + + return LAG_ID_ALLOCATOR_ERROR_DB_ERROR; +} + +int32_t LagIdAllocator::lagIdGet( + _In_ const string &pcname) +{ + SWSS_LOG_ENTER(); + + vector keys; + keys.push_back(CHASSIS_APP_LAG_ID_START_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_END_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_TABLE_NAME); + keys.push_back(CHASSIS_APP_LAG_ID_SET_NAME); + + vector args; + args.push_back("get"); + args.push_back(pcname); + + set ret = runRedisScript(*m_dbConnector, m_shaLagId, keys, args); + + if (!ret.empty()) + { + // We expect only one value in the set returned + + auto rv_lag_id = ret.begin(); + + return (stoi(*rv_lag_id)); + } + + return LAG_ID_ALLOCATOR_ERROR_DB_ERROR; +} diff --git a/orchagent/lagid.h b/orchagent/lagid.h new file mode 100644 index 0000000000..9b043741a1 --- /dev/null +++ b/orchagent/lagid.h @@ -0,0 +1,44 @@ +#ifndef SWSS_LAGID_H +#define SWSS_LAGID_H + +#include "dbconnector.h" +#include "sal.h" +#include "schema.h" +#include "redisapi.h" + +using namespace swss; +using namespace std; + +#define LAG_ID_ALLOCATOR_ERROR_DELETE_ENTRY_NOT_FOUND 0 +#define LAG_ID_ALLOCATOR_ERROR_TABLE_FULL -1 +#define LAG_ID_ALLOCATOR_ERROR_GET_ENTRY_NOT_FOUND -2 +#define LAG_ID_ALLOCATOR_ERROR_INVALID_OP -3 +#define LAG_ID_ALLOCATOR_ERROR_DB_ERROR -4 + +class LagIdAllocator +{ +public: + + LagIdAllocator( + _In_ DBConnector* chassis_app_db); + +public: + + int32_t lagIdAdd( + _In_ const string &pcname, + _In_ int32_t lag_id); + + int32_t lagIdDel( + _In_ const string &pcname); + + int32_t lagIdGet( + _In_ const string &pcname); + +private: + + DBConnector* m_dbConnector; + + string m_shaLagId; +}; + +#endif // SWSS_LAGID_H diff --git a/orchagent/lagids.lua b/orchagent/lagids.lua new file mode 100644 index 0000000000..979d2eb80f --- /dev/null +++ b/orchagent/lagids.lua @@ -0,0 +1,90 @@ +-- KEYS - lagid_start, lagid_end, lagids, lagid_set +-- ARGV[1] - operation (add/del/get) +-- ARGV[2] - lag name +-- ARGV[3] - current lag id (for "add" operation only) + +-- return lagid if success for "add"/"del" +-- return 0 if lag not exists for "del" +-- return -1 if lag table full for "add" +-- return -2 if log does exist for "get" +-- return -3 if invalid operation + +local op = ARGV[1] +local pcname = ARGV[2] + +local lagid_start = tonumber(redis.call("get", KEYS[1])) +local lagid_end = tonumber(redis.call("get", KEYS[2])) + +if op == "add" then + + local plagid = tonumber(ARGV[3]) + + local dblagid = redis.call("hget", KEYS[3], pcname) + + if dblagid then + dblagid = tonumber(dblagid) + if plagid == 0 then + -- no lagid propsed. Return the existing lagid + redis.call("sadd", KEYS[4], tostring(dblagid)) + return dblagid + end + end + + -- lagid allocation request with a lagid proposal + if plagid >= lagid_start and plagid <= lagid_end then + if plagid == dblagid then + -- proposed lagid is same as the lagid in database + redis.call("sadd", KEYS[4], tostring(plagid)) + return plagid + end + -- proposed lag id is different than that in database OR + -- the portchannel does not exist in the database + -- If proposed lagid is available, return the same proposed lag id + if redis.call("sismember", KEYS[4], tostring(plagid)) == 0 then + redis.call("sadd", KEYS[4], tostring(plagid)) + redis.call("srem", KEYS[4], tostring(dblagid)) + redis.call("hset", KEYS[3], pcname, tostring(plagid)) + return plagid + end + end + + local lagid = lagid_start + while lagid <= lagid_end do + if redis.call("sismember", KEYS[4], tostring(lagid)) == 0 then + redis.call("sadd", KEYS[4], tostring(lagid)) + redis.call("srem", KEYS[4], tostring(dblagid)) + redis.call("hset", KEYS[3], pcname, tostring(lagid)) + return lagid + end + lagid = lagid + 1 + end + + return -1 + +end + +if op == "del" then + + if redis.call("hexists", KEYS[3], pcname) == 1 then + local lagid = redis.call("hget", KEYS[3], pcname) + redis.call("srem", KEYS[4], lagid) + redis.call("hdel", KEYS[3], pcname) + return tonumber(lagid) + end + + return 0 + +end + +if op == "get" then + + if redis.call("hexists", KEYS[3], pcname) == 1 then + local lagid = redis.call("hget", KEYS[3], pcname) + return tonumber(lagid) + end + + return -2 + +end + +return -3 From 85cc7d6b36794e203aae6b89f3816d6bbd5f6e22 Mon Sep 17 00:00:00 2001 From: vedganes Date: Fri, 25 Dec 2020 12:01:31 -0500 Subject: [PATCH 2/3] [lagidallocator] Code review comments fix - 1 Signed-off-by: vedganes Changes to fix code review comments. The lua script evaluation is not passed with keys. The keys for the required data are internally hardcoded in the script itself. --- orchagent/lagid.cpp | 15 +++------------ orchagent/lagids.lua | 40 ++++++++++++++++++++-------------------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/orchagent/lagid.cpp b/orchagent/lagid.cpp index 0fac772e6d..45f489639f 100644 --- a/orchagent/lagid.cpp +++ b/orchagent/lagid.cpp @@ -21,11 +21,8 @@ int32_t LagIdAllocator::lagIdAdd( { SWSS_LOG_ENTER(); + // No keys vector keys; - keys.push_back(CHASSIS_APP_LAG_ID_START_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_END_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_TABLE_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_SET_NAME); vector args; args.push_back("add"); @@ -51,11 +48,8 @@ int32_t LagIdAllocator::lagIdDel( { SWSS_LOG_ENTER(); + // No keys vector keys; - keys.push_back(CHASSIS_APP_LAG_ID_START_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_END_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_TABLE_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_SET_NAME); vector args; args.push_back("del"); @@ -80,11 +74,8 @@ int32_t LagIdAllocator::lagIdGet( { SWSS_LOG_ENTER(); + // No keys vector keys; - keys.push_back(CHASSIS_APP_LAG_ID_START_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_END_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_TABLE_NAME); - keys.push_back(CHASSIS_APP_LAG_ID_SET_NAME); vector args; args.push_back("get"); diff --git a/orchagent/lagids.lua b/orchagent/lagids.lua index 979d2eb80f..f2e902f14d 100644 --- a/orchagent/lagids.lua +++ b/orchagent/lagids.lua @@ -1,4 +1,4 @@ --- KEYS - lagid_start, lagid_end, lagids, lagid_set +-- KEYS - None -- ARGV[1] - operation (add/del/get) -- ARGV[2] - lag name -- ARGV[3] - current lag id (for "add" operation only) @@ -12,20 +12,20 @@ local op = ARGV[1] local pcname = ARGV[2] -local lagid_start = tonumber(redis.call("get", KEYS[1])) -local lagid_end = tonumber(redis.call("get", KEYS[2])) +local lagid_start = tonumber(redis.call("get", "SYSTEM_LAG_ID_START")) +local lagid_end = tonumber(redis.call("get", "SYSTEM_LAG_ID_END")) if op == "add" then local plagid = tonumber(ARGV[3]) - local dblagid = redis.call("hget", KEYS[3], pcname) + local dblagid = redis.call("hget", "SYSTEM_LAG_ID_TABLE", pcname) if dblagid then dblagid = tonumber(dblagid) if plagid == 0 then -- no lagid propsed. Return the existing lagid - redis.call("sadd", KEYS[4], tostring(dblagid)) + redis.call("sadd", "SYSTEM_LAG_ID_SET", tostring(dblagid)) return dblagid end end @@ -34,26 +34,26 @@ if op == "add" then if plagid >= lagid_start and plagid <= lagid_end then if plagid == dblagid then -- proposed lagid is same as the lagid in database - redis.call("sadd", KEYS[4], tostring(plagid)) + redis.call("sadd", "SYSTEM_LAG_ID_SET", tostring(plagid)) return plagid end -- proposed lag id is different than that in database OR -- the portchannel does not exist in the database -- If proposed lagid is available, return the same proposed lag id - if redis.call("sismember", KEYS[4], tostring(plagid)) == 0 then - redis.call("sadd", KEYS[4], tostring(plagid)) - redis.call("srem", KEYS[4], tostring(dblagid)) - redis.call("hset", KEYS[3], pcname, tostring(plagid)) + if redis.call("sismember", "SYSTEM_LAG_ID_SET", tostring(plagid)) == 0 then + redis.call("sadd", "SYSTEM_LAG_ID_SET", tostring(plagid)) + redis.call("srem", "SYSTEM_LAG_ID_SET", tostring(dblagid)) + redis.call("hset", "SYSTEM_LAG_ID_TABLE", pcname, tostring(plagid)) return plagid end end local lagid = lagid_start while lagid <= lagid_end do - if redis.call("sismember", KEYS[4], tostring(lagid)) == 0 then - redis.call("sadd", KEYS[4], tostring(lagid)) - redis.call("srem", KEYS[4], tostring(dblagid)) - redis.call("hset", KEYS[3], pcname, tostring(lagid)) + if redis.call("sismember", "SYSTEM_LAG_ID_SET", tostring(lagid)) == 0 then + redis.call("sadd", "SYSTEM_LAG_ID_SET", tostring(lagid)) + redis.call("srem", "SYSTEM_LAG_ID_SET", tostring(dblagid)) + redis.call("hset", "SYSTEM_LAG_ID_TABLE", pcname, tostring(lagid)) return lagid end lagid = lagid + 1 @@ -65,10 +65,10 @@ end if op == "del" then - if redis.call("hexists", KEYS[3], pcname) == 1 then - local lagid = redis.call("hget", KEYS[3], pcname) - redis.call("srem", KEYS[4], lagid) - redis.call("hdel", KEYS[3], pcname) + if redis.call("hexists", "SYSTEM_LAG_ID_TABLE", pcname) == 1 then + local lagid = redis.call("hget", "SYSTEM_LAG_ID_TABLE", pcname) + redis.call("srem", "SYSTEM_LAG_ID_SET", lagid) + redis.call("hdel", "SYSTEM_LAG_ID_TABLE", pcname) return tonumber(lagid) end @@ -78,8 +78,8 @@ end if op == "get" then - if redis.call("hexists", KEYS[3], pcname) == 1 then - local lagid = redis.call("hget", KEYS[3], pcname) + if redis.call("hexists", "SYSTEM_LAG_ID_TABLE", pcname) == 1 then + local lagid = redis.call("hget", "SYSTEM_LAG_ID_TABLE", pcname) return tonumber(lagid) end From 8688425705348f39b72038e582d330bf18976bcc Mon Sep 17 00:00:00 2001 From: vedganes Date: Tue, 9 Feb 2021 22:43:44 -0500 Subject: [PATCH 3/3] [voq/lagid]Code review comments fix2 Signed-off-by: vedganes Typos fixed. Redundant redis set call is removed. --- orchagent/lagids.lua | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/orchagent/lagids.lua b/orchagent/lagids.lua index f2e902f14d..93a546cad1 100644 --- a/orchagent/lagids.lua +++ b/orchagent/lagids.lua @@ -4,9 +4,9 @@ -- ARGV[3] - current lag id (for "add" operation only) -- return lagid if success for "add"/"del" --- return 0 if lag not exists for "del" +-- return 0 if lag does not exist for "del" -- return -1 if lag table full for "add" --- return -2 if log does exist for "get" +-- return -2 if lag does not exist for "get" -- return -3 if invalid operation local op = ARGV[1] @@ -24,8 +24,7 @@ if op == "add" then if dblagid then dblagid = tonumber(dblagid) if plagid == 0 then - -- no lagid propsed. Return the existing lagid - redis.call("sadd", "SYSTEM_LAG_ID_SET", tostring(dblagid)) + -- no lagid proposed. Return the existing lagid return dblagid end end @@ -34,7 +33,6 @@ if op == "add" then if plagid >= lagid_start and plagid <= lagid_end then if plagid == dblagid then -- proposed lagid is same as the lagid in database - redis.call("sadd", "SYSTEM_LAG_ID_SET", tostring(plagid)) return plagid end -- proposed lag id is different than that in database OR