netdev - Re: [PATCH net 2/2] net/smc: Use correct buffer sizes when switching between TCP and SMC

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: <603d2672855d74e9bfc2619156f4ffe7976de4f5.camel@linux.ibm.com>
Date: Wed, 02 Aug 2023 18:47:58 +0200
From: Gerd Bayer <gbayer@...ux.ibm.com>
To: Tony Lu <tonylu@...ux.alibaba.com>
Cc: Wenjia Zhang <wenjia@...ux.ibm.com>, Jan Karcher <jaka@...ux.ibm.com>,
        Paolo Abeni <pabeni@...hat.com>, Karsten Graul <kgraul@...ux.ibm.com>,
        "D .
 Wythe" <alibuda@...ux.alibaba.com>,
        Wen Gu <guwen@...ux.alibaba.com>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        linux-s390@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 2/2] net/smc: Use correct buffer sizes when
 switching between TCP and SMC

On Wed, 2023-08-02 at 20:43 +0800, Tony Lu wrote:
> On Wed, Aug 02, 2023 at 11:33:13AM +0200, Gerd Bayer wrote:
> > Tuning of the effective buffer size through setsockopts was working
> > for
> > SMC traffic only but not for TCP fall-back connections even before
> > commit 0227f058aa29 ("net/smc: Unbind r/w buffer size from clcsock
> > and
> > make them tunable"). That change made it apparent that TCP fall-
> > back
> > connections would use net.smc.[rw]mem as buffer size instead of
> > net.ipv4_tcp_[rw]mem.
> > 
> > Amend the code that copies attributes between the (TCP) clcsock and
> > the
> > SMC socket and adjust buffer sizes appropriately:
> > - Copy over sk_userlocks so that both sockets agree on whether
> > tuning
> >   via setsockopt is active.
> > - When falling back to TCP use sk_sndbuf or sk_rcvbuf as specified
> > with
> >   setsockopt. Otherwise, use the sysctl value for TCP/IPv4.
> > - Likewise, use either values from setsockopt or from sysctl for
> > SMC
> >   (duplicated) on successful SMC connect.
> > 
> > In smc_tcp_listen_work() drop the explicit copy of buffer sizes as
> > that
> > is taken care of by the attribute copy.
> > 
> > Fixes: 0227f058aa29 ("net/smc: Unbind r/w buffer size from clcsock
> > and make them tunable")
> > Signed-off-by: Gerd Bayer <gbayer@...ux.ibm.com>
> > Reviewed-by: Wenjia Zhang <wenjia@...ux.ibm.com>
> > Reviewed-by: Jan Karcher <jaka@...ux.ibm.com>
> 
> Reviewed-by: Tony Lu <tonylu@...ux.alibaba.com>
> 
> > 
> ^^^^ nit: a extra new line here.
I'll clean that up.

> > ---
> >  net/smc/af_smc.c | 76 ++++++++++++++++++++++++++++++++++----------
> > ----
> >  1 file changed, 54 insertions(+), 22 deletions(-)
> > 
> > 
[...]

> > +/* if set, use value set by setsockopt() - else use IPv4 or SMC
> > sysctl value */
> > +static void smc_adjust_sock_bufsizes(struct sock *nsk, struct sock
> > *osk,
> > +                                    unsigned long mask)
> > +{
> > +       struct net *nnet;
> > +
> > +       nnet = nsk->sk_net.net;
> 
> Better to combine these two lines with existed helper.
> 
> struct net *net = sock_net(nsk);
Yes, looks much cleaner.

[...]
> 

Thank you Tony for your review and comments.
I'll be sending out a v2 with your recommendations - but give people a
little more time to look at this version.

Thanks,
Gerd