Merge pull request #51217 from thaJeztah/hotel_california

libnetwork/drivers/macvlan, ipvlan: remove some redundant code, and cleanups / fixes

daemon/libnetwork/drivers/ipvlan/ipvlan.go

@@ -30,15 +30,12 @@ const (
 	flagVepa    = "vepa"    // ipvlan flag vepa
 )
 
-type endpointTable map[string]*endpoint
-
-type networkTable map[string]*network
-
 type driver struct {
-	networks networkTable
-	sync.Once
-	sync.Mutex
 	store *datastore.Store
+
+	// mu protects the networks map.
+	mu       sync.Mutex
+	networks map[string]*network
 }
 
 type endpoint struct {
@@ -53,18 +50,20 @@ type endpoint struct {
 }
 
 type network struct {
-	id        string
-	endpoints endpointTable
-	driver    *driver
-	config    *configuration
-	sync.Mutex
+	id     string
+	driver *driver
+	config *configuration
+
+	// mu protects the endpoints map.
+	mu        sync.Mutex
+	endpoints map[string]*endpoint
 }
 
 // Register initializes and registers the libnetwork ipvlan driver.
 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

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 {

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.
@@ -41,37 +35,32 @@ 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 {
-		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)
 	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)
+	ep, err := n.endpoint(eid)
+	if err != nil {
+		return err
 	}
+	// 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:
 			// 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",
@@ -79,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",
@@ -91,8 +76,8 @@ 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 {
-				s := n.getSubnetforIPv4(ep.addr)
+			if ep.addr != nil && len(n.config.Ipv4Subnets) > 0 {
+				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,8 +101,8 @@ 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 {
-				s := n.getSubnetforIPv6(ep.addrv6)
+			if ep.addrv6 != nil && len(n.config.Ipv6Subnets) > 0 {
+				s := getSubnetForIP(ep.addrv6, n.config.Ipv6Subnets)
 				if s == nil {
 					return fmt.Errorf("could not find a valid ipv6 subnet for endpoint %s", eid)
 				}
@@ -157,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)
 	}
 
@@ -166,46 +151,10 @@ 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
 }
 
-// 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
-}
-
-// 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)

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: endpointTable{},
+			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:

daemon/libnetwork/drivers/ipvlan/ipvlan_state.go

@@ -5,16 +5,15 @@ package ipvlan
 import (
 	"context"
 	"errors"
-	"fmt"
 
 	"github.com/containerd/log"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
 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)
 	}
@@ -23,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 {
@@ -47,36 +46,30 @@ func (d *driver) getNetworks() []*network {
 	return ls
 }
 
-func (n *network) endpoint(eid string) *endpoint {
-	n.Lock()
-	defer n.Unlock()
+func (n *network) endpoint(eid string) (*endpoint, error) {
+	if eid == "" {
+		return nil, errors.New("invalid endpoint id")
+	}
+	n.mu.Lock()
+	defer n.mu.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) {
-	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()
-}
-
-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
+	n.mu.Unlock()
 }
 
 func validateID(nid, eid string) error {
@@ -91,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
 }

daemon/libnetwork/drivers/macvlan/macvlan.go

@@ -24,15 +24,12 @@ const (
 	driverModeOpt       = "macvlan_mode" // macvlan mode ux opt suffix
 )
 
-type endpointTable map[string]*endpoint
-
-type networkTable map[string]*network
-
 type driver struct {
-	networks networkTable
-	sync.Once
-	sync.Mutex
 	store *datastore.Store
+
+	// mu protects the networks map.
+	mu       sync.Mutex
+	networks map[string]*network
 }
 
 type endpoint struct {
@@ -47,18 +44,20 @@ type endpoint struct {
 }
 
 type network struct {
-	id        string
-	endpoints endpointTable
-	driver    *driver
-	config    *configuration
-	sync.Mutex
+	id     string
+	driver *driver
+	config *configuration
+
+	// mu protects the endpoints map.
+	mu        sync.Mutex
+	endpoints map[string]*endpoint
 }
 
 // Register initializes and registers the libnetwork macvlan driver
 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

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 {

daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go

@@ -29,30 +29,29 @@ 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 {
-		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)
 	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)
+	ep, err := n.endpoint(eid)
+	if err != nil {
+		return err
 	}
-	// parse and match the endpoint address with the available v4 subnets
+	// 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 {
-		if len(n.config.Ipv4Subnets) > 0 {
-			s := n.getSubnetforIPv4(ep.addr)
+		// parse and correlate the endpoint v4 address with the available v4 subnets
+		if ep.addr != nil && len(n.config.Ipv4Subnets) > 0 {
+			s := getSubnetForIP(ep.addr, n.config.Ipv4Subnets)
 			if s == nil {
 				return fmt.Errorf("could not find a valid ipv4 subnet for endpoint %s", eid)
 			}
@@ -75,9 +74,9 @@ 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)
+			s := getSubnetForIP(ep.addrv6, n.config.Ipv6Subnets)
 			if s == nil {
 				return fmt.Errorf("could not find a valid ipv6 subnet for endpoint %s", eid)
 			}
@@ -125,31 +124,10 @@ 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
 }
 
-// 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)

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).
@@ -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,
 		})
 	}
@@ -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,20 +163,16 @@ 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)
-			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)
 			}
 		}
 	}
@@ -182,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
@@ -197,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
 		}
 	}

daemon/libnetwork/drivers/macvlan/macvlan_state.go

@@ -5,16 +5,15 @@ package macvlan
 import (
 	"context"
 	"errors"
-	"fmt"
 
 	"github.com/containerd/log"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
 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)
 	}
@@ -23,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 {
@@ -47,36 +46,30 @@ func (d *driver) getNetworks() []*network {
 	return ls
 }
 
-func (n *network) endpoint(eid string) *endpoint {
-	n.Lock()
-	defer n.Unlock()
+func (n *network) endpoint(eid string) (*endpoint, error) {
+	if eid == "" {
+		return nil, errors.New("invalid endpoint id")
+	}
+	n.mu.Lock()
+	defer n.mu.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) {
-	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()
-}
-
-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
+	n.mu.Unlock()
 }
 
 func validateID(nid, eid string) error {
@@ -90,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
 }

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")
 			}
 		}
 	}()