netdev - Re: [PATCH net 1/2] tcp_metrics: set maximum cwnd from the dst entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250617133935.60f621db@mordecai.tesarici.cz>
Date: Tue, 17 Jun 2025 13:39:35 +0200
From: Petr Tesarik <ptesarik@...e.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Neal Cardwell <ncardwell@...gle.com>, Kuniyuki
 Iwashima <kuniyu@...gle.com>, "open list:NETWORKING [TCP]"
 <netdev@...r.kernel.org>, David Ahern <dsahern@...nel.org>, Jakub Kicinski
 <kuba@...nel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 1/2] tcp_metrics: set maximum cwnd from the dst
 entry

On Tue, 17 Jun 2025 13:00:53 +0200
Paolo Abeni <pabeni@...hat.com> wrote:

> On 6/13/25 12:20 PM, Petr Tesarik wrote:
> > diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
> > index 4251670e328c8..dd8f3457bd72e 100644
> > --- a/net/ipv4/tcp_metrics.c
> > +++ b/net/ipv4/tcp_metrics.c
> > @@ -477,6 +477,9 @@ void tcp_init_metrics(struct sock *sk)
> >  	if (!dst)
> >  		goto reset;
> >  
> > +	if (dst_metric_locked(dst, RTAX_CWND))
> > +		tp->snd_cwnd_clamp = dst_metric(dst, RTAX_CWND);
> > +
> >  	rcu_read_lock();
> >  	tm = tcp_get_metrics(sk, dst, false);
> >  	if (!tm) {
> > @@ -484,9 +487,6 @@ void tcp_init_metrics(struct sock *sk)
> >  		goto reset;
> >  	}
> >  
> > -	if (tcp_metric_locked(tm, TCP_METRIC_CWND))
> > -		tp->snd_cwnd_clamp = tcp_metric_get(tm, TCP_METRIC_CWND);
> > -
> >  	val = READ_ONCE(net->ipv4.sysctl_tcp_no_ssthresh_metrics_save) ?
> >  	      0 : tcp_metric_get(tm, TCP_METRIC_SSTHRESH);
> >  	if (val) {  
> 
> It's unclear to me why you drop the tcp_metric_get() here. It looks like
> the above will cause a functional regression, with unlocked cached
> metrics no longer taking effects?

Unlocked cached TCP_METRIC_CWND has never taken effects. As you can
see, tcp_metric_get() was executed only if the metric was locked. 

In fact, the cwnd parameter in the route does not have any effect
either. It's even documented in the manual page of ip-route(8):

              cwnd NUMBER (Linux 2.3.15+ only)
                     the clamp for congestion window. It is ignored if
                     the lock flag is not used.

Note that here is also an initcwnd parameter, and I'm not changing
anything about the handling of that one.

Now, if you think that this TCP_METRIC_CWND is quite useless, then I
wholeheartedly agree with you, but we cannot simply remove it, as it
has become part of uapi, defined in include/uapi/linux/tcp_metrics.h.

Petr T