From 099d40c9daa159f2c57a031af43c667bb36ab23c Mon Sep 17 00:00:00 2001 From: Saikrishna Arcot Date: Mon, 29 Apr 2024 13:38:57 -0700 Subject: [PATCH] Run `ip neigh flush` before removing the IP address from the interface (#3281) * Ignore any error returned from `ip neigh flush` In the test_po_update test case, one of the things done there is to remove an IP address from a port channel interface. As part of that, the current handling for that issues a `ip neigh flush dev ...` command, added in sonic-net/sonic-utilities#606, presumably to remove old neighbor entries that would no longer be valid. I would think that the kernel would automatically do this, but maybe it didn't back then; I'm not sure if there's been a behavior change here since then. In some cases, this command is returning an error, saying "Failed to send flush request: No such file or directory". I'm not sure why this is; maybe when iproute2 is going through the list of neighbors, some neighbor entry was there, but then by the time it issued the deletion request, that neighbor entry was removed by the kernel since the IP address was removed. Either way, I don't believe a failure here is critical. Therefore, ignore any failures from running this command. Signed-off-by: Saikrishna Arcot * Move the IP neighbor flush to be before the IP address removal This should make sure that the IP neighbor flush should always work. This also requires the tests to be updated, to mock out the flush command call since that interface won't exist. Signed-off-by: Saikrishna Arcot --------- Signed-off-by: Saikrishna Arcot --- config/main.py | 11 +++-- tests/ip_config_test.py | 94 ++++++++++++++++++++++++----------------- tests/vlan_test.py | 36 ++++++++++------ 3 files changed, 83 insertions(+), 58 deletions(-) diff --git a/config/main.py b/config/main.py index 8f3b7245bd..ea2a5154e3 100644 --- a/config/main.py +++ b/config/main.py @@ -4745,17 +4745,16 @@ def remove(ctx, interface_name, ip_addr): if output != "": if any(interface_name in output_line for output_line in output.splitlines()): ctx.fail("Cannot remove the last IP entry of interface {}. A static {} route is still bound to the RIF.".format(interface_name, ip_ver)) - remove_router_interface_ip_address(config_db, interface_name, ip_address) - interface_addresses = get_interface_ipaddresses(config_db, interface_name) - if len(interface_addresses) == 0 and is_interface_bind_to_vrf(config_db, interface_name) is False and get_intf_ipv6_link_local_mode(ctx, interface_name, table_name) != "enable": - if table_name != "VLAN_SUB_INTERFACE": - config_db.set_entry(table_name, interface_name, None) - if multi_asic.is_multi_asic(): command = ['sudo', 'ip', 'netns', 'exec', str(ctx.obj['namespace']), 'ip', 'neigh', 'flush', 'dev', str(interface_name), str(ip_address)] else: command = ['ip', 'neigh', 'flush', 'dev', str(interface_name), str(ip_address)] clicommon.run_command(command) + remove_router_interface_ip_address(config_db, interface_name, ip_address) + interface_addresses = get_interface_ipaddresses(config_db, interface_name) + if len(interface_addresses) == 0 and is_interface_bind_to_vrf(config_db, interface_name) is False and get_intf_ipv6_link_local_mode(ctx, interface_name, table_name) != "enable": + if table_name != "VLAN_SUB_INTERFACE": + config_db.set_entry(table_name, interface_name, None) # # 'loopback-action' subcommand diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index b227c76ff3..6003e7401a 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -99,22 +99,28 @@ def test_add_del_interface_valid_ipv4(self): assert ('Eth36.10', '32.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE') # config int ip remove Ethernet64 10.10.10.1/24 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet64", "10.10.10.1/24"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Ethernet64', '10.10.10.1/24') not in db.cfgdb.get_table('INTERFACE') + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet64", "10.10.10.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Ethernet64', '10.10.10.1/24') not in db.cfgdb.get_table('INTERFACE') # config int ip remove Ethernet0.10 10.11.10.1/24 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet0.10", "10.11.10.1/24"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Ethernet0.10', '10.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet0.10", "10.11.10.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Ethernet0.10', '10.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') # config int ip remove Eth36.10 32.11.10.1/24 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "32.11.10.1/24"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Eth36.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "32.11.10.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Eth36.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') def test_add_interface_invalid_ipv4(self): db = Db() @@ -185,20 +191,26 @@ def test_add_del_interface_valid_ipv6(self): assert ('Eth36.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE') # config int ip remove Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('INTERFACE') - - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet0.10", "1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') - - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Eth36.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('INTERFACE') + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet0.10", "1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Eth36.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE') def test_del_interface_case_sensitive_ipv6(self): db = Db() @@ -209,10 +221,12 @@ def test_del_interface_case_sensitive_ipv6(self): assert ('Ethernet72', 'FC00::1/126') in db.cfgdb.get_table('INTERFACE') # config int ip remove Ethernet72 FC00::1/126 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "FC00::1/126"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Ethernet72', 'FC00::1/126') not in db.cfgdb.get_table('INTERFACE') + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "FC00::1/126"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Ethernet72', 'FC00::1/126') not in db.cfgdb.get_table('INTERFACE') def test_add_interface_invalid_ipv6(self): db = Db() @@ -248,10 +262,12 @@ def test_add_del_interface_ipv6_with_leading_zeros(self): assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') in db.cfgdb.get_table('INTERFACE') # config int ip remove Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') not in db.cfgdb.get_table('INTERFACE') + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') not in db.cfgdb.get_table('INTERFACE') def test_add_del_interface_shortened_ipv6_with_leading_zeros(self): db = Db() @@ -265,10 +281,12 @@ def test_add_del_interface_shortened_ipv6_with_leading_zeros(self): assert ('Ethernet68', '3000::1/64') in db.cfgdb.get_table('INTERFACE') # config int ip remove Ethernet68 3000::001/64 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "3000::001/64"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 - assert ('Ethernet68', '3000::1/64') not in db.cfgdb.get_table('INTERFACE') + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "3000::001/64"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + assert ('Ethernet68', '3000::1/64') not in db.cfgdb.get_table('INTERFACE') def test_intf_vrf_bind_unbind(self): runner = CliRunner() diff --git a/tests/vlan_test.py b/tests/vlan_test.py index 436e309281..5a84737b2a 100644 --- a/tests/vlan_test.py +++ b/tests/vlan_test.py @@ -368,13 +368,17 @@ def test_config_vlan_del_vlan(self, mock_restart_dhcp_relay_service): assert "Error: Vlan1000 can not be removed. First remove IP addresses assigned to this VLAN" in result.output # remove vlan IP`s - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Vlan1000", "192.168.0.1/21"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Vlan1000", "192.168.0.1/21"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Vlan1000", "fc02:1000::1/64"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Vlan1000", "fc02:1000::1/64"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 # del vlan with IP result = runner.invoke(config.config.commands["vlan"].commands["del"], ["1000"], obj=db) @@ -778,15 +782,19 @@ def test_config_vlan_del_dhcp_relay_restart(self): obj = {"config_db": db.cfgdb} # remove vlan IP`s - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], - ["Vlan1000", "192.168.0.1/21"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Vlan1000", "192.168.0.1/21"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], - ["Vlan1000", "fc02:1000::1/64"], obj=obj) - print(result.exit_code, result.output) - assert result.exit_code != 0 + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Vlan1000", "fc02:1000::1/64"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 # remove vlan members vlan_member = db.cfgdb.get_table("VLAN_MEMBER")