Skip to content

Commit

Permalink
Merge pull request #2368 from pradipd/WindowsPortMappingFix
Browse files Browse the repository at this point in the history
(windows) Pick a random host port if the user does not specify a host port.
  • Loading branch information
mavenugo committed Apr 23, 2019
2 parents 48f8463 + e6b4a7a commit 9ff9b57
Show file tree
Hide file tree
Showing 10 changed files with 284 additions and 50 deletions.
16 changes: 15 additions & 1 deletion drivers/windows/overlay/ov_endpoint_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,19 @@ func (d *driver) CreateEndpoint(nid, eid string, ifInfo driverapi.InterfaceInfo,
return err
}

pbPolicy, err := windows.ConvertPortBindings(epConnectivity.PortBindings)
ep.portMapping = epConnectivity.PortBindings
ep.portMapping, err = windows.AllocatePorts(n.portMapper, ep.portMapping, ep.addr.IP)
if err != nil {
return err
}

defer func() {
if err != nil {
windows.ReleasePorts(n.portMapper, ep.portMapping)
}
}()

pbPolicy, err := windows.ConvertPortBindings(ep.portMapping)
if err != nil {
return err
}
Expand Down Expand Up @@ -229,6 +241,8 @@ func (d *driver) DeleteEndpoint(nid, eid string) error {
return fmt.Errorf("endpoint id %q not found", eid)
}

windows.ReleasePorts(n.portMapper, ep.portMapping)

n.deleteEndpoint(eid)

_, err := endpointRequest("DELETE", ep.profileID, "")
Expand Down
11 changes: 7 additions & 4 deletions drivers/windows/overlay/ov_network_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/Microsoft/hcsshim"
"github.com/docker/libnetwork/driverapi"
"github.com/docker/libnetwork/netlabel"
"github.com/docker/libnetwork/portmapper"
"github.com/docker/libnetwork/types"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -46,6 +47,7 @@ type network struct {
initErr error
subnets []*subnet
secure bool
portMapper *portmapper.PortMapper
sync.Mutex
}

Expand Down Expand Up @@ -89,10 +91,11 @@ func (d *driver) CreateNetwork(id string, option map[string]interface{}, nInfo d
}

n := &network{
id: id,
driver: d,
endpoints: endpointTable{},
subnets: []*subnet{},
id: id,
driver: d,
endpoints: endpointTable{},
subnets: []*subnet{},
portMapper: portmapper.New(""),
}

genData, ok := option[netlabel.GenericData].(map[string]string)
Expand Down
125 changes: 125 additions & 0 deletions drivers/windows/port_mapping.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// +build windows

package windows

import (
"bytes"
"errors"
"fmt"
"net"

"github.com/docker/libnetwork/portmapper"
"github.com/docker/libnetwork/types"
"github.com/ishidawataru/sctp"
"github.com/sirupsen/logrus"
)

const (
maxAllocatePortAttempts = 10
)

// ErrUnsupportedAddressType is returned when the specified address type is not supported.
type ErrUnsupportedAddressType string

func (uat ErrUnsupportedAddressType) Error() string {
return fmt.Sprintf("unsupported address type: %s", string(uat))
}

// AllocatePorts allocates ports specified in bindings from the portMapper
func AllocatePorts(portMapper *portmapper.PortMapper, bindings []types.PortBinding, containerIP net.IP) ([]types.PortBinding, error) {
bs := make([]types.PortBinding, 0, len(bindings))
for _, c := range bindings {
b := c.GetCopy()
if err := allocatePort(portMapper, &b, containerIP); err != nil {
// On allocation failure, release previously allocated ports. On cleanup error, just log a warning message
if cuErr := ReleasePorts(portMapper, bs); cuErr != nil {
logrus.Warnf("Upon allocation failure for %v, failed to clear previously allocated port bindings: %v", b, cuErr)
}
return nil, err
}
bs = append(bs, b)
}
return bs, nil
}

func allocatePort(portMapper *portmapper.PortMapper, bnd *types.PortBinding, containerIP net.IP) error {
var (
host net.Addr
err error
)

// 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.
for i := 0; i < maxAllocatePortAttempts; i++ {
if host, err = portMapper.MapRange(container, bnd.HostIP, int(bnd.HostPort), int(bnd.HostPortEnd), false); err == nil {
break
}
// There is no point in immediately retrying to map an explicitly chosen port.
if bnd.HostPort != 0 {
logrus.Warnf("Failed to allocate and map port %d-%d: %s", bnd.HostPort, bnd.HostPortEnd, err)
break
}
logrus.Warnf("Failed to allocate and map port: %s, retry: %d", err, i+1)
}
if err != nil {
return 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 ErrUnsupportedAddressType(fmt.Sprintf("%T", netAddr))
}
//Windows does not support host port ranges.
bnd.HostPortEnd = bnd.HostPort
return nil
}

// ReleasePorts releases ports specified in bindings from the portMapper
func ReleasePorts(portMapper *portmapper.PortMapper, 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 {
errorBuf.WriteString(fmt.Sprintf("\ncould not release %v because of %v", m, err))
}
}

if errorBuf.Len() != 0 {
return errors.New(errorBuf.String())
}
return nil
}

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)
}
47 changes: 37 additions & 10 deletions drivers/windows/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/docker/libnetwork/discoverapi"
"github.com/docker/libnetwork/driverapi"
"github.com/docker/libnetwork/netlabel"
"github.com/docker/libnetwork/portmapper"
"github.com/docker/libnetwork/types"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -88,11 +89,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
id string
created bool
config *networkConfiguration
endpoints map[string]*hnsEndpoint // key: endpoint id
driver *driver // The network's driver
portMapper *portmapper.PortMapper
sync.Mutex
}

Expand Down Expand Up @@ -252,10 +254,11 @@ func (d *driver) DecodeTableEntry(tablename string, key string, value []byte) (s

func (d *driver) createNetwork(config *networkConfiguration) error {
network := &hnsNetwork{
id: config.ID,
endpoints: make(map[string]*hnsEndpoint),
config: config,
driver: d,
id: config.ID,
endpoints: make(map[string]*hnsEndpoint),
config: config,
driver: d,
portMapper: portmapper.New(""),
}

d.Lock()
Expand Down Expand Up @@ -610,7 +613,27 @@ func (d *driver) CreateEndpoint(nid, eid string, ifInfo driverapi.InterfaceInfo,
endpointStruct.MacAddress = strings.Replace(macAddress.String(), ":", "-", -1)
}

endpointStruct.Policies, err = ConvertPortBindings(epConnectivity.PortBindings)
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)
if err != nil {
return err
}

defer func() {
if err != nil {
ReleasePorts(n.portMapper, portMapping)
}
}()
}

endpointStruct.Policies, err = ConvertPortBindings(portMapping)
if err != nil {
return err
}
Expand Down Expand Up @@ -721,6 +744,10 @@ func (d *driver) DeleteEndpoint(nid, eid string) error {
return err
}

if n.config.Type == "l2bridge" || n.config.Type == "l2tunnel" {
ReleasePorts(n.portMapper, ep.portMapping)
}

n.Lock()
delete(n.endpoints, eid)
n.Unlock()
Expand Down
2 changes: 0 additions & 2 deletions portallocator/portallocator.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build !windows

package portallocator

import (
Expand Down
9 changes: 9 additions & 0 deletions portallocator/portallocator_windows.go
Original file line number Diff line number Diff line change
@@ -1 +1,10 @@
package portallocator

const (
StartPortRange = 60000
EndPortRange = 65000
)

func getDynamicPortRange() (start int, end int, err error) {
return StartPortRange, EndPortRange, nil
}
37 changes: 4 additions & 33 deletions portmapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import (
"errors"
"fmt"
"net"
"sync"

"github.com/docker/libnetwork/iptables"
"github.com/docker/libnetwork/portallocator"
"github.com/ishidawataru/sctp"
"github.com/sirupsen/logrus"
Expand All @@ -32,20 +30,6 @@ var (
ErrSCTPAddrNoIP = errors.New("sctp address does not contain any IP address")
)

// PortMapper manages the network address translation
type PortMapper struct {
chain *iptables.ChainInfo
bridgeName string

// udp:ip:port
currentMappings map[string]*mapping
lock sync.Mutex

proxyPath string

Allocator *portallocator.PortAllocator
}

// New returns a new instance of PortMapper
func New(proxyPath string) *PortMapper {
return NewWithPortAllocator(portallocator.Get(), proxyPath)
Expand All @@ -60,12 +44,6 @@ func NewWithPortAllocator(allocator *portallocator.PortAllocator, proxyPath stri
}
}

// SetIptablesChain sets the specified chain into portmapper
func (pm *PortMapper) SetIptablesChain(c *iptables.ChainInfo, bridgeName string) {
pm.chain = c
pm.bridgeName = bridgeName
}

// Map maps the specified container transport address to the host's network address and transport port
func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int, useProxy bool) (host net.Addr, err error) {
return pm.MapRange(container, hostIP, hostPort, hostPort, useProxy)
Expand Down Expand Up @@ -174,7 +152,7 @@ func (pm *PortMapper) MapRange(container net.Addr, hostIP net.IP, hostPortStart,

containerIP, containerPort := getIPAndPort(m.container)
if hostIP.To4() != nil {
if err := pm.forward(iptables.Append, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort); err != nil {
if err := pm.AppendForwardingTableEntry(m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort); err != nil {
return nil, err
}
}
Expand All @@ -183,7 +161,7 @@ func (pm *PortMapper) MapRange(container net.Addr, hostIP net.IP, hostPortStart,
// need to undo the iptables rules before we return
m.userlandProxy.Stop()
if hostIP.To4() != nil {
pm.forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
pm.DeleteForwardingTableEntry(m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
if err := pm.Allocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil {
return err
}
Expand Down Expand Up @@ -222,7 +200,7 @@ func (pm *PortMapper) Unmap(host net.Addr) error {

containerIP, containerPort := getIPAndPort(data.container)
hostIP, hostPort := getIPAndPort(data.host)
if err := pm.forward(iptables.Delete, data.proto, hostIP, hostPort, containerIP.String(), containerPort); err != nil {
if err := pm.DeleteForwardingTableEntry(data.proto, hostIP, hostPort, containerIP.String(), containerPort); err != nil {
logrus.Errorf("Error on iptables delete: %s", err)
}

Expand All @@ -248,7 +226,7 @@ func (pm *PortMapper) ReMapAll() {
for _, data := range pm.currentMappings {
containerIP, containerPort := getIPAndPort(data.container)
hostIP, hostPort := getIPAndPort(data.host)
if err := pm.forward(iptables.Append, data.proto, hostIP, hostPort, containerIP.String(), containerPort); err != nil {
if err := pm.AppendForwardingTableEntry(data.proto, hostIP, hostPort, containerIP.String(), containerPort); err != nil {
logrus.Errorf("Error on iptables add: %s", err)
}
}
Expand Down Expand Up @@ -285,10 +263,3 @@ func getIPAndPort(a net.Addr) (net.IP, int) {
}
return nil, 0
}

func (pm *PortMapper) forward(action iptables.Action, proto string, sourceIP net.IP, sourcePort int, containerIP string, containerPort int) error {
if pm.chain == nil {
return nil
}
return pm.chain.Forward(action, sourceIP, sourcePort, proto, containerIP, containerPort, pm.bridgeName)
}
Loading

0 comments on commit 9ff9b57

Please sign in to comment.