From d0f2c0664b394370face42c183ea79982f4a7bd7 Mon Sep 17 00:00:00 2001
From: Maisem Ali <maisem@tailscale.com>
Date: Tue, 24 Oct 2023 08:26:08 -0700
Subject: [PATCH] wgengine/netstack: standardize var names in UpdateNetstackIPs

Updates #cleanup

Signed-off-by: Maisem Ali <maisem@tailscale.com>
---
 wgengine/netstack/netstack.go | 68 +++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go
index a2c9217e9..21e330aff 100644
--- a/wgengine/netstack/netstack.go
+++ b/wgengine/netstack/netstack.go
@@ -333,7 +333,7 @@ func (ns *Impl) UpdateNetstackIPs(nm *netmap.NetworkMap) {
 		ns.atomicIsLocalIPFunc.Store(tsaddr.FalseContainsIPFunc())
 	}
 
-	oldIPs := make(map[netip.Prefix]bool)
+	oldPfx := make(map[netip.Prefix]bool)
 	for _, protocolAddr := range ns.ipstack.AllAddresses()[nicID] {
 		ap := protocolAddr.AddressWithPrefix
 		ip := netaddrIPFromNetstackIP(ap.Address)
@@ -343,71 +343,77 @@ func (ns *Impl) UpdateNetstackIPs(nm *netmap.NetworkMap) {
 			// ours to delete.
 			continue
 		}
-		oldIPs[netip.PrefixFrom(ip, ap.PrefixLen)] = true
+		p := netip.PrefixFrom(ip, ap.PrefixLen)
+		oldPfx[p] = true
 	}
-	newIPs := make(map[netip.Prefix]bool)
+	newPfx := make(map[netip.Prefix]bool)
 
 	if selfNode.Valid() {
 		for i := range selfNode.Addresses().LenIter() {
-			ipp := selfNode.Addresses().At(i)
-			newIPs[ipp] = true
+			p := selfNode.Addresses().At(i)
+			newPfx[p] = true
 		}
 		if ns.ProcessSubnets {
 			for i := range selfNode.AllowedIPs().LenIter() {
-				ipp := selfNode.AllowedIPs().At(i)
-				newIPs[ipp] = true
+				p := selfNode.AllowedIPs().At(i)
+				newPfx[p] = true
 			}
 		}
 	}
 
-	ipsToBeAdded := make(map[netip.Prefix]bool)
-	for ipp := range newIPs {
-		if !oldIPs[ipp] {
-			ipsToBeAdded[ipp] = true
+	pfxToAdd := make(map[netip.Prefix]bool)
+	for p := range newPfx {
+		if !oldPfx[p] {
+			pfxToAdd[p] = true
 		}
 	}
-	ipsToBeRemoved := make(map[netip.Prefix]bool)
-	for ip := range oldIPs {
-		if !newIPs[ip] {
-			ipsToBeRemoved[ip] = true
+	pfxToRemove := make(map[netip.Prefix]bool)
+	for p := range oldPfx {
+		if !newPfx[p] {
+			pfxToRemove[p] = true
 		}
 	}
 	ns.mu.Lock()
 	for ip := range ns.connsOpenBySubnetIP {
-		delete(ipsToBeRemoved, netip.PrefixFrom(ip, ip.BitLen()))
+		// TODO(maisem): this looks like a bug, remove or document. It seems as
+		// though we might end up either leaking the address on the netstack
+		// NIC, or where we do accounting for connsOpenBySubnetIP from 1 to 0,
+		// we might end up removing the address from the netstack NIC that was
+		// still being advertised.
+		delete(pfxToRemove, netip.PrefixFrom(ip, ip.BitLen()))
 	}
 	ns.mu.Unlock()
 
-	for ipp := range ipsToBeRemoved {
-		err := ns.ipstack.RemoveAddress(nicID, tcpip.AddrFromSlice(ipp.Addr().AsSlice()))
+	for p := range pfxToRemove {
+		err := ns.ipstack.RemoveAddress(nicID, tcpip.AddrFromSlice(p.Addr().AsSlice()))
 		if err != nil {
-			ns.logf("netstack: could not deregister IP %s: %v", ipp, err)
+			ns.logf("netstack: could not deregister IP %s: %v", p, err)
 		} else {
-			ns.logf("[v2] netstack: deregistered IP %s", ipp)
+			ns.logf("[v2] netstack: deregistered IP %s", p)
 		}
 	}
-	for ipp := range ipsToBeAdded {
-		if !ipp.IsValid() {
-			ns.logf("netstack: [unexpected] skipping invalid IP (%v/%v)", ipp.Addr(), ipp.Bits())
+	for p := range pfxToAdd {
+		if !p.IsValid() {
+			ns.logf("netstack: [unexpected] skipping invalid IP (%v/%v)", p.Addr(), p.Bits())
 			continue
 		}
-		pa := tcpip.ProtocolAddress{
-			AddressWithPrefix: ipPrefixToAddressWithPrefix(ipp),
+		tcpAddr := tcpip.ProtocolAddress{
+			AddressWithPrefix: ipPrefixToAddressWithPrefix(p),
 		}
-		if ipp.Addr().Is6() {
-			pa.Protocol = ipv6.ProtocolNumber
+		if p.Addr().Is6() {
+			tcpAddr.Protocol = ipv6.ProtocolNumber
 		} else {
-			pa.Protocol = ipv4.ProtocolNumber
+			tcpAddr.Protocol = ipv4.ProtocolNumber
 		}
 		var tcpErr tcpip.Error // not error
-		tcpErr = ns.ipstack.AddProtocolAddress(nicID, pa, stack.AddressProperties{
+		tcpErr = ns.ipstack.AddProtocolAddress(nicID, tcpAddr, stack.AddressProperties{
 			PEB:        stack.CanBePrimaryEndpoint, // zero value default
 			ConfigType: stack.AddressConfigStatic,  // zero value default
 		})
 		if tcpErr != nil {
-			ns.logf("netstack: could not register IP %s: %v", ipp, tcpErr)
+			ns.logf("netstack: could not register IP %s: %v", p, tcpErr)
 		} else {
-			ns.logf("[v2] netstack: registered IP %s", ipp)
+			ns.logf("[v2] netstack: registered IP %s", p)
 		}
 	}
 }