Skip to content

Commit

Permalink
Add delete SAG global MAC while VLAN is enabled test case
Browse files Browse the repository at this point in the history 
* Fix test cases errors
* Refine logic to keep it simple
  • Loading branch information
@superchild
superchild committed Jun 23, 2023
1 parent 8548d1b commit 371be65
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 33 deletions.
51 changes: 40 additions & 11 deletions cfgmgr/intfmgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,27 @@ bool IntfMgr::setIntfMpls(const string &alias, const string& mpls)
return true;
}

void IntfMgr::setIntfState(const string &alias, bool isUp)
{
stringstream cmd;
string res;

if (isUp)
{
cmd << IP_CMD << " link set " << shellquote(alias) << " up";
}
else
{
cmd << IP_CMD << " link set " << shellquote(alias) << " down";
}

int ret = swss::exec(cmd.str(), res);
if (ret)
{
SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret);
}
}

void IntfMgr::addLoopbackIntf(const string &alias)
{
stringstream cmd;
Expand Down Expand Up @@ -997,23 +1018,21 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
string gwmac = "";
if (m_cfgSagTable.hget("GLOBAL", "gateway_mac", gwmac))
{
// before change interface MAC, update sub-interface admin staus to down,
// it can prevent unwanted events received during interface changed in kernel
// bring it up after the changed
// before change interface MAC, set interface down and up to regenerate IPv6 LL by MAC
if (sag == "true")
{
updateSubIntfAdminStatus(alias, "down");
setIntfState(alias, false);
setIntfMac(alias, gwmac);
updateSubIntfAdminStatus(alias, "up");
setIntfState(alias, true);

FieldValueTuple fvTuple("mac_addr", gwmac);
data.push_back(fvTuple);
}
else if (sag == "false")
{
updateSubIntfAdminStatus(alias, "down");
setIntfState(alias, false);
setIntfMac(alias, gMacAddress.to_string());
updateSubIntfAdminStatus(alias, "up");
setIntfState(alias, true);

FieldValueTuple fvTuple("mac_addr", MacAddress().to_string());
data.push_back(fvTuple);
Expand Down Expand Up @@ -1326,16 +1345,26 @@ void IntfMgr::updateSagMac(const std::string &macAddr)
{
SWSS_LOG_NOTICE("set %s mac address to %s", key.c_str(), macAddr.c_str());

// enable SAG
updateSubIntfAdminStatus(key, "down");
// enable SAG, set device down and up to regenerate IPv6 LL by MAC
setIntfState(key, false);
setIntfMac(key, macAddr);
updateSubIntfAdminStatus(key, "up");
setIntfState(key, true);

vector<FieldValueTuple> vlanIntFv;
FieldValueTuple fvTuple("mac_addr", macAddr);

// keep consistent with default MAC 00:00:00:00:00:00
string entryMac = MacAddress().to_string();
if (macAddr != gMacAddress.to_string())
{
entryMac = macAddr;
}

FieldValueTuple fvTuple("mac_addr", entryMac);
vlanIntFv.push_back(fvTuple);
m_appIntfTableProducer.set(key, vlanIntFv);
}
} else {
SWSS_LOG_INFO("can't get %s in VLAN_INTERFACE table", key.c_str());
}
}
}
Expand Down
1 change: 1 addition & 0 deletions cfgmgr/intfmgr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class IntfMgr : public Orch
void setIntfVrf(const std::string &alias, const std::string &vrfName);
void setIntfMac(const std::string &alias, const std::string &macAddr);
bool setIntfMpls(const std::string &alias, const std::string &mpls);
void setIntfState(const std::string &alias, bool isUp);

bool doIntfGeneralTask(const std::vector<std::string>& keys, std::vector<FieldValueTuple> data, const std::string& op);
bool doIntfAddrTask(const std::vector<std::string>& keys, const std::vector<FieldValueTuple>& data, const std::string& op);
Expand Down
56 changes: 36 additions & 20 deletions orchagent/intfsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,6 @@ bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPre
else
{
intfs_entry.mac = gMacAddress;
port.m_mac = gMacAddress;
gPortsOrch->setPort(alias, port);
}
m_syncdIntfses[alias] = intfs_entry;
m_vrfOrch->increaseVrfRefCount(vrf_id);
Expand Down Expand Up @@ -979,31 +977,49 @@ void IntfsOrch::doTask(Consumer &consumer)
}
}

// the interface MAC is updated by setIntf(), it can get from PortsOrch
if (mac && gPortsOrch->getPort(alias, port))
if (!mac)
{
sai_attribute_t attr;
attr.id = SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS;

memcpy(attr.value.mac, port.m_mac.getMac(), sizeof(sai_mac_t));
sai_status_t status = sai_router_intfs_api->set_router_interface_attribute(port.m_rif_id, &attr);
mac = gMacAddress;
}

if (status != SAI_STATUS_SUCCESS)
// update mac if it is changed
if (m_syncdIntfses.find(alias) != m_syncdIntfses.end())
{
if (m_syncdIntfses[alias].mac != mac)
{
SWSS_LOG_ERROR("Failed to set router interface mac %s for port %s, rv:%d",
port.m_mac.to_string().c_str(), port.m_alias.c_str(), status);
if (handleSaiSetStatus(SAI_API_ROUTER_INTERFACE, status) == task_need_retry)
// port.m_rif_id is set in setIntf(), need to get port again
if (gPortsOrch->getPort(alias, port))
{
it++;
continue;
sai_attribute_t attr;
attr.id = SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS;

memcpy(attr.value.mac, mac.getMac(), sizeof(sai_mac_t));
sai_status_t status = sai_router_intfs_api->set_router_interface_attribute(port.m_rif_id, &attr);

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set router interface mac %s for port %s, rv:%d",
mac.to_string().c_str(), port.m_alias.c_str(), status);
if (handleSaiSetStatus(SAI_API_ROUTER_INTERFACE, status) == task_need_retry)
{
it++;
continue;
}
}
else
{
SWSS_LOG_NOTICE("Set router interface mac %s for port %s success",
mac.to_string().c_str(), alias.c_str());
m_syncdIntfses[alias].mac = mac;
}
}
else
{
SWSS_LOG_ERROR("Failed to set router interface mac %s for port %s, get port fail",
mac.to_string().c_str(), alias.c_str());
}
}
}
else
{
SWSS_LOG_ERROR("Failed to set router interface mac %s for port %s, getPort fail",
mac.to_string().c_str(), alias.c_str());
}

if (!proxy_arp.empty())
{
Expand Down
69 changes: 67 additions & 2 deletions tests/test_sag.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def remove_vrf(self, vrf_name):
time.sleep(1)

def add_sag_mac(self, mac):
self.cfg_db.create_entry(swsscommon.CFG_SAG_TABLE_NAME, "GLOBAL", {"gwmac": mac})
self.cfg_db.create_entry(swsscommon.CFG_SAG_TABLE_NAME, "GLOBAL", {"gateway_mac": mac})
time.sleep(1)

def remove_sag_mac(self):
Expand Down Expand Up @@ -88,7 +88,7 @@ def check_kernel_intf_ipv6_addr(self, dvs, interface, addr):
assert addr in result

def check_app_db_sag_mac(self, fvs, mac):
assert fvs.get("gwmac") == mac
assert fvs.get("gateway_mac") == mac

def check_app_db_intf(self, fvs, mac, sag):
assert fvs.get("mac_addr") == mac and fvs.get("static_anycast_gateway") == sag
Expand Down Expand Up @@ -191,6 +191,71 @@ def test_SagAddRemove(self, dvs):
# reset interface
self.reset_interface(dvs, interface, vlan)

def test_SagRemoveWhenSagVlanEnabled(self, dvs):
self.setup_db(dvs)

default_mac = "00:00:00:00:00:00"
default_vrf_oid = self.get_asic_db_default_vrf_oid()
system_mac = self.get_system_mac(dvs)

interface = "Ethernet0"
vlan = "100"
vlan_intf = "Vlan{}".format(vlan)
self.setup_interface(dvs, interface, vlan)

ip = "1.1.1.1/24"
dvs.add_ip_address(vlan_intf, ip)

# add SAG global MAC address
mac = "00:11:22:33:44:55"
self.add_sag_mac(mac)
fvs = dvs.get_app_db().wait_for_entry(swsscommon.APP_SAG_TABLE_NAME, "GLOBAL")
self.check_app_db_sag_mac(fvs, mac)

ipv6_ll_route = self.generate_ipv6_link_local_addr(mac, 128)
self.check_asic_db_route_entry(str(ipv6_ll_route), default_vrf_oid, exist=True)

# enable SAG on the VLAN interface
self.enable_sag(vlan)
fvs = self.app_db.wait_for_field_match(
swsscommon.APP_INTF_TABLE_NAME,
vlan_intf,
{"mac_addr": mac})

self.check_app_db_intf(fvs, mac, "true")
self.check_kernel_intf_mac(dvs, vlan_intf, mac)

ipv6_ll = self.generate_ipv6_link_local_addr(mac, 64)
self.check_kernel_intf_ipv6_addr(dvs, vlan_intf, str(ipv6_ll))
self.check_asic_db_router_interface(dvs.getVlanOid(vlan), mac, default_vrf_oid)

# delete SAG global MAC address
self.remove_sag_mac()
self.app_db.wait_for_deleted_entry(swsscommon.APP_SAG_TABLE_NAME, "GLOBAL")

ipv6_ll_route = self.generate_ipv6_link_local_addr(mac, 128)
self.check_asic_db_route_entry(str(ipv6_ll_route), default_vrf_oid, exist=False)

fvs = self.app_db.wait_for_field_match(
swsscommon.APP_INTF_TABLE_NAME,
vlan_intf,
{"NULL": "NULL"}
)

self.check_app_db_intf(fvs, default_mac, "true")
self.check_kernel_intf_mac(dvs, vlan_intf, system_mac)

ipv6_ll = self.generate_ipv6_link_local_addr(system_mac, 64)
self.check_kernel_intf_ipv6_addr(dvs, vlan_intf, str(ipv6_ll))
self.check_asic_db_router_interface(dvs.getVlanOid(vlan), system_mac, default_vrf_oid)

# remove ip
dvs.remove_ip_address(vlan_intf, ip)

# reset interface
self.reset_interface(dvs, interface, vlan)


def test_SagAddRemoveInVrf(self, dvs):
self.setup_db(dvs)

Expand Down

0 comments on commit 371be65

Please sign in to comment.