From c37cc1c56336d242fdcc3e728f8e86be992dd382 Mon Sep 17 00:00:00 2001 From: Venkatesan Mahalingam <34145258+venkatmahalingam@users.noreply.github.com> Date: Wed, 23 Jun 2021 20:23:15 -0700 Subject: [PATCH] Support for in-band-mgmt via management VRF (#1726) * Support for in-band-mgmt via management VRF. Signed-off-by: Venkatesan Mahalingam --- cfgmgr/intfmgr.cpp | 4 +- cfgmgr/vrfmgr.cpp | 69 +++++++++++- cfgmgr/vrfmgrd.cpp | 1 + fpmsyncd/routesync.cpp | 13 ++- orchagent/intfsorch.cpp | 18 +++ orchagent/intfsorch.h | 1 + orchagent/neighorch.cpp | 3 +- orchagent/vrforch.cpp | 5 + orchagent/vrforch.h | 4 +- tests/test_inband_intf_mgmt_vrf.py | 169 +++++++++++++++++++++++++++++ 10 files changed, 281 insertions(+), 6 deletions(-) create mode 100644 tests/test_inband_intf_mgmt_vrf.py diff --git a/cfgmgr/intfmgr.cpp b/cfgmgr/intfmgr.cpp index 836f8acf9218..fa2207b32b17 100644 --- a/cfgmgr/intfmgr.cpp +++ b/cfgmgr/intfmgr.cpp @@ -19,6 +19,7 @@ using namespace swss; #define VNET_PREFIX "Vnet" #define MTU_INHERITANCE "0" #define VRF_PREFIX "Vrf" +#define VRF_MGMT "mgmt" #define LOOPBACK_DEFAULT_MTU_STR "65536" @@ -399,7 +400,8 @@ bool IntfMgr::isIntfStateOk(const string &alias) return true; } } - else if (!alias.compare(0, strlen(VRF_PREFIX), VRF_PREFIX)) + else if ((!alias.compare(0, strlen(VRF_PREFIX), VRF_PREFIX)) || + (alias == VRF_MGMT)) { if (m_stateVrfTable.get(alias, temp)) { diff --git a/cfgmgr/vrfmgr.cpp b/cfgmgr/vrfmgr.cpp index 06c2a7b8cde3..ea203412b247 100644 --- a/cfgmgr/vrfmgr.cpp +++ b/cfgmgr/vrfmgr.cpp @@ -12,6 +12,8 @@ #define VRF_TABLE_START 1001 #define VRF_TABLE_END 2000 #define TABLE_LOCAL_PREF 1001 // after l3mdev-table +#define MGMT_VRF_TABLE_ID 5000 +#define MGMT_VRF "mgmt" using namespace swss; @@ -143,6 +145,13 @@ bool VrfMgr::delLink(const string& vrfName) return false; } + if (vrfName == MGMT_VRF) + { + recycleTable(m_vrfTableMap[vrfName]); + m_vrfTableMap.erase(vrfName); + return true; + } + cmd << IP_CMD << " link del " << shellquote(vrfName); EXEC_WITH_ERROR_THROW(cmd.str(), res); @@ -163,6 +172,15 @@ bool VrfMgr::setLink(const string& vrfName) { return true; } + + if (vrfName == MGMT_VRF) + { + // Mgmt VRF is initialised as part of hostcfgd, + // just return the reserved table_id for mgmt VRF from here. + uint32_t table_id = MGMT_VRF_TABLE_ID; + m_vrfTableMap.emplace(vrfName, table_id); + return true; + } uint32_t table = getFreeTable(); if (table == 0) @@ -207,6 +225,51 @@ void VrfMgr::doTask(Consumer &consumer) auto vrfName = kfvKey(t); string op = kfvOp(t); + // Mgmt VRF table event handling for in-band management + if (consumer.getTableName() == CFG_MGMT_VRF_CONFIG_TABLE_NAME) + { + SWSS_LOG_DEBUG("Event for mgmt VRF op %s", op.c_str()); + if (op == SET_COMMAND) + { + bool in_band_mgmt_enabled = false; + bool mgmt_vrf_enabled = false; + for (auto i : kfvFieldsValues(t)) + { + if (fvField(i) == "mgmtVrfEnabled") + { + if (fvValue(i) == "true") + { + mgmt_vrf_enabled = true; + } + SWSS_LOG_DEBUG("Event for mgmt VRF table mgmt_vrf_enabled is set val:%s", fvValue(i).c_str()); + } + else if (fvField(i) == "in_band_mgmt_enabled") + { + if (fvValue(i) == "true") + { + in_band_mgmt_enabled = true; + } + SWSS_LOG_DEBUG("Event for mgmt VRF table in_band_mgmt_enabled is set val:%s", fvValue(i).c_str()); + } + } + // If mgmt VRF is not enabled or in-band-mgmt is not enabled delete the in-band-mgmt + // related VRF table map information + if ((op == SET_COMMAND) && ((mgmt_vrf_enabled == false) || (in_band_mgmt_enabled == false))) + { + op = DEL_COMMAND; + } + } + vrfName = MGMT_VRF; + if (((op == DEL_COMMAND) && (m_vrfTableMap.find(vrfName) == m_vrfTableMap.end())) || + ((op == SET_COMMAND) && (m_vrfTableMap.find(vrfName) != m_vrfTableMap.end()))) + { + // If the mgmt VRF is not populated already, return + it = consumer.m_toSync.erase(it); + continue; + } + SWSS_LOG_DEBUG("Event for mgmt VRF op %s", op.c_str()); + } + SWSS_LOG_DEBUG("Event for table %s vrf netdev %s id %s", consumer.getTableName().c_str(), vrfName.c_str(), op.c_str()); if (op == SET_COMMAND) { if (consumer.getTableName() == CFG_VXLAN_EVPN_NVO_TABLE_NAME) @@ -226,7 +289,8 @@ void VrfMgr::doTask(Consumer &consumer) m_stateVrfTable.set(vrfName, fvVector); SWSS_LOG_NOTICE("Created vrf netdev %s", vrfName.c_str()); - if (consumer.getTableName() == CFG_VRF_TABLE_NAME) + if ((consumer.getTableName() == CFG_VRF_TABLE_NAME) || + (consumer.getTableName() == CFG_MGMT_VRF_CONFIG_TABLE_NAME)) { status = doVrfVxlanTableCreateTask (t); if (status == false) @@ -256,7 +320,8 @@ void VrfMgr::doTask(Consumer &consumer) { doVrfEvpnNvoDelTask (t); } - else if (consumer.getTableName() == CFG_VRF_TABLE_NAME) + else if ((consumer.getTableName() == CFG_VRF_TABLE_NAME) || + (consumer.getTableName() == CFG_MGMT_VRF_CONFIG_TABLE_NAME)) { vector temp; diff --git a/cfgmgr/vrfmgrd.cpp b/cfgmgr/vrfmgrd.cpp index af8e78bce8bf..556b937901f6 100644 --- a/cfgmgr/vrfmgrd.cpp +++ b/cfgmgr/vrfmgrd.cpp @@ -46,6 +46,7 @@ int main(int argc, char **argv) CFG_VRF_TABLE_NAME, CFG_VNET_TABLE_NAME, CFG_VXLAN_EVPN_NVO_TABLE_NAME, + CFG_MGMT_VRF_CONFIG_TABLE_NAME }; DBConnector cfgDb("CONFIG_DB", 0); diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 7fae01eb3d71..9910dca07e1f 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -19,6 +19,7 @@ using namespace swss; #define VXLAN_IF_NAME_PREFIX "Brvxlan" #define VNET_PREFIX "Vnet" #define VRF_PREFIX "Vrf" +#define MGMT_VRF_PREFIX "mgmt" #ifndef ETH_ALEN #define ETH_ALEN 6 @@ -625,7 +626,17 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) */ if (memcmp(vrf, VRF_PREFIX, strlen(VRF_PREFIX))) { - SWSS_LOG_ERROR("Invalid VRF name %s (ifindex %u)", vrf, rtnl_route_get_table(route_obj)); + if(memcmp(vrf, MGMT_VRF_PREFIX, strlen(MGMT_VRF_PREFIX))) + { + SWSS_LOG_ERROR("Invalid VRF name %s (ifindex %u)", vrf, rtnl_route_get_table(route_obj)); + } + else + { + dip = rtnl_route_get_dst(route_obj); + nl_addr2str(dip, destipprefix, MAX_ADDR_SIZE); + SWSS_LOG_INFO("Skip routes for Mgmt VRF name %s (ifindex %u) prefix: %s", vrf, + rtnl_route_get_table(route_obj), destipprefix); + } return; } memcpy(destipprefix, vrf, strlen(vrf)); diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp index f3fab3cc61e1..27deb78d0473 100644 --- a/orchagent/intfsorch.cpp +++ b/orchagent/intfsorch.cpp @@ -42,6 +42,7 @@ const int intfsorch_pri = 35; #define RIF_FLEX_STAT_COUNTER_POLL_MSECS "1000" #define UPDATE_MAPS_SEC 1 +#define MGMT_VRF "mgmt" static const vector rifStatIds = { @@ -159,6 +160,23 @@ string IntfsOrch::getRouterIntfsAlias(const IpAddress &ip, const string &vrf_nam return string(); } +bool IntfsOrch::isInbandIntfInMgmtVrf(const string& alias) +{ + if (m_syncdIntfses.find(alias) == m_syncdIntfses.end()) + { + return false; + } + + string vrf_name = ""; + vrf_name = m_vrfOrch->getVRFname(m_syncdIntfses[alias].vrf_id); + if ((!vrf_name.empty()) && (vrf_name == MGMT_VRF)) + { + return true; + } + + return false; +} + void IntfsOrch::increaseRouterIntfsRefCount(const string &alias) { SWSS_LOG_ENTER(); diff --git a/orchagent/intfsorch.h b/orchagent/intfsorch.h index 11b947ec2986..89167fb80462 100644 --- a/orchagent/intfsorch.h +++ b/orchagent/intfsorch.h @@ -36,6 +36,7 @@ class IntfsOrch : public Orch sai_object_id_t getRouterIntfsId(const string&); bool isPrefixSubnet(const IpPrefix&, const string&); + bool isInbandIntfInMgmtVrf(const string& alias); string getRouterIntfsAlias(const IpAddress &ip, const string &vrf_name = ""); string getRifRateFlexCounterTableKey(string key); void increaseRouterIntfsRefCount(const string&); diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index c7c18b3e31e6..9f9bcde829cc 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -551,7 +551,8 @@ void NeighOrch::doTask(Consumer &consumer) string alias = key.substr(0, found); - if (alias == "eth0" || alias == "lo" || alias == "docker0") + if (alias == "eth0" || alias == "lo" || alias == "docker0" + || ((op == SET_COMMAND) && m_intfsOrch->isInbandIntfInMgmtVrf(alias))) { it = consumer.m_toSync.erase(it); continue; diff --git a/orchagent/vrforch.cpp b/orchagent/vrforch.cpp index 60117c366d25..19ca5c0fd885 100644 --- a/orchagent/vrforch.cpp +++ b/orchagent/vrforch.cpp @@ -68,6 +68,11 @@ bool VRFOrch::addOperation(const Request& request) vni = static_cast(request.getAttrUint(name)); continue; } + else if ((name == "mgmtVrfEnabled") || (name == "in_band_mgmt_enabled")) + { + SWSS_LOG_INFO("MGMT VRF field: %s ignored", name.c_str()); + continue; + } else { SWSS_LOG_ERROR("Logic error: Unknown attribute: %s", name.c_str()); diff --git a/orchagent/vrforch.h b/orchagent/vrforch.h index 68a871b8091b..195015fa083b 100644 --- a/orchagent/vrforch.h +++ b/orchagent/vrforch.h @@ -32,7 +32,9 @@ const request_description_t request_description = { { "ip_opt_action", REQ_T_PACKET_ACTION }, { "l3_mc_action", REQ_T_PACKET_ACTION }, { "fallback", REQ_T_BOOL }, - { "vni", REQ_T_UINT } + { "vni", REQ_T_UINT }, + { "mgmtVrfEnabled", REQ_T_BOOL }, + { "in_band_mgmt_enabled", REQ_T_BOOL } }, { } // no mandatory attributes }; diff --git a/tests/test_inband_intf_mgmt_vrf.py b/tests/test_inband_intf_mgmt_vrf.py new file mode 100644 index 000000000000..05aa1f7389b4 --- /dev/null +++ b/tests/test_inband_intf_mgmt_vrf.py @@ -0,0 +1,169 @@ +import time +import pytest + +from swsscommon import swsscommon + +MGMT_VRF_NAME = 'mgmt' +INBAND_INTF_NAME = 'Ethernet4' + +class TestInbandInterface(object): + def setup_db(self, dvs): + self.appl_db = swsscommon.DBConnector(0, dvs.redis_sock, 0) + self.asic_db = dvs.get_asic_db() + self.cfg_db = swsscommon.DBConnector(4, dvs.redis_sock, 0) + + def add_mgmt_vrf(self, dvs): + initial_entries = set(self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER")) + #dvs.runcmd("config vrf add mgmt") + dvs.runcmd("ip link add mgmt type vrf table 5000") + dvs.runcmd("ifconfig mgmt up") + time.sleep(2) + + # check application database + tbl = swsscommon.Table(self.appl_db, 'VRF_TABLE') + vrf_keys = tbl.getKeys() + assert len(vrf_keys) == 0 + + tbl = swsscommon.Table(self.cfg_db, 'MGMT_VRF_CONFIG') + fvs = swsscommon.FieldValuePairs([('mgmtVrfEnabled', 'true'), ('in_band_mgmt_enabled', 'true')]) + tbl.set('vrf_global', fvs) + time.sleep(1) + + # check application database + tbl = swsscommon.Table(self.appl_db, 'VRF_TABLE') + vrf_keys = tbl.getKeys() + assert len(vrf_keys) == 1 + assert vrf_keys[0] == MGMT_VRF_NAME + + # check SAI database info present in ASIC_DB + self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER", len(initial_entries) + 1) + current_entries = set(self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER")) + assert len(current_entries - initial_entries) == 1 + return list(current_entries - initial_entries)[0] + + def del_inband_mgmt_vrf(self): + tbl = swsscommon.Table(self.cfg_db, 'MGMT_VRF_CONFIG') + fvs = swsscommon.FieldValuePairs([('mgmtVrfEnabled', 'true'), ('in_band_mgmt_enabled', 'false')]) + tbl.set('vrf_global', fvs) + time.sleep(5) + + # check application database + tbl = swsscommon.Table(self.appl_db, 'VRF_TABLE') + vrf_keys = tbl.getKeys() + assert len(vrf_keys) == 0 + + def del_mgmt_vrf(self, dvs): + dvs.runcmd("ip link del mgmt") + tbl = swsscommon.Table(self.cfg_db, 'MGMT_VRF_CONFIG') + tbl._del('vrf_global') + time.sleep(5) + + def create_inband_intf(self, interface): + cfg_tbl = cfg_key = cfg_fvs = None + if interface.startswith('PortChannel'): + tbl_name = 'PORTCHANNEL_INTERFACE' + cfg_tbl = 'PORTCHANNEL' + cfg_key = interface + cfg_fvs = swsscommon.FieldValuePairs([("admin_status", "up"), + ("mtu", "9100")]) + elif interface.startswith('Vlan'): + tbl_name = 'VLAN_INTERFACE' + cfg_tbl = 'VLAN' + vlan_id = interface[len('Vlan'):] + cfg_key = 'Vlan' + vlan_id + cfg_fvs = swsscommon.FieldValuePairs([("vlanid", vlan_id)]) + elif interface.startswith('Loopback'): + tbl_name = 'LOOPBACK_INTERFACE' + else: + tbl_name = 'INTERFACE' + if cfg_tbl is not None: + tbl = swsscommon.Table(self.cfg_db, cfg_tbl) + tbl.set(cfg_key, cfg_fvs) + time.sleep(1) + fvs = swsscommon.FieldValuePairs([('vrf_name', MGMT_VRF_NAME)]) + tbl = swsscommon.Table(self.cfg_db, tbl_name) + tbl.set(interface, fvs) + time.sleep(1) + + def remove_inband_intf(self, interface): + cfg_tbl = cfg_key = None + if interface.startswith('PortChannel'): + tbl_name = 'PORTCHANNEL_INTERFACE' + cfg_tbl = 'PORTCHANNEL' + cfg_key = interface + elif interface.startswith('Vlan'): + tbl_name = 'VLAN_INTERFACE' + cfg_tbl = 'VLAN' + vlan_id = interface[len('Vlan'):] + cfg_key = 'Vlan' + vlan_id + elif interface.startswith('Loopback'): + tbl_name = 'LOOPBACK_INTERFACE' + else: + tbl_name = 'INTERFACE' + tbl = swsscommon.Table(self.cfg_db, tbl_name) + tbl._del(interface) + time.sleep(1) + if cfg_tbl is not None: + tbl = swsscommon.Table(self.cfg_db, cfg_tbl) + tbl._del(cfg_key) + time.sleep(1) + + def test_MgmtVrf(self, dvs, testlog): + self.setup_db(dvs) + + vrf_oid = self.add_mgmt_vrf(dvs) + self.del_inband_mgmt_vrf() + self.del_mgmt_vrf(dvs) + + @pytest.mark.parametrize('intf_name', ['Ethernet4', 'Vlan100', 'PortChannel5', 'Loopback1']) + def test_InbandIntf(self, intf_name, dvs, testlog): + self.setup_db(dvs) + + vrf_oid = self.add_mgmt_vrf(dvs) + self.create_inband_intf(intf_name) + + # check application database + tbl = swsscommon.Table(self.appl_db, 'INTF_TABLE') + intf_keys = tbl.getKeys() + status, fvs = tbl.get(intf_name) + assert status == True + for fv in fvs: + if fv[0] == 'vrf_name': + assert fv[1] == MGMT_VRF_NAME + + if not intf_name.startswith('Loopback'): + # check ASIC router interface database + # one loopback router interface one port based router interface + intf_entries = self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE", 2) + for key in intf_entries: + fvs = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE", key) + loopback = False + intf_vrf_oid = None + for k, v in fvs.items(): + if k == 'SAI_ROUTER_INTERFACE_ATTR_TYPE' and v == 'SAI_ROUTER_INTERFACE_TYPE_LOOPBACK': + loopback = True + break + if k == 'SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID': + intf_vrf_oid = v + if loopback: + continue + assert intf_vrf_oid == vrf_oid + + self.remove_inband_intf(intf_name) + time.sleep(1) + # check application database + tbl = swsscommon.Table(self.appl_db, 'INTF_TABLE') + intf_keys = tbl.getKeys() + assert len(intf_keys) == 0 + + if not intf_name.startswith('Loopback'): + self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE", 1) + + self.del_inband_mgmt_vrf() + self.del_mgmt_vrf(dvs) + + +# 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(): + pass