Revert "Merge pull request #600 from Metaswitch/issue549arp"

This reverts commit 70cb6f3, reversing
changes made to 4f647bb.

Conflicts:
	CHANGES.md

CHANGES.md

@@ -14,7 +14,6 @@
   many local endpoints.
 - Neutron mechanism driver patches and docs for OpenStack Kilo release.
 - Correct IPv6 documentation for Juno and Kilo.
-- Reset ARP configuration when endpoint MAC changes.
 
 ## 0.21
 

calico/felix/devices.py

@@ -179,30 +179,23 @@ def del_route(ip_type, ip, interface):
         futils.check_call(["ip", "-6", "route", "del", ip, "dev", interface])
 
 
-def set_routes(ip_type, ips, interface, mac=None, reset_arp=False):
+def set_routes(ip_type, ips, interface, mac=None):
     """
     Set the routes on the interface to be the specified set.
 
     :param ip_type: Type of IP (IPV4 or IPV6)
     :param set ips: IPs to set up (any not in the set are removed)
     :param str interface: Interface name
     :param str mac|NoneType: MAC address. May not be none unless ips is empty.
-    :param bool reset_arp: Reset arp. Only valid if IPv4.
     """
     if mac is None and ips:
         raise ValueError("mac must be supplied if ips is not empty")
-    if reset_arp and ip_type != futils.IPV4:
-        raise ValueError("reset_arp may only be supplied for IPv4")
 
     current_ips = list_interface_ips(ip_type, interface)
-
     for ip in (current_ips - ips):
         del_route(ip_type, ip, interface)
     for ip in (ips - current_ips):
         add_route(ip_type, ip, interface, mac)
-    if reset_arp:
-        for ip in (ips & current_ips):
-            futils.check_call(['arp', '-s', ip, mac, '-i', interface])
 
 
 def interface_up(if_name):
@@ -221,6 +214,7 @@ def interface_up(if_name):
     try:
         with open(flags_file, 'r') as f:
             flags = f.read().strip()
+
             _log.debug("Interface %s has flags %s", if_name, flags)
     except IOError as e:
         # If we fail to check that the interface is up, then it has probably

calico/felix/endpoint.py

@@ -189,7 +189,6 @@ def __init__(self, config, endpoint_id, ip_type, iptables_updater,
         # Will be filled in as we learn about the OS interface and the
         # endpoint config.
         self.endpoint = None
-        self._mac = None
         self._iface_name = None
         self._suffix = None
 
@@ -206,19 +205,12 @@ def on_endpoint_update(self, endpoint):
         :param dict[str] endpoint: endpoint parameter dictionary.
         """
         _log.info("%s updated: %s", self, endpoint)
-
-        mac_changed = False
-
         if endpoint and not self.endpoint:
             # This is the first time we have seen the endpoint, so extract the
             # interface name and endpoint ID.
             self._iface_name = endpoint["name"]
             self._suffix = interface_to_suffix(self.config,
                                                self._iface_name)
-            if endpoint['mac'] != self._mac:
-                self._mac = endpoint['mac']
-                mac_changed = True
-
         was_ready = self._ready
 
         # Activate the required profile IDs (and deactivate any that we no
@@ -240,7 +232,7 @@ def on_endpoint_update(self, endpoint):
         if endpoint:
             # Configure the network interface; may fail if not there yet (in
             # which case we'll just do it when the interface comes up).
-            self._configure_interface(mac_changed)
+            self._configure_interface()
         else:
             # Remove the network programming.
             self._deconfigure_interface()
@@ -347,12 +339,9 @@ def _remove_chains(self):
             _log.exception("Failed to delete chains for %s", self)
             self._failed = True
 
-    def _configure_interface(self, mac_changed):
+    def _configure_interface(self):
         """
         Applies sysctls and routes to the interface.
-
-        :param: bool mac_changed: Has the MAC address changed since it was last
-        configured? If so, we reconfigure ARP for the interface.
         """
         try:
             if self.ip_type == IPV4:
@@ -368,8 +357,7 @@ def _configure_interface(self, mac_changed):
                 ips.add(futils.net_to_ip(ip))
             devices.set_routes(self.ip_type, ips,
                                self._iface_name,
-                               self.endpoint["mac"],
-                               reset_arp=mac_changed)
+                               self.endpoint["mac"])
 
         except (IOError, FailedSystemCall, CalledProcessError):
             if not devices.interface_exists(self._iface_name):

calico/felix/test/test_devices.py

@@ -84,10 +84,6 @@ def test_add_route(self):
             futils.check_call.assert_any_call(['arp', '-s', ip, mac, '-i', tap])
             futils.check_call.assert_called_with(["ip", "route", "replace", ip, "dev", tap])
 
-        with self.assertRaisesRegexp(ValueError,
-                                     "mac must be supplied if ip is provided"):
-            devices.add_route(type, ip, tap, None)
-
         type = futils.IPV6
         ip = "2001::"
         with mock.patch('calico.felix.futils.check_call', return_value=retcode):
@@ -115,82 +111,6 @@ def test_del_route(self):
             devices.del_route(type, ip, tap)
             futils.check_call.assert_called_once_with(["ip", "-6", "route", "del", ip, "dev", tap])
 
-    def test_set_routes(self):
-        type = futils.IPV4
-        ips = set(["1.2.3.4", "2.3.4.5"])
-        interface = "tapabcdef"
-        mac = stub_utils.get_mac()
-        with self.assertRaisesRegexp(ValueError,
-                                     "mac must be supplied if ips is not empty"):
-            devices.set_routes(type, ips, interface)
-        with self.assertRaisesRegexp(ValueError,
-                                     "reset_arp may only be supplied for IPv4"):
-            devices.set_routes(futils.IPV6, ips, interface, mac=mac,
-                               reset_arp=True)
-
-        retcode = futils.CommandOutput("", "")
-        current_ips = set()
-        calls = [mock.call(['arp', '-s', "1.2.3.4", mac, '-i', interface]),
-                 mock.call(["ip", "route", "replace", "1.2.3.4", "dev", interface]),
-                 mock.call(['arp', '-s', "2.3.4.5", mac, '-i', interface]),
-                 mock.call(["ip", "route", "replace", "2.3.4.5", "dev", interface])]
-
-        with mock.patch('calico.felix.futils.check_call', return_value=retcode):
-            with mock.patch('calico.felix.devices.list_interface_ips',
-                            return_value=current_ips):
-                devices.set_routes(type, ips, interface, mac)
-                self.assertEqual(futils.check_call.call_count, len(calls))
-                futils.check_call.assert_has_calls(calls, any_order=True)
-
-        with mock.patch('calico.felix.futils.check_call', return_value=retcode):
-            with mock.patch('calico.felix.devices.list_interface_ips',
-                            return_value=ips):
-                self.assertEqual(futils.check_call.call_count, 0)
-
-        current_ips = set(["2.3.4.5", "3.4.5.6"])
-        calls = [mock.call(['arp', '-s', "1.2.3.4", mac, '-i', interface]),
-                 mock.call(["ip", "route", "replace", "1.2.3.4", "dev", interface]),
-                 mock.call(['arp', '-d', "3.4.5.6", '-i', interface]),
-                 mock.call(["ip", "route", "del", "3.4.5.6", "dev", interface])]
-        with mock.patch('calico.felix.futils.check_call', return_value=retcode):
-            with mock.patch('calico.felix.devices.list_interface_ips',
-                            return_value=current_ips):
-                devices.set_routes(type, ips, interface, mac)
-                self.assertEqual(futils.check_call.call_count, len(calls))
-                futils.check_call.assert_has_calls(calls, any_order=True)
-
-        retcode = futils.CommandOutput("", "")
-        current_ips = set()
-        calls = [mock.call(['arp', '-s', "1.2.3.4", mac, '-i', interface]),
-                 mock.call(["ip", "route", "replace", "1.2.3.4", "dev", interface]),
-                 mock.call(['arp', '-s', "2.3.4.5", mac, '-i', interface]),
-                 mock.call(["ip", "route", "replace", "2.3.4.5", "dev", interface])]
-
-        with mock.patch('calico.felix.futils.check_call', return_value=retcode):
-            with mock.patch('calico.felix.devices.list_interface_ips',
-                            return_value=current_ips):
-                devices.set_routes(type, ips, interface, mac, reset_arp=True)
-                self.assertEqual(futils.check_call.call_count, len(calls))
-                futils.check_call.assert_has_calls(calls, any_order=True)
-
-        with mock.patch('calico.felix.futils.check_call', return_value=retcode):
-            with mock.patch('calico.felix.devices.list_interface_ips',
-                            return_value=ips):
-                self.assertEqual(futils.check_call.call_count, 0)
-
-        current_ips = set(["2.3.4.5", "3.4.5.6"])
-        calls = [mock.call(['arp', '-s', "1.2.3.4", mac, '-i', interface]),
-                 mock.call(["ip", "route", "replace", "1.2.3.4", "dev", interface]),
-                 mock.call(['arp', '-s', "2.3.4.5", mac, '-i', interface]),
-                 mock.call(['arp', '-d', "3.4.5.6", '-i', interface]),
-                 mock.call(["ip", "route", "del", "3.4.5.6", "dev", interface])]
-        with mock.patch('calico.felix.futils.check_call', return_value=retcode):
-            with mock.patch('calico.felix.devices.list_interface_ips',
-                            return_value=current_ips):
-                devices.set_routes(type, ips, interface, mac, reset_arp=True)
-                self.assertEqual(futils.check_call.call_count, len(calls))
-                futils.check_call.assert_has_calls(calls, any_order=True)
-
 
     def test_list_interface_ips(self):
         type = futils.IPV4

debian/changelog

@@ -10,7 +10,6 @@ calico (0.22) trusty; urgency=medium
     many local endpoints.
   * Neutron mechanism driver patches and docs for OpenStack Kilo release.
   * Correct IPv6 documentation for Juno and Kilo.
-  * Reset ARP configuration when endpoint MAC changes.
 
  -- Matt Dupre <[email protected]>  Tue, 01 Jun 2015 17:15:48 +0100
 

rpm/calico.spec

@@ -215,7 +215,6 @@ rm -rf $RPM_BUILD_ROOT
     many local endpoints.
   - Neutron mechanism driver patches and docs for OpenStack Kilo release.
   - Correct IPv6 documentation for Juno and Kilo.
-  - Reset ARP configuration when endpoint MAC changes.
 
 * Tue May 26 2015 Matt Dupre <[email protected]> 0.21
   - Support for running multiple neutron-server instances in OpenStack