From ddc57ed6ae6acb9afc6aabb4a430fdaf9db655d1 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 26 Jun 2026 12:43:19 +0200 Subject: [PATCH] confd: fix veth teardown regression When a veth pair has one end in a container and the other in the host namespace, removing it failed: the container end's teardown deleted the whole pair, so the host end's own delete then failed ("Cannot find device") and aborted the teardown, leaving the interface behind. The host-namespace delete added for the both-ends-in-container case was emitted for every container veth end. Restrict it to the primary end, which for a host/container pair is the host end that already deletes the pair itself. Extend the container veth tests to remove the pair and verify it is gone, covering both the single-end (regression) and both-ends teardown paths. Fixes: #1546 Signed-off-by: Joachim Wiberg --- src/confd/src/cni.c | 14 ++++++++------ test/case/containers/internal_link/test.adoc | 1 + test/case/containers/internal_link/test.py | 17 +++++++++++++++++ test/case/containers/veth/test.adoc | 1 + test/case/containers/veth/test.py | 17 +++++++++++++++++ 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/confd/src/cni.c b/src/confd/src/cni.c index 9f7980aa..a7ebfb70 100644 --- a/src/confd/src/cni.c +++ b/src/confd/src/cni.c @@ -415,13 +415,15 @@ int cni_netdag_gen_iface(struct dagger *net, const char *ifname, fprintf(fp, "container -a -f delete network %s >/dev/null\n", ifname); - /* If this end belongs to a veth pair, the kernel keeps the pair - * alive after CNI host-device returns the interface to the host - * namespace. Remove it here, once the container is gone, so the - * pair does not linger and block a later re-creation. Tolerant: - * the peer's teardown may already have removed it. + /* When both ends of a veth pair are in containers, neither end + * is torn down from the host namespace, so the kernel keeps the + * pair alive after CNI host-device returns the interfaces here. + * Have the primary end delete it, once its container is gone. + * When the peer is a host interface it is the primary and deletes + * the pair itself (veth_gen_del); we must not race it here. + * Tolerant: the pair may already be gone. */ - if (lydx_get_child(dif, "veth")) + if (lydx_get_child(dif, "veth") && veth_is_primary(dif)) fprintf(fp, "ip link del dev %s 2>/dev/null || true\n", ifname); fclose(fp); diff --git a/test/case/containers/internal_link/test.adoc b/test/case/containers/internal_link/test.adoc index f52c311a..d1ce703e 100644 --- a/test/case/containers/internal_link/test.adoc +++ b/test/case/containers/internal_link/test.adoc @@ -29,5 +29,6 @@ image::topology.svg[VETH Pair Between Two Containers topology, align=center, sca . Create VETH pair with both ends assigned to containers . Verify both containers have started . Verify {LEFT} reaches {RIGHT} over the internal VETH pair +. Remove containers and VETH pair, verify clean teardown diff --git a/test/case/containers/internal_link/test.py b/test/case/containers/internal_link/test.py index 26c342eb..24ea7e1f 100755 --- a/test/case/containers/internal_link/test.py +++ b/test/case/containers/internal_link/test.py @@ -98,4 +98,21 @@ with infamy.Test() as test: until(reachable, attempts=30) + with test.step("Remove containers and VETH pair, verify clean teardown"): + # Removing a pair with both ends in containers must tear it down + # cleanly, leaving nothing behind in the host namespace. + target.delete_xpaths([ + f"/infix-containers:containers/container[name='{LEFT}']", + f"/infix-containers:containers/container[name='{RIGHT}']", + f"/ietf-interfaces:interfaces/interface[name='{IFACE_LEFT}']", + f"/ietf-interfaces:interfaces/interface[name='{IFACE_RIGHT}']", + ]) + + def removed(): + ifaces = target.get_data("/ietf-interfaces:interfaces")["interfaces"]["interface"] + names = [iface["name"] for iface in ifaces] + return IFACE_LEFT not in names and IFACE_RIGHT not in names + + until(removed, attempts=30) + test.succeed() diff --git a/test/case/containers/veth/test.adoc b/test/case/containers/veth/test.adoc index 7adb242e..8ddfdddf 100644 --- a/test/case/containers/veth/test.adoc +++ b/test/case/containers/veth/test.adoc @@ -28,5 +28,6 @@ image::topology.svg[Container with VETH Pair topology, align=center, scaledwidth . Verify container 'web-br0-veth' has started . Verify basic DUT connectivity, host:data can ping DUT 10.0.0.2 . Verify container 'web-br0-veth' is reachable on http://10.0.0.2:91 +. Remove container and VETH pair, verify clean teardown diff --git a/test/case/containers/veth/test.py b/test/case/containers/veth/test.py index 8d50ec9f..cf85076e 100755 --- a/test/case/containers/veth/test.py +++ b/test/case/containers/veth/test.py @@ -108,4 +108,21 @@ with infamy.Test() as test: with test.step("Verify container 'web-br0-veth' is reachable on http://10.0.0.2:91"): until(lambda: MESG in ns.call(lambda: curl(URL)), attempts=10) + with test.step("Remove container and VETH pair, verify clean teardown"): + # Regression test for #941/#1546: removing a veth pair with one end + # in a container must tear the pair down cleanly, leaving nothing + # behind in the host namespace. + target.delete_xpaths([ + f"/infix-containers:containers/container[name='{NAME}']", + f"/ietf-interfaces:interfaces/interface[name='{NAME}']", + "/ietf-interfaces:interfaces/interface[name='veth0b']", + ]) + + def removed(): + ifaces = target.get_data("/ietf-interfaces:interfaces")["interfaces"]["interface"] + names = [iface["name"] for iface in ifaces] + return NAME not in names and "veth0b" not in names + + until(removed, attempts=30) + test.succeed()