From 9cd1bf37be7be416b4d6a11d9b3ab11856d6c174 Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Tue, 21 Jul 2026 00:11:33 +0500 Subject: [PATCH] portmap: flush UDP conntrack on DEL to avoid hostPort blackhole cmdAdd already deletes stale UDP conntrack entries after installing DNAT rules so a replacement pod with a new IP is not blackholed. cmdDel only tore down the iptables/nftables rules and left conntrack alone. That breaks the common case of flipping a pod to hostNetwork:true on the same hostPort: DEL removes the NAT mapping, but leftover UDP conntrack still steers packets at the gone container IP. Flush on DEL the same way ADD does (best-effort; failures are logged only). Fixes #1190 Signed-off-by: Dean Chen <862469039@qq.com> --- plugins/meta/portmap/main.go | 17 ++++++++++++++++- plugins/meta/portmap/portmap_test.go | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/plugins/meta/portmap/main.go b/plugins/meta/portmap/main.go index 28dab921c..913e371a9 100644 --- a/plugins/meta/portmap/main.go +++ b/plugins/meta/portmap/main.go @@ -161,7 +161,22 @@ func cmdDel(args *skel.CmdArgs) error { // We don't need to parse out whether or not we're using v6 or snat, // deletion is idempotent - return netConf.mapper.unforwardPorts(netConf) + if err := netConf.mapper.unforwardPorts(netConf); err != nil { + return err + } + + // Flush UDP conntrack after removing the DNAT rules. Without this, a later + // consumer of the same hostPort (for example a pod that switches to + // hostNetwork:true and binds the port directly) can be blackholed by stale + // NAT state left from the previous mapping. cmdAdd already flushes on the + // setup path; DEL needs the same cleanup. Failures are informative only. + if err := deletePortmapStaleConnections(netConf.RuntimeConfig.PortMaps, unix.AF_INET); err != nil { + log.Printf("failed to delete stale UDP conntrack entries on DEL: %v", err) + } + if err := deletePortmapStaleConnections(netConf.RuntimeConfig.PortMaps, unix.AF_INET6); err != nil { + log.Printf("failed to delete stale UDP conntrack entries on DEL: %v", err) + } + return nil } func main() { diff --git a/plugins/meta/portmap/portmap_test.go b/plugins/meta/portmap/portmap_test.go index 7c7aea415..e6aa28e0f 100644 --- a/plugins/meta/portmap/portmap_test.go +++ b/plugins/meta/portmap/portmap_test.go @@ -23,6 +23,23 @@ import ( "github.com/containernetworking/cni/pkg/types" ) +// deletePortmapStaleConnections is a no-op for non-UDP mappings and must not +// fail when given only TCP entries (no conntrack call is made). +var _ = Describe("deletePortmapStaleConnections", func() { + It("skips non-UDP port mappings without error", func() { + err := deletePortmapStaleConnections([]PortMapEntry{ + {HostPort: 8080, ContainerPort: 80, Protocol: "tcp"}, + {HostPort: 8081, ContainerPort: 81, Protocol: "TCP"}, + }, 2 /* AF_INET */) + Expect(err).NotTo(HaveOccurred()) + }) + + It("accepts an empty mapping list", func() { + err := deletePortmapStaleConnections(nil, 2) + Expect(err).NotTo(HaveOccurred()) + }) +}) + var _ = Describe("portmapping configuration", func() { for _, ver := range []string{"0.3.0", "0.3.1", "0.4.0", "1.0.0"} { // Redefine ver inside for scope so real value is picked up by each dynamically defined It()