tstest/natlab/vnet: standardize on 1-based naming of nodes, networks, MACs

We had a mix of 0-based and 1-based nodes and MACs in logs.

Updates #13038

Change-Id: I36d1b00f7f94b37b4ae2cd439bcdc5dbee6eda4d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

tstest/integration/nat/nat_test.go

@@ -283,35 +283,35 @@ func (nt *natTest) runTest(node1, node2 addNodeFunc) pingRoute {
 	for i, c := range clients {
 		i, c := i, c
 		eg.Go(func() error {
+			node := nodes[i]
 			st, err := c.Status(ctx)
 			if err != nil {
-				return fmt.Errorf("node%d status: %w", i, err)
+				return fmt.Errorf("%v status: %w", node, err)
 			}
-			t.Logf("node%d status: %v", i, st)
+			t.Logf("%v status: %v", node, st)
 
-			node := nodes[i]
 			if node.HostFirewall() {
 				if err := c.EnableHostFirewall(ctx); err != nil {
-					return fmt.Errorf("node%d firewall: %w", i, err)
+					return fmt.Errorf("%v firewall: %w", node, err)
 				}
-				t.Logf("node%d firewalled", i)
+				t.Logf("%v firewalled", node)
 			}
 
 			if err := up(ctx, c); err != nil {
-				return fmt.Errorf("node%d up: %w", i, err)
+				return fmt.Errorf("%v up: %w", node, err)
 			}
-			t.Logf("node%d up!", i)
+			t.Logf("%v up!", node)
 
 			st, err = c.Status(ctx)
 			if err != nil {
-				return fmt.Errorf("node%d status: %w", i, err)
+				return fmt.Errorf("%v status: %w", node, err)
 			}
 			sts[i] = st
 
 			if st.BackendState != "Running" {
-				return fmt.Errorf("node%d state = %q", i, st.BackendState)
+				return fmt.Errorf("%v state = %q", node, st.BackendState)
 			}
-			t.Logf("node%d up with %v", i, sts[i].Self.TailscaleIPs)
+			t.Logf("%v up with %v", node, sts[i].Self.TailscaleIPs)
 			return nil
 		})
 	}

tstest/natlab/vnet/conf.go

@@ -60,7 +60,8 @@ func (c *Config) FirstNetwork() *Network {
 func (c *Config) AddNode(opts ...any) *Node {
 	num := len(c.nodes)
 	n := &Node{
-		mac: MAC{0x52, 0xcc, 0xcc, 0xcc, 0xcc, byte(num)}, // 52=TS then 0xcc for ccclient
+		num: num + 1,
+		mac: MAC{0x52, 0xcc, 0xcc, 0xcc, 0xcc, byte(num) + 1}, // 52=TS then 0xcc for ccclient
 	}
 	c.nodes = append(c.nodes, n)
 	for _, o := range opts {
@@ -119,7 +120,7 @@ type TailscaledEnv struct {
 func (c *Config) AddNetwork(opts ...any) *Network {
 	num := len(c.networks)
 	n := &Network{
-		mac: MAC{0x52, 0xee, 0xee, 0xee, 0xee, byte(num)}, // 52=TS then 0xee for 'etwork
+		mac: MAC{0x52, 0xee, 0xee, 0xee, 0xee, byte(num) + 1}, // 52=TS then 0xee for 'etwork
 	}
 	c.networks = append(c.networks, n)
 	for _, o := range opts {
@@ -150,6 +151,7 @@ func (c *Config) AddNetwork(opts ...any) *Network {
 // Node is the configuration of a node in the virtual network.
 type Node struct {
 	err error
+	num int   // 1-based node number
 	n   *node // nil until NewServer called
 
 	env           []TailscaledEnv
@@ -163,6 +165,16 @@ type Node struct {
 	nets []*Network
 }
 
+// Num returns the 1-based node number.
+func (n *Node) Num() int {
+	return n.num
+}
+
+// String returns the string "nodeN" where N is the 1-based node number.
+func (n *Node) String() string {
+	return fmt.Sprintf("node%d", n.num)
+}
+
 // MAC returns the MAC address of the node.
 func (n *Node) MAC() MAC {
 	return n.mac
@@ -285,17 +297,18 @@ func (s *Server) initFromConfig(c *Config) error {
 			LinkType: layers.LinkTypeIPv4,
 		}))
 	}
-	for i, conf := range c.nodes {
+	for _, conf := range c.nodes {
 		if conf.err != nil {
 			return conf.err
 		}
 		n := &node{
+			num:           conf.num,
 			mac:           conf.mac,
 			net:           netOfConf[conf.Network()],
 			verboseSyslog: conf.VerboseSyslog(),
 		}
 		n.interfaceID = must.Get(s.pcapWriter.AddInterface(pcapgo.NgInterface{
-			Name:     fmt.Sprintf("node%d", i+1),
+			Name:     n.String(),
 			LinkType: layers.LinkTypeEthernet,
 		}))
 		conf.n = n

tstest/natlab/vnet/conf_test.go

@@ -69,3 +69,12 @@ func TestConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestNodeString(t *testing.T) {
+	if g, w := (&Node{num: 1}).String(), "node1"; g != w {
+		t.Errorf("got %q; want %q", g, w)
+	}
+	if g, w := (&node{num: 1}).String(), "node1"; g != w {
+		t.Errorf("got %q; want %q", g, w)
+	}
+}

tstest/natlab/vnet/vnet.go

@@ -277,14 +277,21 @@ func (n *network) acceptTCP(r *tcp.ForwarderRequest) {
 	}
 
 	if destPort == 124 {
+		node, ok := n.nodesByIP[clientRemoteIP]
+		if !ok {
+			log.Printf("no node for TCP 124 connection from %v", clientRemoteIP)
+			r.Complete(true)
+			return
+		}
 		r.Complete(false)
 		tc := gonet.NewTCPConn(&wq, ep)
+
 		go func() {
 			defer tc.Close()
 			bs := bufio.NewScanner(tc)
 			for bs.Scan() {
 				line := bs.Text()
-				log.Printf("LOG from guest %v: %s", clientRemoteIP, line)
+				log.Printf("LOG from %v: %s", node, line)
 			}
 		}()
 		return
@@ -503,6 +510,7 @@ func (n *network) MACOfIP(ip netip.Addr) (_ MAC, ok bool) {
 
 type node struct {
 	mac           MAC
+	num           int // 1-based node number
 	interfaceID   int
 	net           *network
 	lanIP         netip.Addr // must be in net.lanIP prefix + unique in net
@@ -516,6 +524,11 @@ type node struct {
 	logCatcherWrites int
 }
 
+// String returns the string "nodeN" where N is the 1-based node number.
+func (n *node) String() string {
+	return fmt.Sprintf("node%d", n.num)
+}
+
 type derpServer struct {
 	srv       *derp.Server
 	handler   http.Handler
@@ -801,8 +814,13 @@ func (s *Server) routeUDPPacket(up UDPPacket) {
 		return
 	}
 
-	netw, ok := s.networkByWAN[up.Dst.Addr()]
+	dstIP := up.Dst.Addr()
+	netw, ok := s.networkByWAN[dstIP]
 	if !ok {
+		if dstIP.IsPrivate() {
+			// Not worth spamming logs. RFC 1918 space doesn't route.
+			return
+		}
 		log.Printf("no network to route UDP packet for %v", up.Dst)
 		return
 	}
@@ -1018,7 +1036,7 @@ func (n *network) HandleEthernetIPv4PacketForRouter(ep EthernetPacket) {
 		if node.verboseSyslog {
 			// TODO(bradfitz): parse this and capture it, structured, into
 			// node's log buffer.
-			log.Printf("syslog from %v: %s", srcIP, udp.Payload)
+			log.Printf("syslog from %v: %s", node, udp.Payload)
 		}
 		return
 	}