From 32710d3e5ee39c9b160db236a536307fbf6e95dc Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Mon, 30 Jun 2025 04:13:36 +0200 Subject: [PATCH 01/10] libnet/portmapper: remove dead AppendForwardingTableEntry Prior to commit 4f09af626, AppendForwardingTableEntry had a Linux implementation. That's not the case anymore, and it's a no-op on Windows. Remove it. Signed-off-by: Albin Kerouanton --- daemon/libnetwork/portmapper/mapper.go | 3 --- daemon/libnetwork/portmapper/mapper_windows.go | 5 ----- 2 files changed, 8 deletions(-) diff --git a/daemon/libnetwork/portmapper/mapper.go b/daemon/libnetwork/portmapper/mapper.go index 0fa82a8b26..3f6cf2efec 100644 --- a/daemon/libnetwork/portmapper/mapper.go +++ b/daemon/libnetwork/portmapper/mapper.go @@ -124,9 +124,6 @@ func (pm *PortMapper) MapRange(container net.Addr, hostIP net.IP, hostPortStart, } containerIP, containerPort := getIPAndPort(m.container) - if err := pm.AppendForwardingTableEntry(m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort); err != nil { - return nil, err - } var err error m.stopUserlandProxy, err = newDummyProxy(m.proto, hostIP, allocatedHostPort) diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portmapper/mapper_windows.go index cfa07ca38b..c10101c728 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portmapper/mapper_windows.go @@ -20,11 +20,6 @@ type PortMapper struct { allocator *portallocator.PortAllocator } -// AppendForwardingTableEntry adds a port mapping to the forwarding table -func (pm *PortMapper) AppendForwardingTableEntry(proto string, sourceIP net.IP, sourcePort int, containerIP string, containerPort int) error { - return nil -} - // DeleteForwardingTableEntry removes a port mapping from the forwarding table func (pm *PortMapper) DeleteForwardingTableEntry(proto string, sourceIP net.IP, sourcePort int, containerIP string, containerPort int) error { return nil From b48442db4cadfbe8cd11b92fe62ec11ec294e2f5 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Mon, 30 Jun 2025 04:14:17 +0200 Subject: [PATCH 02/10] libnet/portmapper: remove dead DeleteForwardingTableEntry Prior to commit 4f09af626, DeleteForwardingTableEntry had a Linux implementation. That's not the case anymore, and it's a no-op on Windows. Remove it. Signed-off-by: Albin Kerouanton --- daemon/libnetwork/portmapper/mapper.go | 10 ---------- daemon/libnetwork/portmapper/mapper_windows.go | 6 ------ 2 files changed, 16 deletions(-) diff --git a/daemon/libnetwork/portmapper/mapper.go b/daemon/libnetwork/portmapper/mapper.go index 3f6cf2efec..00914999e0 100644 --- a/daemon/libnetwork/portmapper/mapper.go +++ b/daemon/libnetwork/portmapper/mapper.go @@ -123,15 +123,11 @@ func (pm *PortMapper) MapRange(container net.Addr, hostIP net.IP, hostPortStart, return nil, ErrPortMappedForIP } - containerIP, containerPort := getIPAndPort(m.container) - var err error m.stopUserlandProxy, err = newDummyProxy(m.proto, hostIP, allocatedHostPort) if err != nil { // FIXME(thaJeztah): both stopping the proxy and deleting iptables rules can produce an error, and both are not currently handled. m.stopUserlandProxy() - // need to undo the iptables rules before we return - pm.DeleteForwardingTableEntry(m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort) return nil, err } @@ -156,12 +152,6 @@ func (pm *PortMapper) Unmap(host net.Addr) error { delete(pm.currentMappings, key) - containerIP, containerPort := getIPAndPort(data.container) - hostIP, hostPort := getIPAndPort(data.host) - if err := pm.DeleteForwardingTableEntry(data.proto, hostIP, hostPort, containerIP.String(), containerPort); err != nil { - log.G(context.TODO()).Errorf("Error on iptables delete: %s", err) - } - switch a := host.(type) { case *net.TCPAddr: pm.allocator.ReleasePort(a.IP, "tcp", a.Port) diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portmapper/mapper_windows.go index c10101c728..463c524d0d 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portmapper/mapper_windows.go @@ -1,7 +1,6 @@ package portmapper import ( - "net" "sync" "github.com/moby/moby/v2/daemon/libnetwork/portallocator" @@ -19,8 +18,3 @@ type PortMapper struct { allocator *portallocator.PortAllocator } - -// DeleteForwardingTableEntry removes a port mapping from the forwarding table -func (pm *PortMapper) DeleteForwardingTableEntry(proto string, sourceIP net.IP, sourcePort int, containerIP string, containerPort int) error { - return nil -} From f6c59f9779ff06a74c9e92156fd06f5cd7d400b6 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Mon, 30 Jun 2025 04:15:36 +0200 Subject: [PATCH 03/10] libnet/portmapper: merge mapper.go & mapper_windows.go The portmapper struct provided by libnet/portmapper is only available on Windows. Merge both files to reflect that. Signed-off-by: Albin Kerouanton --- daemon/libnetwork/portmapper/mapper.go | 202 ------------------ .../libnetwork/portmapper/mapper_windows.go | 194 +++++++++++++++++ 2 files changed, 194 insertions(+), 202 deletions(-) delete mode 100644 daemon/libnetwork/portmapper/mapper.go diff --git a/daemon/libnetwork/portmapper/mapper.go b/daemon/libnetwork/portmapper/mapper.go deleted file mode 100644 index 00914999e0..0000000000 --- a/daemon/libnetwork/portmapper/mapper.go +++ /dev/null @@ -1,202 +0,0 @@ -//go:build windows - -package portmapper - -import ( - "context" - "errors" - "fmt" - "net" - - "github.com/containerd/log" - "github.com/ishidawataru/sctp" - "github.com/moby/moby/v2/daemon/libnetwork/portallocator" -) - -type mapping struct { - proto string - stopUserlandProxy func() error - host net.Addr - container net.Addr -} - -var ( - // ErrUnknownBackendAddressType refers to an unknown container or unsupported address type - ErrUnknownBackendAddressType = errors.New("unknown container address type not supported") - // ErrPortMappedForIP refers to a port already mapped to an ip address - ErrPortMappedForIP = errors.New("port is already mapped to ip") - // ErrPortNotMapped refers to an unmapped port - ErrPortNotMapped = errors.New("port is not mapped") - // ErrSCTPAddrNoIP refers to a SCTP address without IP address. - ErrSCTPAddrNoIP = errors.New("sctp address does not contain any IP address") -) - -// New returns a new instance of PortMapper -func New() *PortMapper { - return NewWithPortAllocator(portallocator.Get(), "") -} - -// NewWithPortAllocator returns a new instance of PortMapper which will use the specified PortAllocator -func NewWithPortAllocator(allocator *portallocator.PortAllocator, proxyPath string) *PortMapper { - return &PortMapper{ - currentMappings: make(map[string]*mapping), - allocator: allocator, - proxyPath: proxyPath, - } -} - -// MapRange maps the specified container transport address to the host's network address and transport port range -func (pm *PortMapper) MapRange(container net.Addr, hostIP net.IP, hostPortStart, hostPortEnd int) (host net.Addr, retErr error) { - pm.lock.Lock() - defer pm.lock.Unlock() - - var ( - m *mapping - proto string - allocatedHostPort int - ) - - switch container.(type) { - case *net.TCPAddr: - proto = "tcp" - - var err error - allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) - if err != nil { - return nil, err - } - defer func() { - if retErr != nil { - pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) - } - }() - - m = &mapping{ - proto: proto, - host: &net.TCPAddr{IP: hostIP, Port: allocatedHostPort}, - container: container, - } - case *net.UDPAddr: - proto = "udp" - - var err error - allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) - if err != nil { - return nil, err - } - defer func() { - if retErr != nil { - pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) - } - }() - - m = &mapping{ - proto: proto, - host: &net.UDPAddr{IP: hostIP, Port: allocatedHostPort}, - container: container, - } - case *sctp.SCTPAddr: - proto = "sctp" - - var err error - allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) - if err != nil { - return nil, err - } - defer func() { - if retErr != nil { - pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) - } - }() - - m = &mapping{ - proto: proto, - host: &sctp.SCTPAddr{IPAddrs: []net.IPAddr{{IP: hostIP}}, Port: allocatedHostPort}, - container: container, - } - default: - return nil, ErrUnknownBackendAddressType - } - - key := getKey(m.host) - if _, exists := pm.currentMappings[key]; exists { - return nil, ErrPortMappedForIP - } - - var err error - m.stopUserlandProxy, err = newDummyProxy(m.proto, hostIP, allocatedHostPort) - if err != nil { - // FIXME(thaJeztah): both stopping the proxy and deleting iptables rules can produce an error, and both are not currently handled. - m.stopUserlandProxy() - return nil, err - } - - pm.currentMappings[key] = m - return m.host, nil -} - -// Unmap removes stored mapping for the specified host transport address -func (pm *PortMapper) Unmap(host net.Addr) error { - pm.lock.Lock() - defer pm.lock.Unlock() - - key := getKey(host) - data, exists := pm.currentMappings[key] - if !exists { - return ErrPortNotMapped - } - - if data.stopUserlandProxy != nil { - data.stopUserlandProxy() - } - - delete(pm.currentMappings, key) - - switch a := host.(type) { - case *net.TCPAddr: - pm.allocator.ReleasePort(a.IP, "tcp", a.Port) - case *net.UDPAddr: - pm.allocator.ReleasePort(a.IP, "udp", a.Port) - case *sctp.SCTPAddr: - if len(a.IPAddrs) == 0 { - return ErrSCTPAddrNoIP - } - pm.allocator.ReleasePort(a.IPAddrs[0].IP, "sctp", a.Port) - default: - return ErrUnknownBackendAddressType - } - - return nil -} - -func getKey(a net.Addr) string { - switch t := a.(type) { - case *net.TCPAddr: - return fmt.Sprintf("%s:%d/%s", t.IP.String(), t.Port, "tcp") - case *net.UDPAddr: - return fmt.Sprintf("%s:%d/%s", t.IP.String(), t.Port, "udp") - case *sctp.SCTPAddr: - if len(t.IPAddrs) == 0 { - log.G(context.TODO()).Error(ErrSCTPAddrNoIP) - return "" - } - return fmt.Sprintf("%s:%d/%s", t.IPAddrs[0].IP.String(), t.Port, "sctp") - } - return "" -} - -func getIPAndPort(a net.Addr) (net.IP, int) { - switch t := a.(type) { - case *net.TCPAddr: - return t.IP, t.Port - case *net.UDPAddr: - return t.IP, t.Port - case *sctp.SCTPAddr: - if len(t.IPAddrs) == 0 { - log.G(context.TODO()).Error(ErrSCTPAddrNoIP) - return nil, 0 - } - return t.IPAddrs[0].IP, t.Port - } - return nil, 0 -} diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portmapper/mapper_windows.go index 463c524d0d..d6c1f88b8f 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portmapper/mapper_windows.go @@ -1,9 +1,33 @@ package portmapper import ( + "context" + "errors" + "fmt" + "net" "sync" "github.com/moby/moby/v2/daemon/libnetwork/portallocator" + "github.com/containerd/log" + "github.com/ishidawataru/sctp" +) + +type mapping struct { + proto string + stopUserlandProxy func() error + host net.Addr + container net.Addr +} + +var ( + // ErrUnknownBackendAddressType refers to an unknown container or unsupported address type + ErrUnknownBackendAddressType = errors.New("unknown container address type not supported") + // ErrPortMappedForIP refers to a port already mapped to an ip address + ErrPortMappedForIP = errors.New("port is already mapped to ip") + // ErrPortNotMapped refers to an unmapped port + ErrPortNotMapped = errors.New("port is not mapped") + // ErrSCTPAddrNoIP refers to a SCTP address without IP address. + ErrSCTPAddrNoIP = errors.New("sctp address does not contain any IP address") ) // PortMapper manages the network address translation @@ -18,3 +42,173 @@ type PortMapper struct { allocator *portallocator.PortAllocator } + +// New returns a new instance of PortMapper +func New() *PortMapper { + return NewWithPortAllocator(portallocator.Get(), "") +} + +// NewWithPortAllocator returns a new instance of PortMapper which will use the specified PortAllocator +func NewWithPortAllocator(allocator *portallocator.PortAllocator, proxyPath string) *PortMapper { + return &PortMapper{ + currentMappings: make(map[string]*mapping), + allocator: allocator, + proxyPath: proxyPath, + } +} + +// MapRange maps the specified container transport address to the host's network address and transport port range +func (pm *PortMapper) MapRange(container net.Addr, hostIP net.IP, hostPortStart, hostPortEnd int) (host net.Addr, retErr error) { + pm.lock.Lock() + defer pm.lock.Unlock() + + var ( + m *mapping + proto string + allocatedHostPort int + ) + + switch container.(type) { + case *net.TCPAddr: + proto = "tcp" + + var err error + allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) + if err != nil { + return nil, err + } + defer func() { + if retErr != nil { + pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) + } + }() + + m = &mapping{ + proto: proto, + host: &net.TCPAddr{IP: hostIP, Port: allocatedHostPort}, + container: container, + } + case *net.UDPAddr: + proto = "udp" + + var err error + allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) + if err != nil { + return nil, err + } + defer func() { + if retErr != nil { + pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) + } + }() + + m = &mapping{ + proto: proto, + host: &net.UDPAddr{IP: hostIP, Port: allocatedHostPort}, + container: container, + } + case *sctp.SCTPAddr: + proto = "sctp" + + var err error + allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) + if err != nil { + return nil, err + } + defer func() { + if retErr != nil { + pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) + } + }() + + m = &mapping{ + proto: proto, + host: &sctp.SCTPAddr{IPAddrs: []net.IPAddr{{IP: hostIP}}, Port: allocatedHostPort}, + container: container, + } + default: + return nil, ErrUnknownBackendAddressType + } + + key := getKey(m.host) + if _, exists := pm.currentMappings[key]; exists { + return nil, ErrPortMappedForIP + } + + var err error + m.stopUserlandProxy, err = newDummyProxy(m.proto, hostIP, allocatedHostPort) + if err != nil { + // FIXME(thaJeztah): both stopping the proxy and deleting iptables rules can produce an error, and both are not currently handled. + m.stopUserlandProxy() + return nil, err + } + + pm.currentMappings[key] = m + return m.host, nil +} + +// Unmap removes stored mapping for the specified host transport address +func (pm *PortMapper) Unmap(host net.Addr) error { + pm.lock.Lock() + defer pm.lock.Unlock() + + key := getKey(host) + data, exists := pm.currentMappings[key] + if !exists { + return ErrPortNotMapped + } + + if data.stopUserlandProxy != nil { + data.stopUserlandProxy() + } + + delete(pm.currentMappings, key) + + switch a := host.(type) { + case *net.TCPAddr: + pm.allocator.ReleasePort(a.IP, "tcp", a.Port) + case *net.UDPAddr: + pm.allocator.ReleasePort(a.IP, "udp", a.Port) + case *sctp.SCTPAddr: + if len(a.IPAddrs) == 0 { + return ErrSCTPAddrNoIP + } + pm.allocator.ReleasePort(a.IPAddrs[0].IP, "sctp", a.Port) + default: + return ErrUnknownBackendAddressType + } + + return nil +} + +func getKey(a net.Addr) string { + switch t := a.(type) { + case *net.TCPAddr: + return fmt.Sprintf("%s:%d/%s", t.IP.String(), t.Port, "tcp") + case *net.UDPAddr: + return fmt.Sprintf("%s:%d/%s", t.IP.String(), t.Port, "udp") + case *sctp.SCTPAddr: + if len(t.IPAddrs) == 0 { + log.G(context.TODO()).Error(ErrSCTPAddrNoIP) + return "" + } + return fmt.Sprintf("%s:%d/%s", t.IPAddrs[0].IP.String(), t.Port, "sctp") + } + return "" +} + +func getIPAndPort(a net.Addr) (net.IP, int) { + switch t := a.(type) { + case *net.TCPAddr: + return t.IP, t.Port + case *net.UDPAddr: + return t.IP, t.Port + case *sctp.SCTPAddr: + if len(t.IPAddrs) == 0 { + log.G(context.TODO()).Error(ErrSCTPAddrNoIP) + return nil, 0 + } + return t.IPAddrs[0].IP, t.Port + } + return nil, 0 +} From 9e7de1b6790909fdcdd7633818ce486df53f0758 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Mon, 30 Jun 2025 19:34:11 +0200 Subject: [PATCH 04/10] libnet/portmapper: remove unused field proxyPath Signed-off-by: Albin Kerouanton --- daemon/libnetwork/portmapper/mapper_windows.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portmapper/mapper_windows.go index d6c1f88b8f..d168286070 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portmapper/mapper_windows.go @@ -38,22 +38,19 @@ type PortMapper struct { currentMappings map[string]*mapping lock sync.Mutex - proxyPath string - allocator *portallocator.PortAllocator } // New returns a new instance of PortMapper func New() *PortMapper { - return NewWithPortAllocator(portallocator.Get(), "") + return NewWithPortAllocator(portallocator.Get()) } // NewWithPortAllocator returns a new instance of PortMapper which will use the specified PortAllocator -func NewWithPortAllocator(allocator *portallocator.PortAllocator, proxyPath string) *PortMapper { +func NewWithPortAllocator(allocator *portallocator.PortAllocator) *PortMapper { return &PortMapper{ currentMappings: make(map[string]*mapping), allocator: allocator, - proxyPath: proxyPath, } } From f6e5b3afc529a9f53870f6cea337137d4e7856a1 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Tue, 1 Jul 2025 01:18:57 +0200 Subject: [PATCH 05/10] libnet/portmapper: drop unused NewWithPortAllocator This function is only called by New, and it takes the singleton PortAllocator exposed by the portallocator package. Remove this function and instantiate the PortMapper directly from New constructor. Signed-off-by: Albin Kerouanton --- daemon/libnetwork/portmapper/mapper_windows.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portmapper/mapper_windows.go index d168286070..6808335b5d 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portmapper/mapper_windows.go @@ -43,14 +43,9 @@ type PortMapper struct { // New returns a new instance of PortMapper func New() *PortMapper { - return NewWithPortAllocator(portallocator.Get()) -} - -// NewWithPortAllocator returns a new instance of PortMapper which will use the specified PortAllocator -func NewWithPortAllocator(allocator *portallocator.PortAllocator) *PortMapper { return &PortMapper{ currentMappings: make(map[string]*mapping), - allocator: allocator, + allocator: portallocator.Get(), } } From 90f31c6c2722b21070950f39d861804f0cc1f756 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Tue, 1 Jul 2025 01:21:06 +0200 Subject: [PATCH 06/10] libnet/portmapper: remove dead field bridgeName This field is not referenced by anything. Drop it. Signed-off-by: Albin Kerouanton --- daemon/libnetwork/portmapper/mapper_windows.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portmapper/mapper_windows.go index 6808335b5d..fe573ca264 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portmapper/mapper_windows.go @@ -32,8 +32,6 @@ var ( // PortMapper manages the network address translation type PortMapper struct { - bridgeName string - // udp:ip:port currentMappings map[string]*mapping lock sync.Mutex From af677b61a52439cbd027722f65b30ab1f6cff1b8 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Mon, 30 Jun 2025 17:22:33 +0200 Subject: [PATCH 07/10] libnet/portmapper: clean up windows port mapper The windows port mapper is needlessly complex while its job is pretty straightforward: reserve a port through the port allocator, and start a dummy proxy to allocate it from the OS. The biggest source of complexity is the use of the `net.Addr` interface to pass the host IP, port and proto. `MapRange` now has a proto arg, and returns the allocated port. `MapRange` is also instantiating a `mapping` struct whose fields are all unused, except for its `stopUserlandProxy`. Instead, store `stopProxy` callbacks directly into the `PortMapper`. Signed-off-by: Albin Kerouanton --- .../windows/overlay/ov_endpoint_windows.go | 2 +- .../drivers/windows/port_mapping.go | 59 +---- daemon/libnetwork/drivers/windows/windows.go | 7 +- .../libnetwork/portmapper/mapper_windows.go | 250 +++++++----------- daemon/libnetwork/portmapper/proxy_windows.go | 85 ------ daemon/libnetwork/types/types.go | 14 - 6 files changed, 116 insertions(+), 301 deletions(-) delete mode 100644 daemon/libnetwork/portmapper/proxy_windows.go diff --git a/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go b/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go index 718c9d4edb..9973ae1bde 100644 --- a/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go +++ b/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go @@ -159,7 +159,7 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri } ep.portMapping = epConnectivity.PortBindings - ep.portMapping, err = windows.AllocatePorts(n.portMapper, ep.portMapping, ep.addr.IP) + ep.portMapping, err = windows.AllocatePorts(n.portMapper, ep.portMapping) if err != nil { return err } diff --git a/daemon/libnetwork/drivers/windows/port_mapping.go b/daemon/libnetwork/drivers/windows/port_mapping.go index b534add2b6..23ef4dd1d8 100644 --- a/daemon/libnetwork/drivers/windows/port_mapping.go +++ b/daemon/libnetwork/drivers/windows/port_mapping.go @@ -10,7 +10,6 @@ import ( "net" "github.com/containerd/log" - "github.com/ishidawataru/sctp" "github.com/moby/moby/v2/daemon/libnetwork/portmapper" "github.com/moby/moby/v2/daemon/libnetwork/types" ) @@ -18,11 +17,11 @@ import ( const maxAllocatePortAttempts = 10 // AllocatePorts allocates ports specified in bindings from the portMapper -func AllocatePorts(portMapper *portmapper.PortMapper, bindings []types.PortBinding, containerIP net.IP) ([]types.PortBinding, error) { +func AllocatePorts(portMapper *portmapper.PortMapper, bindings []types.PortBinding) ([]types.PortBinding, error) { bs := make([]types.PortBinding, 0, len(bindings)) for _, c := range bindings { - b := c.Copy() - if err := allocatePort(portMapper, &b, containerIP); err != nil { + b, err := allocatePort(portMapper, c) + if err != nil { // On allocation failure, release previously allocated ports. On cleanup error, just log a warning message if cuErr := ReleasePorts(portMapper, bs); cuErr != nil { log.G(context.TODO()).Warnf("Upon allocation failure for %v, failed to clear previously allocated port bindings: %v", b, cuErr) @@ -34,35 +33,24 @@ func AllocatePorts(portMapper *portmapper.PortMapper, bindings []types.PortBindi return bs, nil } -func allocatePort(portMapper *portmapper.PortMapper, bnd *types.PortBinding, containerIP net.IP) error { - var ( - host net.Addr - err error - ) - +func allocatePort(portMapper *portmapper.PortMapper, bnd types.PortBinding) (types.PortBinding, error) { // Windows does not support a host ip for port bindings (this is validated in ConvertPortBindings()). // If the HostIP is nil, force it to be 0.0.0.0 for use as the key in portMapper. if bnd.HostIP == nil { bnd.HostIP = net.IPv4zero } - // Store the container interface address in the operational binding - bnd.IP = containerIP - // Adjust HostPortEnd if this is not a range. if bnd.HostPortEnd == 0 { bnd.HostPortEnd = bnd.HostPort } - // Construct the container side transport address - container, err := bnd.ContainerAddr() - if err != nil { - return err - } - // Try up to maxAllocatePortAttempts times to get a port that's not already allocated. + var allocatedPort int + var err error for i := 0; i < maxAllocatePortAttempts; i++ { - if host, err = portMapper.MapRange(container, bnd.HostIP, int(bnd.HostPort), int(bnd.HostPortEnd)); err == nil { + allocatedPort, err = portMapper.MapRange(bnd.HostIP, bnd.Proto, int(bnd.HostPort), int(bnd.HostPortEnd)) + if err == nil { break } // There is no point in immediately retrying to map an explicitly chosen port. @@ -73,27 +61,13 @@ func allocatePort(portMapper *portmapper.PortMapper, bnd *types.PortBinding, con log.G(context.TODO()).Warnf("Failed to allocate and map port: %s, retry: %d", err, i+1) } if err != nil { - return err + return types.PortBinding{}, err } - // Save the host port (regardless it was or not specified in the binding) - switch netAddr := host.(type) { - case *net.TCPAddr: - bnd.HostPort = uint16(host.(*net.TCPAddr).Port) - break - case *net.UDPAddr: - bnd.HostPort = uint16(host.(*net.UDPAddr).Port) - break - case *sctp.SCTPAddr: - bnd.HostPort = uint16(host.(*sctp.SCTPAddr).Port) - break - default: - // For completeness - return fmt.Errorf("unsupported address type: %T", netAddr) - } - // Windows does not support host port ranges. - bnd.HostPortEnd = bnd.HostPort - return nil + bnd.HostPort = uint16(allocatedPort) + bnd.HostPortEnd = uint16(allocatedPort) + + return bnd, nil } // ReleasePorts releases ports specified in bindings from the portMapper @@ -114,10 +88,5 @@ func ReleasePorts(portMapper *portmapper.PortMapper, bindings []types.PortBindin } func releasePort(portMapper *portmapper.PortMapper, bnd types.PortBinding) error { - // Construct the host side transport address - host, err := bnd.HostAddr() - if err != nil { - return err - } - return portMapper.Unmap(host) + return portMapper.Unmap(bnd.HostIP, bnd.Proto, int(bnd.HostPort)) } diff --git a/daemon/libnetwork/drivers/windows/windows.go b/daemon/libnetwork/drivers/windows/windows.go index da7292b2a7..8e373ae599 100644 --- a/daemon/libnetwork/drivers/windows/windows.go +++ b/daemon/libnetwork/drivers/windows/windows.go @@ -701,12 +701,7 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri portMapping := epConnectivity.PortBindings if n.config.Type == "l2bridge" || n.config.Type == "l2tunnel" { - ip := net.IPv4(0, 0, 0, 0) - if ifInfo.Address() != nil { - ip = ifInfo.Address().IP - } - - portMapping, err = AllocatePorts(n.portMapper, portMapping, ip) + portMapping, err = AllocatePorts(n.portMapper, portMapping) if err != nil { return err } diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portmapper/mapper_windows.go index fe573ca264..6487da2300 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portmapper/mapper_windows.go @@ -4,37 +4,31 @@ import ( "context" "errors" "fmt" + "io" "net" + "net/netip" "sync" - "github.com/moby/moby/v2/daemon/libnetwork/portallocator" "github.com/containerd/log" "github.com/ishidawataru/sctp" + "github.com/moby/moby/v2/daemon/libnetwork/portallocator" + "github.com/moby/moby/v2/daemon/libnetwork/types" ) -type mapping struct { - proto string - stopUserlandProxy func() error - host net.Addr - container net.Addr -} - var ( - // ErrUnknownBackendAddressType refers to an unknown container or unsupported address type - ErrUnknownBackendAddressType = errors.New("unknown container address type not supported") // ErrPortMappedForIP refers to a port already mapped to an ip address ErrPortMappedForIP = errors.New("port is already mapped to ip") // ErrPortNotMapped refers to an unmapped port ErrPortNotMapped = errors.New("port is not mapped") - // ErrSCTPAddrNoIP refers to a SCTP address without IP address. - ErrSCTPAddrNoIP = errors.New("sctp address does not contain any IP address") ) // PortMapper manages the network address translation type PortMapper struct { - // udp:ip:port - currentMappings map[string]*mapping - lock sync.Mutex + // osListeners stores listening sockets used by active port mappings + // to reserve ports from the OS. Outer map is keyed by protocol, and inner + // map is keyed by host address and port. + osListeners map[types.Protocol]map[netip.AddrPort]io.Closer + lock sync.Mutex allocator *portallocator.PortAllocator } @@ -42,163 +36,119 @@ type PortMapper struct { // New returns a new instance of PortMapper func New() *PortMapper { return &PortMapper{ - currentMappings: make(map[string]*mapping), - allocator: portallocator.Get(), + osListeners: make(map[types.Protocol]map[netip.AddrPort]io.Closer), + allocator: portallocator.Get(), } } // MapRange maps the specified container transport address to the host's network address and transport port range -func (pm *PortMapper) MapRange(container net.Addr, hostIP net.IP, hostPortStart, hostPortEnd int) (host net.Addr, retErr error) { +func (pm *PortMapper) MapRange(hostIP net.IP, proto types.Protocol, hostPortStart, hostPortEnd int) (_ int, retErr error) { pm.lock.Lock() defer pm.lock.Unlock() - var ( - m *mapping - proto string - allocatedHostPort int - ) - - switch container.(type) { - case *net.TCPAddr: - proto = "tcp" - - var err error - allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) - if err != nil { - return nil, err - } - defer func() { - if retErr != nil { - pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) - } - }() - - m = &mapping{ - proto: proto, - host: &net.TCPAddr{IP: hostIP, Port: allocatedHostPort}, - container: container, - } - case *net.UDPAddr: - proto = "udp" - - var err error - allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) - if err != nil { - return nil, err - } - defer func() { - if retErr != nil { - pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) - } - }() - - m = &mapping{ - proto: proto, - host: &net.UDPAddr{IP: hostIP, Port: allocatedHostPort}, - container: container, - } - case *sctp.SCTPAddr: - proto = "sctp" - - var err error - allocatedHostPort, err = pm.allocator.RequestPortInRange(hostIP, proto, hostPortStart, hostPortEnd) - if err != nil { - return nil, err - } - defer func() { - if retErr != nil { - pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort) - } - }() - - m = &mapping{ - proto: proto, - host: &sctp.SCTPAddr{IPAddrs: []net.IPAddr{{IP: hostIP}}, Port: allocatedHostPort}, - container: container, - } - default: - return nil, ErrUnknownBackendAddressType - } - - key := getKey(m.host) - if _, exists := pm.currentMappings[key]; exists { - return nil, ErrPortMappedForIP - } - - var err error - m.stopUserlandProxy, err = newDummyProxy(m.proto, hostIP, allocatedHostPort) + allocatedHostPort, err := pm.allocator.RequestPortInRange(hostIP, proto.String(), hostPortStart, hostPortEnd) if err != nil { - // FIXME(thaJeztah): both stopping the proxy and deleting iptables rules can produce an error, and both are not currently handled. - m.stopUserlandProxy() - return nil, err + return 0, err + } + defer func() { + if retErr != nil { + pm.allocator.ReleasePort(hostIP, proto.String(), allocatedHostPort) + } + }() + + if pm.osListeners[proto] == nil { + pm.osListeners[proto] = make(map[netip.AddrPort]io.Closer) } - pm.currentMappings[key] = m - return m.host, nil + addr, ok := netip.AddrFromSlice(hostIP) + if !ok { + return 0, fmt.Errorf("invalid HostIP: %s", hostIP) + } + + hAddrPort := netip.AddrPortFrom(addr, uint16(allocatedHostPort)) + if _, exists := pm.osListeners[proto][hAddrPort]; exists { + return 0, ErrPortMappedForIP + } + + var osListener io.Closer + osListener, err = allocateHostPort(proto.String(), hostIP, allocatedHostPort) + if err != nil { + if osListener != nil { + if err := osListener.Close(); err != nil { + // Prior to v29.0, this error was never checked. So, instead of + // returning an error, log it and proceed. + log.G(context.TODO()).Infof("failed to stop dummy proxy for %s/%s: %v", hostIP, proto, err) + } + } + return 0, err + } + + pm.osListeners[proto][hAddrPort] = osListener + return allocatedHostPort, nil +} + +// allocateHostPort allocates a host port by binding to the specified host IP and port. +func allocateHostPort(proto string, hostIP net.IP, hostPort int) (io.Closer, error) { + // detect version of hostIP to bind only to correct version + protoVer := proto + "4" + if hostIP.To4() == nil { + protoVer = proto + "6" + } + + switch proto { + case "tcp": + l, err := net.ListenTCP(protoVer, &net.TCPAddr{IP: hostIP, Port: hostPort}) + if err != nil { + return nil, err + } + return l, nil + case "udp": + l, err := net.ListenUDP(protoVer, &net.UDPAddr{IP: hostIP, Port: hostPort}) + if err != nil { + return nil, err + } + return l, nil + case "sctp": + l, err := sctp.ListenSCTP(protoVer, &sctp.SCTPAddr{IPAddrs: []net.IPAddr{{IP: hostIP}}, Port: hostPort}) + if err != nil { + return nil, err + } + return l, nil + default: + return nil, fmt.Errorf("protocol %s not supported", proto) + } } // Unmap removes stored mapping for the specified host transport address -func (pm *PortMapper) Unmap(host net.Addr) error { +func (pm *PortMapper) Unmap(hostIP net.IP, proto types.Protocol, hostPort int) error { pm.lock.Lock() defer pm.lock.Unlock() - key := getKey(host) - data, exists := pm.currentMappings[key] + addr, ok := netip.AddrFromSlice(hostIP) + if !ok { + return fmt.Errorf("invalid HostIP: %s", hostIP) + } + + if pm.osListeners[proto] == nil { + return ErrPortNotMapped + } + + hAddrPort := netip.AddrPortFrom(addr, uint16(hostPort)) + osListener, exists := pm.osListeners[proto][hAddrPort] if !exists { return ErrPortNotMapped } - if data.stopUserlandProxy != nil { - data.stopUserlandProxy() - } - - delete(pm.currentMappings, key) - - switch a := host.(type) { - case *net.TCPAddr: - pm.allocator.ReleasePort(a.IP, "tcp", a.Port) - case *net.UDPAddr: - pm.allocator.ReleasePort(a.IP, "udp", a.Port) - case *sctp.SCTPAddr: - if len(a.IPAddrs) == 0 { - return ErrSCTPAddrNoIP + if osListener != nil { + if err := osListener.Close(); err != nil { + // Prior to v29.0, this error was never checked. So, instead of + // returning an error, log it and proceed. + log.G(context.TODO()).Infof("failed to stop dummy proxy for %s/%s: %v", hostIP, proto, err) } - pm.allocator.ReleasePort(a.IPAddrs[0].IP, "sctp", a.Port) - default: - return ErrUnknownBackendAddressType } + delete(pm.osListeners[proto], hAddrPort) + + pm.allocator.ReleasePort(hostIP, proto.String(), int(hostPort)) return nil } - -func getKey(a net.Addr) string { - switch t := a.(type) { - case *net.TCPAddr: - return fmt.Sprintf("%s:%d/%s", t.IP.String(), t.Port, "tcp") - case *net.UDPAddr: - return fmt.Sprintf("%s:%d/%s", t.IP.String(), t.Port, "udp") - case *sctp.SCTPAddr: - if len(t.IPAddrs) == 0 { - log.G(context.TODO()).Error(ErrSCTPAddrNoIP) - return "" - } - return fmt.Sprintf("%s:%d/%s", t.IPAddrs[0].IP.String(), t.Port, "sctp") - } - return "" -} - -func getIPAndPort(a net.Addr) (net.IP, int) { - switch t := a.(type) { - case *net.TCPAddr: - return t.IP, t.Port - case *net.UDPAddr: - return t.IP, t.Port - case *sctp.SCTPAddr: - if len(t.IPAddrs) == 0 { - log.G(context.TODO()).Error(ErrSCTPAddrNoIP) - return nil, 0 - } - return t.IPAddrs[0].IP, t.Port - } - return nil, 0 -} diff --git a/daemon/libnetwork/portmapper/proxy_windows.go b/daemon/libnetwork/portmapper/proxy_windows.go deleted file mode 100644 index 6d868b4027..0000000000 --- a/daemon/libnetwork/portmapper/proxy_windows.go +++ /dev/null @@ -1,85 +0,0 @@ -package portmapper - -import ( - "fmt" - "io" - "net" - - "github.com/ishidawataru/sctp" -) - -// ipVersion refers to IP version - v4 or v6 -type ipVersion string - -const ( - // IPv4 is version 4 - ipv4 ipVersion = "4" - // IPv4 is version 6 - ipv6 ipVersion = "6" -) - -// dummyProxy just listen on some port, it is needed to prevent accidental -// port allocations on bound port, because without userland proxy we using -// iptables rules and not net.Listen -type dummyProxy struct { - listener io.Closer - addr net.Addr - ipVersion ipVersion -} - -func newDummyProxy(proto string, hostIP net.IP, hostPort int) (stop func() error, retErr error) { - // detect version of hostIP to bind only to correct version - version := ipv4 - if hostIP.To4() == nil { - version = ipv6 - } - var addr net.Addr - switch proto { - case "tcp": - addr = &net.TCPAddr{IP: hostIP, Port: hostPort} - case "udp": - addr = &net.UDPAddr{IP: hostIP, Port: hostPort} - case "sctp": - addr = &sctp.SCTPAddr{IPAddrs: []net.IPAddr{{IP: hostIP}}, Port: hostPort} - default: - return nil, fmt.Errorf("Unknown addr type: %s", proto) - } - p := &dummyProxy{addr: addr, ipVersion: version} - if err := p.start(); err != nil { - return nil, err - } - return p.stop, nil -} - -func (p *dummyProxy) start() error { - switch addr := p.addr.(type) { - case *net.TCPAddr: - l, err := net.ListenTCP("tcp"+string(p.ipVersion), addr) - if err != nil { - return err - } - p.listener = l - case *net.UDPAddr: - l, err := net.ListenUDP("udp"+string(p.ipVersion), addr) - if err != nil { - return err - } - p.listener = l - case *sctp.SCTPAddr: - l, err := sctp.ListenSCTP("sctp"+string(p.ipVersion), addr) - if err != nil { - return err - } - p.listener = l - default: - return fmt.Errorf("Unknown addr type: %T", p.addr) - } - return nil -} - -func (p *dummyProxy) stop() error { - if p.listener != nil { - return p.listener.Close() - } - return nil -} diff --git a/daemon/libnetwork/types/types.go b/daemon/libnetwork/types/types.go index 9d83537461..9a37f736d6 100644 --- a/daemon/libnetwork/types/types.go +++ b/daemon/libnetwork/types/types.go @@ -72,20 +72,6 @@ func (p PortBinding) HostAddr() (net.Addr, error) { } } -// ContainerAddr returns the container side transport address -func (p PortBinding) ContainerAddr() (net.Addr, error) { - switch p.Proto { - case UDP: - return &net.UDPAddr{IP: p.IP, Port: int(p.Port)}, nil - case TCP: - return &net.TCPAddr{IP: p.IP, Port: int(p.Port)}, nil - case SCTP: - return &sctp.SCTPAddr{IPAddrs: []net.IPAddr{{IP: p.IP}}, Port: int(p.Port)}, nil - default: - return nil, fmt.Errorf("invalid transport protocol: %s", p.Proto.String()) - } -} - // Copy returns a deep copy of the PortBinding. func (p PortBinding) Copy() PortBinding { return PortBinding{ From 9efc1cc264c28cbf5df71cd5b4ed341ebd51f089 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Tue, 1 Jul 2025 02:04:57 +0200 Subject: [PATCH 08/10] libnet/portmapper: rename, move PortMapper to portallocator The only viable way to allocate a port is to bind and listen to it. So, the windows PortMapper was really a PortAllocator in disguise. Rename it to OSAllocator and move it to the portallocator package. Signed-off-by: Albin Kerouanton --- .../windows/overlay/ov_endpoint_windows.go | 6 +-- .../windows/overlay/ov_network_windows.go | 14 ++--- .../drivers/windows/port_mapping.go | 26 ++++----- daemon/libnetwork/drivers/windows/windows.go | 30 +++++------ .../osallocator_windows.go} | 53 +++++++++---------- 5 files changed, 64 insertions(+), 65 deletions(-) rename daemon/libnetwork/{portmapper/mapper_windows.go => portallocator/osallocator_windows.go} (69%) diff --git a/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go b/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go index 9973ae1bde..14d5520fa5 100644 --- a/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go +++ b/daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go @@ -159,14 +159,14 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri } ep.portMapping = epConnectivity.PortBindings - ep.portMapping, err = windows.AllocatePorts(n.portMapper, ep.portMapping) + ep.portMapping, err = windows.AllocatePorts(n.pa, ep.portMapping) if err != nil { return err } defer func() { if err != nil { - windows.ReleasePorts(n.portMapper, ep.portMapping) + windows.ReleasePorts(n.pa, ep.portMapping) } }() @@ -227,7 +227,7 @@ func (d *driver) DeleteEndpoint(nid, eid string) error { return fmt.Errorf("endpoint id %q not found", eid) } - windows.ReleasePorts(n.portMapper, ep.portMapping) + windows.ReleasePorts(n.pa, ep.portMapping) n.deleteEndpoint(eid) diff --git a/daemon/libnetwork/drivers/windows/overlay/ov_network_windows.go b/daemon/libnetwork/drivers/windows/overlay/ov_network_windows.go index f0ac35732b..649b475e98 100644 --- a/daemon/libnetwork/drivers/windows/overlay/ov_network_windows.go +++ b/daemon/libnetwork/drivers/windows/overlay/ov_network_windows.go @@ -15,7 +15,7 @@ import ( "github.com/moby/moby/v2/daemon/libnetwork/driverapi" "github.com/moby/moby/v2/daemon/libnetwork/drivers/overlay" "github.com/moby/moby/v2/daemon/libnetwork/netlabel" - "github.com/moby/moby/v2/daemon/libnetwork/portmapper" + "github.com/moby/moby/v2/daemon/libnetwork/portallocator" "github.com/moby/moby/v2/daemon/libnetwork/types" ) @@ -50,7 +50,7 @@ type network struct { initErr error subnets []*subnet secure bool - portMapper *portmapper.PortMapper + pa *portallocator.OSAllocator sync.Mutex } @@ -86,11 +86,11 @@ func (d *driver) CreateNetwork(ctx context.Context, id string, option map[string } n := &network{ - id: id, - driver: d, - endpoints: endpointTable{}, - subnets: []*subnet{}, - portMapper: portmapper.New(), + id: id, + driver: d, + endpoints: endpointTable{}, + subnets: []*subnet{}, + pa: portallocator.New(), } genData, ok := option[netlabel.GenericData].(map[string]string) diff --git a/daemon/libnetwork/drivers/windows/port_mapping.go b/daemon/libnetwork/drivers/windows/port_mapping.go index 23ef4dd1d8..00cebd7bd4 100644 --- a/daemon/libnetwork/drivers/windows/port_mapping.go +++ b/daemon/libnetwork/drivers/windows/port_mapping.go @@ -10,20 +10,20 @@ import ( "net" "github.com/containerd/log" - "github.com/moby/moby/v2/daemon/libnetwork/portmapper" + "github.com/moby/moby/v2/daemon/libnetwork/portallocator" "github.com/moby/moby/v2/daemon/libnetwork/types" ) const maxAllocatePortAttempts = 10 -// AllocatePorts allocates ports specified in bindings from the portMapper -func AllocatePorts(portMapper *portmapper.PortMapper, bindings []types.PortBinding) ([]types.PortBinding, error) { +// AllocatePorts allocates ports specified in bindings from the port allocator. +func AllocatePorts(pa *portallocator.OSAllocator, bindings []types.PortBinding) ([]types.PortBinding, error) { bs := make([]types.PortBinding, 0, len(bindings)) for _, c := range bindings { - b, err := allocatePort(portMapper, c) + b, err := allocatePort(pa, c) if err != nil { // On allocation failure, release previously allocated ports. On cleanup error, just log a warning message - if cuErr := ReleasePorts(portMapper, bs); cuErr != nil { + if cuErr := ReleasePorts(pa, bs); cuErr != nil { log.G(context.TODO()).Warnf("Upon allocation failure for %v, failed to clear previously allocated port bindings: %v", b, cuErr) } return nil, err @@ -33,9 +33,9 @@ func AllocatePorts(portMapper *portmapper.PortMapper, bindings []types.PortBindi return bs, nil } -func allocatePort(portMapper *portmapper.PortMapper, bnd types.PortBinding) (types.PortBinding, error) { +func allocatePort(pa *portallocator.OSAllocator, bnd types.PortBinding) (types.PortBinding, error) { // Windows does not support a host ip for port bindings (this is validated in ConvertPortBindings()). - // If the HostIP is nil, force it to be 0.0.0.0 for use as the key in portMapper. + // If the HostIP is nil, force it to be 0.0.0.0 for use as the key in the port allocator. if bnd.HostIP == nil { bnd.HostIP = net.IPv4zero } @@ -49,7 +49,7 @@ func allocatePort(portMapper *portmapper.PortMapper, bnd types.PortBinding) (typ var allocatedPort int var err error for i := 0; i < maxAllocatePortAttempts; i++ { - allocatedPort, err = portMapper.MapRange(bnd.HostIP, bnd.Proto, int(bnd.HostPort), int(bnd.HostPortEnd)) + allocatedPort, err = pa.AllocateHostPort(bnd.HostIP, bnd.Proto, int(bnd.HostPort), int(bnd.HostPortEnd)) if err == nil { break } @@ -70,13 +70,13 @@ func allocatePort(portMapper *portmapper.PortMapper, bnd types.PortBinding) (typ return bnd, nil } -// ReleasePorts releases ports specified in bindings from the portMapper -func ReleasePorts(portMapper *portmapper.PortMapper, bindings []types.PortBinding) error { +// ReleasePorts releases ports specified in bindings from the portAlloc +func ReleasePorts(pa *portallocator.OSAllocator, bindings []types.PortBinding) error { var errorBuf bytes.Buffer // Attempt to release all port bindings, do not stop on failure for _, m := range bindings { - if err := releasePort(portMapper, m); err != nil { + if err := releasePort(pa, m); err != nil { errorBuf.WriteString(fmt.Sprintf("\ncould not release %v because of %v", m, err)) } } @@ -87,6 +87,6 @@ func ReleasePorts(portMapper *portmapper.PortMapper, bindings []types.PortBindin return nil } -func releasePort(portMapper *portmapper.PortMapper, bnd types.PortBinding) error { - return portMapper.Unmap(bnd.HostIP, bnd.Proto, int(bnd.HostPort)) +func releasePort(pa *portallocator.OSAllocator, bnd types.PortBinding) error { + return pa.Deallocate(bnd.HostIP, bnd.Proto, int(bnd.HostPort)) } diff --git a/daemon/libnetwork/drivers/windows/windows.go b/daemon/libnetwork/drivers/windows/windows.go index 8e373ae599..c61a93a847 100644 --- a/daemon/libnetwork/drivers/windows/windows.go +++ b/daemon/libnetwork/drivers/windows/windows.go @@ -27,7 +27,7 @@ import ( "github.com/moby/moby/v2/daemon/libnetwork/datastore" "github.com/moby/moby/v2/daemon/libnetwork/driverapi" "github.com/moby/moby/v2/daemon/libnetwork/netlabel" - "github.com/moby/moby/v2/daemon/libnetwork/portmapper" + "github.com/moby/moby/v2/daemon/libnetwork/portallocator" "github.com/moby/moby/v2/daemon/libnetwork/scope" "github.com/moby/moby/v2/daemon/libnetwork/types" "go.opentelemetry.io/otel" @@ -94,12 +94,12 @@ type hnsEndpoint struct { } type hnsNetwork struct { - id string - created bool - config *networkConfiguration - endpoints map[string]*hnsEndpoint // key: endpoint id - driver *driver // The network's driver - portMapper *portmapper.PortMapper + id string + created bool + config *networkConfiguration + endpoints map[string]*hnsEndpoint // key: endpoint id + driver *driver // The network's driver + pa *portallocator.OSAllocator sync.Mutex } @@ -308,11 +308,11 @@ func (ncfg *networkConfiguration) processIPAM(id string, ipamV4Data, ipamV6Data func (d *driver) createNetwork(config *networkConfiguration) *hnsNetwork { network := &hnsNetwork{ - id: config.ID, - endpoints: make(map[string]*hnsEndpoint), - config: config, - driver: d, - portMapper: portmapper.New(), + id: config.ID, + endpoints: make(map[string]*hnsEndpoint), + config: config, + driver: d, + pa: portallocator.New(), } d.Lock() @@ -701,14 +701,14 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri portMapping := epConnectivity.PortBindings if n.config.Type == "l2bridge" || n.config.Type == "l2tunnel" { - portMapping, err = AllocatePorts(n.portMapper, portMapping) + portMapping, err = AllocatePorts(n.pa, portMapping) if err != nil { return err } defer func() { if err != nil { - ReleasePorts(n.portMapper, portMapping) + ReleasePorts(n.pa, portMapping) } }() } @@ -823,7 +823,7 @@ func (d *driver) DeleteEndpoint(nid, eid string) error { } if n.config.Type == "l2bridge" || n.config.Type == "l2tunnel" { - ReleasePorts(n.portMapper, ep.portMapping) + ReleasePorts(n.pa, ep.portMapping) } n.Lock() diff --git a/daemon/libnetwork/portmapper/mapper_windows.go b/daemon/libnetwork/portallocator/osallocator_windows.go similarity index 69% rename from daemon/libnetwork/portmapper/mapper_windows.go rename to daemon/libnetwork/portallocator/osallocator_windows.go index 6487da2300..df99f7b429 100644 --- a/daemon/libnetwork/portmapper/mapper_windows.go +++ b/daemon/libnetwork/portallocator/osallocator_windows.go @@ -1,4 +1,4 @@ -package portmapper +package portallocator import ( "context" @@ -11,7 +11,6 @@ import ( "github.com/containerd/log" "github.com/ishidawataru/sctp" - "github.com/moby/moby/v2/daemon/libnetwork/portallocator" "github.com/moby/moby/v2/daemon/libnetwork/types" ) @@ -22,42 +21,42 @@ var ( ErrPortNotMapped = errors.New("port is not mapped") ) -// PortMapper manages the network address translation -type PortMapper struct { +// OSAllocator allocates ports from the OS by creating listening sockets. +type OSAllocator struct { // osListeners stores listening sockets used by active port mappings // to reserve ports from the OS. Outer map is keyed by protocol, and inner // map is keyed by host address and port. osListeners map[types.Protocol]map[netip.AddrPort]io.Closer lock sync.Mutex - allocator *portallocator.PortAllocator + allocator *PortAllocator } -// New returns a new instance of PortMapper -func New() *PortMapper { - return &PortMapper{ +// New returns a new instance of OSAllocator +func New() *OSAllocator { + return &OSAllocator{ osListeners: make(map[types.Protocol]map[netip.AddrPort]io.Closer), - allocator: portallocator.Get(), + allocator: Get(), } } -// MapRange maps the specified container transport address to the host's network address and transport port range -func (pm *PortMapper) MapRange(hostIP net.IP, proto types.Protocol, hostPortStart, hostPortEnd int) (_ int, retErr error) { - pm.lock.Lock() - defer pm.lock.Unlock() +// AllocateHostPort allocates a port from the OS (by creating a listening socket). +func (pa *OSAllocator) AllocateHostPort(hostIP net.IP, proto types.Protocol, hostPortStart, hostPortEnd int) (_ int, retErr error) { + pa.lock.Lock() + defer pa.lock.Unlock() - allocatedHostPort, err := pm.allocator.RequestPortInRange(hostIP, proto.String(), hostPortStart, hostPortEnd) + allocatedHostPort, err := pa.allocator.RequestPortInRange(hostIP, proto.String(), hostPortStart, hostPortEnd) if err != nil { return 0, err } defer func() { if retErr != nil { - pm.allocator.ReleasePort(hostIP, proto.String(), allocatedHostPort) + pa.allocator.ReleasePort(hostIP, proto.String(), allocatedHostPort) } }() - if pm.osListeners[proto] == nil { - pm.osListeners[proto] = make(map[netip.AddrPort]io.Closer) + if pa.osListeners[proto] == nil { + pa.osListeners[proto] = make(map[netip.AddrPort]io.Closer) } addr, ok := netip.AddrFromSlice(hostIP) @@ -66,7 +65,7 @@ func (pm *PortMapper) MapRange(hostIP net.IP, proto types.Protocol, hostPortStar } hAddrPort := netip.AddrPortFrom(addr, uint16(allocatedHostPort)) - if _, exists := pm.osListeners[proto][hAddrPort]; exists { + if _, exists := pa.osListeners[proto][hAddrPort]; exists { return 0, ErrPortMappedForIP } @@ -83,7 +82,7 @@ func (pm *PortMapper) MapRange(hostIP net.IP, proto types.Protocol, hostPortStar return 0, err } - pm.osListeners[proto][hAddrPort] = osListener + pa.osListeners[proto][hAddrPort] = osListener return allocatedHostPort, nil } @@ -119,22 +118,22 @@ func allocateHostPort(proto string, hostIP net.IP, hostPort int) (io.Closer, err } } -// Unmap removes stored mapping for the specified host transport address -func (pm *PortMapper) Unmap(hostIP net.IP, proto types.Protocol, hostPort int) error { - pm.lock.Lock() - defer pm.lock.Unlock() +// Deallocate removes stored mapping for the specified host transport address +func (pa *OSAllocator) Deallocate(hostIP net.IP, proto types.Protocol, hostPort int) error { + pa.lock.Lock() + defer pa.lock.Unlock() addr, ok := netip.AddrFromSlice(hostIP) if !ok { return fmt.Errorf("invalid HostIP: %s", hostIP) } - if pm.osListeners[proto] == nil { + if pa.osListeners[proto] == nil { return ErrPortNotMapped } hAddrPort := netip.AddrPortFrom(addr, uint16(hostPort)) - osListener, exists := pm.osListeners[proto][hAddrPort] + osListener, exists := pa.osListeners[proto][hAddrPort] if !exists { return ErrPortNotMapped } @@ -147,8 +146,8 @@ func (pm *PortMapper) Unmap(hostIP net.IP, proto types.Protocol, hostPort int) e } } - delete(pm.osListeners[proto], hAddrPort) + delete(pa.osListeners[proto], hAddrPort) - pm.allocator.ReleasePort(hostIP, proto.String(), int(hostPort)) + pa.allocator.ReleasePort(hostIP, proto.String(), int(hostPort)) return nil } From fc8641135309cb10faa9d6a47785d27717f010b4 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Tue, 1 Jul 2025 02:24:16 +0200 Subject: [PATCH 09/10] libnet/d/windows: inline releasePort releasePort is a one-liner and is called only in one place. Inline it. Signed-off-by: Albin Kerouanton --- daemon/libnetwork/drivers/windows/port_mapping.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/daemon/libnetwork/drivers/windows/port_mapping.go b/daemon/libnetwork/drivers/windows/port_mapping.go index 00cebd7bd4..7b5ea62f53 100644 --- a/daemon/libnetwork/drivers/windows/port_mapping.go +++ b/daemon/libnetwork/drivers/windows/port_mapping.go @@ -76,7 +76,7 @@ func ReleasePorts(pa *portallocator.OSAllocator, bindings []types.PortBinding) e // Attempt to release all port bindings, do not stop on failure for _, m := range bindings { - if err := releasePort(pa, m); err != nil { + if err := pa.Deallocate(m.HostIP, m.Proto, int(m.HostPort)); err != nil { errorBuf.WriteString(fmt.Sprintf("\ncould not release %v because of %v", m, err)) } } @@ -86,7 +86,3 @@ func ReleasePorts(pa *portallocator.OSAllocator, bindings []types.PortBinding) e } return nil } - -func releasePort(pa *portallocator.OSAllocator, bnd types.PortBinding) error { - return pa.Deallocate(bnd.HostIP, bnd.Proto, int(bnd.HostPort)) -} From 2f1015482ff8532befdde2cd2271f352ed3e7fcb Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Tue, 1 Jul 2025 02:27:46 +0200 Subject: [PATCH 10/10] libnet/d/windows: ReleasePorts: use errors.Join Signed-off-by: Albin Kerouanton --- daemon/libnetwork/drivers/windows/port_mapping.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/daemon/libnetwork/drivers/windows/port_mapping.go b/daemon/libnetwork/drivers/windows/port_mapping.go index 7b5ea62f53..4849587598 100644 --- a/daemon/libnetwork/drivers/windows/port_mapping.go +++ b/daemon/libnetwork/drivers/windows/port_mapping.go @@ -3,7 +3,6 @@ package windows import ( - "bytes" "context" "errors" "fmt" @@ -72,17 +71,14 @@ func allocatePort(pa *portallocator.OSAllocator, bnd types.PortBinding) (types.P // ReleasePorts releases ports specified in bindings from the portAlloc func ReleasePorts(pa *portallocator.OSAllocator, bindings []types.PortBinding) error { - var errorBuf bytes.Buffer + var errs []error // Attempt to release all port bindings, do not stop on failure for _, m := range bindings { if err := pa.Deallocate(m.HostIP, m.Proto, int(m.HostPort)); err != nil { - errorBuf.WriteString(fmt.Sprintf("\ncould not release %v because of %v", m, err)) + errs = append(errs, fmt.Errorf("could not release %v because of %v", m, err)) } } - if errorBuf.Len() != 0 { - return errors.New(errorBuf.String()) - } - return nil + return errors.Join(errs...) }