From 84adfa1ba311a4ed2b274458e379bc95e08e699b Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 13 Aug 2024 08:20:43 -0700 Subject: [PATCH] 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 --- tstest/integration/nat/nat_test.go | 20 ++++++++++---------- tstest/natlab/vnet/conf.go | 21 +++++++++++++++++---- tstest/natlab/vnet/conf_test.go | 9 +++++++++ tstest/natlab/vnet/vnet.go | 24 +++++++++++++++++++++--- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/tstest/integration/nat/nat_test.go b/tstest/integration/nat/nat_test.go index 4aa68d55b..a469859e6 100644 --- a/tstest/integration/nat/nat_test.go +++ b/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 }) } diff --git a/tstest/natlab/vnet/conf.go b/tstest/natlab/vnet/conf.go index d0d3fadca..0f880694f 100644 --- a/tstest/natlab/vnet/conf.go +++ b/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 diff --git a/tstest/natlab/vnet/conf_test.go b/tstest/natlab/vnet/conf_test.go index ae731d127..15d3c69ef 100644 --- a/tstest/natlab/vnet/conf_test.go +++ b/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) + } +} diff --git a/tstest/natlab/vnet/vnet.go b/tstest/natlab/vnet/vnet.go index 38355e51f..0205559c9 100644 --- a/tstest/natlab/vnet/vnet.go +++ b/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 }