From 31d65a909de2630c6b23b4aa31287b6b4432d92b Mon Sep 17 00:00:00 2001 From: Jordan Whited Date: Tue, 17 Mar 2026 15:12:57 -0700 Subject: [PATCH] net/batching: eliminate gso helper func indirection These were previously swappable for historical reasons that are no longer relevant. Removing the indirection enables future inlining optimizations if we simplify further. Updates tailscale/corp#38703 Signed-off-by: Jordan Whited --- net/batching/conn_linux.go | 35 +++++++++++++------------- net/batching/conn_linux_test.go | 44 +++++++++++++-------------------- 2 files changed, 34 insertions(+), 45 deletions(-) diff --git a/net/batching/conn_linux.go b/net/batching/conn_linux.go index b3702b72a..036fa1318 100644 --- a/net/batching/conn_linux.go +++ b/net/batching/conn_linux.go @@ -52,14 +52,12 @@ var ( // linuxBatchingConn is a UDP socket that provides batched i/o. It implements // [Conn]. type linuxBatchingConn struct { - pc *net.UDPConn - xpc xnetBatchReaderWriter - rxOffload bool // supports UDP GRO or similar - txOffload atomic.Bool // supports UDP GSO or similar - setGSOSizeInControl func(control *[]byte, gsoSize uint16) // typically setGSOSizeInControl(); swappable for testing - getGSOSizeFromControl func(control []byte) (int, error) // typically getGSOSizeFromControl(); swappable for testing - sendBatchPool sync.Pool - rxqOverflowsMetric *clientmetric.Metric + pc *net.UDPConn + xpc xnetBatchReaderWriter + rxOffload bool // supports UDP GRO or similar + txOffload atomic.Bool // supports UDP GSO or similar + sendBatchPool sync.Pool + rxqOverflowsMetric *clientmetric.Metric // readOpMu guards read operations that must perform accounting against // rxqOverflows in single-threaded fashion. There are no concurrent usages @@ -134,7 +132,7 @@ func (c *linuxBatchingConn) coalesceMessages(addr *net.UDPAddr, geneve packet.Ge msgs[base].Buffers[0] = append(msgs[base].Buffers[0], make([]byte, msgLen)...) copy(msgs[base].Buffers[0][baseLenBefore:], buff) if i == len(buffs)-1 { - c.setGSOSizeInControl(&msgs[base].OOB, uint16(gsoSize)) + setGSOSizeInControl(&msgs[base].OOB, uint16(gsoSize)) } dgramCnt++ if msgLen < gsoSize { @@ -146,7 +144,7 @@ func (c *linuxBatchingConn) coalesceMessages(addr *net.UDPAddr, geneve packet.Ge } } if dgramCnt > 1 { - c.setGSOSizeInControl(&msgs[base].OOB, uint16(gsoSize)) + setGSOSizeInControl(&msgs[base].OOB, uint16(gsoSize)) } // Reset prior to incrementing base since we are preparing to start a // new potential batch. @@ -262,7 +260,7 @@ func (c *linuxBatchingConn) splitCoalescedMessages(msgs []ipv6.Message, firstMsg end = msg.N numToSplit = 1 ) - gsoSize, err = c.getGSOSizeFromControl(msg.OOB[:msg.NN]) + gsoSize, err = getGSOSizeFromControl(msg.OOB[:msg.NN]) if err != nil { return n, err } @@ -426,9 +424,10 @@ func tryEnableUDPOffload(pconn nettype.PacketConn) (hasTX bool, hasRX bool) { return hasTX, hasRX } -// getGSOSizeFromControl returns the GSO size found in control. If no GSO size -// is found or the len(control) < unix.SizeofCmsghdr, this function returns 0. -// A non-nil error will be returned if control is malformed. +// getGSOSizeFromControl returns the GSO size found in control associated with a +// cmsg type of UDP_GRO, which the kernel populates in the read direction. If no +// GSO size is found or the len(control) < unix.SizeofCmsghdr, this function +// returns 0. A non-nil error will be returned if control is malformed. func getGSOSizeFromControl(control []byte) (int, error) { data, err := getDataFromControl(control, unix.SOL_UDP, unix.UDP_GRO, 2) if err != nil { @@ -441,7 +440,9 @@ func getGSOSizeFromControl(control []byte) (int, error) { } // setGSOSizeInControl sets a socket control message in control containing -// gsoSize. If len(control) < controlMessageSize control's len will be set to 0. +// gsoSize with an associated cmsg type of UDP_SEGMENT, which we are responsible +// for populating prior to writing towards the kernel. If len(control) < controlMessageSize +// control's len will be set to 0. func setGSOSizeInControl(control *[]byte, gsoSize uint16) { *control = (*control)[:0] if cap(*control) < int(unsafe.Sizeof(unix.Cmsghdr{})) { @@ -513,9 +514,7 @@ func TryUpgradeToConn(pconn nettype.PacketConn, network string, batchSize int, r return pconn } b := &linuxBatchingConn{ - pc: uc, - getGSOSizeFromControl: getGSOSizeFromControl, - setGSOSizeInControl: setGSOSizeInControl, + pc: uc, sendBatchPool: sync.Pool{ New: func() any { ua := &net.UDPAddr{ diff --git a/net/batching/conn_linux_test.go b/net/batching/conn_linux_test.go index bc9e55a9d..62bdb9d81 100644 --- a/net/batching/conn_linux_test.go +++ b/net/batching/conn_linux_test.go @@ -18,33 +18,17 @@ import ( "tailscale.com/net/packet" ) -func setGSOSize(control *[]byte, gsoSize uint16) { - *control = (*control)[:cap(*control)] - binary.LittleEndian.PutUint16(*control, gsoSize) -} - -func getGSOSize(control []byte) (int, error) { - if len(control) < 2 { - return 0, nil - } - return int(binary.LittleEndian.Uint16(control)), nil -} - func Test_linuxBatchingConn_splitCoalescedMessages(t *testing.T) { - c := &linuxBatchingConn{ - setGSOSizeInControl: setGSOSize, - getGSOSizeFromControl: getGSOSize, - } + c := &linuxBatchingConn{} - newMsg := func(n, gso int) ipv6.Message { + newMsg := func(n int, gso uint16) ipv6.Message { msg := ipv6.Message{ Buffers: [][]byte{make([]byte, 1024)}, N: n, - OOB: make([]byte, 2), + OOB: gsoControl(gso), } - binary.LittleEndian.PutUint16(msg.OOB, uint16(gso)) if gso > 0 { - msg.NN = 2 + msg.NN = len(msg.OOB) } return msg } @@ -156,10 +140,7 @@ func Test_linuxBatchingConn_splitCoalescedMessages(t *testing.T) { } func Test_linuxBatchingConn_coalesceMessages(t *testing.T) { - c := &linuxBatchingConn{ - setGSOSizeInControl: setGSOSize, - getGSOSizeFromControl: getGSOSize, - } + c := &linuxBatchingConn{} withGeneveSpace := func(len, cap int) []byte { return make([]byte, len+packet.GeneveFixedHeaderLength, cap+packet.GeneveFixedHeaderLength) @@ -285,7 +266,7 @@ func Test_linuxBatchingConn_coalesceMessages(t *testing.T) { msgs := make([]ipv6.Message, len(tt.buffs)) for i := range msgs { msgs[i].Buffers = make([][]byte, 1) - msgs[i].OOB = make([]byte, 0, 2) + msgs[i].OOB = make([]byte, controlMessageSize) } got := c.coalesceMessages(addr, tt.geneve, tt.buffs, msgs, packet.GeneveFixedHeaderLength) if got != len(tt.wantLens) { @@ -299,9 +280,18 @@ func Test_linuxBatchingConn_coalesceMessages(t *testing.T) { if gotLen != tt.wantLens[i] { t.Errorf("len(msgs[%d].Buffers[0]) %d != %d", i, gotLen, tt.wantLens[i]) } - gotGSO, err := getGSOSize(msgs[i].OOB) + // coalesceMessages calls setGSOSizeInControl, which uses a cmsg + // type of UDP_SEGMENT, and getGSOSizeInControl scans for a cmsg + // type of UDP_GRO. Therefore, we have to use the lower-level + // getDataFromControl in order to specify the cmsg type of + // interest for this test. + data, err := getDataFromControl(msgs[i].OOB, unix.SOL_UDP, unix.UDP_SEGMENT, 2) if err != nil { - t.Fatalf("msgs[%d] getGSOSize err: %v", i, err) + t.Fatalf("msgs[%d] getDataFromControl err: %v", i, err) + } + var gotGSO int + if len(data) >= 2 { + gotGSO = int(binary.NativeEndian.Uint16(data)) } if gotGSO != tt.wantGSO[i] { t.Errorf("msgs[%d] gsoSize %d != %d", i, gotGSO, tt.wantGSO[i])