Skip to content

Commit

Permalink
Code-review comments addressed
Browse files Browse the repository at this point in the history
  • Loading branch information
Vasant committed May 14, 2020
1 parent aa72b67 commit 7cd3814
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 82 deletions.
28 changes: 19 additions & 9 deletions orchagent/fdborch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_obj
break;

case SAI_FDB_EVENT_FLUSHED:
if ((bridge_port_id == SAI_NULL_OBJECT_ID && entry->bv_id == SAI_NULL_OBJECT_ID)
|| (bridge_port_id && entry->bv_id == SAI_NULL_OBJECT_ID))
if (entry->bv_id == SAI_NULL_OBJECT_ID)
{
for (auto itr = m_entries.begin(); itr != m_entries.end();)
{
Expand All @@ -175,21 +174,32 @@ void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_obj
update.add = false;
itr++;

storeFdbEntryState(update);
if (SAI_NULL_OBJECT_ID != bridge_port_id)
{
if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port))
{
SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id);
return;
}
SWSS_LOG_DEBUG("FDB flush: mac: %s port: %s bridge port OID: 0x%" PRIx64 ".",
update.entry.mac.to_string().c_str(),
update.port.m_alias.c_str(),
bridge_port_id);
}
else
{
SWSS_LOG_DEBUG("FDB flush: mac: %s",
update.entry.mac.to_string().c_str());
}

SWSS_LOG_DEBUG("FdbOrch notification: mac %s was removed", update.entry.mac.to_string().c_str());
storeFdbEntryState(update);

for (auto observer: m_observers)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
}
}
}
else if (bridge_port_id && entry->bv_id == SAI_NULL_OBJECT_ID)
{
/*this is a placeholder for flush port fdb case, not supported yet.*/
SWSS_LOG_ERROR("FdbOrch notification: not supported flush port fdb action, port_id = 0x%" PRIx64 ", bv_id = 0x%" PRIx64 ".", bridge_port_id, entry->bv_id);
}
else if (bridge_port_id == SAI_NULL_OBJECT_ID && entry->bv_id != SAI_NULL_OBJECT_ID)
{
/*this is a placeholder for flush vlan fdb case, not supported yet.*/
Expand Down
28 changes: 13 additions & 15 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from dvslib import dvs_database as dvs_db
from dvslib import dvs_acl
from dvslib import dvs_vlan
from dvslib import dvs_lag

def ensure_system(cmd):
(rc, output) = commands.getstatusoutput(cmd)
Expand Down Expand Up @@ -982,14 +983,6 @@ def get_state_db(self):

return self.state_db

def get_dvs_acl(self):
if not self.dvs_acl:
self.dvs_acl = dvs_acl.DVSAcl(self.get_asic_db(),
self.get_config_db(),
self.get_state_db(),
self.get_counters_db())
return self.dvs_acl

@pytest.yield_fixture(scope="module")
def dvs(request):
name = request.config.getoption("--dvsname")
Expand All @@ -1010,21 +1003,26 @@ def testlog(request, dvs):
yield testlog
dvs.runcmd("logger === finish test %s ===" % request.node.name)

@pytest.yield_fixture(scope="class")
def dvs_vlan_manager(request, dvs):
request.cls.dvs_vlan = dvs_vlan.DVSVlan(dvs.get_asic_db(),
dvs.get_config_db(),
dvs.get_state_db(),
dvs.get_counters_db(),
dvs.get_app_db())

################# DVSLIB module manager fixtures #############################
@pytest.yield_fixture(scope="class")
def dvs_acl_manager(request, dvs):
request.cls.dvs_acl = dvs_acl.DVSAcl(dvs.get_asic_db(),
dvs.get_config_db(),
dvs.get_state_db(),
dvs.get_counters_db())

@pytest.yield_fixture(scope="class")
def dvs_lag_manager(request, dvs):
request.cls.dvs_lag = dvs_lag.DVSLag(dvs.get_config_db())

@pytest.yield_fixture(scope="class")
def dvs_vlan_manager(request, dvs):
request.cls.dvs_vlan = dvs_vlan.DVSVlan(dvs.get_asic_db(),
dvs.get_config_db(),
dvs.get_state_db(),
dvs.get_counters_db(),
dvs.get_app_db())
##################### DPB fixtures ###########################################
@pytest.yield_fixture(scope="module")
def create_dpb_config_file(dvs):
Expand Down
113 changes: 59 additions & 54 deletions tests/test_port_dpb_vlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,79 +10,85 @@ def check_syslog(self, dvs, marker, log, expected_cnt):
(exitcode, num) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \"%s\" | wc -l" % (marker, log)])
assert num.strip() >= str(expected_cnt)

'''
@pytest.mark.skip()
'''
def test_dependency(self, dvs):

vlan = "100"
p = Port(dvs, "Ethernet0")
p.sync_from_config_db()

# 1. Create VLAN100
self.dvs_vlan.create_vlan(vlan)
#print "Created VLAN100"

# 2. Add Ethernet0 to VLAN100
self.dvs_vlan.create_vlan_member(vlan, p.get_name())
#print "Added Ethernet0 to VLAN100"

# 3. Add log marker to syslog
marker = dvs.add_log_marker()

# 4. Delete Ethernet0 from config DB. Sleep for 2 seconds.
p.delete_from_config_db()
#Verify that we are looping on dependency
time.sleep(2)

# 5. Verify that we are waiting in portsorch for the port
# to be removed from VLAN, by looking at the log
self.check_syslog(dvs, marker, "Cannot remove port as bridge port OID is present", 1)

# 6. Also verify that port is still present in ASIC DB.
assert(p.exists_in_asic_db() == True)

# 7. Remove port from VLAN
self.dvs_vlan.remove_vlan_member(vlan, p.get_name())
# Verify that port is deleted

# 8. Verify that port is removed from ASIC DB
assert(p.not_exists_in_asic_db() == True)

#Create the port back and delete the VLAN
# 9. Re-create port Ethernet0 and verify that it is
# present in CONFIG, APPL, and ASIC DBs
p.write_to_config_db()
#print "Added port:%s to config DB"%p.get_name()
p.verify_config_db()
#print "Config DB verification passed!"
p.verify_app_db()
#print "Application DB verification passed!"
p.verify_asic_db()
#print "ASIC DB verification passed!"

# 10. Remove VLAN100 and verify that its removed.
self.dvs_vlan.remove_vlan(vlan)
self.dvs_vlan.get_and_verify_vlan_ids(0)

'''
@pytest.mark.skip()
'''
def test_one_port_one_vlan(self, dvs):
dpb = DPB()
vlan = "100"

# Breakout testing with VLAN dependency
# 1. Create VLAN100 and add Ethernet0 as member
self.dvs_vlan.create_vlan(vlan)
#print "Created VLAN100"
self.dvs_vlan.create_vlan_member(vlan, "Ethernet0")
self.dvs_vlan.get_and_verify_vlan_member_ids(1)
#print "Added Ethernet0 to VLAN100"

# 2. Delete Ethernet0 from config DB. Verify that its deleted
# CONFIG and APPL DB and its still present in ASIC DB
p = Port(dvs, "Ethernet0")
p.sync_from_config_db()
p.delete_from_config_db()
assert(p.exists_in_config_db() == False)
assert(p.exists_in_app_db() == False)
assert(p.exists_in_asic_db() == True)
#print "Ethernet0 deleted from config DB and APP DB, waiting to be removed from VLAN"

# 3. Verify that Ethernet0 gets deleted from ASIC DB once
# its removed from VLAN100.
self.dvs_vlan.remove_vlan_member(vlan, "Ethernet0")
self.dvs_vlan.get_and_verify_vlan_member_ids(0)
assert(p.not_exists_in_asic_db() == True)
#print "Ethernet0 removed from VLAN and also from ASIC DB"

# 4. To simulate port breakout, 1x100G --> 4x25G, create 4 ports
dpb.create_child_ports(dvs, p, 4)

# Breakin testing with VLAN dependency
# 5. Add all 4 ports to VLAN100
port_names = ["Ethernet0", "Ethernet1", "Ethernet2", "Ethernet3"]
vlan_member_count = 0
for pname in port_names:
self.dvs_vlan.create_vlan_member(vlan, pname)
vlan_member_count = vlan_member_count + 1
self.dvs_vlan.get_and_verify_vlan_member_ids(vlan_member_count)
#print "Add %s to VLAN"%port_names

# 6. Delete 4 ports from CONFIG DB. Verify that they are all deleted
# from CONFIG and APPL DB but still present in ASIC DB
child_ports = []
for pname in port_names:
cp = Port(dvs, pname)
Expand All @@ -92,69 +98,70 @@ def test_one_port_one_vlan(self, dvs):
assert(cp.exists_in_app_db() == False)
assert(cp.exists_in_asic_db() == True)
child_ports.append(cp)
#print "Deleted %s from config DB and APP DB"%port_names

# 7. Remove all 4 ports from VLAN100 and verify that they all get
# deleted from ASIC DB
for cp in child_ports:
self.dvs_vlan.remove_vlan_member(vlan, cp.get_name())
vlan_member_count = vlan_member_count - 1
self.dvs_vlan.get_and_verify_vlan_member_ids(vlan_member_count)
assert(cp.not_exists_in_asic_db() == True)
#print "Deleted %s from VLAN"%port_names

# 8. Re-create Ethernet0 and verify that its created all 3 DBs
p.write_to_config_db()
#print "Added port:%s to config DB"%p.get_name()
p.verify_config_db()
#print "Config DB verification passed!"
p.verify_app_db()
#print "Application DB verification passed!"
p.verify_asic_db()
#print "ASIC DB verification passed!"

# 9. Remove VLAN100 and verify the same
self.dvs_vlan.remove_vlan(vlan)
self.dvs_vlan.get_and_verify_vlan_ids(0)

'''
@pytest.mark.skip()
'''
def test_one_port_multiple_vlan(self, dvs):

dpb = DPB()
vlans = ["100", "101", "102"]

# 1. Create 3 VLANs
for vlan in vlans:
self.dvs_vlan.create_vlan(vlan)
#print "Created VLAN100, VLAN101, and VLAN102"

# 2. Add Ethernet0 to all 3 VLANs and verify
for vlan in vlans:
self.dvs_vlan.create_vlan_member(vlan, "Ethernet0")
self.dvs_vlan.get_and_verify_vlan_member_ids(len(vlans))
#print "Added Ethernet0 to all three VLANs"

# 3. Delete Ethernet0 from CONFIG DB. Verify that it is deleted
# from CONFIG and APPl DB, whereas still present in ASIC DB.
p = Port(dvs, "Ethernet0")
p.sync_from_config_db()
p.delete_from_config_db()
assert(p.exists_in_config_db() == False)
assert(p.exists_in_app_db() == False)
assert(p.exists_in_asic_db() == True)
#print "Ethernet0 deleted from config DB and APP DB, waiting to be removed from VLANs"

# 4. Remove Ethernet0 from one of the VLANs and verify that
# its still present in ASIC DB
self.dvs_vlan.remove_vlan_member(vlans[0], "Ethernet0")
self.dvs_vlan.get_and_verify_vlan_member_ids(len(vlans)-1)
assert(p.exists_in_asic_db() == True)
#print "Ethernet0 removed from VLAN100 and its still present in ASIC DB"

# 5. Remove Ethernet0 from one more VLAN and verify that
# its still present in ASIC DB
self.dvs_vlan.remove_vlan_member(vlans[1], "Ethernet0")
self.dvs_vlan.get_and_verify_vlan_member_ids(len(vlans)-2)
assert(p.exists_in_asic_db() == True)
#print "Ethernet0 removed from VLAN101 and its still present in ASIC DB"

# 6. Remove Ethernet0 from last VLAN as well and verify that
# its deleted from ASIC DB
self.dvs_vlan.remove_vlan_member(vlans[2], "Ethernet0")
self.dvs_vlan.get_and_verify_vlan_member_ids(0)
assert(p.not_exists_in_asic_db() == True)
#print "Ethernet0 removed from VLAN102 and also from ASIC DB"

# 7. To Simulate 1x40G --> 4x10G, create 4 ports
dpb.create_child_ports(dvs, p, 4)
#print "1X40G ---> 4x10G verified"

# Breakin
# 8. To Simulate 4x10G --> 1x40G, delete all 4 ports and re-create Ethernet0
port_names = ["Ethernet0", "Ethernet1", "Ethernet2", "Ethernet3"]
for pname in port_names:
cp = Port(dvs, pname)
Expand All @@ -163,24 +170,18 @@ def test_one_port_multiple_vlan(self, dvs):
assert(cp.exists_in_config_db() == False)
assert(cp.exists_in_app_db() == False)
assert(cp.not_exists_in_asic_db() == True)
#print "Deleted %s and verified all DBs"%port_names

#Add back Ethernet0
p.write_to_config_db()
p.verify_config_db()
p.verify_app_db()
p.verify_asic_db()
#print "Added port:%s and verified all DBs"%p.get_name()

# Remove all three VLANs
# 9. Remove all 3 VLANs and verify the same
self.dvs_vlan.remove_vlan("100")
self.dvs_vlan.remove_vlan("101")
self.dvs_vlan.remove_vlan("102")
#print "All three VLANs removed"
self.dvs_vlan.get_and_verify_vlan_ids(0)

'''
@pytest.mark.skip()
'''
def test_all_port_10_vlans(self, dvs):
num_vlans = 10
start_vlan = 100
Expand All @@ -195,23 +196,27 @@ def test_all_port_10_vlans(self, dvs):
for i in range(num_vlans):
vlan_names.append(str(start_vlan + i))

# 1. Create 10 VLANs
for vlan_name in vlan_names:
self.dvs_vlan.create_vlan(vlan_name)
#print "%d VLANs created"%num_vlans

# 2. Add all 32 ports to all 10 VLANs
for port_name in port_names:
for vlan_name in vlan_names:
self.dvs_vlan.create_vlan_member(vlan_name, port_name, tagging_mode = "tagged")
#print "All %d ports are added to all %d VLANs"%(num_ports,num_vlans)
self.dvs_vlan.get_and_verify_vlan_member_ids(num_ports*num_vlans)

# 3. Do the following for each port
# 3.1. Delete port from CONFIG DB and verify that its deleted from
# CONFIG DB and APPL DB but not from ASIC DB.
# 3.2. Remove the port from all 10 VLANs and verify that it
# gets deleted from ASIC DB
ports = []
for port_name in port_names:
p = Port(dvs, port_name)
ports.append(p)
p.sync_from_config_db()
p.delete_from_config_db()
#print "Deleted %s from config DB"%port_name
assert(p.exists_in_config_db() == False)
assert(p.exists_in_app_db() == False)
assert(p.exists_in_asic_db() == True)
Expand All @@ -220,15 +225,15 @@ def test_all_port_10_vlans(self, dvs):

self.dvs_vlan.get_and_verify_vlan_member_ids((num_ports*num_vlans)-(len(ports)*num_vlans))
assert(p.not_exists_in_asic_db() == True)
#print "All %d ports are removed from all %d VLANs and deleted"%(num_ports,num_vlans)

# 4. Re-create all ports and verify the same
for p in ports:
p.write_to_config_db()
p.verify_config_db()
p.verify_app_db()
p.verify_asic_db()
#print "Re-created all %d ports"%num_ports

# 5. Remove all VLANs and verify the same
for vlan_name in vlan_names:
self.dvs_vlan.remove_vlan(vlan_name)
#print "All %d VLANs removed"%num_vlans
self.dvs_vlan.get_and_verify_vlan_ids(0)
Loading

0 comments on commit 7cd3814

Please sign in to comment.