Both tests started flaking after my 910735448 ("tstest/natlab/vnet:
send unsolicited IPv6 Router Advertisements") added background RA
traffic on v6-enabled networks.
TestPacketSideEffects races the periodic unsolicited-RA goroutine
against its synchronous packet-count assertions: when the multicast
RA fires after the test has registered its sinks, both sinks receive
it and "got 1 packet, want N" becomes "got N+2".
TestProtocolQEMU's reader was doing raw Read on the SOCK_STREAM unix
socket and comparing the whole result to the expected length-prefixed
packet. The kernel is free to coalesce the on-register RA frame and
the test packet into one Read, in which case bytes.Equal fails and
the entire chunk (including the test packet's bytes) gets discarded
as "unexpected", leading to a 5s i/o timeout. Parse the QEMU uint32
length-prefix framing with io.ReadFull instead so we read exactly one
frame per iteration regardless of how the kernel buffers them. The
SOCK_DGRAM path (TestProtocolUnixDgram) keeps the original raw Read
since datagram boundaries are preserved.
These where the top two flakes in oss on the flakes dashboards.
Updates #13038
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I32983656b692921a0f43a4a5e9a8a6ab2555ee49
vnet only ever sent IPv6 RAs in response to a Router Solicitation. In
practice this meant gokrazy VMs running with a dual-stack LAN never
installed vnet's IPv6 default route: gokrazy brings the link up via
DHCPv4 and the kernel never emits an RS on its own under that init
path. Off-link IPv6 destinations like the fake DERP servers were
therefore unreachable from any gokrazy test node that also had v4
on the same interface. (Pure-v6 nodes happened to work because the
kernel sends an RS as part of v6-only autoconf.)
Fix this in two complementary ways:
- Send an unsolicited RA every 5s to the link-local all-nodes group
on every v6-enabled network. This matches what real routers do
(RFC 4861 §6.2.1, MaxRtrAdvInterval; we use a much shorter
interval than the spec's 200s default so short-lived tests don't
have to wait).
- Send a unicast RA to a newly-registered MAC as soon as a client
first transmits on the wire. Without this the first periodic RA
can land before any VM has connected and the next one isn't
until the next tick, which can be longer than the test runs.
Factor the RA serialization out into buildIPv6RouterAdvertisement so
the solicited, periodic, and per-client paths all share one body.
Update TestSelfSignedDERPHashPinning to use a dual-stack hard-NAT
builder and assert zero errors from DebugDERPRegion (instead of
filtering "over IPv6" errors as it had to before this change). The
new builder also sets TS_DEBUG_STRIP_ENDPOINTS=1 on tailscaled so
disco can't find a direct path: without endpoint stripping, the now-
working non-NATted IPv6 LAN gives the two hard-NAT'd nodes a direct
route, defeating the test's "must traverse DERP" assertion. (Hard
NAT alone was enough before this change because v6 routing was
broken.) Also update sendBetweenClients in the vnet unit tests to
tolerate the new on-register RA noise on its read path.
Updates #13038
Updates #19973
Change-Id: Ic281dc53702a25fa773c46313f453837814233e8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Multi-NIC support:
- Add nodeNIC type and node.extraNICs for secondary network interfaces
- Add netForMAC/macForNet to route packets to the correct network by MAC
- Update initFromConfig to allocate a MAC + LAN IP per network
- Fix handleEthernetFrameFromVM, ServeUnixConn to use netForMAC
- Fix MACOfIP, writeEth, WriteUDPPacketNoNAT, gVisor write path, and
createARPResponse to use macForNet (return the MAC actually on that
network, not the node's primary MAC)
- Fix createDHCPResponse for multi-NIC (correct client IP and subnet)
- Add nodeNICMac for secondary NIC MAC generation
- Add Node accessors: NumNICs, NICMac, Networks, LanIP
DHCP fixes:
- Include LeaseTime, SubnetMask, Router, DNS in DHCP Offer (not just
Ack). systemd-networkd requires these to accept an Offer.
- Fix DHCP response source IP: use gateway IP instead of echoing
the request's destination (which was 255.255.255.255 for discovers)
New VIPs:
- cloud-init.tailscale: serves per-node cloud-init meta-data, user-data,
and network-config for VMs booting with nocloud datasource
- files.tailscale: serves binary files (tta, tailscale, tailscaled)
registered via RegisterFile for cloud VM provisioning
- Add ControlServer() accessor for test control server
This is necessary for a three-VM natlab subnet router
integration test, coming later.
Updates #13038
Change-Id: I59f9f356bae9b5509c117265237983972dfdd5af
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This file was never truly necessary and has never actually been used in
the history of Tailscale's open source releases.
A Brief History of AUTHORS files
---
The AUTHORS file was a pattern developed at Google, originally for
Chromium, then adopted by Go and a bunch of other projects. The problem
was that Chromium originally had a copyright line only recognizing
Google as the copyright holder. Because Google (and most open source
projects) do not require copyright assignemnt for contributions, each
contributor maintains their copyright. Some large corporate contributors
then tried to add their own name to the copyright line in the LICENSE
file or in file headers. This quickly becomes unwieldy, and puts a
tremendous burden on anyone building on top of Chromium, since the
license requires that they keep all copyright lines intact.
The compromise was to create an AUTHORS file that would list all of the
copyright holders. The LICENSE file and source file headers would then
include that list by reference, listing the copyright holder as "The
Chromium Authors".
This also become cumbersome to simply keep the file up to date with a
high rate of new contributors. Plus it's not always obvious who the
copyright holder is. Sometimes it is the individual making the
contribution, but many times it may be their employer. There is no way
for the proejct maintainer to know.
Eventually, Google changed their policy to no longer recommend trying to
keep the AUTHORS file up to date proactively, and instead to only add to
it when requested: https://opensource.google/docs/releasing/authors.
They are also clear that:
> Adding contributors to the AUTHORS file is entirely within the
> project's discretion and has no implications for copyright ownership.
It was primarily added to appease a small number of large contributors
that insisted that they be recognized as copyright holders (which was
entirely their right to do). But it's not truly necessary, and not even
the most accurate way of identifying contributors and/or copyright
holders.
In practice, we've never added anyone to our AUTHORS file. It only lists
Tailscale, so it's not really serving any purpose. It also causes
confusion because Tailscalars put the "Tailscale Inc & AUTHORS" header
in other open source repos which don't actually have an AUTHORS file, so
it's ambiguous what that means.
Instead, we just acknowledge that the contributors to Tailscale (whoever
they are) are copyright holders for their individual contributions. We
also have the benefit of using the DCO (developercertificate.org) which
provides some additional certification of their right to make the
contribution.
The source file changes were purely mechanical with:
git ls-files | xargs sed -i -e 's/\(Tailscale Inc &\) AUTHORS/\1 contributors/g'
Updates #cleanup
Change-Id: Ia101a4a3005adb9118051b3416f5a64a4a45987d
Signed-off-by: Will Norris <will@tailscale.com>
No need to make callers specify the redundant IP version or
TTL/HopLimit or EthernetType in the common case. The mkPacket helper
can set those when unset.
And use the mkIPLayer in another place, simplifying some code.
And rename mkPacketErr to just mkPacket, then move mkPacket to
test-only code, as mustPacket.
Updates #13038
Change-Id: Ic216e44dda760c69ab9bfc509370040874a47d30
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And clean up some of the test helpers in the process.
Updates #13038
Change-Id: I3e2b5f7028a32d97af7f91941e59399a8e222b25
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This adds tests for DNS requests, and ignoring IPv6 packets on v4-only
networks.
No behavior changes. But some things are pulled out into functions.
And the mkPacket helpers previously just for tests are moved into
non-test code to be used elsewhere to reduce duplication, doing the
checksum stuff automatically.
Updates #13038
Change-Id: I4dd0b73c75b2b9567b4be3f05a2792999d83f6a3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This adds support for sending packets to 33:33:00:00:01 at IPv6
multicast address ff02::1 to send to all nodes.
Nothing in Tailscale depends on this (yet?), but it makes debugging in
VMs behind natlab easier (e.g. you can ping all nodes), and other
things might depend on this in the future.
Mostly I'm trying to flesh out the IPv6 support in natlab now that we
can write vnet tests.
Updates #13038
Change-Id: If590031fcf075690ca35c7b230a38c3e72e621eb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
To test how virtual machines connect to the natlab vnet code.
Updates #13038
Change-Id: Ia4fd4b0c1803580ee7d94cc9878d777ad4f24f82
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And refactor some of vnet.go for testability.
The only behavioral change (with a new test) is that ethernet
broadcasts no longer get sent back to the sender.
Updates #13038
Change-Id: Ic2e7e7d6d8805b7b7f2b5c52c2c5ba97101cef14
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Brad Fitzpatrick
Will Norris