From: Vincent Bernat <vincent@bernat.im>
Date: Fri, 15 Sep 2017 20:38:52 +0000 (+0200)
Subject: netlink: don't remove interfaces when they are released from a bridge
X-Git-Tag: 0.9.9~12
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=703f6a0d2b1a792a3489e02b06e9f92c7ed9e49c;p=thirdparty%2Flldpd.git

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 f7c490fb..b07f29dd 100644
--- 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 6e80d254..e14ac9b9 100644
--- 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 f8f36021..87d1610d 100644
--- 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 4adbc0ea..9788f6e8 100644
--- 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'])