Re: [PATCH 1/2] ksmbd: fix possibly wrong init value for RDMA buffer size

[Tom Talpey <tom@talpey.com>]

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.

Tom.

> 
> Signed-off-by: He Wang <xw897002528@gmail.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;
>