diff --git a/wgengine/tstun/tun.go b/wgengine/tstun/tun.go index e6202e8c5..c798fc37b 100644 --- a/wgengine/tstun/tun.go +++ b/wgengine/tstun/tun.go @@ -29,11 +29,14 @@ const ( const MaxPacketSize = device.MaxContentSize var ( - ErrClosed = errors.New("device closed") - ErrFiltered = errors.New("packet dropped by filter") - ErrPacketTooBig = errors.New("packet too big") + // ErrClosed is returned when attempting an operation on a closed TUN. + ErrClosed = errors.New("device closed") + // ErrFiltered is returned when the acted-on packet is rejected by a filter. + ErrFiltered = errors.New("packet dropped by filter") ) +var errPacketTooBig = errors.New("packet too big") + // TUN wraps a tun.Device from wireguard-go, // augmenting it with filtering and packet injection. // All the added work happens in Read and Write: @@ -54,11 +57,15 @@ type TUN struct { // errors is the error queue populated by poll. errors chan error // outbound is the queue by which packets leave the TUN device. + // // The directions are relative to the network, not the device: // inbound packets arrive via UDP and are written into the TUN device; // outbound packets are read from the TUN device and sent out via UDP. // This queue is needed because although inbound writes are synchronous, // the other direction must wait on a Wireguard goroutine to poll it. + // + // Empty reads are skipped by Wireguard, so it is always legal + // to discard an empty packet instead of sending it through t.outbound. outbound chan []byte // fitler stores the currently active package filter @@ -142,13 +149,21 @@ func (t *TUN) poll() { // In principle, read errors are not fatal (but wireguard-go disagrees). t.bufferConsumed <- struct{}{} } - } else { - select { - case <-t.closed: - return - case t.outbound <- t.buffer[readOffset : readOffset+n]: - // continue - } + continue + } + + // Wireguard will skip an empty read, + // so we might as well do it here to avoid the send through t.outbound. + if n == 0 { + t.bufferConsumed <- struct{}{} + continue + } + + select { + case <-t.closed: + return + case t.outbound <- t.buffer[readOffset : readOffset+n]: + // continue } } } @@ -180,6 +195,7 @@ func (t *TUN) Read(buf []byte, offset int) (int, error) { n = copy(buf[offset:], packet) // t.buffer has a fixed location in memory, // so this is the easiest way to tell when it has been consumed. + // &packet[0] can be used because empty packets do not reach t.outbound. if &packet[0] == &t.buffer[readOffset] { t.bufferConsumed <- struct{}{} } @@ -240,9 +256,13 @@ func (t *TUN) SetFilter(filt *filter.Filter) { // InjectInbound makes the TUN device behave as if a packet // with the given contents was received from the network. // It blocks and does not take ownership of the packet. +// Injecting an empty packet is a no-op. func (t *TUN) InjectInbound(packet []byte) error { if len(packet) > MaxPacketSize { - return ErrPacketTooBig + return errPacketTooBig + } + if len(packet) == 0 { + return nil } _, err := t.Write(packet, 0) return err @@ -251,9 +271,13 @@ func (t *TUN) InjectInbound(packet []byte) error { // InjectOutbound makes the TUN device behave as if a packet // with the given contents was sent to the network. // It does not block, but takes ownership of the packet. +// Injecting an empty packet is a no-op. func (t *TUN) InjectOutbound(packet []byte) error { if len(packet) > MaxPacketSize { - return ErrPacketTooBig + return errPacketTooBig + } + if len(packet) == 0 { + return nil } select { case <-t.closed: