From 2a2176f3177015a376eae1be24f5020fa601fd22 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 11:12:35 +0200 Subject: [PATCH 1/8] portallocator: RequestPortInRange: fix doc-link in godoc The doc-link was not formatted correctly and didn't work. While updating also slightly touch-up the description to explain "defaultIP". Signed-off-by: Sebastiaan van Stijn --- libnetwork/portallocator/portallocator.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index 0df927748a459..2dacbf08a018d 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -109,9 +109,9 @@ func (p *PortAllocator) RequestPort(ip net.IP, proto string, port int) (int, err return p.RequestPortInRange(ip, proto, port, port) } -// RequestPortInRange is equivalent to [RequestPortsInRange] with a single IP address. -// -// If ip is nil, a port is instead requested for the defaultIP. +// RequestPortInRange is equivalent to [PortAllocator.RequestPortsInRange] with +// a single IP address. If ip is nil, a port is instead requested for the +// default IP (0.0.0.0). func (p *PortAllocator) RequestPortInRange(ip net.IP, proto string, portStart, portEnd int) (int, error) { if ip == nil { ip = defaultIP From 1897a21d60f02182ca3fc135ee1c9ffd15197798 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 11:17:16 +0200 Subject: [PATCH 2/8] portallocator: ReleaseAll: remove unused error-return Signed-off-by: Sebastiaan van Stijn --- libnetwork/drivers/bridge/port_mapping_linux_test.go | 3 +-- libnetwork/portallocator/portallocator.go | 3 +-- libnetwork/portallocator/portallocator_test.go | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/libnetwork/drivers/bridge/port_mapping_linux_test.go b/libnetwork/drivers/bridge/port_mapping_linux_test.go index 2bc21e7eb029d..9511e84fc9a11 100644 --- a/libnetwork/drivers/bridge/port_mapping_linux_test.go +++ b/libnetwork/drivers/bridge/port_mapping_linux_test.go @@ -876,8 +876,7 @@ func TestAddPortMappings(t *testing.T) { return net.ParseIP("127.0.0.1") } - err = portallocator.Get().ReleaseAll() - assert.NilError(t, err) + portallocator.Get().ReleaseAll() pbs, err := n.addPortMappings(tc.epAddrV4, tc.epAddrV6, tc.cfg, tc.defHostIP) if tc.expErr != "" { diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index 2dacbf08a018d..737cf3c5ae5e0 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -221,11 +221,10 @@ func (p *PortAllocator) newPortMap() *portMap { } // ReleaseAll releases all ports for all ips. -func (p *PortAllocator) ReleaseAll() error { +func (p *PortAllocator) ReleaseAll() { p.mutex.Lock() p.ipMap = ipMapping{} p.mutex.Unlock() - return nil } func getRangeKey(portStart, portEnd int) string { diff --git a/libnetwork/portallocator/portallocator_test.go b/libnetwork/portallocator/portallocator_test.go index c48d52406b1ad..fcce81562cb1e 100644 --- a/libnetwork/portallocator/portallocator_test.go +++ b/libnetwork/portallocator/portallocator_test.go @@ -166,9 +166,7 @@ func BenchmarkAllocatePorts(b *testing.B) { b.Fatalf("Expected port %d got %d", expected, port) } } - if err := p.ReleaseAll(); err != nil { - b.Fatal(err) - } + p.ReleaseAll() } } From 630a47177b83037a62e8b62a61c56c0ff9dee25c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 12:01:44 +0200 Subject: [PATCH 3/8] portallocator: use new instance in tests Test the functionality in isolation instead of using the singleton that's returned by the `GET` function; this makes sure tests don't affect each other, and doesn't require resetting the singleton in between tests, potentially allowing these tests to eb run in parallel. Signed-off-by: Sebastiaan van Stijn --- .../portallocator/portallocator_test.go | 40 +++++++------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/libnetwork/portallocator/portallocator_test.go b/libnetwork/portallocator/portallocator_test.go index fcce81562cb1e..9ada774434d74 100644 --- a/libnetwork/portallocator/portallocator_test.go +++ b/libnetwork/portallocator/portallocator_test.go @@ -8,13 +8,8 @@ import ( is "gotest.tools/v3/assert/cmp" ) -func resetPortAllocator() { - instance = newInstance() -} - func TestRequestNewPort(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() port, err := p.RequestPort(defaultIP, "tcp", 0) if err != nil { @@ -27,8 +22,7 @@ func TestRequestNewPort(t *testing.T) { } func TestRequestSpecificPort(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() port, err := p.RequestPort(defaultIP, "tcp", 5000) if err != nil { @@ -41,7 +35,7 @@ func TestRequestSpecificPort(t *testing.T) { } func TestReleasePort(t *testing.T) { - p := Get() + p := newInstance() port, err := p.RequestPort(defaultIP, "tcp", 5000) if err != nil { @@ -55,8 +49,7 @@ func TestReleasePort(t *testing.T) { } func TestReuseReleasedPort(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() port, err := p.RequestPort(defaultIP, "tcp", 5000) if err != nil { @@ -78,8 +71,7 @@ func TestReuseReleasedPort(t *testing.T) { } func TestReleaseUnreadledPort(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() port, err := p.RequestPort(defaultIP, "tcp", 5000) if err != nil { @@ -99,14 +91,15 @@ func TestReleaseUnreadledPort(t *testing.T) { } func TestUnknowProtocol(t *testing.T) { - if _, err := Get().RequestPort(defaultIP, "tcpp", 0); err != ErrUnknownProtocol { + p := newInstance() + + if _, err := p.RequestPort(defaultIP, "tcpp", 0); err != ErrUnknownProtocol { t.Fatalf("Expected error %s got %s", ErrUnknownProtocol, err) } } func TestAllocateAllPorts(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() for i := 0; i <= p.End-p.Begin; i++ { port, err := p.RequestPort(defaultIP, "tcp", 0) @@ -152,8 +145,7 @@ func TestAllocateAllPorts(t *testing.T) { } func BenchmarkAllocatePorts(b *testing.B) { - p := Get() - defer resetPortAllocator() + p := newInstance() for i := 0; i < b.N; i++ { for i := 0; i <= p.End-p.Begin; i++ { @@ -171,8 +163,7 @@ func BenchmarkAllocatePorts(b *testing.B) { } func TestPortAllocation(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() ip := net.ParseIP("192.168.0.1") ip2 := net.ParseIP("192.168.0.2") @@ -231,8 +222,7 @@ func TestPortAllocation(t *testing.T) { } func TestPortAllocationWithCustomRange(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() start, end := 8081, 8082 specificPort := 8000 @@ -297,8 +287,7 @@ func TestPortAllocationWithCustomRange(t *testing.T) { } func TestNoDuplicateBPR(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() if port, err := p.RequestPort(defaultIP, "tcp", p.Begin); err != nil { t.Fatal(err) @@ -314,8 +303,7 @@ func TestNoDuplicateBPR(t *testing.T) { } func TestRequestPortForMultipleIPs(t *testing.T) { - p := Get() - defer resetPortAllocator() + p := newInstance() addrs := []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::")} From 78d88d06dc4b0d4305b07c2c2f1b58f6ff287cf0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 12:41:04 +0200 Subject: [PATCH 4/8] portallocator: use net.IPv4zero for defaultIP, and make it a property Use the variable that's provided by the net package, and make the defaultIP a property of the allocator instead of a package variable. Signed-off-by: Sebastiaan van Stijn --- libnetwork/portallocator/portallocator.go | 21 +++---- .../portallocator/portallocator_test.go | 58 +++++++++---------- 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index 737cf3c5ae5e0..83a4e11d24992 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -21,7 +21,6 @@ var ( ErrAllPortsAllocated = errors.New("all ports are allocated") // ErrUnknownProtocol is returned when an unknown protocol was specified ErrUnknownProtocol = errors.New("unknown protocol") - defaultIP = net.ParseIP("0.0.0.0") once sync.Once instance *PortAllocator ) @@ -62,10 +61,11 @@ func (e ErrPortAlreadyAllocated) Error() string { type ( // PortAllocator manages the transport ports database PortAllocator struct { - mutex sync.Mutex - ipMap ipMapping - Begin int - End int + mutex sync.Mutex + defaultIP net.IP + ipMap ipMapping + Begin int + End int } portRange struct { begin int @@ -96,9 +96,10 @@ func newInstance() *PortAllocator { start, end = defaultPortRangeStart, defaultPortRangeEnd } return &PortAllocator{ - ipMap: ipMapping{}, - Begin: start, - End: end, + ipMap: ipMapping{}, + defaultIP: net.IPv4zero, + Begin: start, + End: end, } } @@ -114,7 +115,7 @@ func (p *PortAllocator) RequestPort(ip net.IP, proto string, port int) (int, err // default IP (0.0.0.0). func (p *PortAllocator) RequestPortInRange(ip net.IP, proto string, portStart, portEnd int) (int, error) { if ip == nil { - ip = defaultIP + ip = p.defaultIP // FIXME(thaJeztah): consider making this a required argument and producing an error instead, or set default when constructing. } return p.RequestPortsInRange([]net.IP{ip}, proto, portStart, portEnd) } @@ -199,7 +200,7 @@ func (p *PortAllocator) ReleasePort(ip net.IP, proto string, port int) { defer p.mutex.Unlock() if ip == nil { - ip = defaultIP + ip = p.defaultIP // FIXME(thaJeztah): consider making this a required argument and producing an error instead, or set default when constructing. } protomap, ok := p.ipMap[ip.String()] if !ok { diff --git a/libnetwork/portallocator/portallocator_test.go b/libnetwork/portallocator/portallocator_test.go index 9ada774434d74..c86d649a3d803 100644 --- a/libnetwork/portallocator/portallocator_test.go +++ b/libnetwork/portallocator/portallocator_test.go @@ -11,7 +11,7 @@ import ( func TestRequestNewPort(t *testing.T) { p := newInstance() - port, err := p.RequestPort(defaultIP, "tcp", 0) + port, err := p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { t.Fatal(err) } @@ -24,7 +24,7 @@ func TestRequestNewPort(t *testing.T) { func TestRequestSpecificPort(t *testing.T) { p := newInstance() - port, err := p.RequestPort(defaultIP, "tcp", 5000) + port, err := p.RequestPort(net.IPv4zero, "tcp", 5000) if err != nil { t.Fatal(err) } @@ -37,7 +37,7 @@ func TestRequestSpecificPort(t *testing.T) { func TestReleasePort(t *testing.T) { p := newInstance() - port, err := p.RequestPort(defaultIP, "tcp", 5000) + port, err := p.RequestPort(net.IPv4zero, "tcp", 5000) if err != nil { t.Fatal(err) } @@ -45,13 +45,13 @@ func TestReleasePort(t *testing.T) { t.Fatalf("Expected port 5000 got %d", port) } - p.ReleasePort(defaultIP, "tcp", 5000) + p.ReleasePort(net.IPv4zero, "tcp", 5000) } func TestReuseReleasedPort(t *testing.T) { p := newInstance() - port, err := p.RequestPort(defaultIP, "tcp", 5000) + port, err := p.RequestPort(net.IPv4zero, "tcp", 5000) if err != nil { t.Fatal(err) } @@ -59,9 +59,9 @@ func TestReuseReleasedPort(t *testing.T) { t.Fatalf("Expected port 5000 got %d", port) } - p.ReleasePort(defaultIP, "tcp", 5000) + p.ReleasePort(net.IPv4zero, "tcp", 5000) - port, err = p.RequestPort(defaultIP, "tcp", 5000) + port, err = p.RequestPort(net.IPv4zero, "tcp", 5000) if err != nil { t.Fatal(err) } @@ -73,7 +73,7 @@ func TestReuseReleasedPort(t *testing.T) { func TestReleaseUnreadledPort(t *testing.T) { p := newInstance() - port, err := p.RequestPort(defaultIP, "tcp", 5000) + port, err := p.RequestPort(net.IPv4zero, "tcp", 5000) if err != nil { t.Fatal(err) } @@ -81,7 +81,7 @@ func TestReleaseUnreadledPort(t *testing.T) { t.Fatalf("Expected port 5000 got %d", port) } - _, err = p.RequestPort(defaultIP, "tcp", 5000) + _, err = p.RequestPort(net.IPv4zero, "tcp", 5000) switch err.(type) { case ErrPortAlreadyAllocated: @@ -93,7 +93,7 @@ func TestReleaseUnreadledPort(t *testing.T) { func TestUnknowProtocol(t *testing.T) { p := newInstance() - if _, err := p.RequestPort(defaultIP, "tcpp", 0); err != ErrUnknownProtocol { + if _, err := p.RequestPort(net.IPv4zero, "tcpp", 0); err != ErrUnknownProtocol { t.Fatalf("Expected error %s got %s", ErrUnknownProtocol, err) } } @@ -102,7 +102,7 @@ func TestAllocateAllPorts(t *testing.T) { p := newInstance() for i := 0; i <= p.End-p.Begin; i++ { - port, err := p.RequestPort(defaultIP, "tcp", 0) + port, err := p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { t.Fatal(err) } @@ -112,19 +112,19 @@ func TestAllocateAllPorts(t *testing.T) { } } - if _, err := p.RequestPort(defaultIP, "tcp", 0); err != ErrAllPortsAllocated { + if _, err := p.RequestPort(net.IPv4zero, "tcp", 0); err != ErrAllPortsAllocated { t.Fatalf("Expected error %s got %s", ErrAllPortsAllocated, err) } - _, err := p.RequestPort(defaultIP, "udp", 0) + _, err := p.RequestPort(net.IPv4zero, "udp", 0) if err != nil { t.Fatal(err) } // release a port in the middle and ensure we get another tcp port port := p.Begin + 5 - p.ReleasePort(defaultIP, "tcp", port) - newPort, err := p.RequestPort(defaultIP, "tcp", 0) + p.ReleasePort(net.IPv4zero, "tcp", port) + newPort, err := p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { t.Fatal(err) } @@ -134,8 +134,8 @@ func TestAllocateAllPorts(t *testing.T) { // now pm.last == newPort, release it so that it's the only free port of // the range, and ensure we get it back - p.ReleasePort(defaultIP, "tcp", newPort) - port, err = p.RequestPort(defaultIP, "tcp", 0) + p.ReleasePort(net.IPv4zero, "tcp", newPort) + port, err = p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { t.Fatal(err) } @@ -149,7 +149,7 @@ func BenchmarkAllocatePorts(b *testing.B) { for i := 0; i < b.N; i++ { for i := 0; i <= p.End-p.Begin; i++ { - port, err := p.RequestPort(defaultIP, "tcp", 0) + port, err := p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { b.Fatal(err) } @@ -228,24 +228,24 @@ func TestPortAllocationWithCustomRange(t *testing.T) { specificPort := 8000 // get an ephemeral port. - port1, err := p.RequestPortInRange(defaultIP, "tcp", 0, 0) + port1, err := p.RequestPortInRange(net.IPv4zero, "tcp", 0, 0) if err != nil { t.Fatal(err) } // request invalid ranges - if _, err := p.RequestPortInRange(defaultIP, "tcp", 0, end); err == nil { + if _, err := p.RequestPortInRange(net.IPv4zero, "tcp", 0, end); err == nil { t.Fatalf("Expected error for invalid range %d-%d", 0, end) } - if _, err := p.RequestPortInRange(defaultIP, "tcp", start, 0); err == nil { + if _, err := p.RequestPortInRange(net.IPv4zero, "tcp", start, 0); err == nil { t.Fatalf("Expected error for invalid range %d-%d", 0, end) } - if _, err := p.RequestPortInRange(defaultIP, "tcp", 8081, 8080); err == nil { + if _, err := p.RequestPortInRange(net.IPv4zero, "tcp", 8081, 8080); err == nil { t.Fatalf("Expected error for invalid range %d-%d", 0, end) } // request a single port - port, err := p.RequestPortInRange(defaultIP, "tcp", specificPort, specificPort) + port, err := p.RequestPortInRange(net.IPv4zero, "tcp", specificPort, specificPort) if err != nil { t.Fatal(err) } @@ -254,7 +254,7 @@ func TestPortAllocationWithCustomRange(t *testing.T) { } // get a port from the range - port2, err := p.RequestPortInRange(defaultIP, "tcp", start, end) + port2, err := p.RequestPortInRange(net.IPv4zero, "tcp", start, end) if err != nil { t.Fatal(err) } @@ -262,7 +262,7 @@ func TestPortAllocationWithCustomRange(t *testing.T) { t.Fatalf("Expected a port between %d and %d, got %d", start, end, port2) } // get another ephemeral port (should be > port1) - port3, err := p.RequestPortInRange(defaultIP, "tcp", 0, 0) + port3, err := p.RequestPortInRange(net.IPv4zero, "tcp", 0, 0) if err != nil { t.Fatal(err) } @@ -270,7 +270,7 @@ func TestPortAllocationWithCustomRange(t *testing.T) { t.Fatalf("Expected new port > %d in the ephemeral range, got %d", port1, port3) } // get another (and in this case the only other) port from the range - port4, err := p.RequestPortInRange(defaultIP, "tcp", start, end) + port4, err := p.RequestPortInRange(net.IPv4zero, "tcp", start, end) if err != nil { t.Fatal(err) } @@ -281,7 +281,7 @@ func TestPortAllocationWithCustomRange(t *testing.T) { t.Fatal("Allocated the same port from a custom range") } // request 3rd port from the range of 2 - if _, err := p.RequestPortInRange(defaultIP, "tcp", start, end); err != ErrAllPortsAllocated { + if _, err := p.RequestPortInRange(net.IPv4zero, "tcp", start, end); err != ErrAllPortsAllocated { t.Fatalf("Expected error %s got %s", ErrAllPortsAllocated, err) } } @@ -289,13 +289,13 @@ func TestPortAllocationWithCustomRange(t *testing.T) { func TestNoDuplicateBPR(t *testing.T) { p := newInstance() - if port, err := p.RequestPort(defaultIP, "tcp", p.Begin); err != nil { + if port, err := p.RequestPort(net.IPv4zero, "tcp", p.Begin); err != nil { t.Fatal(err) } else if port != p.Begin { t.Fatalf("Expected port %d got %d", p.Begin, port) } - if port, err := p.RequestPort(defaultIP, "tcp", 0); err != nil { + if port, err := p.RequestPort(net.IPv4zero, "tcp", 0); err != nil { t.Fatal(err) } else if port == p.Begin { t.Fatalf("Acquire(0) allocated the same port twice: %d", port) From c00f6281d93b721d56d9458c770a835b80e6f2af Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 12:44:15 +0200 Subject: [PATCH 5/8] portallocator: RequestPort: skip RequestPortInRange as intermediate RequestPortInRange is a wrapper for RequestPortsInRange; skip it as intermediate function. Signed-off-by: Sebastiaan van Stijn --- libnetwork/portallocator/portallocator.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index 83a4e11d24992..df472d85e510d 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -107,7 +107,10 @@ func newInstance() *PortAllocator { // If port is 0 it returns first free port. Otherwise it checks port availability // in proto's pool and returns that port or error if port is already busy. func (p *PortAllocator) RequestPort(ip net.IP, proto string, port int) (int, error) { - return p.RequestPortInRange(ip, proto, port, port) + if ip == nil { + ip = p.defaultIP // FIXME(thaJeztah): consider making this a required argument and producing an error instead, or set default when constructing. + } + return p.RequestPortsInRange([]net.IP{ip}, proto, port, port) } // RequestPortInRange is equivalent to [PortAllocator.RequestPortsInRange] with From 05d784d6da658a5a6611ba2c1cf71395e6b569fd Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 12:13:26 +0200 Subject: [PATCH 6/8] portallocator: make newPortMap a regular constructor It was a method on PortAllocator, but not really related, other than reading the default values. Signed-off-by: Sebastiaan van Stijn --- libnetwork/portallocator/portallocator.go | 29 +++++++++++------------ 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index df472d85e510d..3927b1f180c2d 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -141,9 +141,9 @@ func (p *PortAllocator) RequestPortsInRange(ips []net.IP, proto string, portStar ipstr := ip.String() if _, ok := p.ipMap[ipstr]; !ok { p.ipMap[ipstr] = protoMap{ - "tcp": p.newPortMap(), - "udp": p.newPortMap(), - "sctp": p.newPortMap(), + "tcp": newPortMap(p.Begin, p.End), + "udp": newPortMap(p.Begin, p.End), + "sctp": newPortMap(p.Begin, p.End), } } pMaps[i] = p.ipMap[ipstr][proto] @@ -212,18 +212,6 @@ func (p *PortAllocator) ReleasePort(ip net.IP, proto string, port int) { delete(protomap[proto].p, port) } -func (p *PortAllocator) newPortMap() *portMap { - defaultKey := getRangeKey(p.Begin, p.End) - pm := &portMap{ - p: map[int]struct{}{}, - defaultRange: defaultKey, - portRanges: map[string]*portRange{ - defaultKey: newPortRange(p.Begin, p.End), - }, - } - return pm -} - // ReleaseAll releases all ports for all ips. func (p *PortAllocator) ReleaseAll() { p.mutex.Lock() @@ -243,6 +231,17 @@ func newPortRange(portStart, portEnd int) *portRange { } } +func newPortMap(portStart, portEnd int) *portMap { + defaultKey := getRangeKey(portStart, portEnd) + return &portMap{ + p: map[int]struct{}{}, + defaultRange: defaultKey, + portRanges: map[string]*portRange{ + defaultKey: newPortRange(portStart, portEnd), + }, + } +} + func (pm *portMap) getPortRange(portStart, portEnd int) (*portRange, error) { var key string if portStart == 0 && portEnd == 0 { From fb1ae4bdb71f8af21f2a4e79d8f943153c887233 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 13:17:32 +0200 Subject: [PATCH 7/8] portallocator: RequestPortsInRange: validate range once RequestPortsInRange calls portMap.getPortRange() in a loop, but the given port-range is always the same. Perform validation once instead of for each iteration. Signed-off-by: Sebastiaan van Stijn --- libnetwork/portallocator/portallocator.go | 24 +++++++++++------------ 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index 3927b1f180c2d..7eee0e9a8c918 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -133,6 +133,13 @@ func (p *PortAllocator) RequestPortsInRange(ips []net.IP, proto string, portStar return 0, ErrUnknownProtocol } + if portStart != 0 || portEnd != 0 { + // Validate custom port-range + if portStart == 0 || portEnd == 0 || portEnd < portStart { + return 0, fmt.Errorf("invalid port range: %d-%d", portStart, portEnd) + } + } + p.mutex.Lock() defer p.mutex.Unlock() @@ -167,11 +174,7 @@ func (p *PortAllocator) RequestPortsInRange(ips []net.IP, proto string, portStar // Create/fetch ranges for each portMap. pRanges := make([]*portRange, len(pMaps)) for i, pMap := range pMaps { - var err error - pRanges[i], err = pMap.getPortRange(portStart, portEnd) - if err != nil { - return 0, err - } + pRanges[i] = pMap.getPortRange(portStart, portEnd) } // Starting after the last port allocated for the first address, search @@ -242,26 +245,21 @@ func newPortMap(portStart, portEnd int) *portMap { } } -func (pm *portMap) getPortRange(portStart, portEnd int) (*portRange, error) { +func (pm *portMap) getPortRange(portStart, portEnd int) *portRange { var key string if portStart == 0 && portEnd == 0 { key = pm.defaultRange } else { key = getRangeKey(portStart, portEnd) - if portStart == portEnd || - portStart == 0 || portEnd == 0 || - portEnd < portStart { - return nil, fmt.Errorf("invalid port range: %s", key) - } } // Return existing port range, if already known. if pr, exists := pm.portRanges[key]; exists { - return pr, nil + return pr } // Otherwise create a new port range. pr := newPortRange(portStart, portEnd) pm.portRanges[key] = pr - return pr, nil + return pr } From 8e580efb73572bc55a49e7d67afc3033238da26e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Aug 2024 14:37:35 +0200 Subject: [PATCH 8/8] portallocator: un-export PortAllocator.Begin, PortAllocator.End These values are configured when instantiating the allocator, and not intended to be mutated externally. They're only used internally with the exception of a test in the bridge driver that uses it to pick a port that can be used for testing. This patch: - un-exports the Begin and End fields - introduces a GetPortRange() utility to allow the bridge driver to get the port, but marking it as a function for internal use. Signed-off-by: Sebastiaan van Stijn --- .../drivers/bridge/port_mapping_linux_test.go | 2 +- libnetwork/portallocator/portallocator.go | 23 +++++++++++++------ .../portallocator/portallocator_test.go | 22 +++++++++--------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/libnetwork/drivers/bridge/port_mapping_linux_test.go b/libnetwork/drivers/bridge/port_mapping_linux_test.go index 9511e84fc9a11..82e1fc8a134f2 100644 --- a/libnetwork/drivers/bridge/port_mapping_linux_test.go +++ b/libnetwork/drivers/bridge/port_mapping_linux_test.go @@ -413,7 +413,7 @@ func TestAddPortMappings(t *testing.T) { ctrIP4 := newIPNet(t, "172.19.0.2/16") ctrIP4Mapped := newIPNet(t, "::ffff:172.19.0.2/112") ctrIP6 := newIPNet(t, "fdf8:b88e:bb5c:3483::2/64") - firstEphemPort := uint16(portallocator.Get().Begin) + firstEphemPort, _ := portallocator.GetPortRange() testcases := []struct { name string diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index 7eee0e9a8c918..4e3b970642db4 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -64,8 +64,8 @@ type ( mutex sync.Mutex defaultIP net.IP ipMap ipMapping - Begin int - End int + begin int + end int } portRange struct { begin int @@ -80,6 +80,15 @@ type ( protoMap map[string]*portMap ) +// GetPortRange returns the PortAllocator's default port range. +// +// This function is for internal use in tests, and must not be used +// for other purposes. +func GetPortRange() (start, end uint16) { + p := Get() + return uint16(p.begin), uint16(p.end) +} + // Get returns the PortAllocator func Get() *PortAllocator { // Port Allocator is a singleton @@ -98,8 +107,8 @@ func newInstance() *PortAllocator { return &PortAllocator{ ipMap: ipMapping{}, defaultIP: net.IPv4zero, - Begin: start, - End: end, + begin: start, + end: end, } } @@ -148,9 +157,9 @@ func (p *PortAllocator) RequestPortsInRange(ips []net.IP, proto string, portStar ipstr := ip.String() if _, ok := p.ipMap[ipstr]; !ok { p.ipMap[ipstr] = protoMap{ - "tcp": newPortMap(p.Begin, p.End), - "udp": newPortMap(p.Begin, p.End), - "sctp": newPortMap(p.Begin, p.End), + "tcp": newPortMap(p.begin, p.end), + "udp": newPortMap(p.begin, p.end), + "sctp": newPortMap(p.begin, p.end), } } pMaps[i] = p.ipMap[ipstr][proto] diff --git a/libnetwork/portallocator/portallocator_test.go b/libnetwork/portallocator/portallocator_test.go index c86d649a3d803..cc0831d722143 100644 --- a/libnetwork/portallocator/portallocator_test.go +++ b/libnetwork/portallocator/portallocator_test.go @@ -16,7 +16,7 @@ func TestRequestNewPort(t *testing.T) { t.Fatal(err) } - if expected := p.Begin; port != expected { + if expected := p.begin; port != expected { t.Fatalf("Expected port %d got %d", expected, port) } } @@ -101,13 +101,13 @@ func TestUnknowProtocol(t *testing.T) { func TestAllocateAllPorts(t *testing.T) { p := newInstance() - for i := 0; i <= p.End-p.Begin; i++ { + for i := 0; i <= p.end-p.begin; i++ { port, err := p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { t.Fatal(err) } - if expected := p.Begin + i; port != expected { + if expected := p.begin + i; port != expected { t.Fatalf("Expected port %d got %d", expected, port) } } @@ -122,7 +122,7 @@ func TestAllocateAllPorts(t *testing.T) { } // release a port in the middle and ensure we get another tcp port - port := p.Begin + 5 + port := p.begin + 5 p.ReleasePort(net.IPv4zero, "tcp", port) newPort, err := p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { @@ -148,13 +148,13 @@ func BenchmarkAllocatePorts(b *testing.B) { p := newInstance() for i := 0; i < b.N; i++ { - for i := 0; i <= p.End-p.Begin; i++ { + for i := 0; i <= p.end-p.begin; i++ { port, err := p.RequestPort(net.IPv4zero, "tcp", 0) if err != nil { b.Fatal(err) } - if expected := p.Begin + i; port != expected { + if expected := p.begin + i; port != expected { b.Fatalf("Expected port %d got %d", expected, port) } } @@ -289,15 +289,15 @@ func TestPortAllocationWithCustomRange(t *testing.T) { func TestNoDuplicateBPR(t *testing.T) { p := newInstance() - if port, err := p.RequestPort(net.IPv4zero, "tcp", p.Begin); err != nil { + if port, err := p.RequestPort(net.IPv4zero, "tcp", p.begin); err != nil { t.Fatal(err) - } else if port != p.Begin { - t.Fatalf("Expected port %d got %d", p.Begin, port) + } else if port != p.begin { + t.Fatalf("Expected port %d got %d", p.begin, port) } if port, err := p.RequestPort(net.IPv4zero, "tcp", 0); err != nil { t.Fatal(err) - } else if port == p.Begin { + } else if port == p.begin { t.Fatalf("Acquire(0) allocated the same port twice: %d", port) } } @@ -310,7 +310,7 @@ func TestRequestPortForMultipleIPs(t *testing.T) { // Default port range. port, err := p.RequestPortsInRange(addrs, "tcp", 0, 0) assert.Check(t, err) - assert.Check(t, is.Equal(port, p.Begin)) + assert.Check(t, is.Equal(port, p.begin)) // Single-port range. port, err = p.RequestPortsInRange(addrs, "tcp", 10000, 10000)