From 38e76ebea905de636e9b5b11675d97e278153aa9 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Fri, 10 Jan 2025 10:37:51 +0000 Subject: [PATCH] Only allocate a gateway if the n/w driver wants one A gateway address is always reserved before the network driver is asked to create the network. But, the driver doesn't always need a gateway address, so the address reservation can be unnecessary. This means, for example, an "--internal" IPv4 "/31" network cannot be used as a point-to-point link, because one of its two addresses is always reserved for a gateway. So, before allocating a gateway address, ask the network driver if it will need one (based on options that only the network driver can interpret). Implement that as an optional interface for network drivers. Signed-off-by: Rob Murray --- libnetwork/controller.go | 8 +++++++- libnetwork/driverapi/driverapi.go | 9 +++++++++ libnetwork/network.go | 29 ++++++++++++++++++++++++----- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/libnetwork/controller.go b/libnetwork/controller.go index f1b0b7495f..89134aee85 100644 --- a/libnetwork/controller.go +++ b/libnetwork/controller.go @@ -562,8 +562,14 @@ func (c *Controller) NewNetwork(networkType, name string, id string, options ... // Make sure we have a driver available for this network type // before we allocate anything. - if _, err := nw.driver(true); err != nil { + if d, err := nw.driver(true); err != nil { return nil, err + } else if gac, ok := d.(driverapi.GwAllocChecker); ok { + // Give the driver a chance to say it doesn't need a gateway IP address. + nw.skipGwAllocIPv4, nw.skipGwAllocIPv6, err = gac.GetSkipGwAlloc(nw.generic) + if err != nil { + return nil, err + } } // From this point on, we need the network specific configuration, diff --git a/libnetwork/driverapi/driverapi.go b/libnetwork/driverapi/driverapi.go index 0450281b81..a8b7144a9e 100644 --- a/libnetwork/driverapi/driverapi.go +++ b/libnetwork/driverapi/driverapi.go @@ -3,6 +3,8 @@ package driverapi import ( "context" "net" + + "github.com/docker/docker/libnetwork/options" ) // NetworkPluginEndpointType represents the Endpoint Type used by Plugin system @@ -85,6 +87,13 @@ type Driver interface { IsBuiltIn() bool } +// GwAllocChecker is an optional interface for a network driver. +type GwAllocChecker interface { + // GetSkipGwAlloc returns true if the opts describe a network + // that does not need a gateway IPv4/IPv6 address, else false. + GetSkipGwAlloc(opts options.Generic) (skipIPv4, skipIPv6 bool, err error) +} + // NetworkInfo provides a go interface for drivers to provide network // specific information to libnetwork. type NetworkInfo interface { diff --git a/libnetwork/network.go b/libnetwork/network.go index 466eccb21c..607287d955 100644 --- a/libnetwork/network.go +++ b/libnetwork/network.go @@ -206,6 +206,8 @@ type Network struct { configFrom string loadBalancerIP net.IP loadBalancerMode string + skipGwAllocIPv4 bool + skipGwAllocIPv6 bool platformNetwork //nolint:nolintlint,unused // only populated on windows mu sync.Mutex } @@ -466,6 +468,8 @@ func (n *Network) CopyTo(o datastore.KVObject) error { dstN.configFrom = n.configFrom dstN.loadBalancerIP = n.loadBalancerIP dstN.loadBalancerMode = n.loadBalancerMode + dstN.skipGwAllocIPv4 = n.skipGwAllocIPv4 + dstN.skipGwAllocIPv6 = n.skipGwAllocIPv6 // copy labels if dstN.labels == nil { @@ -584,6 +588,8 @@ func (n *Network) MarshalJSON() ([]byte, error) { netMap["configFrom"] = n.configFrom netMap["loadBalancerIP"] = n.loadBalancerIP netMap["loadBalancerMode"] = n.loadBalancerMode + netMap["skipGwAllocIPv4"] = n.skipGwAllocIPv4 + netMap["skipGwAllocIPv6"] = n.skipGwAllocIPv6 return json.Marshal(netMap) } @@ -707,6 +713,12 @@ func (n *Network) UnmarshalJSON(b []byte) (err error) { if v, ok := netMap["loadBalancerMode"]; ok { n.loadBalancerMode = v.(string) } + if v, ok := netMap["skipGwAllocIPv4"]; ok { + n.skipGwAllocIPv4 = v.(bool) + } + if v, ok := netMap["skipGwAllocIPv6"]; ok { + n.skipGwAllocIPv6 = v.(bool) + } return nil } @@ -1515,18 +1527,21 @@ func (n *Network) ipamAllocate() (retErr error) { func (n *Network) ipamAllocateVersion(ipVer int, ipam ipamapi.Ipam) error { var ( - cfgList *[]*IpamConf - infoList *[]*IpamInfo - err error + cfgList *[]*IpamConf + infoList *[]*IpamInfo + skipGwAlloc bool + err error ) switch ipVer { case 4: cfgList = &n.ipamV4Config infoList = &n.ipamV4Info + skipGwAlloc = n.skipGwAllocIPv4 case 6: cfgList = &n.ipamV6Config infoList = &n.ipamV6Info + skipGwAlloc = n.skipGwAllocIPv6 default: return types.InternalErrorf("incorrect ip version passed to ipam allocate: %d", ipVer) } @@ -1587,8 +1602,9 @@ func (n *Network) ipamAllocateVersion(ipVer int, ipam ipamapi.Ipam) error { } } - // If there's still no gateway, reserve cfg.Gateway or let the IPAM driver select an address. - if d.Gateway == nil { + // If there's still no gateway, reserve cfg.Gateway if the user specified it. Else, + // if the driver wants a gateway, let the IPAM driver select an address. + if d.Gateway == nil && (cfg.Gateway != "" || !skipGwAlloc) { gatewayOpts := map[string]string{ ipamapi.RequestAddressType: netlabel.Gateway, } @@ -1654,6 +1670,9 @@ func (n *Network) ipamReleaseVersion(ipVer int, ipam ipamapi.Ipam) { for _, d := range *infoList { if d.Gateway != nil { + // FIXME(robmry) - if an IPAM driver returned a gateway in Meta[netlabel.Gateway], and + // no user config overrode that address, it wasn't explicitly allocated so it shouldn't + // be released here? if err := ipam.ReleaseAddress(d.PoolID, d.Gateway.IP); err != nil { log.G(context.TODO()).Warnf("Failed to release gateway ip address %s on delete of network %s (%s): %v", d.Gateway.IP, n.Name(), n.ID(), err) }