From 17425cff08e844ec414897ecaa8a663315a26dc5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 13:17:56 +0200 Subject: [PATCH 01/10] libnetwork/drivers/macvlan, ipvlan: driver.Join: don't fetch endpoint twice The function was fetching a reference to the endpoint twice; while this did give the option for an early return, in practice it didn't mean much, because it could still fail if the endpoint was removed in between. This code still has a race condition, because while a reference to the endpoint is retrieved while acquiring a lock, the result is mutated without. This probably needs to either have some accessor, or the function should keep a lock for the whole operation (possibly switching to an RWMutex). Signed-off-by: Sebastiaan van Stijn --- daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go | 11 +++++------ .../libnetwork/drivers/macvlan/macvlan_joinleave.go | 11 +++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go index 60950e85f0..aec8947b82 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go @@ -41,10 +41,6 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if err != nil { return err } - endpoint := n.endpoint(eid) - if endpoint == nil { - return fmt.Errorf("could not find endpoint with id %s", eid) - } // generate a name for the iface that will be renamed to eth0 in the sbox containerIfName, err := netutils.GenerateIfaceName(ns.NlHandle(), vethPrefix, vethLen) if err != nil { @@ -55,12 +51,15 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if err != nil { return err } - // bind the generated iface name to the endpoint - endpoint.srcName = vethName ep := n.endpoint(eid) if ep == nil { return fmt.Errorf("could not find endpoint with id %s", eid) } + // bind the generated iface name to the endpoint + // + // TODO(thaJeztah): this should really be done under a lock. + ep.srcName = vethName + if !n.config.Internal { switch n.config.IpvlanMode { case modeL3, modeL3S: diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go index a447591b96..cf3e38bc61 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go @@ -29,10 +29,6 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if err != nil { return err } - endpoint := n.endpoint(eid) - if endpoint == nil { - return fmt.Errorf("could not find endpoint with id %s", eid) - } // generate a name for the iface that will be renamed to eth0 in the sbox containerIfName, err := netutils.GenerateIfaceName(ns.NlHandle(), vethPrefix, vethLen) if err != nil { @@ -43,12 +39,15 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if err != nil { return err } - // bind the generated iface name to the endpoint - endpoint.srcName = vethName ep := n.endpoint(eid) if ep == nil { return fmt.Errorf("could not find endpoint with id %s", eid) } + // bind the generated iface name to the endpoint + // + // TODO(thaJeztah): this should really be done under a lock. + ep.srcName = vethName + // parse and match the endpoint address with the available v4 subnets if !n.config.Internal { if len(n.config.Ipv4Subnets) > 0 { From a29444534525e2f6ba8d29cbe34f9df9b0f16391 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 13:35:25 +0200 Subject: [PATCH 02/10] ibnetwork/drivers/macvlan, ipvlan: align and fix potential panic There were some missing checks whether ep.addr, ep.addrv6 were nil, which could panic in getSubnetForIP. Signed-off-by: Sebastiaan van Stijn --- daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go | 6 +++--- daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go index aec8947b82..f23862d0bb 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go @@ -44,7 +44,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf // generate a name for the iface that will be renamed to eth0 in the sbox containerIfName, err := netutils.GenerateIfaceName(ns.NlHandle(), vethPrefix, vethLen) if err != nil { - return fmt.Errorf("error generating an interface name: %v", err) + return fmt.Errorf("error generating an interface name: %w", err) } // create the netlink ipvlan interface vethName, err := createIPVlan(containerIfName, n.config.Parent, n.config.IpvlanMode, n.config.IpvlanFlag) @@ -90,7 +90,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf } case modeL2: // parse and correlate the endpoint v4 address with the available v4 subnets - if len(n.config.Ipv4Subnets) > 0 { + if ep.addr != nil && len(n.config.Ipv4Subnets) > 0 { s := n.getSubnetforIPv4(ep.addr) if s == nil { return fmt.Errorf("could not find a valid ipv4 subnet for endpoint %s", eid) @@ -115,7 +115,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf ep.addr.IP.String(), s.GwIP, n.config.IpvlanMode, n.config.Parent) } // parse and correlate the endpoint v6 address with the available v6 subnets - if len(n.config.Ipv6Subnets) > 0 { + if ep.addrv6 != nil && len(n.config.Ipv6Subnets) > 0 { s := n.getSubnetforIPv6(ep.addrv6) if s == nil { return fmt.Errorf("could not find a valid ipv6 subnet for endpoint %s", eid) diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go index cf3e38bc61..339e8cd5e2 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go @@ -32,7 +32,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf // generate a name for the iface that will be renamed to eth0 in the sbox containerIfName, err := netutils.GenerateIfaceName(ns.NlHandle(), vethPrefix, vethLen) if err != nil { - return fmt.Errorf("error generating an interface name: %s", err) + return fmt.Errorf("error generating an interface name: %w", err) } // create the netlink macvlan interface vethName, err := createMacVlan(containerIfName, n.config.Parent, n.config.MacvlanMode) @@ -48,9 +48,9 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf // TODO(thaJeztah): this should really be done under a lock. ep.srcName = vethName - // parse and match the endpoint address with the available v4 subnets if !n.config.Internal { - if len(n.config.Ipv4Subnets) > 0 { + // parse and correlate the endpoint v4 address with the available v4 subnets + if ep.addr != nil && len(n.config.Ipv4Subnets) > 0 { s := n.getSubnetforIPv4(ep.addr) if s == nil { return fmt.Errorf("could not find a valid ipv4 subnet for endpoint %s", eid) @@ -74,7 +74,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf log.G(ctx).Debugf("Macvlan Endpoint Joined with IPv4_Addr: %s, Gateway: %s, MacVlan_Mode: %s, Parent: %s", ep.addr.IP.String(), s.GwIP, n.config.MacvlanMode, n.config.Parent) } - // parse and match the endpoint address with the available v6 subnets + // parse and correlate the endpoint v6 address with the available v6 subnets if ep.addrv6 != nil && len(n.config.Ipv6Subnets) > 0 { s := n.getSubnetforIPv6(ep.addrv6) if s == nil { From bf7277f8fe4df3b7b354ac6f887147d76ee06ab6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 13:48:29 +0200 Subject: [PATCH 03/10] libnetwork/drivers/macvlan, ipvlan: remove getSubnetforIPv6, getSubnetforIPv4 These methods were just wrappers around getSubnetforIP; let's peel away the abstraction and call it directly; we're already checking for n.config.Ipv4Subnet and n.config.Ipv6Subnets on the call-site, so may as well just pass it in. Signed-off-by: Sebastiaan van Stijn --- .../libnetwork/drivers/ipvlan/ipvlan_joinleave.go | 15 +++------------ .../drivers/macvlan/macvlan_joinleave.go | 15 +++------------ 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go index f23862d0bb..c43cc39c3e 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go @@ -91,7 +91,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf case modeL2: // parse and correlate the endpoint v4 address with the available v4 subnets if ep.addr != nil && len(n.config.Ipv4Subnets) > 0 { - s := n.getSubnetforIPv4(ep.addr) + s := getSubnetForIP(ep.addr, n.config.Ipv4Subnets) if s == nil { return fmt.Errorf("could not find a valid ipv4 subnet for endpoint %s", eid) } @@ -116,7 +116,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf } // parse and correlate the endpoint v6 address with the available v6 subnets if ep.addrv6 != nil && len(n.config.Ipv6Subnets) > 0 { - s := n.getSubnetforIPv6(ep.addrv6) + s := getSubnetForIP(ep.addrv6, n.config.Ipv6Subnets) if s == nil { return fmt.Errorf("could not find a valid ipv6 subnet for endpoint %s", eid) } @@ -195,16 +195,7 @@ func ifaceGateway(dfNet string) (*staticRoute, error) { return defaultRoute, nil } -// getSubnetforIPv4 returns the ipv4 subnet to which the given IP belongs -func (n *network) getSubnetforIPv4(ip *net.IPNet) *ipSubnet { - return getSubnetForIP(ip, n.config.Ipv4Subnets) -} - -// getSubnetforIPv6 returns the ipv6 subnet to which the given IP belongs -func (n *network) getSubnetforIPv6(ip *net.IPNet) *ipSubnet { - return getSubnetForIP(ip, n.config.Ipv6Subnets) -} - +// getSubnetForIP returns the (IPv4 or IPv6) subnet to which the given IP belongs. func getSubnetForIP(ip *net.IPNet, subnets []*ipSubnet) *ipSubnet { for _, s := range subnets { _, snet, err := net.ParseCIDR(s.SubnetIP) diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go index 339e8cd5e2..cd339ddfc1 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go @@ -51,7 +51,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if !n.config.Internal { // parse and correlate the endpoint v4 address with the available v4 subnets if ep.addr != nil && len(n.config.Ipv4Subnets) > 0 { - s := n.getSubnetforIPv4(ep.addr) + s := getSubnetForIP(ep.addr, n.config.Ipv4Subnets) if s == nil { return fmt.Errorf("could not find a valid ipv4 subnet for endpoint %s", eid) } @@ -76,7 +76,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf } // parse and correlate the endpoint v6 address with the available v6 subnets if ep.addrv6 != nil && len(n.config.Ipv6Subnets) > 0 { - s := n.getSubnetforIPv6(ep.addrv6) + s := getSubnetForIP(ep.addrv6, n.config.Ipv6Subnets) if s == nil { return fmt.Errorf("could not find a valid ipv6 subnet for endpoint %s", eid) } @@ -139,16 +139,7 @@ func (d *driver) Leave(nid, eid string) error { return nil } -// getSubnetforIPv4 returns the ipv4 subnet to which the given IP belongs -func (n *network) getSubnetforIPv4(ip *net.IPNet) *ipSubnet { - return getSubnetForIP(ip, n.config.Ipv4Subnets) -} - -// getSubnetforIPv6 returns the ipv6 subnet to which the given IP belongs -func (n *network) getSubnetforIPv6(ip *net.IPNet) *ipSubnet { - return getSubnetForIP(ip, n.config.Ipv6Subnets) -} - +// getSubnetForIP returns the (IPv4 or IPv6) subnet to which the given IP belongs. func getSubnetForIP(ip *net.IPNet, subnets []*ipSubnet) *ipSubnet { for _, s := range subnets { _, snet, err := net.ParseCIDR(s.SubnetIP) From aec6e7f7b6cd7bb4cb59becf855969894ffc2142 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 13:51:50 +0200 Subject: [PATCH 04/10] libnetwork/drivers/macvlan, ipvlan: remove networkTable, endpointTable These types were just a straight map[string]XXX, with no methods or other properties attached. Signed-off-by: Sebastiaan van Stijn --- daemon/libnetwork/drivers/ipvlan/ipvlan.go | 10 +++------- daemon/libnetwork/drivers/ipvlan/ipvlan_network.go | 2 +- daemon/libnetwork/drivers/macvlan/macvlan.go | 10 +++------- daemon/libnetwork/drivers/macvlan/macvlan_network.go | 2 +- 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan.go b/daemon/libnetwork/drivers/ipvlan/ipvlan.go index cde2312351..523bb7d314 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan.go @@ -30,12 +30,8 @@ const ( flagVepa = "vepa" // ipvlan flag vepa ) -type endpointTable map[string]*endpoint - -type networkTable map[string]*network - type driver struct { - networks networkTable + networks map[string]*network sync.Once sync.Mutex store *datastore.Store @@ -54,7 +50,7 @@ type endpoint struct { type network struct { id string - endpoints endpointTable + endpoints map[string]*endpoint driver *driver config *configuration sync.Mutex @@ -64,7 +60,7 @@ type network struct { func Register(r driverapi.Registerer, store *datastore.Store) error { d := &driver{ store: store, - networks: networkTable{}, + networks: map[string]*network{}, } if err := d.initStore(); err != nil { return err diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go index 6238bf68a4..c19870ac04 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go @@ -113,7 +113,7 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { n := &network{ id: config.ID, driver: d, - endpoints: endpointTable{}, + endpoints: map[string]*endpoint{}, config: config, } // add the network diff --git a/daemon/libnetwork/drivers/macvlan/macvlan.go b/daemon/libnetwork/drivers/macvlan/macvlan.go index 0e6afa32ec..049439576c 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan.go @@ -24,12 +24,8 @@ const ( driverModeOpt = "macvlan_mode" // macvlan mode ux opt suffix ) -type endpointTable map[string]*endpoint - -type networkTable map[string]*network - type driver struct { - networks networkTable + networks map[string]*network sync.Once sync.Mutex store *datastore.Store @@ -48,7 +44,7 @@ type endpoint struct { type network struct { id string - endpoints endpointTable + endpoints map[string]*endpoint driver *driver config *configuration sync.Mutex @@ -58,7 +54,7 @@ type network struct { func Register(r driverapi.Registerer, store *datastore.Store) error { d := &driver{ store: store, - networks: networkTable{}, + networks: map[string]*network{}, } if err := d.initStore(); err != nil { return err diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_network.go b/daemon/libnetwork/drivers/macvlan/macvlan_network.go index 08f62f4fb8..460d2f0c5c 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_network.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_network.go @@ -131,7 +131,7 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { d.addNetwork(&network{ id: config.ID, driver: d, - endpoints: endpointTable{}, + endpoints: map[string]*endpoint{}, config: config, }) } From d481c09fa77053b789a0d4e8f94438b0482d8953 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 18 Oct 2025 20:26:36 +0200 Subject: [PATCH 05/10] libnetwork/drivers/macvlan, ipvlan: make driver.leave a stub These drivers did not do anything meaningful in the `Leave` method; they would check if the network and/or endpoint were missing, in which case they produced an error, but the network and endpoint (if present) would not be used, so it was only validation. Such validation could still be relevant elsewhere, but looking at where this method is called; the `Driver.Leave()` is called in two places, both of which don't handle the error, other than logging it as a warning / error; It's called by `Endpoint.sbJoin()`, as part of the rollback; https://github.com/moby/moby/blob/d5c838dc5ee1a692c2fb6cde6da6fa185e7538aa/daemon/libnetwork/endpoint.go#L539-L545 And `Endpoint.sbLeave()`, which also discards the error; https://github.com/moby/moby/blob/d5c838dc5ee1a692c2fb6cde6da6fa185e7538aa/daemon/libnetwork/endpoint.go#L772-L776 Based on he above, this code looks to be redundant, so replacing it with a stub; returning `nil`. As replacing the code removed the use of network.getEndpoint, which was effectively a copy of network.endpoint (which didn't have error handling), I merged the two methods, and removed custom error-handling elsewhere. Signed-off-by: Sebastiaan van Stijn --- .../drivers/ipvlan/ipvlan_endpoint.go | 6 ++--- .../drivers/ipvlan/ipvlan_joinleave.go | 18 +++---------- .../libnetwork/drivers/ipvlan/ipvlan_state.go | 25 +++++++------------ .../drivers/macvlan/macvlan_endpoint.go | 6 ++--- .../drivers/macvlan/macvlan_joinleave.go | 18 +++---------- .../drivers/macvlan/macvlan_state.go | 25 +++++++------------ daemon/libnetwork/endpoint.go | 4 +-- 7 files changed, 32 insertions(+), 70 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_endpoint.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_endpoint.go index 728ebb3b0e..a4e3fa7bcb 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_endpoint.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_endpoint.go @@ -68,9 +68,9 @@ func (d *driver) DeleteEndpoint(nid, eid string) error { if n == nil { return fmt.Errorf("network id %q not found", nid) } - ep := n.endpoint(eid) - if ep == nil { - return fmt.Errorf("endpoint id %q not found", eid) + ep, err := n.endpoint(eid) + if err != nil { + return err } if link, err := ns.NlHandle().LinkByName(ep.srcName); err == nil { if err := ns.NlHandle().LinkDel(link); err != nil { diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go index c43cc39c3e..78fec45806 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go @@ -51,9 +51,9 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if err != nil { return err } - ep := n.endpoint(eid) - if ep == nil { - return fmt.Errorf("could not find endpoint with id %s", eid) + ep, err := n.endpoint(eid) + if err != nil { + return err } // bind the generated iface name to the endpoint // @@ -165,18 +165,6 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf // Leave method is invoked when a Sandbox detaches from an endpoint. func (d *driver) Leave(nid, eid string) error { - network, err := d.getNetwork(nid) - if err != nil { - return err - } - endpoint, err := network.getEndpoint(eid) - if err != nil { - return err - } - if endpoint == nil { - return fmt.Errorf("could not find endpoint with id %s", eid) - } - return nil } diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go index a67ca15cc4..d4b2066de3 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go @@ -5,7 +5,6 @@ package ipvlan import ( "context" "errors" - "fmt" "github.com/containerd/log" "github.com/moby/moby/v2/daemon/libnetwork/types" @@ -47,11 +46,18 @@ func (d *driver) getNetworks() []*network { return ls } -func (n *network) endpoint(eid string) *endpoint { +func (n *network) endpoint(eid string) (*endpoint, error) { + if eid == "" { + return nil, errors.New("invalid endpoint id") + } n.Lock() defer n.Unlock() - return n.endpoints[eid] + ep, ok := n.endpoints[eid] + if !ok || ep == nil { + return nil, errors.New("could not find endpoint with id " + eid) + } + return ep, nil } func (n *network) addEndpoint(ep *endpoint) { @@ -66,19 +72,6 @@ func (n *network) deleteEndpoint(eid string) { n.Unlock() } -func (n *network) getEndpoint(eid string) (*endpoint, error) { - n.Lock() - defer n.Unlock() - if eid == "" { - return nil, fmt.Errorf("endpoint id %s not found", eid) - } - if ep, ok := n.endpoints[eid]; ok { - return ep, nil - } - - return nil, nil -} - func validateID(nid, eid string) error { if nid == "" { return errors.New("invalid network id") diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_endpoint.go b/daemon/libnetwork/drivers/macvlan/macvlan_endpoint.go index 5cfab2e165..6ef71470f1 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_endpoint.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_endpoint.go @@ -72,9 +72,9 @@ func (d *driver) DeleteEndpoint(nid, eid string) error { if n == nil { return fmt.Errorf("network id %q not found", nid) } - ep := n.endpoint(eid) - if ep == nil { - return fmt.Errorf("endpoint id %q not found", eid) + ep, err := n.endpoint(eid) + if err != nil { + return err } if link, err := ns.NlHandle().LinkByName(ep.srcName); err == nil { if err := ns.NlHandle().LinkDel(link); err != nil { diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go index cd339ddfc1..a41edeb7c0 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go @@ -39,9 +39,9 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if err != nil { return err } - ep := n.endpoint(eid) - if ep == nil { - return fmt.Errorf("could not find endpoint with id %s", eid) + ep, err := n.endpoint(eid) + if err != nil { + return err } // bind the generated iface name to the endpoint // @@ -124,18 +124,6 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf // Leave method is invoked when a Sandbox detaches from an endpoint. func (d *driver) Leave(nid, eid string) error { - network, err := d.getNetwork(nid) - if err != nil { - return err - } - endpoint, err := network.getEndpoint(eid) - if err != nil { - return err - } - if endpoint == nil { - return fmt.Errorf("could not find endpoint with id %s", eid) - } - return nil } diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_state.go b/daemon/libnetwork/drivers/macvlan/macvlan_state.go index 5700731eac..ad52d683e0 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_state.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_state.go @@ -5,7 +5,6 @@ package macvlan import ( "context" "errors" - "fmt" "github.com/containerd/log" "github.com/moby/moby/v2/daemon/libnetwork/types" @@ -47,11 +46,18 @@ func (d *driver) getNetworks() []*network { return ls } -func (n *network) endpoint(eid string) *endpoint { +func (n *network) endpoint(eid string) (*endpoint, error) { + if eid == "" { + return nil, errors.New("invalid endpoint id") + } n.Lock() defer n.Unlock() - return n.endpoints[eid] + ep, ok := n.endpoints[eid] + if !ok || ep == nil { + return nil, errors.New("could not find endpoint with id " + eid) + } + return ep, nil } func (n *network) addEndpoint(ep *endpoint) { @@ -66,19 +72,6 @@ func (n *network) deleteEndpoint(eid string) { n.Unlock() } -func (n *network) getEndpoint(eid string) (*endpoint, error) { - n.Lock() - defer n.Unlock() - if eid == "" { - return nil, fmt.Errorf("endpoint id %s not found", eid) - } - if ep, ok := n.endpoints[eid]; ok { - return ep, nil - } - - return nil, nil -} - func validateID(nid, eid string) error { if nid == "" { return errors.New("invalid network id") diff --git a/daemon/libnetwork/endpoint.go b/daemon/libnetwork/endpoint.go index a8f13ad2a2..90ac96111c 100644 --- a/daemon/libnetwork/endpoint.go +++ b/daemon/libnetwork/endpoint.go @@ -538,8 +538,8 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint } defer func() { if retErr != nil { - if e := d.Leave(nid, epid); e != nil { - log.G(ctx).Warnf("driver leave failed while rolling back join: %v", e) + if err := d.Leave(nid, epid); err != nil { + log.G(ctx).WithError(err).Warnf("driver leave failed while rolling back join") } } }() From 5276dd8e9a63f28df902da7d9078ce6f30509164 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 18 Oct 2025 20:37:03 +0200 Subject: [PATCH 06/10] libnetwork/drivers/ipvlan: remove ifaceGateway utility This utility was only called with two constant values; const ( defaultV4RouteCidr = "0.0.0.0/0" defaultV6RouteCidr = "::/0" ) However; - calling it would always execute a `net.ParseCIDR` - verify if it would produce an error (which would be very unlikely) - it used a `staticRoute` struct that was ONLY used for this function - and immediately deconstructed into its components - furthermore, the `NextHop` field would be discarded by jinfo.AddStaticRoute, which only used the third argument for `routeType == types.NEXTHOP` This patch: - removes the `ifaceGateway` and associated `staticRoute` and consts - defines two package-level vars for `defaultV4Net` and `defaultV6Net`, which can be reused (no need to parse / construct them for every join) Signed-off-by: Sebastiaan van Stijn --- .../drivers/ipvlan/ipvlan_joinleave.go | 39 +++---------------- 1 file changed, 5 insertions(+), 34 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go index 78fec45806..f376de8a3a 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go @@ -18,15 +18,9 @@ import ( "go.opentelemetry.io/otel/trace" ) -type staticRoute struct { - Destination *net.IPNet - RouteType types.RouteType - NextHop net.IP -} - -const ( - defaultV4RouteCidr = "0.0.0.0/0" - defaultV6RouteCidr = "::/0" +var ( + defaultV4Net = &net.IPNet{IP: net.IPv4zero, Mask: net.CIDRMask(0, 32)} // "0.0.0.0/0" + defaultV6Net = &net.IPNet{IP: net.IPv6zero, Mask: net.CIDRMask(0, 128)} // "::/0" ) // Join method is invoked when a Sandbox is attached to an endpoint. @@ -66,11 +60,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf // disable gateway services to add a default gw using dev eth0 only jinfo.DisableGatewayService() if ep.addr != nil { - defaultRoute, err := ifaceGateway(defaultV4RouteCidr) - if err != nil { - return err - } - if err := jinfo.AddStaticRoute(defaultRoute.Destination, defaultRoute.RouteType, defaultRoute.NextHop); err != nil { + if err := jinfo.AddStaticRoute(defaultV4Net, types.CONNECTED, nil); err != nil { return fmt.Errorf("failed to set an ipvlan l3/l3s mode ipv4 default gateway: %v", err) } log.G(ctx).Debugf("Ipvlan Endpoint Joined with IPv4_Addr: %s, Ipvlan_Mode: %s, Parent: %s", @@ -78,11 +68,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf } // If the endpoint has a v6 address, set a v6 default route if ep.addrv6 != nil { - default6Route, err := ifaceGateway(defaultV6RouteCidr) - if err != nil { - return err - } - if err = jinfo.AddStaticRoute(default6Route.Destination, default6Route.RouteType, default6Route.NextHop); err != nil { + if err := jinfo.AddStaticRoute(defaultV6Net, types.CONNECTED, nil); err != nil { return fmt.Errorf("failed to set an ipvlan l3/l3s mode ipv6 default gateway: %v", err) } log.G(ctx).Debugf("Ipvlan Endpoint Joined with IPv6_Addr: %s, Ipvlan_Mode: %s, Parent: %s", @@ -168,21 +154,6 @@ func (d *driver) Leave(nid, eid string) error { return nil } -// ifaceGateway returns a static route for either v4/v6 to be set to the container eth0 -func ifaceGateway(dfNet string) (*staticRoute, error) { - nh, dst, err := net.ParseCIDR(dfNet) - if err != nil { - return nil, fmt.Errorf("unable to parse default route %v", err) - } - defaultRoute := &staticRoute{ - Destination: dst, - RouteType: types.CONNECTED, - NextHop: nh, - } - - return defaultRoute, nil -} - // getSubnetForIP returns the (IPv4 or IPv6) subnet to which the given IP belongs. func getSubnetForIP(ip *net.IPNet, subnets []*ipSubnet) *ipSubnet { for _, s := range subnets { From a2f4f09f91d1e35970347a7cbbc090528071fb27 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 14:02:59 +0200 Subject: [PATCH 07/10] libnetwork/drivers/macvlan, ipvlan: remove unused sync.Once Both were added as part of the initial implementation in commit [moby@ea30113] ([libnetwork@1d6f2c5]), but never used. [moby@ea30113]: https://github.com/moby/moby/commit/ea301133039d9852455893a4ada1ab887ebe8459 [libnetwork@1d6f2c5]: https://github.com/moby/libnetwork/commit/1d6f2c59c46fa6533d8cbf3d648f001a5da7c19c Signed-off-by: Sebastiaan van Stijn --- daemon/libnetwork/drivers/ipvlan/ipvlan.go | 1 - daemon/libnetwork/drivers/macvlan/macvlan.go | 1 - 2 files changed, 2 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan.go b/daemon/libnetwork/drivers/ipvlan/ipvlan.go index 523bb7d314..72f4c42bff 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan.go @@ -32,7 +32,6 @@ const ( type driver struct { networks map[string]*network - sync.Once sync.Mutex store *datastore.Store } diff --git a/daemon/libnetwork/drivers/macvlan/macvlan.go b/daemon/libnetwork/drivers/macvlan/macvlan.go index 049439576c..b0e54c85b6 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan.go @@ -26,7 +26,6 @@ const ( type driver struct { networks map[string]*network - sync.Once sync.Mutex store *datastore.Store } From 65296cd0e7b9b77f46cb549f27cd08162fe848b8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 14:48:06 +0200 Subject: [PATCH 08/10] libnetwork/drivers/macvlan, ipvlan: un-embed mutexes Signed-off-by: Sebastiaan van Stijn --- daemon/libnetwork/drivers/ipvlan/ipvlan.go | 16 ++++--- .../libnetwork/drivers/ipvlan/ipvlan_state.go | 42 +++++++++--------- daemon/libnetwork/drivers/macvlan/macvlan.go | 16 ++++--- .../drivers/macvlan/macvlan_state.go | 43 ++++++++++--------- 4 files changed, 63 insertions(+), 54 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan.go b/daemon/libnetwork/drivers/ipvlan/ipvlan.go index 72f4c42bff..bbf1bb3759 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan.go @@ -31,9 +31,11 @@ const ( ) type driver struct { - networks map[string]*network - sync.Mutex store *datastore.Store + + // mu protects the networks map. + mu sync.Mutex + networks map[string]*network } type endpoint struct { @@ -48,11 +50,13 @@ type endpoint struct { } type network struct { - id string + id string + driver *driver + config *configuration + + // mu protects the endpoints map. + mu sync.Mutex endpoints map[string]*endpoint - driver *driver - config *configuration - sync.Mutex } // Register initializes and registers the libnetwork ipvlan driver. diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go index d4b2066de3..f9c3253b52 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go @@ -11,9 +11,9 @@ import ( ) func (d *driver) network(nid string) *network { - d.Lock() + d.mu.Lock() n, ok := d.networks[nid] - d.Unlock() + d.mu.Unlock() if !ok { log.G(context.TODO()).Errorf("network id %s not found", nid) } @@ -22,21 +22,21 @@ func (d *driver) network(nid string) *network { } func (d *driver) addNetwork(n *network) { - d.Lock() + d.mu.Lock() d.networks[n.id] = n - d.Unlock() + d.mu.Unlock() } func (d *driver) deleteNetwork(nid string) { - d.Lock() + d.mu.Lock() delete(d.networks, nid) - d.Unlock() + d.mu.Unlock() } // getNetworks Safely returns a slice of existing networks func (d *driver) getNetworks() []*network { - d.Lock() - defer d.Unlock() + d.mu.Lock() + defer d.mu.Unlock() ls := make([]*network, 0, len(d.networks)) for _, nw := range d.networks { @@ -50,8 +50,8 @@ func (n *network) endpoint(eid string) (*endpoint, error) { if eid == "" { return nil, errors.New("invalid endpoint id") } - n.Lock() - defer n.Unlock() + n.mu.Lock() + defer n.mu.Unlock() ep, ok := n.endpoints[eid] if !ok || ep == nil { @@ -61,15 +61,15 @@ func (n *network) endpoint(eid string) (*endpoint, error) { } func (n *network) addEndpoint(ep *endpoint) { - n.Lock() + n.mu.Lock() n.endpoints[ep.id] = ep - n.Unlock() + n.mu.Unlock() } func (n *network) deleteEndpoint(eid string) { - n.Lock() + n.mu.Lock() delete(n.endpoints, eid) - n.Unlock() + n.mu.Unlock() } func validateID(nid, eid string) error { @@ -84,15 +84,15 @@ func validateID(nid, eid string) error { } func (d *driver) getNetwork(id string) (*network, error) { - d.Lock() - defer d.Unlock() if id == "" { - return nil, types.InvalidParameterErrorf("invalid network id: %s", id) + return nil, types.InvalidParameterErrorf("invalid network id") } - if nw, ok := d.networks[id]; ok { - return nw, nil + d.mu.Lock() + defer d.mu.Unlock() + nw, ok := d.networks[id] + if !ok || nw == nil { + return nil, types.NotFoundErrorf("network not found: %s", id) } - - return nil, types.NotFoundErrorf("network not found: %s", id) + return nw, nil } diff --git a/daemon/libnetwork/drivers/macvlan/macvlan.go b/daemon/libnetwork/drivers/macvlan/macvlan.go index b0e54c85b6..4103a1c588 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan.go @@ -25,9 +25,11 @@ const ( ) type driver struct { - networks map[string]*network - sync.Mutex store *datastore.Store + + // mu protects the networks map. + mu sync.Mutex + networks map[string]*network } type endpoint struct { @@ -42,11 +44,13 @@ type endpoint struct { } type network struct { - id string + id string + driver *driver + config *configuration + + // mu protects the endpoints map. + mu sync.Mutex endpoints map[string]*endpoint - driver *driver - config *configuration - sync.Mutex } // Register initializes and registers the libnetwork macvlan driver diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_state.go b/daemon/libnetwork/drivers/macvlan/macvlan_state.go index ad52d683e0..5faab1eb8f 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_state.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_state.go @@ -11,9 +11,9 @@ import ( ) func (d *driver) network(nid string) *network { - d.Lock() + d.mu.Lock() n, ok := d.networks[nid] - d.Unlock() + d.mu.Unlock() if !ok { log.G(context.TODO()).Errorf("network id %s not found", nid) } @@ -22,21 +22,21 @@ func (d *driver) network(nid string) *network { } func (d *driver) addNetwork(n *network) { - d.Lock() + d.mu.Lock() d.networks[n.id] = n - d.Unlock() + d.mu.Unlock() } func (d *driver) deleteNetwork(nid string) { - d.Lock() + d.mu.Lock() delete(d.networks, nid) - d.Unlock() + d.mu.Unlock() } // getNetworks Safely returns a slice of existing networks func (d *driver) getNetworks() []*network { - d.Lock() - defer d.Unlock() + d.mu.Lock() + defer d.mu.Unlock() ls := make([]*network, 0, len(d.networks)) for _, nw := range d.networks { @@ -50,8 +50,8 @@ func (n *network) endpoint(eid string) (*endpoint, error) { if eid == "" { return nil, errors.New("invalid endpoint id") } - n.Lock() - defer n.Unlock() + n.mu.Lock() + defer n.mu.Unlock() ep, ok := n.endpoints[eid] if !ok || ep == nil { @@ -61,15 +61,15 @@ func (n *network) endpoint(eid string) (*endpoint, error) { } func (n *network) addEndpoint(ep *endpoint) { - n.Lock() + n.mu.Lock() n.endpoints[ep.id] = ep - n.Unlock() + n.mu.Unlock() } func (n *network) deleteEndpoint(eid string) { - n.Lock() + n.mu.Lock() delete(n.endpoints, eid) - n.Unlock() + n.mu.Unlock() } func validateID(nid, eid string) error { @@ -83,14 +83,15 @@ func validateID(nid, eid string) error { } func (d *driver) getNetwork(id string) (*network, error) { - d.Lock() - defer d.Unlock() if id == "" { - return nil, types.InvalidParameterErrorf("invalid network id: %s", id) - } - if nw, ok := d.networks[id]; ok { - return nw, nil + return nil, types.InvalidParameterErrorf("invalid network id") } - return nil, types.NotFoundErrorf("network not found: %s", id) + d.mu.Lock() + defer d.mu.Unlock() + nw, ok := d.networks[id] + if !ok || nw == nil { + return nil, types.NotFoundErrorf("network not found: %s", id) + } + return nw, nil } From a013147c40ed9cc8707dd0239608970ea3a3a56f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 15:20:42 +0200 Subject: [PATCH 09/10] libnetwork/drivers/macvlan: parentHasSingleUser: don't create copy of networks This function was calling driver.getNetworks, which copies the networks map into a new slice. As we're not mutating the networks, we can just use the networks map itself to check if there's any networks configured with the same parent. While changing; - Also change the signature to accept the parent to compare to as a string - Return early once we determined there's more than one user Signed-off-by: Sebastiaan van Stijn --- .../drivers/macvlan/macvlan_network.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_network.go b/daemon/libnetwork/drivers/macvlan/macvlan_network.go index 460d2f0c5c..d5a112c43b 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_network.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_network.go @@ -139,14 +139,20 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { return foundExisting, nil } -func (d *driver) parentHasSingleUser(n *network) bool { +func (d *driver) parentHasSingleUser(parent string) bool { + d.mu.Lock() + defer d.mu.Unlock() + users := 0 - networkList := d.getNetworks() - for _, testN := range networkList { - if n.config.Parent == testN.config.Parent { + for _, nw := range d.networks { + if nw.config.Parent == parent { users++ } + if users > 1 { + return false + } } + // TODO(thaJeztah): "zero users" should also return "true?" (this would be theoretical as we're checking a network to be the last remaining user) return users == 1 } @@ -157,7 +163,7 @@ func (d *driver) DeleteNetwork(nid string) error { return fmt.Errorf("network id %s not found", nid) } // if the driver created the slave interface and this network is the last user, delete it, otherwise leave it - if n.config.CreatedSlaveLink && parentExists(n.config.Parent) && d.parentHasSingleUser(n) { + if n.config.CreatedSlaveLink && parentExists(n.config.Parent) && d.parentHasSingleUser(n.config.Parent) { // only delete the link if it is named the net_id if n.config.Parent == getDummyName(nid) { err := delDummyLink(n.config.Parent) From 10faa629feaf864b512ce6b01a66ef2355982e9a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 19 Oct 2025 15:31:14 +0200 Subject: [PATCH 10/10] libnetwork/drivers/macvlan, ipvlan: assorted minor cleanups - Inline some vars and align between drivers - Remove nested if's where possible - Use `WithError` for some logs, and use the context if available - Scope variables locally where only used locally and, the reverse, make it clear where a (function-)global variable is used. Signed-off-by: Sebastiaan van Stijn --- .../drivers/ipvlan/ipvlan_joinleave.go | 2 +- .../drivers/ipvlan/ipvlan_network.go | 53 ++++++++----------- .../drivers/macvlan/macvlan_network.go | 31 +++++------ 3 files changed, 36 insertions(+), 50 deletions(-) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go index f376de8a3a..eff1b875ed 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go @@ -142,7 +142,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf if err != nil { return err } - if err = d.storeUpdate(ep); err != nil { + if err := d.storeUpdate(ep); err != nil { return fmt.Errorf("failed to save ipvlan endpoint %.7s to store: %v", ep.id, err) } diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go index c19870ac04..9619ac940a 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go +++ b/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go @@ -64,7 +64,7 @@ func (d *driver) CreateNetwork(ctx context.Context, nid string, option map[strin err = d.storeUpdate(config) if err != nil { d.deleteNetwork(config.ID) - log.G(context.TODO()).Debugf("encountered an error rolling back a network create for %s : %v", config.ID, err) + log.G(ctx).Debugf("encountered an error rolling back a network create for %s : %v", config.ID, err) return err } @@ -110,14 +110,12 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { } } if !foundExisting { - n := &network{ + d.addNetwork(&network{ id: config.ID, driver: d, endpoints: map[string]*endpoint{}, config: config, - } - // add the network - d.addNetwork(n) + }) } return foundExisting, nil @@ -138,23 +136,17 @@ func (d *driver) DeleteNetwork(nid string) error { return fmt.Errorf("network id %s not found", nid) } // if the driver created the slave interface, delete it, otherwise leave it - if ok := n.config.CreatedSlaveLink; ok { - // if the interface exists, only delete if it matches iface.vlan or dummy.net_id naming - if ok := parentExists(n.config.Parent); ok { - // only delete the link if it is named the net_id - if n.config.Parent == getDummyName(nid) { - err := delDummyLink(n.config.Parent) - if err != nil { - log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", - n.config.Parent, err) - } - } else { - // only delete the link if it matches iface.vlan naming - err := delVlanLink(n.config.Parent) - if err != nil { - log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", - n.config.Parent, err) - } + // if the interface exists, only delete if it matches iface.vlan or dummy.net_id naming + if n.config.CreatedSlaveLink && parentExists(n.config.Parent) { + // only delete the link if it is named the net_id + if n.config.Parent == getDummyName(nid) { + if err := delDummyLink(n.config.Parent); err != nil { + log.G(context.TODO()).WithError(err).Debugf("link %s was not deleted, continuing the delete network operation", n.config.Parent) + } + } else { + // only delete the link if it matches iface.vlan naming + if err := delVlanLink(n.config.Parent); err != nil { + log.G(context.TODO()).WithError(err).Debugf("link %s was not deleted, continuing the delete network operation", n.config.Parent) } } } @@ -166,14 +158,13 @@ func (d *driver) DeleteNetwork(nid string) error { } if err := d.storeDelete(ep); err != nil { - log.G(context.TODO()).Warnf("Failed to remove ipvlan endpoint %.7s from store: %v", ep.id, err) + log.G(context.TODO()).WithError(err).Warnf("Failed to remove ipvlan endpoint %.7s from store", ep.id) } } // delete the *network d.deleteNetwork(nid) // delete the network record from persistent cache - err := d.storeDelete(n.config) - if err != nil { + if err := d.storeDelete(n.config); err != nil { return fmt.Errorf("error deleting id %s from datastore: %v", nid, err) } return nil @@ -181,13 +172,13 @@ func (d *driver) DeleteNetwork(nid string) error { // parseNetworkOptions parses docker network options func parseNetworkOptions(id string, option options.Generic) (*configuration, error) { - var ( - err error - config = &configuration{} - ) + var config = &configuration{} + // parse generic labels first if genData, ok := option[netlabel.GenericData]; ok && genData != nil { - if config, err = parseNetworkGenericOptions(genData); err != nil { + var err error + config, err = parseNetworkGenericOptions(genData) + if err != nil { return nil, err } } @@ -233,7 +224,7 @@ func parseNetworkOptions(id string, option options.Generic) (*configuration, err return config, nil } -// parseNetworkGenericOptions parse generic driver docker network options +// parseNetworkGenericOptions parses generic driver docker network options func parseNetworkGenericOptions(data any) (*configuration, error) { switch opt := data.(type) { case *configuration: diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_network.go b/daemon/libnetwork/drivers/macvlan/macvlan_network.go index d5a112c43b..1e2379a78e 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_network.go +++ b/daemon/libnetwork/drivers/macvlan/macvlan_network.go @@ -55,14 +55,14 @@ func (d *driver) CreateNetwork(ctx context.Context, nid string, option map[strin err = d.storeUpdate(config) if err != nil { d.deleteNetwork(config.ID) - log.G(context.TODO()).Debugf("encountered an error rolling back a network create for %s : %v", config.ID, err) + log.G(ctx).Debugf("encountered an error rolling back a network create for %s : %v", config.ID, err) return err } return nil } -func (d *driver) GetSkipGwAlloc(opts options.Generic) (ipv4, ipv6 bool, _ error) { +func (d *driver) GetSkipGwAlloc(options.Generic) (ipv4, ipv6 bool, _ error) { // Only set up a default gateway if the user configured one (the gateway // must be external to the Docker macvlan network, the driver doesn't assign // the address to anything). @@ -166,17 +166,13 @@ func (d *driver) DeleteNetwork(nid string) error { if n.config.CreatedSlaveLink && parentExists(n.config.Parent) && d.parentHasSingleUser(n.config.Parent) { // only delete the link if it is named the net_id if n.config.Parent == getDummyName(nid) { - err := delDummyLink(n.config.Parent) - if err != nil { - log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", - n.config.Parent, err) + if err := delDummyLink(n.config.Parent); err != nil { + log.G(context.TODO()).WithError(err).Debugf("link %s was not deleted, continuing the delete network operation", n.config.Parent) } } else { // only delete the link if it matches iface.vlan naming - err := delVlanLink(n.config.Parent) - if err != nil { - log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", - n.config.Parent, err) + if err := delVlanLink(n.config.Parent); err != nil { + log.G(context.TODO()).WithError(err).Debugf("link %s was not deleted, continuing the delete network operation", n.config.Parent) } } } @@ -188,14 +184,13 @@ func (d *driver) DeleteNetwork(nid string) error { } if err := d.storeDelete(ep); err != nil { - log.G(context.TODO()).Warnf("Failed to remove macvlan endpoint %.7s from store: %v", ep.id, err) + log.G(context.TODO()).WithError(err).Warnf("Failed to remove macvlan endpoint %.7s from store", ep.id) } } // delete the *network d.deleteNetwork(nid) // delete the network record from persistent cache - err := d.storeDelete(n.config) - if err != nil { + if err := d.storeDelete(n.config); err != nil { return fmt.Errorf("error deleting id %s from datastore: %v", nid, err) } return nil @@ -203,13 +198,13 @@ func (d *driver) DeleteNetwork(nid string) error { // parseNetworkOptions parses docker network options func parseNetworkOptions(id string, option options.Generic) (*configuration, error) { - var ( - err error - config = &configuration{} - ) + var config = &configuration{} + // parse generic labels first if genData, ok := option[netlabel.GenericData]; ok && genData != nil { - if config, err = parseNetworkGenericOptions(genData); err != nil { + var err error + config, err = parseNetworkGenericOptions(genData) + if err != nil { return nil, err } }