Re: [PATCH v2] ksmbd: fix missing RDMA-capable flag for IPoIB device in ksmbd_rdma_capable_netdev()

[Kangjing Huang <huangkangjing@gmail.com>]

On 10/20/23 19:08, Namjae Jeon wrote:
> 2023-10-21 5:27 GMT+09:00, Tom Talpey <tom@talpey.com>:
>> On 10/20/2023 10:45 AM, Namjae Jeon wrote:
>>> 2023-10-20 22:02 GMT+09:00, Kangjing "Chaser" Huang
>>> <huangkangjing@gmail.com>:
>>>> Physical ib_device does not have an underlying net_device, thus its
>>>> association with IPoIB net_device cannot be retrieved via
>>>> ops.get_netdev() or ib_device_get_by_netdev(). ksmbd reads physical
>>>> ib_device port GUID from the lower 16 bytes of the hardware addresses on
>>>> IPoIB net_device and match its underlying ib_device using ib_find_gid()
>>>>
>>>> Signed-off-by: Kangjing Huang <huangkangjing@gmail.com>
>>>> Acked-by: Namjae Jeon <linkinjeon@kernel.org>
>>> Can you fix the warnings from checkpatch.pl ?  You need to run it like
>>> this. ./scripts/checkpatch.pl [patch]
>>>
>>> WARNING: Block comments use a trailing */ on a separate line
>>> #148: FILE: fs/smb/server/transport_rdma.c:2256:
>>> +			 * check for matching infiniband GUID in hw_addr */
>>>
>>> WARNING: From:/Signed-off-by: email name mismatch: 'From: "Kangjing
>>> \Chaser\ Huang" <huangkangjing@gmail.com>' != 'Signed-off-by: Kangjing
>>> Huang <huangkangjing@gmail.com>'
>>>
>>> total: 0 errors, 2 warnings, 54 lines checked
>>>
>>>
>>> And have you made this patch on linux 6.6-rc6 kernel ? because I can't
>>> apply this patch with the following error.
>>>
>>> checking file fs/smb/server/transport_rdma.c
>>> Hunk #1 FAILED at 2140.
>>> Hunk #2 FAILED at 2241.
>>> 2 out of 2 hunks FAILED
>>>

Sorry about that. I did do it on v6.6-rc6. It may be my mail client's 
issue with plain text emails, I'm sending again together with the 
comment format fixed and the review tag using git send-email.


>>> Thanks.
>>
>> With these glitches addressed, please add my
>>
>> Reviewed-by: Tom Talpey <tom@talpey.com>
> Thanks for your review!
>>
>> Thanks for the update.
>>
>>>>
>>>> v2 -> v1:
>>>> * Add more detailed description to comment
>>>> ---
>>>>     fs/smb/server/transport_rdma.c | 39
>>>> +++++++++++++++++++++++++---------
>>>>     1 file changed, 29 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/fs/smb/server/transport_rdma.c
>>>> b/fs/smb/server/transport_rdma.c
>>>> index 3b269e1f523a..a623e29b2760 100644
>>>> --- a/fs/smb/server/transport_rdma.c
>>>> +++ b/fs/smb/server/transport_rdma.c
>>>> @@ -2140,8 +2140,7 @@ static int smb_direct_ib_client_add(struct
>>>> ib_device
>>>> *ib_dev)
>>>>     	if (ib_dev->node_type != RDMA_NODE_IB_CA)
>>>>     		smb_direct_port = SMB_DIRECT_PORT_IWARP;
>>>>
>>>> -	if (!ib_dev->ops.get_netdev ||
>>>> -	    !rdma_frwr_is_supported(&ib_dev->attrs))
>>>> +	if (!rdma_frwr_is_supported(&ib_dev->attrs))
>>>>     		return 0;
>>>>
>>>>     	smb_dev = kzalloc(sizeof(*smb_dev), GFP_KERNEL);
>>>> @@ -2241,17 +2240,37 @@ bool ksmbd_rdma_capable_netdev(struct net_device
>>>> *netdev)
>>>>     		for (i = 0; i < smb_dev->ib_dev->phys_port_cnt; i++) {
>>>>     			struct net_device *ndev;
>>>>
>>>> -			ndev = smb_dev->ib_dev->ops.get_netdev(smb_dev->ib_dev,
>>>> -							       i + 1);
>>>> -			if (!ndev)
>>>> -				continue;
>>>> +			if (smb_dev->ib_dev->ops.get_netdev) {
>>>> +				ndev = smb_dev->ib_dev->ops.get_netdev(
>>>> +					smb_dev->ib_dev, i + 1);
>>>> +				if (!ndev)
>>>> +					continue;
>>>>
>>>> -			if (ndev == netdev) {
>>>> +				if (ndev == netdev) {
>>>> +					dev_put(ndev);
>>>> +					rdma_capable = true;
>>>> +					goto out;
>>>> +				}
>>>>     				dev_put(ndev);
>>>> -				rdma_capable = true;
>>>> -				goto out;
>>>> +			/* if ib_dev does not implement ops.get_netdev
>>>> +			 * check for matching infiniband GUID in hw_addr */
>>>> +			} else if (netdev->type == ARPHRD_INFINIBAND) {
>>>> +				struct netdev_hw_addr *ha;
>>>> +				union ib_gid gid;
>>>> +				u32 port_num;
>>>> +				int ret;
>>>> +
>>>> +				netdev_hw_addr_list_for_each(
>>>> +					ha, &netdev->dev_addrs) {
>>>> +					memcpy(&gid, ha->addr + 4, sizeof(gid));
>>>> +					ret = ib_find_gid(smb_dev->ib_dev, &gid,
>>>> +							  &port_num, NULL);
>>>> +					if (!ret) {
>>>> +						rdma_capable = true;
>>>> +						goto out;
>>>> +					}
>>>> +				}
>>>>     			}
>>>> -			dev_put(ndev);
>>>>     		}
>>>>     	}
>>>>     out:
>>>> --
>>>> 2.42.0
>>>>
>>>>
>>>
>>