Incrementing the reference count of a packet as a means of granting ownership
is unsafe when the packet is shared across gorountines. The underlying buffer's
reference count is unchanged since it "technically" has the same owning
PacketBuffer, which means different goroutines operating on the underlying
buffer (and packet itself) race.
Clones are roughly as fast as IncRefs because the PacketBuffers allocate from
a pool and the underlying buffers are cloned with copy-on-write
semantics.
I've left IncRef in places where the original packet in obviously going out of
scope at the end of the function or in some tests.
Reported-by: syzbot+e026046f4bf8ad09ae1f@syzkaller.appspotmail.com
Reported-by: syzbot+559365d6050db4b30e0f@syzkaller.appspotmail.com
Reported-by: syzbot+63c78a2c88a5744c636b@syzkaller.appspotmail.com
PiperOrigin-RevId: 705676806
The helper function is deprecated. The package gvisor.dev/gvisor/pkg/rand
depends on crypto/rand which performs worse thatn math/rand, the changes
are fine since they are not at any gVisor's hot path.
The ultimate goal is to migrate math/rand to math/rand/v2.
We weren't verifying that inbound MAC addresses match the NIC, which led to
netstack ingesting packets not meant for it.
Fixes#10908.
PiperOrigin-RevId: 675330632
Our Bazel build infrastructure explicitly recognizes some suffixes that can
appear at the end of filenames (tools/bazeldefs/tags.bzl). It uses these tags
to group files passed to go_stateify (tools/defs.bzl:go_library() =>
calculate_sets()).
Each invocation of go_stateify infers build tags from its input files. Before
this CL, all Go files in pkg/tcpip/link/fdbased (except for fdbased_unsafe.go)
are grouped together to generate fdbased_state_autogen.go, so go_stateify
infers builds tags "!linux !amd64,!arm64" (from mmap_stub.go) and "linux ..."
(from most other files); these contradictory build tags mean that
fdbased_state_autogen.go is never actually compiled, preventing its types from
being registered. After this CL, mmap_nonlinux.go independently generates
fdbased_nonlinux_state_autogen.go.
Fixing this causes fdbased_state_autogen.go to be included for the first time,
exposing a second problem:
```
pkg/tcpip/link/fdbased/endpoint_test.go:80:6: context already declared through import of package context ("context")
.../pkg/tcpip/link/fdbased/fdbased_state_autogen.go:12:2: other declaration of context
```
(Apparently Go imports are a pessimal hybrid of "shared between files in a
package" and "not shared between files in a package".) To fix this, rename the
test type to testContext.
PiperOrigin-RevId: 663522480
Having read more GRO kernel code and NAPI, I believe the previous design was
overly complex and resulted in poor performance.
- Netstack GRO uses a `time.Timer` to periodically clear GRO'd packets. It's
got... several problems.
1. The timer can be configured via CLI flag with arbitrary granularity, but
(IIUC) `time.Timer` relies on the netpoller, which [has millisecond
granularity].
2. The timer creates new goroutines when it fires.
3. There's a complex atomic value song and dance to setting up the timer,
canceling it when no packets are pending, and resuming it for incoming
packets.
- Linux GRO doesn't quite work this way.
- Typically, [Linux flushes GRO whenever it can] (unless running with high
HZ; Go preempts goroutines every 10ms so this does not apply). IIUC,
receiving packets triggers the scheduling of a softirq that batch reads as
many packets from a device as possible. softirqs are run as kernel threads,
so this is getting scheduled with jiffy-ish granularity.
- There's no special scheduling of a timer to flush GRO.
In Netstack our "interrupt" is that we return from a poll, then we read
multiple packets at once via recvmmsg/readv/XDP. So we've already got a delay
analogous to "trigger a ksoftirq and wait for the thread to get scheduled."
Thus, we should use zero-timeout GRO that does the following:
- When recvmmsg/poll/etc returns a single packet, just pass it directly and
immediately up the stack.
- When multiple packets are returned, coalesce them with GRO and flush them
without waiting. We never have 1000+ HZ situation.
- We can remove all atomics and locking from GRO, as there will be one
`groDispatcher` per dispatcher goroutine and no timer-spawned goroutines to
synchronize with.
**Performance**: The previous GRO implementation yielded a few percentage points
increase in performance. This is markedly better.
The following is from tcp_benchmark. It is running with host GRO/GSO disabled,
as there's nothing to GRO when the host does it for us. The RecvMMsg dispatcher
is used, as the PacketMMap dispatcher does not return multiple packets at once
(RecvMMsg averages 8 per syscall in these benchmarks).
```
│ /tmp/old.log │ /tmp/new.log │
│ Mb/s │ Mb/s vs base │
TCP/role=server/host-gso=false/host-gro=false 1.764k ± 2% 2.139k ± 2% +21.29% (p=0.000 n=20)
```
PiperOrigin-RevId: 622366744
It is an idea of running codespell as part of our presubmit checks.
Before enabling it for new changes, let's fix what it has found.
Signed-off-by: Andrei Vagin <avagin@gmail.com>
In preparation for improving ARP handling of gratuitous ARPs, set PktType in
regular inbound flow instead of only when handling packet sockets.
PiperOrigin-RevId: 501562322
This change has significant performance implications. bufferv2 is reference
counted and pooled, which alleviates heap/GC pressure. Below are the results
from running the iperf benchmark.
HEAD:
BenchmarkIperf/operation.Upload-16 1552 ns/op 46.6GiB total allocations
BenchmarkIperf/operation.Download-16 1114 ns/op 68.6GiB total allocations
w/ change:
BenchmarkIperf/operation.Upload-16 1139 ns/op (-27%) 1.41GiB total allocations (-97%)
BenchmarkIperf/operation.Download-16 753.2 ns/op (-33%) 706MiB total allocations (-99%)
PiperOrigin-RevId: 462453185
The packetsocket link endpoint is used to enable a packet endpoint to
receive packets right before they are sent to the driver for outgoing
packets or right after they are recieved from the driver for incoming
packets.
Before this change, only packets that are sent/received by the
`stack.nic` were delivered to packet endpoints. However, the packet
endpoint should also receive packets that don't reach `stack.nic`.
Such an example can be when the interface is bridged and an incoming
packet is sent out through a sibling bridge port instead of being
delivered to a `stack.nic`.
The packetsocket link endpoint must wrap a link endpoint that
populates the link headers for ingress packets. It should ideally
be placed as low in the link endpoint heirarchy as possible so that
packets are probed as close to device drivers as possible.
https://github.com/google/gvisor/commit/2d9b33c0fd7d812a7296cd71f14c03925f815f28
removed the original packetsocket link endpoint which only allowed
low-level capturing of outbound packets but not inbound packets.
The CL mentioned above is cl/395761629.
PiperOrigin-RevId: 424947620
The arguments passed to LinkEndpoint.AddHeader are all available in
the packet buffer so just get the values from the packet buffer.
PiperOrigin-RevId: 424463821
This removes the need for the stack to add a link header out-of-line the
write path when delivering outbound packets to a packet socket.
PiperOrigin-RevId: 424444109
...as they are not used in all cases expect in the packet endpoint
which can get the link address directly from the link header.
PiperOrigin-RevId: 424427195
Lucas ManningandgVisor bot
Jing Chen
Kevin KrakauerandgVisor bot
Jamie LiuandgVisor bot
Jing ChenandgVisor bot
Nicolas LacasseandgVisor bot
Andrei Vagin
Bruno Dal BoandgVisor bot
Ayush RanjanandgVisor bot
Ghanan GowripalanandgVisor bot