diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan.go b/daemon/libnetwork/drivers/ipvlan/ipvlan.go index cde2312351..bbf1bb3759 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan.go +++ b/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 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 60950e85f0..eff1b875ed 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. @@ -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) diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_network.go index 6238bf68a4..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: 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: diff --git a/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go b/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go index a67ca15cc4..f9c3253b52 100644 --- a/daemon/libnetwork/drivers/ipvlan/ipvlan_state.go +++ b/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 } diff --git a/daemon/libnetwork/drivers/macvlan/macvlan.go b/daemon/libnetwork/drivers/macvlan/macvlan.go index 0e6afa32ec..4103a1c588 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan.go +++ b/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 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 a447591b96..a41edeb7c0 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go +++ b/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) diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_network.go b/daemon/libnetwork/drivers/macvlan/macvlan_network.go index 08f62f4fb8..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). @@ -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 } } diff --git a/daemon/libnetwork/drivers/macvlan/macvlan_state.go b/daemon/libnetwork/drivers/macvlan/macvlan_state.go index 5700731eac..5faab1eb8f 100644 --- a/daemon/libnetwork/drivers/macvlan/macvlan_state.go +++ b/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 } 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") } } }()