Skip to content

Commit

Permalink
Signed-off-by: Kannan Selvaraj <in62012518@celestica.com>
Browse files Browse the repository at this point in the history
    Why ?
    Static lag support changes in SWSS module
    sonic-net/SONiC#1039

    Patch explanation
    1. static lag supported with option roundrobin.
    2. tlm_teamd -> teamdctl changes to handle json dump for static lag.
    3. test cases -> updated

    UT:-
            Test cases
    1       Create static port channel with static flag     pass    pass
    2       verify static has option flag true or false     pass    pass
    3       Add static member see the portchannel is up     pass    pass
    4       verify teamd is created with round-robin option by default
    pass    pass
    5       Remove last portchannel member check port channel down  pass
    pass
    6       Remove portchannel member check port channel still up   pass
    pass
    7       verify teamdctl config dump     pass    pass
    8       verify teamdctl state dump      pass    pass
    9       shutdown the portchannel check the kernel state pass    pass
    10      no shutdown the portchannel check the kernel state      pass
    pass
    11      "Check the show output matches the review comment
    root@sonic:~# show inter port
    Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not
    available,
           S - selected, D - deselected, * - not synced
      No.  Team Dev       Protocol     Ports
    -----  -------------  -----------
    -----------------------------------------
       01  PortChannel01  LACP(A)(Up)  Ethernet16(S)
       02  PortChannel02  NONE(A)(Up)  Ethernet48(S) Ethernet64(S)
    Ethernet32(S)
    root@sonic:~#
    12      teamnl is set to roundrobin     pass    pass
    13      save and reload and verify portchannel is up    pass    pass
    14      "docker restart teamd
    teamd stopped
    swss stopped
    syncd stopped

    swss started
    syncd started
    teamd started"  pass    pass
    15      warm-reboot fails even without any port channel config  fail
    16      verify teamd settles doesnt hog cpu with 100% cpu usage pass
    17      "trying with static port channel config on non supported
    branches
    port channel will be configured as LACP."               pass

    Not Supported Options
    1. Min links and
    2. fall back are not supported
  • Loading branch information
Kannan Selvaraj committed Mar 25, 2024
1 parent 91bacca commit 3b43772
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 9 deletions.
29 changes: 22 additions & 7 deletions cfgmgr/teammgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ void TeamMgr::doLagTask(Consumer &consumer)
int min_links = 0;
bool fallback = false;
bool fast_rate = false;
bool static_lag = false;
string admin_status = DEFAULT_ADMIN_STATUS_STR;
string mtu = DEFAULT_MTU_STR;
string learn_mode;
Expand Down Expand Up @@ -301,11 +302,16 @@ void TeamMgr::doLagTask(Consumer &consumer)
SWSS_LOG_INFO("Get fast_rate `%s`",
fast_rate ? "true" : "false");
}
else if (fvField(i) == "static")
{
static_lag = true;
SWSS_LOG_INFO ("static %d", static_lag);
}
}

if (m_lagList.find(alias) == m_lagList.end())
{
if (addLag(alias, min_links, fallback, fast_rate) == task_need_retry)
if (addLag(alias, min_links, fallback, fast_rate, static_lag) == task_need_retry)
{
// If LAG creation fails, we need to clean up any potentially orphaned teamd processes
removeLag(alias);
Expand Down Expand Up @@ -562,7 +568,7 @@ bool TeamMgr::setLagLearnMode(const string &alias, const string &learn_mode)
return true;
}

task_process_status TeamMgr::addLag(const string &alias, int min_links, bool fallback, bool fast_rate)
task_process_status TeamMgr::addLag(const string &alias, int min_links, bool fallback, bool fast_rate, bool static_lag)
{
SWSS_LOG_ENTER();

Expand Down Expand Up @@ -603,11 +609,20 @@ task_process_status TeamMgr::addLag(const string &alias, int min_links, bool fal
}
}

conf << "'{\"device\":\"" << alias << "\","
<< "\"hwaddr\":\"" << mac_boot.to_string() << "\","
<< "\"runner\":{"
<< "\"active\":true,"
<< "\"name\":\"lacp\"";
if (static_lag) {
conf << "'{\"device\":\"" << alias << "\","
<< "\"hwaddr\":\"" << mac_boot.to_string() << "\","
<< "\"runner\":{"
<< "\"name\":\"roundrobin\"";

} else {

conf << "'{\"device\":\"" << alias << "\","
<< "\"hwaddr\":\"" << mac_boot.to_string() << "\","
<< "\"runner\":{"
<< "\"active\":true,"
<< "\"name\":\"lacp\"";
}

if (min_links != 0)
{
Expand Down
2 changes: 1 addition & 1 deletion cfgmgr/teammgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class TeamMgr : public Orch
void doLagMemberTask(Consumer &consumer);
void doPortUpdateTask(Consumer &consumer);

task_process_status addLag(const std::string &alias, int min_links, bool fall_back, bool fast_rate);
task_process_status addLag(const std::string &alias, int min_links, bool fall_back, bool fast_rate, bool static_lag);
bool removeLag(const std::string &alias);
task_process_status addLagMember(const std::string &lag, const std::string &member);
bool removeLagMember(const std::string &lag, const std::string &member);
Expand Down
20 changes: 19 additions & 1 deletion tests/mock_tests/teammgrd/teammgr_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,22 @@ namespace teammgr_ut
}
ASSERT_EQ(kill_cmd_called, 1);
}
}

TEST_F(TeamMgrTest, testProcessKilledAfterAddStaticFailure)
{
swss::TeamMgr teammgr(m_config_db.get(), m_app_db.get(), m_state_db.get(), cfg_lag_tables);
swss::Table cfg_lag_table = swss::Table(m_config_db.get(), CFG_LAG_TABLE_NAME);
cfg_lag_table.set("PortChannel1", { { "admin_status", "up" },
{ "mtu", "9100" },
{ "static", "true" } });
teammgr.addExistingData(&cfg_lag_table);
teammgr.doTask();
int kill_cmd_called = 0;
for (auto cmd : mockCallArgs){
if (cmd.find("kill -TERM 1234") != std::string::npos){
kill_cmd_called++;
}
}
ASSERT_EQ(kill_cmd_called, 1);
}
}
69 changes: 69 additions & 0 deletions tests/test_portchannel.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,75 @@ def test_portchannel_member_netdev_oper_status(self, dvs, testlog):
# wait for port-channel deletion
time.sleep(1)


def test_static_portchannel_member_netdev_oper_status(self, dvs, testlog):
config_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0)
state_db = swsscommon.DBConnector(swsscommon.STATE_DB, dvs.redis_sock, 0)
app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)

# create port-channel
tbl = swsscommon.Table(config_db, "PORTCHANNEL")
fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("oper_status", "up"),("static": "true")])
tbl.set("PortChannel111", fvs)

# set port-channel oper status
tbl = swsscommon.ProducerStateTable(app_db, "LAG_TABLE")
fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("oper_status", "up")])
tbl.set("PortChannel111", fvs)

# add members to port-channel
tbl = swsscommon.Table(config_db, "PORTCHANNEL_MEMBER")
fvs = swsscommon.FieldValuePairs([("NULL", "NULL")])
tbl.set("PortChannel111|Ethernet0", fvs)
tbl.set("PortChannel111|Ethernet4", fvs)

# wait for port-channel netdev creation
time.sleep(1)

# set netdev oper status
(exitcode, _) = dvs.runcmd("ip link set up dev Ethernet0")
assert exitcode == 0, "ip link set failed"

(exitcode, _) = dvs.runcmd("ip link set up dev Ethernet4")
assert exitcode == 0, "ip link set failed"

(exitcode, _) = dvs.runcmd("ip link set dev PortChannel111 carrier on")
assert exitcode == 0, "ip link set failed"

# verify port-channel members netdev oper status
tbl = swsscommon.Table(state_db, "PORT_TABLE")
status, fvs = tbl.get("Ethernet0")
assert status is True
fvs = dict(fvs)
assert fvs['netdev_oper_status'] == 'up'

status, fvs = tbl.get("Ethernet4")
assert status is True
fvs = dict(fvs)
assert fvs['netdev_oper_status'] == 'up'

# wait for port-channel netdev creation
time.sleep(1)

(exit_code, output) = dvs.runcmd("teamdctl " + "PortChannel111" + " state dump")
port_state_dump = json.loads(output)
runner_name = port_state_dump["setup"]["runner_name"]
assert runner_name == "roundrobin"


# remove port-channel members
tbl = swsscommon.Table(config_db, "PORTCHANNEL_MEMBER")
tbl._del("PortChannel111|Ethernet0")
tbl._del("PortChannel111|Ethernet4")

# remove port-channel
tbl = swsscommon.Table(config_db, "PORTCHANNEL")
tbl._del("PortChannel111")

# wait for port-channel deletion
time.sleep(1)


# Add Dummy always-pass test at end as workaroud
# for issue when Flaky fail on final test it invokes module tear-down before retrying
def test_nonflaky_dummy():
Expand Down
63 changes: 63 additions & 0 deletions tlm_teamd/values_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,47 @@ std::string ValuesStore::get_value(json_t * root, const std::string & path, Valu
throw std::runtime_error("Reach the end of the ValuesStore::get_value. Path=" + path);
}

///
/// Extract static lag values for LAG with name lag_name, from the parsed json tree with root, to the temporary storage
/// @param lag_name a name of the LAG
/// @param root a pointer to the parsed json tree
/// @param storage a reference to the temporary storage
///

void ValuesStore::extract_values_static(const std::string & lag_name, json_t * root, HashOfRecords & storage)
{

const std::string key = "LAG_TABLE|" + lag_name;
Records lag_values;

for (const auto & p: m_lag_static_paths)
{
const auto & path = p.first;
const auto & type = p.second;
const auto & value = get_value(root, path, type);
lag_values.emplace(path, value);
}
storage.emplace(key, lag_values);

const auto & ports = get_ports(root);
for (const auto & port: ports)
{
const std::string key = "LAG_MEMBER_TABLE|" + lag_name + "|" + port;
Records member_values;
for (const auto & p: m_member_static_paths)
{
const auto & path = p.first;
const auto & type = p.second;
const std::string full_path = "ports." + port + "." + path;
const auto & value = get_value(root, full_path, type);
member_values.emplace(path, value);
}
storage.emplace(key, member_values);
}

return;
}

///
/// Extract values for LAG with name lag_name, from the parsed json tree with root, to the temporary storage
/// @param lag_name a name of the LAG
Expand All @@ -155,6 +196,28 @@ void ValuesStore::extract_values(const std::string & lag_name, json_t * root, Ha

const std::string key = "LAG_TABLE|" + lag_name;
Records lag_values;


bool is_static = false;

for (const auto & p: m_lag_static_paths)
{
const auto & path = p.first;
const auto & type = p.second;
if (path == "setup.runner_name") {
const auto & value = get_value(root, path, type);
if(value == "loadbalance" ||
value == "roundrobin")
is_static = true;
break;
}
}

if (is_static) {
extract_values_static(lag_name, root, storage);
return;
}

for (const auto & p: m_lag_paths)
{
const auto & path = p.first;
Expand Down
14 changes: 14 additions & 0 deletions tlm_teamd/values_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class ValuesStore
std::vector<std::string> update_storage(const HashOfRecords & storage);
void update_db(const HashOfRecords & storage, const std::vector<std::string> & keys_to_refresh);
void extract_values(const std::string & lag_name, json_t * root, HashOfRecords & storage);
void extract_values_static(const std::string & lag_name, json_t * root, HashOfRecords & storage);

HashOfRecords m_storage; // our main storage
const swss::DBConnector * m_db;
Expand Down Expand Up @@ -70,4 +71,17 @@ class ValuesStore
{ "runner.selected", ValuesStore::json_type::boolean },
{ "runner.state", ValuesStore::json_type::string },
};
const std::vector<std::pair<std::string, ValuesStore::json_type>> m_lag_static_paths = {
{ "setup.runner_name", ValuesStore::json_type::string },
{ "setup.kernel_team_mode_name", ValuesStore::json_type::string },
{ "setup.pid", ValuesStore::json_type::integer },
{ "team_device.ifinfo.dev_addr", ValuesStore::json_type::string },
{ "team_device.ifinfo.ifindex", ValuesStore::json_type::integer },
};
const std::vector<std::pair<std::string, ValuesStore::json_type>> m_member_static_paths = {
{ "ifinfo.dev_addr", ValuesStore::json_type::string },
{ "ifinfo.ifindex", ValuesStore::json_type::integer },
{ "link.up", ValuesStore::json_type::boolean },
{ "link_watches.list.link_watch_0.up", ValuesStore::json_type::boolean },
};
};

0 comments on commit 3b43772

Please sign in to comment.