netlink: don't remove interfaces when they are released from a bridge

When released from a bridge, a RTM_DELLINK is sent. We detect there is
an upper device and turn thay into a RTM_NEWLINK. If the interface was
really removed, we would get two RTM_DELLINK (one with master, one
without).

On most setups, another RTM_NEWLINK was sent due to change of promisc
status.

diff --git a/NEWS b/NEWS
index f7c490fb78914db42438e94f09065f0d6598fd6a..b07f29dda24c6f0469db91d16b13fd592f36acc5 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,8 @@ lldpd (0.9.9)
   * Changes:
     + lldpcli can now display local interfaces with LLDP data sent on
       each of them ("show interfaces").
+  * Fix:
+    + Don't remove interfaces when they are released from a bridge.
 
 lldpd (0.9.8)
   * Changes:

diff --git a/src/daemon/netlink.c b/src/daemon/netlink.c
index 6e80d2545ecae37780ec1395a6b418f1771976ec..e14ac9b981eaaa8b56311dc4ab1892d09256c4c2 100644 (file)
--- a/src/daemon/netlink.c
+++ b/src/daemon/netlink.c
@@ -550,6 +550,15 @@ retry:
                                        TAILQ_FOREACH(ifdold, ifs, next) {
                                                if (ifdold->index == ifdnew->index) break;
                                        }
+                                       if (msg->nlmsg_type == RTM_DELLINK && ifdnew->upper_idx != -1) {
+                                               /* This happens for bridges */
+                                               log_debug("netlink",
+                                                   "removal request for %s, but has a master, convert it",
+                                                   ifdnew->name);
+                                               ifdnew->upper_idx = -1;
+                                               msg->nlmsg_type = RTM_NEWLINK;
+                                       }
+
                                        if (msg->nlmsg_type == RTM_NEWLINK) {
                                                if (ifdold == NULL) {
                                                        log_debug("netlink", "interface %s is new",
@@ -567,7 +576,7 @@ retry:
                                                if (ifdold == NULL) {
                                                        log_warnx("netlink",
                                                            "removal request for %s, but no knowledge of it",
-                                                               ifdnew->name);
+                                                           ifdnew->name);
                                                } else {
                                                        log_debug("netlink", "interface %s is to be removed",
                                                            ifdold->name);

diff --git a/tests/integration/fixtures/network.py b/tests/integration/fixtures/network.py
index f8f36021849c147afea626fdc42141252e764093..87d1610dc45dc41e368010045d037caf097902d8 100644 (file)
--- a/tests/integration/fixtures/network.py
+++ b/tests/integration/fixtures/network.py
@@ -134,6 +134,11 @@ class LinksFactory(object):
         idx = ipr.link_lookup(ifname=name)[0]
         ipr.link('del', index=idx)
 
+    def nomaster(self, name):
+        ipr = pyroute2.IPRoute()
+        idx = ipr.link_lookup(ifname=name)[0]
+        ipr.link('set', index=idx, master=0)
+
 
 @pytest.fixture
 def links():

diff --git a/tests/integration/test_interfaces.py b/tests/integration/test_interfaces.py
index 4adbc0ea3f63d1a4276e062942e404fc44583ecf..9788f6e8c4df1fba936b17113d2bfbffd25264e8 100644 (file)
--- a/tests/integration/test_interfaces.py
+++ b/tests/integration/test_interfaces.py
@@ -14,6 +14,27 @@ def test_simple_bridge(lldpd1, lldpd, lldpcli, namespaces, links):
         assert out['lldp.eth0.chassis.Bridge.enabled'] == 'on'
 
 
+def test_remove_bridge(lldpd, lldpcli, namespaces, links):
+    links(namespaces(1), namespaces(2))
+    links(namespaces(3), namespaces(1))  # Another link to setup a bridge
+    with namespaces(1):
+        links.bridge('br42', 'eth0', 'eth3')
+        lldpd("-r")
+    with namespaces(2):
+        lldpd()
+        time.sleep(2)
+        lldpcli("pause")        # Prevent any updates
+    with namespaces(1):
+        out = lldpcli("-f", "keyvalue", "show", "neighbors", "details")
+        assert out['lldp.eth0.port.descr'] == 'eth1'
+        # Remove from bridge
+        links.nomaster('eth0')
+        time.sleep(1)
+        # Check if we still have eth0
+        out = lldpcli("-f", "keyvalue", "show", "neighbors", "details")
+        assert out['lldp.eth0.port.descr'] == 'eth1'
+
+
 @pytest.mark.skipif('Dot1' not in pytest.config.lldpd.features,
                     reason="Dot1 not supported")
 @pytest.mark.parametrize('when', ['before', 'after'])