[PATCH net-next v3 0/2] tcp: keep the route window metric when SO_RCVBUF is set before connect

[Shashank Mohan Jain]

v2 was flagged by the netdev AI reviewer, and the review was right on both
points:

 - The TCP_CLOSE/TCP_LISTEN state check in v2 did not cover every path on
   which SO_RCVBUF is applied before tcp_connect_init() runs. The socket is
   already in TCP_SYN_SENT before tcp_connect_init(), so SO_RCVBUF set in
   that gap - via TCP_FASTOPEN_CONNECT before the first write(), or via
   bpf_setsockopt() in a BPF_SOCK_OPS_TCP_CONNECT_CB program - still wrote
   window_clamp and still overrode the route metric.

 - The v2 changelog was wrong about the "no route metric" case (there both
   the old and the patched kernel end at tcp_full_space(), so nothing
   changes) and overstated the impact.

v3 changes the mechanism: instead of keying off the socket state, it
records whether window_clamp was set by the user via
setsockopt(TCP_WINDOW_CLAMP) and keys tcp_connect_init() and
tcp_openreq_init_rwin() off that, so the route metric is applied on every
pre-connection path, the TCP_SYN_SENT ones included. The changelog now
states the real impact: (1) routes with an RTAX_WINDOW metric and (2) a
TCP_WINDOW_CLAMP overwritten by a later SO_RCVBUF on an unconnected socket
or listener.

Patch 1 is the fix. Patch 2 adds packetdrill tests for the plain
SO_RCVBUF-before-connect case, the TCP_WINDOW_CLAMP-vs-SO_RCVBUF case, and
the TCP_FASTOPEN_CONNECT (TCP_SYN_SENT gap) case; the last fails on both
unpatched net-next and the v2 patch and passes with patch 1.

Changes since v2:
 - fix: distinguish a user TCP_WINDOW_CLAMP from an sk_rcvbuf-derived clamp
   (new tp->window_clamp_user bit) instead of checking the socket state, so
   the route metric survives SO_RCVBUF on the TCP_SYN_SENT paths (TFO,
   sock_ops) too; a genuine TCP_WINDOW_CLAMP is no longer overwritten by a
   later SO_RCVBUF.
 - changelog: drop the inaccurate "no route metric" case and the "any
   application" overstatement; state the impact as RTAX_WINDOW routes and
   an overwritten TCP_WINDOW_CLAMP.
 - selftest: add the TCP_WINDOW_CLAMP-vs-SO_RCVBUF and TCP_FASTOPEN_CONNECT
   cases alongside the original one.

Changes since v1 (single [PATCH net]):
 - retargeted from net to net-next, added the packetdrill selftest, and
   rewrote the changelog (the window scale option is still sent, with a
   shift of 0; since a2cbb1603943 the clamp does not persist past the first
   data segments).

Shashank Mohan Jain (2):
  tcp: keep the route window metric when SO_RCVBUF is set before connect
  selftests/net/packetdrill: cover SO_RCVBUF vs the route window clamp

 include/linux/tcp.h                           |  3 ++-
 net/ipv4/tcp.c                                | 12 +++++++++
 net/ipv4/tcp_minisocks.c                      |  7 +++--
 net/ipv4/tcp_output.c                         |  2 +-
 .../tcp_fastopen_connect_rcvbuf_window.pkt    | 22 +++++++++++++++
 .../tcp_rcvbuf_before_connect_window.pkt      | 27 +++++++++++++++++++
 .../tcp_window_clamp_vs_rcvbuf.pkt            | 24 +++++++++++++++++
 7 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_rcvbuf_before_connect_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt


base-commit: d8674294aefef02266c4d47ad10131f1bffbe534
-- 
2.43.0