Merge pull request #50091 from corhere/libn/overlay-refactor-checkencryption

libnetwork/d/overlay: simplify the encryption logic

libnetwork/drivers/overlay/encryption.go

@@ -114,60 +114,16 @@ func (e *encrMap) String() string {
 	return b.String()
 }
 
-func (d *driver) checkEncryption(nid string, rIP netip.Addr, isLocal, add bool) error {
-	log.G(context.TODO()).Debugf("checkEncryption(%.7s, %v, %t)", nid, rIP, isLocal)
-
-	n := d.network(nid)
-	if n == nil || !n.secure {
-		return nil
-	}
-
-	if len(d.keys) == 0 {
-		return types.ForbiddenErrorf("encryption key is not present")
-	}
-
-	lIP := d.bindAddress
-	aIP := d.advertiseAddress
-	nodes := map[netip.Addr]struct{}{}
-
-	switch {
-	case isLocal:
-		if err := d.peerDbNetworkWalk(nid, func(_ netip.Addr, _ net.HardwareAddr, pEntry *peerEntry) bool {
-			if aIP != pEntry.vtep {
-				nodes[pEntry.vtep] = struct{}{}
-			}
-			return false
-		}); err != nil {
-			log.G(context.TODO()).Warnf("Failed to retrieve list of participating nodes in overlay network %.5s: %v", nid, err)
-		}
-	default:
-		if len(d.network(nid).endpoints) > 0 {
-			nodes[rIP] = struct{}{}
-		}
-	}
-
-	log.G(context.TODO()).Debugf("List of nodes: %s", nodes)
-
-	if add {
-		for rIP := range nodes {
-			if err := setupEncryption(lIP, aIP, rIP, d.secMap, d.keys); err != nil {
-				log.G(context.TODO()).Warnf("Failed to program network encryption between %s and %s: %v", lIP, rIP, err)
-			}
-		}
-	} else {
-		if len(nodes) == 0 {
-			if err := removeEncryption(lIP, rIP, d.secMap); err != nil {
-				log.G(context.TODO()).Warnf("Failed to remove network encryption between %s and %s: %v", lIP, rIP, err)
-			}
-		}
-	}
-
-	return nil
-}
-
 // setupEncryption programs the encryption parameters for secure communication
 // between the local node and a remote node.
-func setupEncryption(localIP, advIP, remoteIP netip.Addr, em *encrMap, keys []*key) error {
+func (d *driver) setupEncryption(remoteIP netip.Addr) error {
+	log.G(context.TODO()).Debugf("setupEncryption(%s)", remoteIP)
+
+	localIP, advIP := d.bindAddress, d.advertiseAddress
+	keys := d.keys // FIXME: data race
+	if len(keys) == 0 {
+		return types.ForbiddenErrorf("encryption key is not present")
+	}
 	log.G(context.TODO()).Debugf("Programming encryption between %s and %s", localIP, remoteIP)
 
 	indices := make([]*spi, 0, len(keys))
@@ -192,17 +148,19 @@ func setupEncryption(localIP, advIP, remoteIP netip.Addr, em *encrMap, keys []*k
 		}
 	}
 
-	em.Lock()
-	em.nodes[remoteIP] = indices
-	em.Unlock()
+	d.secMap.Lock()
+	d.secMap.nodes[remoteIP] = indices
+	d.secMap.Unlock()
 
 	return nil
 }
 
-func removeEncryption(localIP, remoteIP netip.Addr, em *encrMap) error {
-	em.Lock()
-	indices, ok := em.nodes[remoteIP]
-	em.Unlock()
+func (d *driver) removeEncryption(remoteIP netip.Addr) error {
+	log.G(context.TODO()).Debugf("removeEncryption(%s)", remoteIP)
+
+	d.secMap.Lock()
+	indices, ok := d.secMap.nodes[remoteIP]
+	d.secMap.Unlock()
 	if !ok {
 		return nil
 	}
@@ -211,7 +169,7 @@ func removeEncryption(localIP, remoteIP netip.Addr, em *encrMap) error {
 		if i == 0 {
 			dir = bidir
 		}
-		fSA, rSA, err := programSA(localIP.AsSlice(), remoteIP.AsSlice(), idxs, nil, dir, false)
+		fSA, rSA, err := programSA(d.bindAddress.AsSlice(), remoteIP.AsSlice(), idxs, nil, dir, false)
 		if err != nil {
 			log.G(context.TODO()).Warn(err)
 		}

libnetwork/drivers/overlay/joinleave.go

@@ -119,11 +119,7 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf
 		}
 	}
 
-	d.peerAdd(nid, eid, ep.addr, ep.mac, d.advertiseAddress, true)
-
-	if err = d.checkEncryption(nid, netip.Addr{}, true, true); err != nil {
-		log.G(ctx).Warn(err)
-	}
+	d.peerAdd(nid, eid, ep.addr, ep.mac, netip.Addr{})
 
 	buf, err := proto.Marshal(&PeerRecord{
 		EndpointIP:       ep.addr.String(),
@@ -197,11 +193,11 @@ func (d *driver) EventNotify(etype driverapi.EventType, nid, tableName, key stri
 	}
 
 	if etype == driverapi.Delete {
-		d.peerDelete(nid, eid, addr, mac, vtep, false)
+		d.peerDelete(nid, eid, addr, mac, vtep)
 		return
 	}
 
-	d.peerAdd(nid, eid, addr, mac, vtep, false)
+	d.peerAdd(nid, eid, addr, mac, vtep)
 }
 
 // Leave method is invoked when a Sandbox detaches from an endpoint.
@@ -221,7 +217,7 @@ func (d *driver) Leave(nid, eid string) error {
 		return types.InternalMaskableErrorf("could not find endpoint with id %s", eid)
 	}
 
-	d.peerDelete(nid, eid, ep.addr, ep.mac, d.advertiseAddress, true)
+	d.peerDelete(nid, eid, ep.addr, ep.mac, netip.Addr{})
 
 	n.leaveSandbox()
 

libnetwork/drivers/overlay/overlay.go

@@ -30,14 +30,13 @@ var _ discoverapi.Discover = (*driver)(nil)
 type driver struct {
 	bindAddress, advertiseAddress netip.Addr
 
-	config        map[string]interface{}
-	peerDb        peerNetworkMap
-	secMap        *encrMap
-	networks      networkTable
-	initOS        sync.Once
-	localJoinOnce sync.Once
-	keys          []*key
-	peerOpMu      sync.Mutex
+	config   map[string]interface{}
+	peerDb   peerNetworkMap
+	secMap   *encrMap
+	networks networkTable
+	initOS   sync.Once
+	keys     []*key
+	peerOpMu sync.Mutex
 	sync.Mutex
 }
 
@@ -95,12 +94,6 @@ func (d *driver) nodeJoin(data discoverapi.NodeDiscoveryData) error {
 		d.advertiseAddress = advAddr
 		d.bindAddress = bindAddr
 		d.Unlock()
-
-		// If containers are already running on this network update the
-		// advertise address in the peerDB
-		d.localJoinOnce.Do(func() {
-			d.peerDBUpdateSelf()
-		})
 	}
 	return nil
 }

libnetwork/drivers/overlay/peerdb.go

@@ -20,9 +20,12 @@ const ovPeerTable = "overlay_peer_table"
 
 type peerEntry struct {
 	eid        string
-	vtep       netip.Addr
-	prefixBits int // number of 1-bits in network mask of peerIP
-	isLocal    bool
+	vtep       netip.Addr // Virtual Tunnel End Point for non-local peers
+	prefixBits int        // number of 1-bits in network mask of peerIP
+}
+
+func (p *peerEntry) isLocal() bool {
+	return !p.vtep.IsValid()
 }
 
 type peerMap struct {
@@ -37,22 +40,6 @@ type peerNetworkMap struct {
 	sync.Mutex
 }
 
-func (d *driver) peerDbWalk(f func(string, netip.Addr, net.HardwareAddr, *peerEntry) bool) error {
-	d.peerDb.Lock()
-	nids := []string{}
-	for nid := range d.peerDb.mp {
-		nids = append(nids, nid)
-	}
-	d.peerDb.Unlock()
-
-	for _, nid := range nids {
-		d.peerDbNetworkWalk(nid, func(peerIP netip.Addr, peerMac net.HardwareAddr, pEntry *peerEntry) bool {
-			return f(nid, peerIP, peerMac, pEntry)
-		})
-	}
-	return nil
-}
-
 func (d *driver) peerDbNetworkWalk(nid string, f func(netip.Addr, net.HardwareAddr, *peerEntry) bool) error {
 	d.peerDb.Lock()
 	pMap, ok := d.peerDb.mp[nid]
@@ -106,7 +93,7 @@ func (d *driver) peerDbSearch(nid string, peerIP netip.Addr) (netip.Addr, net.Ha
 	return peerIPMatched, peerMacMatched, pEntryMatched, nil
 }
 
-func (d *driver) peerDbAdd(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr, isLocal bool) (bool, int) {
+func (d *driver) peerDbAdd(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr) (bool, int) {
 	d.peerDb.Lock()
 	pMap, ok := d.peerDb.mp[nid]
 	if !ok {
@@ -121,7 +108,6 @@ func (d *driver) peerDbAdd(nid, eid string, peerIP netip.Prefix, peerMac net.Har
 		eid:        eid,
 		vtep:       vtep,
 		prefixBits: peerIP.Bits(),
-		isLocal:    isLocal,
 	}
 
 	pMap.Lock()
@@ -135,7 +121,7 @@ func (d *driver) peerDbAdd(nid, eid string, peerIP netip.Prefix, peerMac net.Har
 	return b, i
 }
 
-func (d *driver) peerDbDelete(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr, isLocal bool) (bool, int) {
+func (d *driver) peerDbDelete(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr) (bool, int) {
 	d.peerDb.Lock()
 	pMap, ok := d.peerDb.mp[nid]
 	if !ok {
@@ -150,7 +136,6 @@ func (d *driver) peerDbDelete(nid, eid string, peerIP netip.Prefix, peerMac net.
 		eid:        eid,
 		vtep:       vtep,
 		prefixBits: peerIP.Bits(),
-		isLocal:    isLocal,
 	}
 
 	pMap.Lock()
@@ -186,26 +171,29 @@ func (d *driver) initSandboxPeerDB(nid string) {
 func (d *driver) peerInitOp(nid string) error {
 	return d.peerDbNetworkWalk(nid, func(peerIP netip.Addr, peerMac net.HardwareAddr, pEntry *peerEntry) bool {
 		// Local entries do not need to be added
-		if pEntry.isLocal {
+		if pEntry.isLocal() {
 			return false
 		}
 
-		d.peerAddOp(nid, pEntry.eid, netip.PrefixFrom(peerIP, pEntry.prefixBits), peerMac, pEntry.vtep, false, pEntry.isLocal)
+		d.peerAddOp(nid, pEntry.eid, netip.PrefixFrom(peerIP, pEntry.prefixBits), peerMac, pEntry.vtep, false)
 		// return false to loop on all entries
 		return false
 	})
 }
 
-func (d *driver) peerAdd(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr, localPeer bool) {
+// peerAdd adds a new entry to the peer database.
+//
+// Local peers are signified by an invalid vtep (i.e. netip.Addr{}).
+func (d *driver) peerAdd(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr) {
 	d.peerOpMu.Lock()
 	defer d.peerOpMu.Unlock()
-	err := d.peerAddOp(nid, eid, peerIP, peerMac, vtep, true, localPeer)
+	err := d.peerAddOp(nid, eid, peerIP, peerMac, vtep, true)
 	if err != nil {
 		log.G(context.TODO()).WithError(err).Warn("Peer add operation failed")
 	}
 }
 
-func (d *driver) peerAddOp(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr, updateDB, localPeer bool) error {
+func (d *driver) peerAddOp(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr, updateDB bool) error {
 	if err := validateID(nid, eid); err != nil {
 		return err
 	}
@@ -213,15 +201,15 @@ func (d *driver) peerAddOp(nid, eid string, peerIP netip.Prefix, peerMac net.Har
 	var dbEntries int
 	var inserted bool
 	if updateDB {
-		inserted, dbEntries = d.peerDbAdd(nid, eid, peerIP, peerMac, vtep, localPeer)
+		inserted, dbEntries = d.peerDbAdd(nid, eid, peerIP, peerMac, vtep)
 		if !inserted {
-			log.G(context.TODO()).Warnf("Entry already present in db: nid:%s eid:%s peerIP:%v peerMac:%v isLocal:%t vtep:%v",
-				nid, eid, peerIP, peerMac, localPeer, vtep)
+			log.G(context.TODO()).Warnf("Entry already present in db: nid:%s eid:%s peerIP:%v peerMac:%v vtep:%v",
+				nid, eid, peerIP, peerMac, vtep)
 		}
 	}
 
 	// Local peers do not need any further configuration
-	if localPeer {
+	if !vtep.IsValid() {
 		return nil
 	}
 
@@ -247,8 +235,10 @@ func (d *driver) peerAddOp(nid, eid string, peerIP netip.Prefix, peerMac net.Har
 		return fmt.Errorf("subnet sandbox join failed for %q: %v", s.subnetIP.String(), err)
 	}
 
-	if err := d.checkEncryption(nid, vtep, false, true); err != nil {
-		log.G(context.TODO()).Warn(err)
+	if n.secure && len(n.endpoints) > 0 {
+		if err := d.setupEncryption(vtep); err != nil {
+			log.G(context.TODO()).Warn(err)
+		}
 	}
 
 	// Add neighbor entry for the peer IP
@@ -270,24 +260,27 @@ func (d *driver) peerAddOp(nid, eid string, peerIP netip.Prefix, peerMac net.Har
 	return nil
 }
 
-func (d *driver) peerDelete(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr, localPeer bool) {
+// peerDelete removes an entry from the peer database.
+//
+// Local peers are signified by an invalid vtep (i.e. netip.Addr{}).
+func (d *driver) peerDelete(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr) {
 	d.peerOpMu.Lock()
 	defer d.peerOpMu.Unlock()
-	err := d.peerDeleteOp(nid, eid, peerIP, peerMac, vtep, localPeer)
+	err := d.peerDeleteOp(nid, eid, peerIP, peerMac, vtep)
 	if err != nil {
 		log.G(context.TODO()).WithError(err).Warn("Peer delete operation failed")
 	}
 }
 
-func (d *driver) peerDeleteOp(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr, localPeer bool) error {
+func (d *driver) peerDeleteOp(nid, eid string, peerIP netip.Prefix, peerMac net.HardwareAddr, vtep netip.Addr) error {
 	if err := validateID(nid, eid); err != nil {
 		return err
 	}
 
-	deleted, dbEntries := d.peerDbDelete(nid, eid, peerIP, peerMac, vtep, localPeer)
+	deleted, dbEntries := d.peerDbDelete(nid, eid, peerIP, peerMac, vtep)
 	if !deleted {
-		log.G(context.TODO()).Warnf("Entry was not in db: nid:%s eid:%s peerIP:%v peerMac:%v isLocal:%t vtep:%v",
-			nid, eid, peerIP, peerMac, localPeer, vtep)
+		log.G(context.TODO()).Warnf("Entry was not in db: nid:%s eid:%s peerIP:%v peerMac:%v vtep:%v",
+			nid, eid, peerIP, peerMac, vtep)
 	}
 
 	n := d.network(nid)
@@ -300,12 +293,14 @@ func (d *driver) peerDeleteOp(nid, eid string, peerIP netip.Prefix, peerMac net.
 		return nil
 	}
 
-	if err := d.checkEncryption(nid, vtep, localPeer, false); err != nil {
-		log.G(context.TODO()).Warn(err)
+	if n.secure && len(n.endpoints) == 0 {
+		if err := d.removeEncryption(vtep); err != nil {
+			log.G(context.TODO()).Warn(err)
+		}
 	}
 
 	// Local peers do not have any local configuration to delete
-	if !localPeer {
+	if vtep.IsValid() {
 		s := n.getSubnetforIP(peerIP)
 		if s == nil {
 			return fmt.Errorf("could not find the subnet %q in network %q", peerIP.String(), n.id)
@@ -338,7 +333,7 @@ func (d *driver) peerDeleteOp(nid, eid string, peerIP netip.Prefix, peerMac net.
 		log.G(context.TODO()).Errorf("peerDeleteOp unable to restore a configuration for nid:%s ip:%v mac:%v err:%s", nid, peerIP, peerMac, err)
 		return err
 	}
-	return d.peerAddOp(nid, peerEntry.eid, netip.PrefixFrom(peerIPAddr, peerEntry.prefixBits), peerMac, peerEntry.vtep, false, peerEntry.isLocal)
+	return d.peerAddOp(nid, peerEntry.eid, netip.PrefixFrom(peerIPAddr, peerEntry.prefixBits), peerMac, peerEntry.vtep, false)
 }
 
 func (d *driver) peerFlush(nid string) {
@@ -359,12 +354,3 @@ func (d *driver) peerFlushOp(nid string) error {
 	delete(d.peerDb.mp, nid)
 	return nil
 }
-
-func (d *driver) peerDBUpdateSelf() {
-	d.peerDbWalk(func(nid string, _ netip.Addr, _ net.HardwareAddr, pEntry *peerEntry) bool {
-		if pEntry.isLocal {
-			pEntry.vtep = d.advertiseAddress
-		}
-		return false
-	})
-}