> -----Original Message----- > From: Neal Cardwell <[email protected]> > Sent: Monday, October 19, 2026 5:53 PM > To: Chia-Yu Chang (Nokia) <[email protected]> > Cc: [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; Koen De Schepper (Nokia) > <[email protected]>; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; Eric Dumazet <[email protected]>; Yuchung Cheng > <[email protected]> > Subject: Re: [PATCH net-next v2 3/3] tcp: Add mss_cache_set_by_cca for CC > algorithm to set MSS > > > CAUTION: This is an external email. Please be very careful when clicking > links or opening attachments. See the URL nok.it/ext for additional > information. > > > > On Sun, Oct 18, 2026 at 5:15 PM <[email protected]> wrote: > > > > From: Chia-Yu Chang <[email protected]> > > > > Make the CC module set the mss_cache smaller than path mtu. This is > > useful > > Since many CC modules will not set the mss_cache value, rather than "Make the > CC module set" I think it's important to say "Allow the CC module to set". > > > for a CC module that maintains an internal fractional cwnd less than 2 > > at very low speed (<100kbps) and very low RTT (<1ms). In this case, > > the minimum snd_cwnd for the stack remains at 2, but the CC module > > will limit the pacing rate to ensure that its internal fractional cwnd > > takes effect. > > Therefore, the CC algorithm can enable fine-grained control without > > causing big rate saw-tooth and delay jitter. > > > > Signed-off-by: Chia-Yu Chang <[email protected]> > > --- > > include/linux/tcp.h | 3 ++- > > net/ipv4/tcp.c | 1 + > > net/ipv4/tcp_output.c | 5 +++-- > > 3 files changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/tcp.h b/include/linux/tcp.h index > > e4d1720f49f0..428d63e46206 100644 > > --- a/include/linux/tcp.h > > +++ b/include/linux/tcp.h > > @@ -232,7 +232,8 @@ struct tcp_sock { > > tcp_usec_ts : 1, /* TSval values in usec */ > > is_sack_reneg:1, /* in recovery from loss with SACK > > reneg? */ > > is_cwnd_limited:1,/* forward progress limited by snd_cwnd? > > */ > > - recvmsg_inq : 1;/* Indicate # of bytes in queue upon > > recvmsg */ > > + recvmsg_inq : 1,/* Indicate # of bytes in queue upon > > recvmsg */ > > + mss_cache_set_by_cca:1;/* mss_cache set by CCA */ > > __cacheline_group_end(tcp_sock_read_txrx); > > > > /* RX read-mostly hotpath cache lines */ diff --git > > a/net/ipv4/tcp.c b/net/ipv4/tcp.c index f4c9101e5996..708e1830227b > > 100644 > > --- a/net/ipv4/tcp.c > > +++ b/net/ipv4/tcp.c > > @@ -457,6 +457,7 @@ void tcp_init_sock(struct sock *sk) > > tp->snd_ssthresh = TCP_INFINITE_SSTHRESH; > > tp->snd_cwnd_clamp = ~0; > > tp->mss_cache = TCP_MSS_DEFAULT; > > + tp->mss_cache_set_by_cca = false; > > > > tp->reordering = > > READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reordering); > > tcp_assign_congestion_control(sk); > > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index > > 3248f3895b15..c53d70718830 100644 > > --- a/net/ipv4/tcp_output.c > > +++ b/net/ipv4/tcp_output.c > > @@ -2082,7 +2082,7 @@ unsigned int tcp_sync_mss(struct sock *sk, u32 pmtu) > > struct inet_connection_sock *icsk = inet_csk(sk); > > int mss_now; > > > > - if (icsk->icsk_mtup.search_high > pmtu) > > + if (icsk->icsk_mtup.search_high > pmtu && > > + !tp->mss_cache_set_by_cca) > > icsk->icsk_mtup.search_high = pmtu; > > This looks suspicious, or at least I don't understand the motivation here. > What's the motivation for this? > > > mss_now = tcp_mtu_to_mss(sk, pmtu); @@ -2096,6 +2096,7 @@ > > unsigned int tcp_sync_mss(struct sock *sk, u32 pmtu) > > > > return mss_now; > > } > > +EXPORT_SYMBOL(tcp_sync_mss); > > > > /* Compute the current effective MSS, taking SACKs and IP options, > > * and even PMTU discovery events into account. > > @@ -2111,7 +2112,7 @@ unsigned int tcp_current_mss(struct sock *sk) > > > > mss_now = tp->mss_cache; > > > > - if (dst) { > > + if (dst && !tp->mss_cache_set_by_cca) { > > u32 mtu = dst_mtu(dst); > > if (mtu != inet_csk(sk)->icsk_pmtu_cookie) > > mss_now = tcp_sync_mss(sk, mtu); > > How is this going to work if the path MTU reduces? For example, if the > sysadmin lowers the MTU to be lower than what's consistent with the MSS value > the CC module chose? Won't the packets from the connection all be bigger than > the MTU, causing weird fragmentation or black-holing? > > AFAICT, to be safe, if the dst_mtu() changes then we still have to call > tcp_sync_mss(), even if the CC module wants some input into the MSS decision. > > Wouldn't it be safer to propose that the CC module optionally sets a ceiling > for the MSS value, rather than giving it direct control over the MSS value? > Along those lines, perhaps CC modules that want to do this could export a new > optional CC module callback that we use in __tcp_mtu_to_mss(), perhaps > something like this? > > /* Clamp it (mss_clamp does not include tcp options) */ > if (mss_now > tp->rx_opt.mss_clamp) > mss_now = tp->rx_opt.mss_clamp; > + /* Optionally clamp MSS based on CC module: */ > + if (ca_ops->max_mss) > + mss_now = min(mss_now, ca_ops->max_mss(sk)); > > Perhaps Eric or Yuchung will have a better idea. :-) > > Best, > neal Hi Neal,
Thanks for the feedback and I've go over this patch again. In the next version, I will add MSS ceiling callback for congestion control. Also, I will avoid exporting tcp_sync_mss by adding one extra tcp_update_mss function exposed for congestion control to update its calculated MSS ceiling. Finally, the applied MSS ceiling will use "mss_now = min_t(u32, mss_now, icsk->icsk_ca_ops->mss_ceiling(sk));". Thanks. Chia-Yu
