From 414dd017b3836c816a2cf74cfe53ed66ca940c4d Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Mon, 6 Dec 2021 12:26:32 +0100 Subject: [PATCH 1/2] Revert "Added API to set ephemeral port allocator range." Since commit 2c4a868f64e6e13bf06589a6670122196651e82e, Docker doesn't use the value of net.ipv4.ip_local_port_range when choosing an ephemeral port. This change reverts back to the previous behavior. Fixes #43054. Signed-off-by: Albin Kerouanton --- libnetwork/config/config.go | 19 ------ libnetwork/portallocator/portallocator.go | 58 ++----------------- .../portallocator/portallocator_test.go | 45 -------------- .../portallocator/portallocator_unix.go | 8 +-- .../portallocator/portallocator_windows.go | 10 ++-- 5 files changed, 14 insertions(+), 126 deletions(-) diff --git a/libnetwork/config/config.go b/libnetwork/config/config.go index 33ece12d9f..a33ffba338 100644 --- a/libnetwork/config/config.go +++ b/libnetwork/config/config.go @@ -1,7 +1,6 @@ package config import ( - "fmt" "os" "strings" @@ -10,7 +9,6 @@ import ( "github.com/docker/docker/libnetwork/ipamutils" "github.com/docker/docker/libnetwork/netlabel" "github.com/docker/docker/libnetwork/osl" - "github.com/docker/docker/libnetwork/portallocator" "github.com/docker/docker/pkg/plugingetter" "github.com/docker/libkv/store" "github.com/pelletier/go-toml" @@ -163,23 +161,6 @@ func OptionExperimental(exp bool) Option { } } -// OptionDynamicPortRange function returns an option setter for service port allocation range -func OptionDynamicPortRange(in string) Option { - return func(c *Config) { - start, end := 0, 0 - if len(in) > 0 { - n, err := fmt.Sscanf(in, "%d-%d", &start, &end) - if n != 2 || err != nil { - logrus.Errorf("Failed to parse range string with err %v", err) - return - } - } - if err := portallocator.Get().SetPortRange(start, end); err != nil { - logrus.Errorf("Failed to set port range with err %v", err) - } - } -} - // OptionNetworkControlPlaneMTU function returns an option setter for control plane MTU func OptionNetworkControlPlaneMTU(exp int) Option { return func(c *Config) { diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index 1dc935cbd7..de96da8623 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -9,22 +9,6 @@ import ( "github.com/sirupsen/logrus" ) -func sanitizePortRange(start int, end int) (newStart, newEnd int, err error) { - if start > defaultPortRangeEnd || end < defaultPortRangeStart || start > end { - return 0, 0, fmt.Errorf("Request out allowed range [%v, %v]", - defaultPortRangeStart, defaultPortRangeEnd) - } - err = nil - newStart, newEnd = start, end - if start < defaultPortRangeStart { - newStart = defaultPortRangeStart - } - if end > defaultPortRangeEnd { - newEnd = defaultPortRangeEnd - } - return -} - type ipMapping map[string]protoMap var ( @@ -104,20 +88,12 @@ func Get() *PortAllocator { return instance } -func getDefaultPortRange() (int, int) { - start, end, err := getDynamicPortRange() - if err == nil { - start, end, err = sanitizePortRange(start, end) - } - if err != nil { - logrus.WithError(err).Infof("falling back to default port range %d-%d", defaultPortRangeStart, defaultPortRangeEnd) - start, end = defaultPortRangeStart, defaultPortRangeEnd - } - return start, end -} - func newInstance() *PortAllocator { - start, end := getDefaultPortRange() + start, end, err := getDynamicPortRange() + if err != nil { + logrus.WithError(err).Infof("falling back to default port range %d-%d", DefaultPortRangeStart, DefaultPortRangeEnd) + start, end = DefaultPortRangeStart, DefaultPortRangeEnd + } return &PortAllocator{ ipMap: ipMapping{}, Begin: start, @@ -191,30 +167,6 @@ func (p *PortAllocator) ReleasePort(ip net.IP, proto string, port int) error { return nil } -// SetPortRange sets dynamic port allocation range. -// if both portBegin and portEnd are 0, the port range reverts to default -// value. Otherwise they are sanitized against the default values to -// ensure their validity. -func (p *PortAllocator) SetPortRange(portBegin, portEnd int) error { - // if begin and end is zero, revert to default values - var begin, end int - var err error - if portBegin == 0 && portEnd == 0 { - begin, end = getDefaultPortRange() - } else if begin, end, err = sanitizePortRange(portBegin, portEnd); err != nil { - return err - } - logrus.Debugf("Setting up port allocator to range %v-%v, current %v-%v", begin, end, p.Begin, p.End) - p.mutex.Lock() - defer p.mutex.Unlock() - if p.Begin == begin && p.End == end { - return nil - } - p.ipMap = ipMapping{} - p.Begin, p.End = begin, end - return nil -} - func (p *PortAllocator) newPortMap() *portMap { defaultKey := getRangeKey(p.Begin, p.End) pm := &portMap{ diff --git a/libnetwork/portallocator/portallocator_test.go b/libnetwork/portallocator/portallocator_test.go index 8a34c3cbbb..4d94885a74 100644 --- a/libnetwork/portallocator/portallocator_test.go +++ b/libnetwork/portallocator/portallocator_test.go @@ -1,7 +1,6 @@ package portallocator import ( - "fmt" "net" "testing" ) @@ -322,47 +321,3 @@ func TestNoDuplicateBPR(t *testing.T) { t.Fatalf("Acquire(0) allocated the same port twice: %d", port) } } - -func TestChangePortRange(t *testing.T) { - var tests = []struct { - begin int - end int - setErr error - reqRlt int - }{ - {defaultPortRangeEnd + 1, defaultPortRangeEnd + 10, fmt.Errorf("begin out of range"), 0}, - {defaultPortRangeStart - 10, defaultPortRangeStart - 1, fmt.Errorf("end out of range"), 0}, - {defaultPortRangeEnd, defaultPortRangeStart, fmt.Errorf("out of order"), 0}, - {defaultPortRangeStart + 100, defaultPortRangeEnd + 10, nil, defaultPortRangeStart + 100}, - {0, 0, nil, defaultPortRangeStart}, // revert to default if no value given - {defaultPortRangeStart - 100, defaultPortRangeEnd, nil, defaultPortRangeStart + 1}, - } - p := Get() - port := 0 - for _, c := range tests { - t.Logf("test: port allocate range %v-%v, setErr=%v, reqPort=%v", - c.begin, c.end, c.setErr, c.reqRlt) - err := p.SetPortRange(c.begin, c.end) - if (c.setErr == nil && c.setErr != err) || - (c.setErr != nil && err == nil) { - t.Fatalf("Unexpected set range result, expected=%v, actual=%v", c.setErr, err) - } - if err != nil { - continue - } - if port > 0 { - err := p.ReleasePort(defaultIP, "tcp", port) - if err != nil { - t.Fatalf("Releasing port %v failed, err=%v", port, err) - } - } - - port, err = p.RequestPort(defaultIP, "tcp", 0) - if err != nil { - t.Fatalf("Request failed, err %v", err) - } - if port != c.reqRlt { - t.Fatalf("Incorrect port returned, expected=%v, actual=%v", c.reqRlt, port) - } - } -} diff --git a/libnetwork/portallocator/portallocator_unix.go b/libnetwork/portallocator/portallocator_unix.go index 21adb62e0c..80e590bf03 100644 --- a/libnetwork/portallocator/portallocator_unix.go +++ b/libnetwork/portallocator/portallocator_unix.go @@ -4,10 +4,10 @@ package portallocator const ( - // defaultPortRangeStart indicates the first port in port range - defaultPortRangeStart = 49153 - // defaultPortRangeEnd indicates the last port in port range + // DefaultPortRangeStart indicates the first port in port range + DefaultPortRangeStart = 49153 + // DefaultPortRangeEnd indicates the last port in port range // consistent with default /proc/sys/net/ipv4/ip_local_port_range // upper bound on linux - defaultPortRangeEnd = 60999 + DefaultPortRangeEnd = 65535 ) diff --git a/libnetwork/portallocator/portallocator_windows.go b/libnetwork/portallocator/portallocator_windows.go index 07c4411eae..160edea90b 100644 --- a/libnetwork/portallocator/portallocator_windows.go +++ b/libnetwork/portallocator/portallocator_windows.go @@ -1,12 +1,12 @@ package portallocator const ( - // defaultPortRangeStart indicates the first port in port range - defaultPortRangeStart = 60000 - // defaultPortRangeEnd indicates the last port in port range - defaultPortRangeEnd = 65000 + // DefaultPortRangeStart indicates the first port in port range + DefaultPortRangeStart = 60000 + // DefaultPortRangeEnd indicates the last port in port range + DefaultPortRangeEnd = 65000 ) func getDynamicPortRange() (start int, end int, err error) { - return defaultPortRangeStart, defaultPortRangeEnd, nil + return DefaultPortRangeStart, DefaultPortRangeEnd, nil } From b4b2a0323b6eff5bd1c3ea842fb70adc2c742806 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 2 Jun 2022 22:59:32 +0200 Subject: [PATCH 2/2] libnetwork/portallocator: un-export consts for defaults Signed-off-by: Sebastiaan van Stijn --- libnetwork/portallocator/portallocator.go | 4 ++-- libnetwork/portallocator/portallocator_unix.go | 8 ++++---- libnetwork/portallocator/portallocator_windows.go | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/portallocator/portallocator.go index de96da8623..a4fd39919c 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/portallocator/portallocator.go @@ -91,8 +91,8 @@ func Get() *PortAllocator { func newInstance() *PortAllocator { start, end, err := getDynamicPortRange() if err != nil { - logrus.WithError(err).Infof("falling back to default port range %d-%d", DefaultPortRangeStart, DefaultPortRangeEnd) - start, end = DefaultPortRangeStart, DefaultPortRangeEnd + logrus.WithError(err).Infof("falling back to default port range %d-%d", defaultPortRangeStart, defaultPortRangeEnd) + start, end = defaultPortRangeStart, defaultPortRangeEnd } return &PortAllocator{ ipMap: ipMapping{}, diff --git a/libnetwork/portallocator/portallocator_unix.go b/libnetwork/portallocator/portallocator_unix.go index 80e590bf03..ac8e863de9 100644 --- a/libnetwork/portallocator/portallocator_unix.go +++ b/libnetwork/portallocator/portallocator_unix.go @@ -4,10 +4,10 @@ package portallocator const ( - // DefaultPortRangeStart indicates the first port in port range - DefaultPortRangeStart = 49153 - // DefaultPortRangeEnd indicates the last port in port range + // defaultPortRangeStart indicates the first port in port range + defaultPortRangeStart = 49153 + // defaultPortRangeEnd indicates the last port in port range // consistent with default /proc/sys/net/ipv4/ip_local_port_range // upper bound on linux - DefaultPortRangeEnd = 65535 + defaultPortRangeEnd = 65535 ) diff --git a/libnetwork/portallocator/portallocator_windows.go b/libnetwork/portallocator/portallocator_windows.go index 160edea90b..07c4411eae 100644 --- a/libnetwork/portallocator/portallocator_windows.go +++ b/libnetwork/portallocator/portallocator_windows.go @@ -1,12 +1,12 @@ package portallocator const ( - // DefaultPortRangeStart indicates the first port in port range - DefaultPortRangeStart = 60000 - // DefaultPortRangeEnd indicates the last port in port range - DefaultPortRangeEnd = 65000 + // defaultPortRangeStart indicates the first port in port range + defaultPortRangeStart = 60000 + // defaultPortRangeEnd indicates the last port in port range + defaultPortRangeEnd = 65000 ) func getDynamicPortRange() (start int, end int, err error) { - return DefaultPortRangeStart, DefaultPortRangeEnd, nil + return defaultPortRangeStart, defaultPortRangeEnd, nil }