From 4f2cf4f47e4f04acd8767b7c4ea4d32ff399ab66 Mon Sep 17 00:00:00 2001 From: liora <liora@nvidia.com> Date: Mon, 18 Apr 2022 14:21:22 +0000 Subject: [PATCH 1/5] Do not ignore netlink messages on interfaces belong to LAG --- portsyncd/linksync.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp index 4a2b351ee0..fc400b5bfc 100644 --- a/portsyncd/linksync.cpp +++ b/portsyncd/linksync.cpp @@ -4,6 +4,7 @@ #include <sys/socket.h> #include <linux/if.h> #include <netlink/route/link.h> +#include <netlink/route/link/bridge.h> #include "logger.h" #include "netmsg.h" #include "dbconnector.h" @@ -212,12 +213,18 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj) return; } - /* If netlink for this port has master, we ignore that for now - * This could be the case where the port was removed from VLAN bridge - */ + /* Ignore netlink on interfaces belong to VLAN bridge */ if (master) { - return; + LinkCache &linkCache = LinkCache::getInstance(); + string masterName = linkCache.ifindexToName(master); + struct rtnl_link *masterLink = linkCache.getLinkByName(masterName.c_str()); + bool isBridge = rtnl_link_is_bridge(masterLink); + + if(isBridge) + { + return; + } } /* In the event of swss restart, it is possible to get netlink messages during bridge From df2d788713954fcd5fb0e2328606028edf569a93 Mon Sep 17 00:00:00 2001 From: liora <liora@nvidia.com> Date: Sun, 1 May 2022 10:07:27 +0000 Subject: [PATCH 2/5] Add VS test for fix in netlink messages handler --- tests/test_portchannel.py | 57 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index ee612ec46d..3e24b6a340 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -382,6 +382,63 @@ def test_Portchannel_tpid(self, dvs, testlog): tbl._del("PortChannel0002") time.sleep(1) + def test_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")]) + 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' + + # 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 From a8228f67ffd6f10d12a8ef49ade141737d4fe5d8 Mon Sep 17 00:00:00 2001 From: liora <liora@nvidia.com> Date: Sun, 22 May 2022 11:00:36 +0000 Subject: [PATCH 3/5] Revert former fix and ignore netlink messages of a port only when it is being removed from bridge --- portsyncd/linksync.cpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp index fc400b5bfc..c39220b47c 100644 --- a/portsyncd/linksync.cpp +++ b/portsyncd/linksync.cpp @@ -213,18 +213,12 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj) return; } - /* Ignore netlink on interfaces belong to VLAN bridge */ - if (master) + /* If netlink for this port has master, we ignore that for now + * This could be the case where the port was removed from VLAN bridge + */ + if (master && nlmsg_type == RTM_DELLINK) { - LinkCache &linkCache = LinkCache::getInstance(); - string masterName = linkCache.ifindexToName(master); - struct rtnl_link *masterLink = linkCache.getLinkByName(masterName.c_str()); - bool isBridge = rtnl_link_is_bridge(masterLink); - - if(isBridge) - { - return; - } + return; } /* In the event of swss restart, it is possible to get netlink messages during bridge From 45232f266c8f0bc2533889333f73a4c98f9456e1 Mon Sep 17 00:00:00 2001 From: liora <liora@nvidia.com> Date: Sun, 22 May 2022 11:41:59 +0000 Subject: [PATCH 4/5] Remove unneeded include --- portsyncd/linksync.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp index c39220b47c..a0bdea0ddb 100644 --- a/portsyncd/linksync.cpp +++ b/portsyncd/linksync.cpp @@ -4,7 +4,6 @@ #include <sys/socket.h> #include <linux/if.h> #include <netlink/route/link.h> -#include <netlink/route/link/bridge.h> #include "logger.h" #include "netmsg.h" #include "dbconnector.h" From 5bb10de9a956f0962ee313a5ad8ed35b504a4e5a Mon Sep 17 00:00:00 2001 From: liora <liora@nvidia.com> Date: Mon, 23 May 2022 17:53:46 +0000 Subject: [PATCH 5/5] Fix comment --- portsyncd/linksync.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp index a0bdea0ddb..d8878f21dc 100644 --- a/portsyncd/linksync.cpp +++ b/portsyncd/linksync.cpp @@ -212,9 +212,8 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj) return; } - /* If netlink for this port has master, we ignore that for now - * This could be the case where the port was removed from VLAN bridge - */ + /* Ignore DELLINK message if port has master, this is applicable to + * the case where port was part of VLAN bridge or LAG */ if (master && nlmsg_type == RTM_DELLINK) { return;