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 #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 <[email protected]> * 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 <[email protected]> --------- Signed-off-by: Saikrishna Arcot <[email protected]>
sonic-net · Apr 29, 2024 · 099d40c · 099d40c
1 parent 45a1901
commit 099d40c
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 58 deletions.
diff --git 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
@@ -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
@@ -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")