mirror of
https://github.com/slackhq/nebula.git
synced 2026-08-15 16:57:02 +02:00
docs: mark six deferred review findings with TODOs
Behavior untouched; each marker records a known gap and the intended fix so the next visit doesn't rediscover it: transient zero-sent sendmmsg errors drop a whole run; the non-vnet Poll queue lacks the post-wake drain loop; recvmmsg controllen resets touch every entry; cached handshake packets flush one syscall each; the routines clamp in activate() would blackhole surplus REUSEPORT sockets if it ever became reachable; darwin WriteBatch burst-drops on EWOULDBLOCK. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ugV2edVqoz3tBvq9J6yWp
This commit is contained in:
@@ -975,7 +975,9 @@ func (hm *HandshakeManager) continueHandshake(via ViaSender, hh *HandshakeHostIn
|
||||
nb := make([]byte, 12, 12)
|
||||
out := make([]byte, mtu)
|
||||
for _, cp := range hh.packetStore {
|
||||
//todo use a sendbatcher
|
||||
// TODO: use a SendBatch here. Each callback lands in
|
||||
// sendNoMetrics -> WriteTo: one syscall per cached packet,
|
||||
// where one sendmmsg could flush the whole store.
|
||||
cp.callback(cp.messageType, cp.messageSubType, hostinfo, cp.packet, nb, out)
|
||||
}
|
||||
f.cachedPacketMetrics.sent.Inc(int64(len(hh.packetStore)))
|
||||
|
||||
@@ -292,6 +292,13 @@ func (f *Interface) activate() error {
|
||||
return err
|
||||
}
|
||||
if len(queues) < f.routines {
|
||||
// TODO: this clamp is only safe because it is unreachable when the
|
||||
// udp side has multiple readers (linux Queues opens exactly n or
|
||||
// errors; every other platform already clamped routines to 1 above).
|
||||
// If a platform ever returns fewer queues than routines with
|
||||
// SO_REUSEPORT sockets already bound, the surplus sockets get no
|
||||
// listenOut and the kernel blackholes every flow it hashes to them —
|
||||
// fail loudly or close the extra sockets instead.
|
||||
f.l.Warn("tun multiqueue is not supported on this platform, falling back to fewer routines",
|
||||
"requested", f.routines, "opened", len(queues))
|
||||
f.routines = len(queues)
|
||||
|
||||
@@ -47,6 +47,11 @@ func (t *Poll) blockOnWrite() error {
|
||||
return blockOn(int32(t.fd), int32(t.shutdownFd), unix.POLLOUT)
|
||||
}
|
||||
|
||||
// TODO: port Offload's post-wake drain loop here so one poll wake amortizes
|
||||
// over a burst (up to tunDrainCap packets) instead of paying a syscall and a
|
||||
// wake per packet. Hosts on the TUNSETOFFLOAD-failure fallback or a tun.fd
|
||||
// config currently lose that batching. blockOn and the EAGAIN plumbing are
|
||||
// already shared; kept one-packet-per-Read for now to preserve behavior.
|
||||
func (t *Poll) Read() ([]Packet, error) {
|
||||
n, err := t.readOne(t.readBuf)
|
||||
if err != nil {
|
||||
|
||||
@@ -142,6 +142,9 @@ func (u *StdConn) WriteTo(b []byte, ap netip.AddrPort) error {
|
||||
|
||||
func (u *StdConn) WriteBatch(bufs [][]byte, addrs []netip.AddrPort, _ []byte) (int, error) {
|
||||
// An un-sendable destination costs its own packet, never the ones behind it in the batch.
|
||||
// TODO: WriteTo maps EWOULDBLOCK to an error, so a full send buffer
|
||||
// silently drops the rest of a burst (linux blocks instead). Poll for
|
||||
// writability on EAGAIN before giving up on the remainder.
|
||||
written := 0
|
||||
for i, b := range bufs {
|
||||
if err := u.WriteTo(b, addrs[i]); err == nil {
|
||||
|
||||
@@ -313,6 +313,9 @@ func (u *StdConn) ListenOut(r EncReader, flush func()) error {
|
||||
|
||||
for {
|
||||
if cmsgSpace > 0 {
|
||||
// TODO: the kernel only rewrites Controllen on entries it fills,
|
||||
// so resetting just the first `n` from the previous wakeup would
|
||||
// save ~(batch-n) stores per wakeup on trickle traffic.
|
||||
for i := range msgs {
|
||||
setMsgControllen(&msgs[i].Hdr, cmsgSpace)
|
||||
}
|
||||
|
||||
@@ -289,6 +289,12 @@ func (w *batchWriter) WriteBatch(bufs [][]byte, addrs []netip.AddrPort, ecns []b
|
||||
i = baseI
|
||||
continue
|
||||
}
|
||||
// TODO: a transient zero-sent errno (ENOBUFS under socket-memory
|
||||
// pressure, or a theoretical EINTR) lands here too and drops
|
||||
// entry 0's entire run (up to 63/127 packets). The RX path
|
||||
// retries EINTR; consider a bounded retry for those two before
|
||||
// falling through to the per-entry drop.
|
||||
//
|
||||
// Any other zero-sent error is a per-entry failure:
|
||||
// an unreachable destination, a firewall EPERM, or a PMTU shrink after a roam
|
||||
// (EINVAL, or EMSGSIZE since kernel 6.14, once gso_size no longer fits the path).
|
||||
|
||||
Reference in New Issue
Block a user