From b037b7d7fd04cabd3e33a1e063f7d1c5d0cb6f26 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Mon, 8 Nov 2021 16:16:35 -0800 Subject: [PATCH] fixed test failures --- config/vlan.py | 132 +++++++------------------------------ scripts/fdbshow | 75 +++++++++------------ tests/vlan_test.py | 160 ++++++++++----------------------------------- 3 files changed, 85 insertions(+), 282 deletions(-) diff --git a/config/vlan.py b/config/vlan.py index 78e77f6820..ddc2cdfc60 100644 --- a/config/vlan.py +++ b/config/vlan.py @@ -156,11 +156,6 @@ def add_vlan_member(db, vid, port, untagged): if (clicommon.interface_is_untagged_member(db.cfgdb, port) and untagged): ctx.fail("{} is already untagged member!".format(port)) - vlan_in_db = db.cfgdb.get_entry('VLAN', vlan) - members = vlan_in_db.get('members', []) - members.append(port) - vlan_in_db['members'] = members - db.cfgdb.set_entry('VLAN', vlan, vlan_in_db) db.cfgdb.set_entry('VLAN_MEMBER', (vlan, port), {'tagging_mode': "untagged" if untagged else "tagged" }) @vlan_member.command('del') @@ -191,91 +186,8 @@ def del_vlan_member(db, vid, port): if not clicommon.is_port_vlan_member(db.cfgdb, port, vlan): ctx.fail("{} is not a member of {}".format(port, vlan)) - vlan_in_db = db.cfgdb.get_entry('VLAN', vlan) - members = vlan_in_db.get('members', []) - vlan_in_db['members'] = members - members.remove(port) - if len(members) == 0: - del vlan_in_db['members'] - else: - vlan_in_db['members'] = members - db.cfgdb.set_entry('VLAN', vlan, vlan_in_db) db.cfgdb.set_entry('VLAN_MEMBER', (vlan, port), None) -@vlan.group(cls=clicommon.AbbreviationGroup, name='dhcp_relay') -def vlan_dhcp_relay(): - pass - -@vlan_dhcp_relay.command('add') -@click.argument('vid', metavar='', required=True, type=int) -@click.argument('dhcp_relay_destination_ip', metavar='', required=True) -@clicommon.pass_db -def add_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip): - """ Add a destination IP address to the VLAN's DHCP relay """ - - ctx = click.get_current_context() - - if not clicommon.is_ipaddress(dhcp_relay_destination_ip): - ctx.fail('{} is invalid IP address'.format(dhcp_relay_destination_ip)) - - vlan_name = 'Vlan{}'.format(vid) - vlan = db.cfgdb.get_entry('VLAN', vlan_name) - if len(vlan) == 0: - ctx.fail("{} doesn't exist".format(vlan_name)) - - dhcp_relay_dests = vlan.get('dhcp_servers', []) - if dhcp_relay_destination_ip in dhcp_relay_dests: - click.echo("{} is already a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name)) - return - - dhcp_relay_dests.append(dhcp_relay_destination_ip) - vlan['dhcp_servers'] = dhcp_relay_dests - db.cfgdb.set_entry('VLAN', vlan_name, vlan) - click.echo("Added DHCP relay destination address {} to {}".format(dhcp_relay_destination_ip, vlan_name)) - try: - click.echo("Restarting DHCP relay service...") - clicommon.run_command("systemctl stop dhcp_relay", display_cmd=False) - clicommon.run_command("systemctl reset-failed dhcp_relay", display_cmd=False) - clicommon.run_command("systemctl start dhcp_relay", display_cmd=False) - except SystemExit as e: - ctx.fail("Restart service dhcp_relay failed with error {}".format(e)) - -@vlan_dhcp_relay.command('del') -@click.argument('vid', metavar='', required=True, type=int) -@click.argument('dhcp_relay_destination_ip', metavar='', required=True) -@clicommon.pass_db -def del_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip): - """ Remove a destination IP address from the VLAN's DHCP relay """ - - ctx = click.get_current_context() - - if not clicommon.is_ipaddress(dhcp_relay_destination_ip): - ctx.fail('{} is invalid IP address'.format(dhcp_relay_destination_ip)) - - vlan_name = 'Vlan{}'.format(vid) - vlan = db.cfgdb.get_entry('VLAN', vlan_name) - if len(vlan) == 0: - ctx.fail("{} doesn't exist".format(vlan_name)) - - dhcp_relay_dests = vlan.get('dhcp_servers', []) - if not dhcp_relay_destination_ip in dhcp_relay_dests: - ctx.fail("{} is not a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name)) - - dhcp_relay_dests.remove(dhcp_relay_destination_ip) - if len(dhcp_relay_dests) == 0: - del vlan['dhcp_servers'] - else: - vlan['dhcp_servers'] = dhcp_relay_dests - db.cfgdb.set_entry('VLAN', vlan_name, vlan) - click.echo("Removed DHCP relay destination address {} from {}".format(dhcp_relay_destination_ip, vlan_name)) - try: - click.echo("Restarting DHCP relay service...") - clicommon.run_command("systemctl stop dhcp_relay", display_cmd=False) - clicommon.run_command("systemctl reset-failed dhcp_relay", display_cmd=False) - clicommon.run_command("systemctl start dhcp_relay", display_cmd=False) - except SystemExit as e: - ctx.fail("Restart service dhcp_relay failed with error {}".format(e)) - def vlan_id_is_valid(vid): """Check if the vlan id is in acceptable range (between 1 and 4094) """ @@ -329,6 +241,9 @@ def add_vlan_range(db, vid1, vid2, warning): vid2 = vid2+1 warning_vlans_list = [] + ctx = click.get_current_context() + ctx.obj = {'db': db.cfgdb} + curr_vlan_count = 0 clients = db.cfgdb.get_redis_client(db.cfgdb.CONFIG_DB) pipe = clients.pipeline() @@ -362,25 +277,27 @@ def del_vlan_range(db, vid1, vid2, warning): warning_vlans_list = [] warning_membership_list = [] warning_ip_list = [] + ctx = click.get_current_context() + ctx.obj = {'db': db.cfgdb} client = db.cfgdb.get_redis_client(db.cfgdb.CONFIG_DB) pipe = client.pipeline() - cur, vlan_member_keys = client.scan(cursor=0, match='*VLAN_MEMBER*', count=50) + cur, vlan_member_keys = client.scan(cursor=0, match='VLAN_MEMBER*', count=50) while cur != 0: - cur, keys = client.scan(cursor=cur, match='*VLAN_MEMBER*', count=50) + cur, keys = client.scan(cursor=cur, match='VLAN_MEMBER*', count=50) vlan_member_keys.extend(keys) - - cur, vlan_temp_member_keys = client.scan(cursor=0, match='*VLAN_MEMBER*', count=50) + + cur, vlan_temp_member_keys = client.scan(cursor=0, match='VLAN_MEMBER*', count=50) while cur != 0: - cur, keys = client.scan(cursor=cur, match='*VLAN_MEMBER*', count=50) + cur, keys = client.scan(cursor=cur, match='VLAN_MEMBER*', count=50) vlan_temp_member_keys.extend(keys) - cur, vlan_ip_keys = client.scan(cursor=0, match='*VLAN_INTERFACE*', count=50) + cur, vlan_ip_keys = client.scan(cursor=0, match='VLAN_INTERFACE*', count=50) while cur != 0: - cur, keys = client.scan(cursor=cur, match='*VLAN_INTERFACE*', count=50) + cur, keys = client.scan(cursor=cur, match='VLAN_INTERFACE*', count=50) vlan_ip_keys.extend(keys) - # Fetch the interfaces from config_db associated with *VLAN_MEMBER* + # Fetch the interfaces from config_db associated with VLAN_MEMBER* stored_intf_list = [] if vlan_temp_member_keys is not None: for x in range(len(vlan_temp_member_keys)): @@ -390,7 +307,7 @@ def del_vlan_range(db, vid1, vid2, warning): stored_intf_list = list(set(stored_intf_list)) list_length = len(stored_intf_list) - # Fetch VLAN participation list for each interface + # Fetch VLAN participation list for each interface vid = range(vid1, vid2) if vlan_temp_member_keys is not None and list_length != 0: for i in range(list_length): @@ -567,6 +484,7 @@ def add_vlan_member_range(db, vid1, vid2, interface_name, untagged, warning): if v == interface_name: ctx.fail(" {} is configured as a port channel member".format(interface_name)) + vlan_ports_data = db.cfgdb.get_table('VLAN_MEMBER') for vid in range(vid1, vid2): vlan_name = 'Vlan{}'.format(vid) vlan = db.cfgdb.get_entry('VLAN', vlan_name) @@ -576,8 +494,7 @@ def add_vlan_member_range(db, vid1, vid2, interface_name, untagged, warning): warning_vlans_list.append(vid) continue - members = vlan.get('members', []) - if interface_name in members: + if (vlan_name, interface_name) in vlan_ports_data.keys(): if warning is True: warning_membership_list.append(vid) if clicommon.get_interface_naming_mode() == "alias": @@ -588,11 +505,9 @@ def add_vlan_member_range(db, vid1, vid2, interface_name, untagged, warning): else: continue - members.append(interface_name) - vlan['members'] = members - pipe.hmset('VLAN|{}'.format(vlan_name), vlan_member_data(vlan)) pipe.hmset('VLAN_MEMBER|{}'.format(vlan_name+'|'+interface_name), {'tagging_mode': "untagged" if untagged else "tagged" }) # If port is being made L2 port, enable STP + ctx.obj = {'db': db.cfgdb} pipe.execute() # Log warning messages if 'warning' option is enabled if warning is True and len(warning_vlans_list) != 0: @@ -630,6 +545,7 @@ def del_vlan_member_range(db, vid1, vid2, interface_name, warning): clients = db.cfgdb.get_redis_client(db.cfgdb.CONFIG_DB) pipe = clients.pipeline() + vlan_ports_data = db.cfgdb.get_table('VLAN_MEMBER') for vid in range(vid1, vid2): vlan_name = 'Vlan{}'.format(vid) vlan = db.cfgdb.get_entry('VLAN', vlan_name) @@ -639,8 +555,7 @@ def del_vlan_member_range(db, vid1, vid2, interface_name, warning): warning_vlans_list.append(vid) continue - members = vlan.get('members', []) - if interface_name not in members: + if (vlan_name, interface_name) not in vlan_ports_data.keys(): if warning is True: warning_membership_list.append(vid) if clicommon.get_interface_naming_mode() == "alias": @@ -651,16 +566,13 @@ def del_vlan_member_range(db, vid1, vid2, interface_name, warning): else: continue - members.remove(interface_name) - if len(members) == 0: - pipe.hdel('VLAN|{}'.format(vlan_name), 'members@') - else: - vlan['members'] = members pipe.hmset('VLAN|{}'.format(vlan_name), vlan_member_data(vlan)) pipe.delete('VLAN_MEMBER|{}'.format(vlan_name+'|'+interface_name)) - pipe.delete('STP_VLAN_INTF|{}'.format(vlan_name + '|' + interface_name)) + pipe.delete('STP_VLAN_PORT|{}'.format(vlan_name + '|' + interface_name)) pipe.execute() + # If port is being made non-L2 port, disable STP on interface + ctx.obj = {'db': db.cfgdb} # Log warning messages if 'warning' option is enabled if warning is True and len(warning_vlans_list) != 0: logging.warning('Non-existent VLANs: {}'.format(get_hyphenated_string(warning_vlans_list))) diff --git a/scripts/fdbshow b/scripts/fdbshow index 68f72ee70a..9b83b07191 100755 --- a/scripts/fdbshow +++ b/scripts/fdbshow @@ -1,4 +1,4 @@ -#!/usr/bin/python3 +#!/usr/bin/env python3 """ Script to show MAC/FDB entries learnt in Hardware @@ -54,8 +54,8 @@ except KeyError: # pragma: no cover pass from natsort import natsorted -from swsssdk import SonicV2Connector, port_util -#from swsscommon.swsscommon import SonicV2Connector +from swsssdk import port_util +from swsscommon.swsscommon import SonicV2Connector from tabulate import tabulate class FdbShow(object): @@ -75,7 +75,7 @@ class FdbShow(object): def fetch_fdb_data(self): """ Fetch FDB entries from ASIC DB. - FDB entries are sorted on "VlanID" plus MAC addr, and stored as a list of tuples + FDB entries are sorted on "VlanID" and stored as a list of tuples """ self.db.connect(self.db.ASIC_DB) self.bridge_mac_list = [] @@ -89,61 +89,46 @@ class FdbShow(object): bvid_tlb = {} oid_pfx = len("oid:0x") - - client = self.db.get_redis_client(self.db.ASIC_DB) - pipe = client.pipeline() - values = [] - fdb_list = [] for s in fdb_str: fdb_entry = s fdb = json.loads(fdb_entry .split(":", 2)[-1]) if not fdb: continue - fdb_list.append(fdb) - pipe.hgetall(s) - values = pipe.execute() - client = self.db.get_redis_client(self.db.ASIC_DB) - pipe = client.pipeline() - vlans = [] - vlan_keys = self.db.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:*") - for v in vlan_keys: - pipe.hgetall(v) - vlans = pipe.execute() - - posi = 0 - for ent in values: - if 'SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID' not in ent: - posi = posi + 1 - continue + + ent = self.db.get_all('ASIC_DB', s, blocking=True) br_port_id = ent["SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID"][oid_pfx:] ent_type = ent["SAI_FDB_ENTRY_ATTR_TYPE"] fdb_type = ['Dynamic','Static'][ent_type == "SAI_FDB_ENTRY_TYPE_STATIC"] - if (br_port_id not in self.if_br_oid_map): - posi = posi + 1 + if br_port_id not in self.if_br_oid_map: continue - port_id = self.if_br_oid_map[br_port_id] - - #FIXME: When if_name not mapped to port_id in DB, using port_id as if_name in the display. - try: + if port_id in self.if_oid_map: if_name = self.if_oid_map[port_id] - except Exception as e: + else: if_name = port_id - - #FIXME: When VLAN Id for fdb entry not present in DB, then not displaying the fdb entry. if 'vlan' in fdb: vlan_id = fdb["vlan"] - elif 'bvid' in fdb: - try: - vlan_id = vlans[vlan_keys.index("ASIC_STATE:SAI_OBJECT_TYPE_VLAN:{}".format(fdb_list[posi][u'bvid']))] ["SAI_VLAN_ATTR_VLAN_ID"] - except Exception as e: - posi = posi + 1 + else: + if 'bvid' not in fdb: + # no possibility to find the Vlan id. skip the FDB entry continue - - self.bridge_mac_list.append((int(vlan_id),) + (fdb_list[posi]["mac"],) + (if_name,) + (fdb_type,)) - posi = posi + 1 - - self.bridge_mac_list.sort(key = lambda x: (x[0], x[1])) + bvid = fdb["bvid"] + if bvid in bvid_tlb: + vlan_id = bvid_tlb[bvid] + else: + try: + vlan_id = port_util.get_vlan_id_from_bvid(self.db, bvid) + bvid_tlb[bvid] = vlan_id + if vlan_id is None: + # the situation could be faced if the system has an FDB entries, + # which are linked to default Vlan(caused by untagged traffic) + continue + except Exception: + vlan_id = bvid + print("Failed to get Vlan id for bvid {}\n".format(bvid)) + self.bridge_mac_list.append((int(vlan_id),) + (fdb["mac"],) + (if_name,) + (fdb_type,)) + + self.bridge_mac_list.sort(key = lambda x: x[0]) return @@ -180,7 +165,7 @@ class FdbShow(object): output.append([self.FDB_COUNT, fdb[0], fdb[1], fdb[2], fdb[3]]) print(tabulate(output, self.HEADER)) - print("Total number of entries {0} ".format(self.FDB_COUNT)) + print("Total number of entries {0}".format(self.FDB_COUNT)) def main(): diff --git a/tests/vlan_test.py b/tests/vlan_test.py index 99140e43c3..a861de79d5 100644 --- a/tests/vlan_test.py +++ b/tests/vlan_test.py @@ -91,20 +91,38 @@ """ show_vlan_config_output_range="""\ -Name VID Member Mode --------- ----- ---------- -------- -Vlan1000 1000 Ethernet4 untagged -Vlan1000 1000 Ethernet8 untagged -Vlan1000 1000 Ethernet12 untagged -Vlan1000 1000 Ethernet16 untagged -Vlan2000 2000 Ethernet24 untagged -Vlan2000 2000 Ethernet28 untagged -Vlan3001 3001 Ethernet4 tagged -Vlan3001 3001 Ethernet8 tagged -Vlan3002 3002 Ethernet4 tagged -Vlan3002 3002 Ethernet8 tagged -Vlan3003 3003 Ethernet4 tagged -Vlan3003 3003 Ethernet8 tagged +Name VID Member Mode +-------- ----- --------------- -------- +Vlan1000 1000 Ethernet4 untagged +Vlan1000 1000 Ethernet8 untagged +Vlan1000 1000 Ethernet12 untagged +Vlan1000 1000 Ethernet16 untagged +Vlan2000 2000 Ethernet24 untagged +Vlan2000 2000 Ethernet28 untagged +Vlan3000 3000 +Vlan3001 3001 Ethernet4 tagged +Vlan3001 3001 Ethernet8 tagged +Vlan3002 3002 Ethernet4 tagged +Vlan3002 3002 Ethernet8 tagged +Vlan3003 3003 Ethernet4 tagged +Vlan3003 3003 Ethernet8 tagged +Vlan4000 4000 PortChannel1001 tagged +""" + +show_vlan_config_output_range_after_del="""\ +Name VID Member Mode +-------- ----- --------------- -------- +Vlan1000 1000 Ethernet4 untagged +Vlan1000 1000 Ethernet8 untagged +Vlan1000 1000 Ethernet12 untagged +Vlan1000 1000 Ethernet16 untagged +Vlan2000 2000 Ethernet24 untagged +Vlan2000 2000 Ethernet28 untagged +Vlan3000 3000 +Vlan3001 3001 +Vlan3002 3002 +Vlan3003 3003 +Vlan4000 4000 PortChannel1001 tagged """ show_vlan_config_in_alias_mode_output="""\ @@ -477,92 +495,6 @@ def test_config_add_del_vlan_and_vlan_member_in_alias_mode(self): os.environ['SONIC_CLI_IFACE_MODE'] = "default" - def test_config_vlan_add_dhcp_relay_with_nonexist_vlanid(self): - runner = CliRunner() - - with mock.patch('utilities_common.cli.run_command') as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"], - ["1001", "192.0.0.100"]) - print(result.exit_code) - print(result.output) - # traceback.print_tb(result.exc_info[2]) - assert result.exit_code != 0 - assert "Error: Vlan1001 doesn't exist" in result.output - assert mock_run_command.call_count == 0 - - def test_config_vlan_add_dhcp_relay_with_invalid_vlanid(self): - runner = CliRunner() - - with mock.patch('utilities_common.cli.run_command') as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"], - ["4096", "192.0.0.100"]) - print(result.exit_code) - print(result.output) - # traceback.print_tb(result.exc_info[2]) - assert result.exit_code != 0 - assert "Error: Vlan4096 doesn't exist" in result.output - assert mock_run_command.call_count == 0 - - def test_config_vlan_add_dhcp_relay_with_invalid_ip(self): - runner = CliRunner() - - with mock.patch('utilities_common.cli.run_command') as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"], - ["1000", "192.0.0.1000"]) - print(result.exit_code) - print(result.output) - # traceback.print_tb(result.exc_info[2]) - assert result.exit_code != 0 - assert "Error: 192.0.0.1000 is invalid IP address" in result.output - assert mock_run_command.call_count == 0 - - def test_config_vlan_add_dhcp_relay_with_exist_ip(self): - runner = CliRunner() - - with mock.patch('utilities_common.cli.run_command') as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"], - ["1000", "192.0.0.1"]) - print(result.exit_code) - print(result.output) - # traceback.print_tb(result.exc_info[2]) - assert result.exit_code == 0 - assert "192.0.0.1 is already a DHCP relay destination for Vlan1000" in result.output - assert mock_run_command.call_count == 0 - - def test_config_vlan_add_del_dhcp_relay_dest(self): - runner = CliRunner() - db = Db() - - # add new relay dest - with mock.patch("utilities_common.cli.run_command") as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"], - ["1000", "192.0.0.100"], obj=db) - print(result.exit_code) - print(result.output) - assert result.exit_code == 0 - assert result.output == config_vlan_add_dhcp_relay_output - assert mock_run_command.call_count == 3 - - # show output - result = runner.invoke(show.cli.commands["vlan"].commands["brief"], [], obj=db) - print(result.output) - assert result.output == show_vlan_brief_output_with_new_dhcp_relay_address - - # del relay dest - with mock.patch("utilities_common.cli.run_command") as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"], - ["1000", "192.0.0.100"], obj=db) - print(result.exit_code) - print(result.output) - assert result.exit_code == 0 - assert result.output == config_vlan_del_dhcp_relay_output - assert mock_run_command.call_count == 3 - - # show output - result = runner.invoke(show.cli.commands["vlan"].commands["brief"], [], obj=db) - print(result.output) - assert result.output == show_vlan_brief_output - def test_vlan_config_range(self): runner = CliRunner() db = Db() @@ -595,33 +527,7 @@ def test_vlan_config_range(self): print(result.exit_code) print(result.output) assert result.exit_code == 0 - assert result.output == show_vlan_config_output - - def test_config_vlan_remove_nonexist_dhcp_relay_dest(self): - runner = CliRunner() - - with mock.patch('utilities_common.cli.run_command') as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"], - ["1000", "192.0.0.100"]) - print(result.exit_code) - print(result.output) - # traceback.print_tb(result.exc_info[2]) - assert result.exit_code != 0 - assert "Error: 192.0.0.100 is not a DHCP relay destination for Vlan1000" in result.output - assert mock_run_command.call_count == 0 - - def test_config_vlan_remove_dhcp_relay_dest_with_nonexist_vlanid(self): - runner = CliRunner() - - with mock.patch('utilities_common.cli.run_command') as mock_run_command: - result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"], - ["1001", "192.0.0.1"]) - print(result.exit_code) - print(result.output) - # traceback.print_tb(result.exc_info[2]) - assert result.exit_code != 0 - assert "Error: Vlan1001 doesn't exist" in result.output - assert mock_run_command.call_count == 0 + assert result.output == show_vlan_config_output_range_after_del def test_config_vlan_proxy_arp_with_nonexist_vlan_intf_table(self): modes = ["enabled", "disabled"]