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: <CAKYAXd_Dj+YCKYQNO_g4WwARcxkKCB4TZLriuzhm-u9o+kuAoA@mail.gmail.com>
Date: Wed, 8 Jan 2025 08:14:15 +0900
From: Namjae Jeon <linkinjeon@...nel.org>
To: Tom Talpey <tom@...pey.com>, Steve French <smfrench@...il.com>
Cc: He Wang <xw897002528@...il.com>, Steve French <sfrench@...ba.org>, 
	Sergey Senozhatsky <senozhatsky@...omium.org>, linux-cifs@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ksmbd: fix possibly wrong init value for RDMA buffer size

On Wed, Jan 8, 2025 at 6:04 AM Tom Talpey <tom@...pey.com> wrote:
>
> On 1/5/2025 10:39 PM, He Wang wrote:
> > Field `initiator_depth` is for incoming request.
> >
> > According to the man page, `max_qp_rd_atom` is the maximum number of
> > outstanding packaets, and `max_qp_init_rd_atom` is the maximum depth of
> > incoming requests.
>
> I do not believe this is correct, what "man page" are you referring to?
> The commit message is definitely wrong. Neither value is referring to
> generic "maximum packets" nor "incoming requests".
>
> The max_qp_rd_atom is the "ORD" or outgoing read/atomic request depth.
> The ksmbd server uses this to control RDMA Read requests to fetch data
> from the client for certain SMB3_WRITE operations. (SMB Direct does not
> use atomics)
>
> The max_qp_init_rd_atom is the "IRD" or incoming read/atomic request
> depth. The SMB3 protocol does not allow clients to request data from
> servers via RDMA Read. This is absolutely by design, and the server
> therefore does not use this value.
>
> In practice, many RDMA providers set the rd_atom and rd_init_atom to
> the same value, but this change would appear to break SMB Direct write
> functionality when operating over providers that do not.
>
> So, NAK.
>
> Namjae, you should revert your upstream commit.
Okay, Thanks for your review:)
Steve, Please revert it in ksmbd-for-next also.

>
> Tom.
>
> >
> > Signed-off-by: He Wang <xw897002528@...il.com>
> > ---
> >   fs/smb/server/transport_rdma.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/smb/server/transport_rdma.c b/fs/smb/server/transport_rdma.c
> > index 0ef3c9f0b..c6dbbbb32 100644
> > --- a/fs/smb/server/transport_rdma.c
> > +++ b/fs/smb/server/transport_rdma.c
> > @@ -1640,7 +1640,7 @@ static int smb_direct_accept_client(struct smb_direct_transport *t)
> >       int ret;
> >
> >       memset(&conn_param, 0, sizeof(conn_param));
> > -     conn_param.initiator_depth = min_t(u8, t->cm_id->device->attrs.max_qp_rd_atom,
> > +     conn_param.initiator_depth = min_t(u8, t->cm_id->device->attrs.max_qp_init_rd_atom,
> >                                          SMB_DIRECT_CM_INITIATOR_DEPTH);
> >       conn_param.responder_resources = 0;
> >
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ