NAS-141771 / 27.0.0-BETA.1 / fix NTB TCP 0 sized (permanent) window clamp

[yocalebo]

TCP connections crossing the NTB interconnect between HA controllers can
stall with the peer advertising a zero receive window that never reopens.
File copies hang silently, DLM drops its connection for roughly 20 seconds
at a time, and long lived websocket sessions see unexplained disconnects.
This change fixes the root cause in the ntb_netdev receive path.

Root cause

ntb_netdev posts full MTU sized receive buffers into the transport ring and
delivers every received frame to the network stack in that buffer no matter
how small the frame actually is. At the default 64KB transport MTU each
buffer rounds up to a 128KB allocation, so a 54 byte pure TCP ACK reaches
the socket with a truesize near 131KB, a charge of roughly 2400 times the
real data size against the socket receive buffer.

TCP sizes its advertised window from truesize based accounting. A burst of
small control messages fills the receive buffer after a few dozen packets,
the advertised window collapses to zero, and the window regrowth paths are
gated on incoming packets having a sane length to truesize ratio, which
never happens on this device. Bidirectional ssh traffic can then deadlock
permanently because application level flow control wedges on both sides
while both receive windows are shut.

Fix

Copy received frames of rx_copybreak bytes or less (default 2048, runtime
tunable) into a right sized skb allocated from the page frag cache and
recycle the jumbo ring buffer immediately. Small control frames then reach
the stack with honest truesize and receive windows stay open. Bulk frames
keep the existing zero copy buffer swap. The receive hot path also stops
performing a high order atomic allocation for every small frame.

Verification

Measured on H20 and F60 HA pairs at 64KB MTU using a reproduction harness
that drives bidirectional bulk copies, small message chatter and CPU load,
comparing builds that differ only by this patch.

• truesize per small packet drops from 131328 bytes to 960 bytes
• receive buffer overcommit drops from about 1651x to about 12x
• TCPWantZeroWindowAdv over a four minute load window drops from 24392 to
  about 1000
• the unpatched build parks multiple sockets in the TCP persist state
  within three seconds of load while the patched build shows zero wedged
  sockets across repeated full runs
• a production configured F60 accumulated over 600k wanted zero window
  events in ten hours of ordinary operation before the fix and near zero
  after
• on the 6.12 stable train the same defect surfaces as receive queue
  collapse storms of roughly 170k TCPRcvCollapsed per four minute load
  window, driven by the same truesize inflation this patch eliminates

[anodos325]

I think there's a problem where if len <= rx_copybreak < ETH_HLEN, it can reach netdev_alloc_skb_ip_align(ndev, len) and eth_type_trans() will read uninitialized heap past the copied bytes. Because of the changes here I think we need to add guard against if (len < ETH_HLEN) rather than simply if (len < 0) in line 123. Though this area seems rather hairy and the driver is unfamiliar.

[amotin]

I think we need to add guard against if (len < ETH_HLEN) rather than simply if (len < 0) in line 123

if (len < 0) is a check for errors returned by ntb_transport. But I agree that receive of less then Ethernet header here should be illegal. We might count it as a different error kind, but yeah, I agree, I should not have sense to pass it to the network stack.

[amotin]

I think you've nailed the real cause. It seems TCP limits amount of memory used by the socket, and if it is too high, it blocks new receives until the existing ones are processed. Since for small receives memory use will be terribly inefficient, I wonder if socket might not accumulate enough data to reach the threshold making application to actually read the received data from socket to free the memory.

I don't know if 2KB is enough here, but this could be one of the solutions. I'd probably prefer 3648 AKA SKB_MAX_ORDER(NET_SKB_PAD, 0) as a copybreak to take the most out of single page allocations. Another alternative I can think of is making ntb_transport to allocate memory, but since it does not know anything about networking, it might be too much for it to know about netdev_alloc_skb(), unless it will be wrapped into some callback, which means its API change.

In addition to this we might want to adjust MTU in a way to make the allocated skb's to fit into 64KB without rounding to 128KB, so that we waste less memory on large packets too. That would be 65074, so our earlier attempt to use 64000 actually had some sense, and we could keep it.

[amotin]

PS: "so a 54 byte pure TCP ACK reaches the socket with a truesize near 131KB" -- I'd guess, and Claude agrees, that pure ACK should not be put on a socket queue, so should be irrelevant to this problem. I guess there should be at least one data byte actually received.

[yocalebo]

I've addressed all reviews

[bugclerk]

Jira URL: https://ixsystems.atlassian.net/browse/NAS-141771

[bugclerk]

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.