From a2bd0b4b53db8468681eb2905e2fbc2f9073869a Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Mon, 12 Sep 2016 22:27:33 -0700 Subject: [PATCH] LinuxBridge: Use ifindex for logical 'timestamp' With Xenial (and maybe older versions), the modified timestamps in /sys/class/net/(device_name) are not stable. They appear to work for a period of time, and then when some kind of cache clears on the kernel side, all of the timestamps are reset to the latest access time. This was causing the Linux Bridge agent to think that the interfaces were experiencing local changes much more frequently than they actually were, resulting in more polling to the Neutron server and subsequently more BUILD->ACTIVE->BUILD->ACTIVE transitions in the logical model. The purpose of the timestamp patch was to catch rapid server REBUILD operations where the interface would be deleted and re-added within a polling interval. Without it, these would be stuck in the BUILD state since the agent wouldn't realize it needed to wire the ports. This patch switches to looking at the IFINDEX of the interfaces to use as a sort of logical timestamp. If an interface gets removed and readded, it will get a different index, so the original timestamp comparison logic will still work. In the future, the agent should undergo a larger refactor to just watch 'ip monitor' for netlink events to replace the polling of the interface listing and the timestamp logic entirely. However, this approach was taken due to the near term release and the ability to back-port it to older releases. This was verified with both Nova rebuild actions and Nova interface attach/detach actions. Change-Id: I016019885446bff6806268ab49cd5476d93ec61f Closes-Bug: #1622833 --- neutron/agent/linux/bridge_lib.py | 7 ++++--- .../linuxbridge/agent/linuxbridge_neutron_agent.py | 7 ++++++- neutron/tests/common/net_helpers.py | 7 ++++--- .../functional/agent/linux/test_bridge_lib.py | 14 ++++++++------ 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/neutron/agent/linux/bridge_lib.py b/neutron/agent/linux/bridge_lib.py index e3bf52502c6..dfb1100ff2f 100644 --- a/neutron/agent/linux/bridge_lib.py +++ b/neutron/agent/linux/bridge_lib.py @@ -39,10 +39,11 @@ def is_bridged_interface(interface): return os.path.exists(BRIDGE_PORT_FS_FOR_DEVICE % interface) -def get_interface_bridged_time(interface): +def get_interface_ifindex(interface): try: - return os.stat(BRIDGE_PORT_FS_FOR_DEVICE % interface).st_mtime - except OSError: + with open(os.path.join(BRIDGE_FS, interface, 'ifindex'), 'r') as fh: + return int(fh.read().strip()) + except (IOError, ValueError): pass diff --git a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py index 1e5b0728d99..aa65eec6b40 100644 --- a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py @@ -597,7 +597,12 @@ class LinuxBridgeManager(amb.CommonAgentManagerBase): LOG.debug("Done deleting interface %s", interface) def get_devices_modified_timestamps(self, devices): - return {d: bridge_lib.get_interface_bridged_time(d) for d in devices} + # NOTE(kevinbenton): we aren't returning real timestamps here. We + # are returning interface indexes instead which change when the + # interface is removed/re-added. This works for the direct + # comparison the common agent loop performs with these. + # See bug/1622833 for details. + return {d: bridge_lib.get_interface_ifindex(d) for d in devices} def get_all_devices(self): devices = set() diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index ff819f1a14c..e5a2efe031b 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -857,10 +857,11 @@ class LinuxBridgePortFixture(PortFixture): super(LinuxBridgePortFixture, self)._setUp() br_port_name = self._get_port_name() if br_port_name: - self.br_port, self.port = self.useFixture( - NamedVethFixture(veth0_prefix=br_port_name)).ports + self.veth_fixture = self.useFixture( + NamedVethFixture(veth0_prefix=br_port_name)) else: - self.br_port, self.port = self.useFixture(VethFixture()).ports + self.veth_fixture = self.useFixture(VethFixture()) + self.br_port, self.port = self.veth_fixture.ports if self.mac: self.port.link.set_address(self.mac) diff --git a/neutron/tests/functional/agent/linux/test_bridge_lib.py b/neutron/tests/functional/agent/linux/test_bridge_lib.py index a0a1772f9a0..88f0e4981d8 100644 --- a/neutron/tests/functional/agent/linux/test_bridge_lib.py +++ b/neutron/tests/functional/agent/linux/test_bridge_lib.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_utils import uuidutils from neutron.agent.linux import bridge_lib from neutron.tests.common import net_helpers @@ -28,7 +29,8 @@ class BridgeLibTestCase(base.BaseSudoTestCase): bridge = self.useFixture( net_helpers.LinuxBridgeFixture(namespace=None)).bridge port_fixture = self.useFixture( - net_helpers.LinuxBridgePortFixture(bridge)) + net_helpers.LinuxBridgePortFixture( + bridge, port_id=uuidutils.generate_uuid())) return bridge, port_fixture def test_is_bridged_interface(self): @@ -42,12 +44,12 @@ class BridgeLibTestCase(base.BaseSudoTestCase): def test_get_bridge_names(self): self.assertIn(self.bridge.name, bridge_lib.get_bridge_names()) - def test_get_interface_bridged_time(self): + def test_get_interface_ifindex(self): port = self.port_fixture.br_port - t1 = bridge_lib.get_interface_bridged_time(port) - self.bridge.delif(port) - self.bridge.addif(port) - t2 = bridge_lib.get_interface_bridged_time(port) + t1 = bridge_lib.get_interface_ifindex(str(port)) + self.port_fixture.veth_fixture.destroy() + self.port_fixture.veth_fixture._setUp() + t2 = bridge_lib.get_interface_ifindex(str(port)) self.assertIsNotNone(t1) self.assertIsNotNone(t2) self.assertGreaterEqual(t2, t1)