Skip to content

Commit 278770d

Browse files
wendaniwendani
authored
[sub intf] Fix kernel side processing to enslave sub interface to non-default vrf (sonic-net#1521)
Signed-off-by: Wenda Ni <[email protected]> What I did Change in IntfMgr: Use alias (instead of subIntfAlias before the change) to hold complete sub interface name; use parentAlias (instead of alias before the change) to hold parent port name. Move sub interface creation to the first place before processing fields vrf, proxy arp, and garp. In doing above, a sub interface can receive the same treatment as other types of interfaces (interface type port, type vlan) on vrf, proxy arp, and garp fields, while maintaining an indicator (parentAlias not empty) to receive treatment specific to a sub interface (creation, deletion). Why I did it Fix sonic-net#1277, sonic-net#1510 How I verified it vs test. Extended all test cases to test a sub interface ingress linked to a non-default vrf.
1 parent 031f536 commit 278770d

File tree

4 files changed

+521
-158
lines changed

4 files changed

+521
-158
lines changed

‎cfgmgr/intfmgr.cpp

+67-67
Original file line numberDiff line numberDiff line change
@@ -428,16 +428,15 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
428428

429429
string alias(keys[0]);
430430
string vlanId;
431-
string subIntfAlias;
431+
string parentAlias;
432432
size_t found = alias.find(VLAN_SUB_INTERFACE_SEPARATOR);
433433
if (found != string::npos)
434434
{
435435
// This is a sub interface
436-
// subIntfAlias holds the complete sub interface name
437-
// while alias becomes the parent interface
438-
subIntfAlias = alias;
436+
// alias holds the complete sub interface name
437+
// while parentAlias holds the parent port name
439438
vlanId = alias.substr(found + 1);
440-
alias = alias.substr(0, found);
439+
parentAlias = alias.substr(0, found);
441440
}
442441
bool is_lo = !alias.compare(0, strlen(LOOPBACK_PREFIX), LOOPBACK_PREFIX);
443442
string mac = "";
@@ -482,7 +481,7 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
482481

483482
if (op == SET_COMMAND)
484483
{
485-
if (!isIntfStateOk(alias))
484+
if (!isIntfStateOk(parentAlias.empty() ? alias : parentAlias))
486485
{
487486
SWSS_LOG_DEBUG("Interface is not ready, skipping %s", alias.c_str());
488487
return false;
@@ -520,74 +519,28 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
520519
}
521520
}
522521

523-
if (!vrf_name.empty())
524-
{
525-
setIntfVrf(alias, vrf_name);
526-
}
527-
528-
/*Set the mac of interface*/
529-
if (!mac.empty())
530-
{
531-
setIntfMac(alias, mac);
532-
}
533-
else
534-
{
535-
FieldValueTuple fvTuple("mac_addr", MacAddress().to_string());
536-
data.push_back(fvTuple);
537-
}
538-
539-
if (!proxy_arp.empty())
522+
if (!parentAlias.empty())
540523
{
541-
if (!setIntfProxyArp(alias, proxy_arp))
542-
{
543-
SWSS_LOG_ERROR("Failed to set proxy ARP to \"%s\" state for the \"%s\" interface", proxy_arp.c_str(), alias.c_str());
544-
return false;
545-
}
546-
547-
if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
548-
{
549-
FieldValueTuple fvTuple("proxy_arp", proxy_arp);
550-
data.push_back(fvTuple);
551-
}
552-
}
553-
554-
if (!grat_arp.empty())
555-
{
556-
if (!setIntfGratArp(alias, grat_arp))
557-
{
558-
SWSS_LOG_ERROR("Failed to set ARP accept to \"%s\" state for the \"%s\" interface", grat_arp.c_str(), alias.c_str());
559-
return false;
560-
}
561-
562-
if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
563-
{
564-
FieldValueTuple fvTuple("grat_arp", grat_arp);
565-
data.push_back(fvTuple);
566-
}
567-
}
568-
569-
if (!subIntfAlias.empty())
570-
{
571-
if (m_subIntfList.find(subIntfAlias) == m_subIntfList.end())
524+
if (m_subIntfList.find(alias) == m_subIntfList.end())
572525
{
573526
try
574527
{
575-
addHostSubIntf(alias, subIntfAlias, vlanId);
528+
addHostSubIntf(parentAlias, alias, vlanId);
576529
}
577530
catch (const std::runtime_error &e)
578531
{
579532
SWSS_LOG_NOTICE("Sub interface ip link add failure. Runtime error: %s", e.what());
580533
return false;
581534
}
582535

583-
m_subIntfList.insert(subIntfAlias);
536+
m_subIntfList.insert(alias);
584537
}
585538

586539
if (!mtu.empty())
587540
{
588541
try
589542
{
590-
setHostSubIntfMtu(subIntfAlias, mtu);
543+
setHostSubIntfMtu(alias, mtu);
591544
}
592545
catch (const std::runtime_error &e)
593546
{
@@ -609,7 +562,7 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
609562
}
610563
try
611564
{
612-
setHostSubIntfAdminStatus(subIntfAlias, adminStatus);
565+
setHostSubIntfAdminStatus(alias, adminStatus);
613566
}
614567
catch (const std::runtime_error &e)
615568
{
@@ -618,10 +571,57 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
618571
}
619572

620573
// set STATE_DB port state
621-
setSubIntfStateOk(subIntfAlias);
574+
setSubIntfStateOk(alias);
622575
}
623-
m_appIntfTableProducer.set(subIntfAlias.empty() ? alias : subIntfAlias, data);
624-
m_stateIntfTable.hset(subIntfAlias.empty() ? alias : subIntfAlias, "vrf", vrf_name);
576+
577+
if (!vrf_name.empty())
578+
{
579+
setIntfVrf(alias, vrf_name);
580+
}
581+
582+
/*Set the mac of interface*/
583+
if (!mac.empty())
584+
{
585+
setIntfMac(alias, mac);
586+
}
587+
else
588+
{
589+
FieldValueTuple fvTuple("mac_addr", MacAddress().to_string());
590+
data.push_back(fvTuple);
591+
}
592+
593+
if (!proxy_arp.empty())
594+
{
595+
if (!setIntfProxyArp(alias, proxy_arp))
596+
{
597+
SWSS_LOG_ERROR("Failed to set proxy ARP to \"%s\" state for the \"%s\" interface", proxy_arp.c_str(), alias.c_str());
598+
return false;
599+
}
600+
601+
if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
602+
{
603+
FieldValueTuple fvTuple("proxy_arp", proxy_arp);
604+
data.push_back(fvTuple);
605+
}
606+
}
607+
608+
if (!grat_arp.empty())
609+
{
610+
if (!setIntfGratArp(alias, grat_arp))
611+
{
612+
SWSS_LOG_ERROR("Failed to set ARP accept to \"%s\" state for the \"%s\" interface", grat_arp.c_str(), alias.c_str());
613+
return false;
614+
}
615+
616+
if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
617+
{
618+
FieldValueTuple fvTuple("grat_arp", grat_arp);
619+
data.push_back(fvTuple);
620+
}
621+
}
622+
623+
m_appIntfTableProducer.set(alias, data);
624+
m_stateIntfTable.hset(alias, "vrf", vrf_name);
625625
}
626626
else if (op == DEL_COMMAND)
627627
{
@@ -640,16 +640,16 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
640640
m_loopbackIntfList.erase(alias);
641641
}
642642

643-
if (!subIntfAlias.empty())
643+
if (!parentAlias.empty())
644644
{
645-
removeHostSubIntf(subIntfAlias);
646-
m_subIntfList.erase(subIntfAlias);
645+
removeHostSubIntf(alias);
646+
m_subIntfList.erase(alias);
647647

648-
removeSubIntfState(subIntfAlias);
648+
removeSubIntfState(alias);
649649
}
650650

651-
m_appIntfTableProducer.del(subIntfAlias.empty() ? alias : subIntfAlias);
652-
m_stateIntfTable.del(subIntfAlias.empty() ? alias : subIntfAlias);
651+
m_appIntfTableProducer.del(alias);
652+
m_stateIntfTable.del(alias);
653653
}
654654
else
655655
{

‎orchagent/intfsorch.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,7 @@ void IntfsOrch::doTask(Consumer &consumer)
731731
{
732732
IntfsEntry intfs_entry;
733733
intfs_entry.ref_count = 0;
734+
intfs_entry.proxy_arp = false;
734735
intfs_entry.vrf_id = vrf_id;
735736
m_syncdIntfses[alias] = intfs_entry;
736737
m_vrfOrch->increaseVrfRefCount(vrf_id);

‎tests/conftest.py

+6-3
Original file line numberDiff line numberDiff line change
@@ -1003,16 +1003,19 @@ def set_interface_status(self, interface, admin_status):
10031003
tbl.set(interface, fvs)
10041004
time.sleep(1)
10051005

1006-
# deps: acl, fdb_update, fdb, mirror_port_erspan, vlan
1007-
def add_ip_address(self, interface, ip):
1006+
# deps: acl, fdb_update, fdb, mirror_port_erspan, vlan, sub port intf
1007+
def add_ip_address(self, interface, ip, vrf_name=None):
10081008
if interface.startswith("PortChannel"):
10091009
tbl_name = "PORTCHANNEL_INTERFACE"
10101010
elif interface.startswith("Vlan"):
10111011
tbl_name = "VLAN_INTERFACE"
10121012
else:
10131013
tbl_name = "INTERFACE"
10141014
tbl = swsscommon.Table(self.cdb, tbl_name)
1015-
fvs = swsscommon.FieldValuePairs([("NULL", "NULL")])
1015+
pairs = [("NULL", "NULL")]
1016+
if vrf_name:
1017+
pairs = [("vrf_name", vrf_name)]
1018+
fvs = swsscommon.FieldValuePairs(pairs)
10161019
tbl.set(interface, fvs)
10171020
tbl.set(interface + "|" + ip, fvs)
10181021
time.sleep(1)

0 commit comments

Comments
 (0)