[PATCH net-next] net/smc: use new helper to get the netdev associated to an ibdev

[PATCH net-next] net/smc: use new helper to get the netdev associated to an ibdev

[Wen Gu]

Patch [1] provides common interfaces to store and get net devices
associated to an IB device port and removes the ops->get_netdev()
callback of mlx5 driver. So use the new interface in smc.

[1]: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev functions")

Reported-by: D. Wythe <alibuda@linux.alibaba.com>
Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/smc_ib.c   | 8 ++------
 net/smc/smc_pnet.c | 4 +---
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 9297dc20bfe2..9c563cdbea90 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -899,9 +899,7 @@ static void smc_copy_netdev_ifindex(struct smc_ib_device *smcibdev, int port)
 	struct ib_device *ibdev = smcibdev->ibdev;
 	struct net_device *ndev;
 
-	if (!ibdev->ops.get_netdev)
-		return;
-	ndev = ibdev->ops.get_netdev(ibdev, port + 1);
+	ndev = ib_device_get_netdev(ibdev, port + 1);
 	if (ndev) {
 		smcibdev->ndev_ifidx[port] = ndev->ifindex;
 		dev_put(ndev);
@@ -921,9 +919,7 @@ void smc_ib_ndev_change(struct net_device *ndev, unsigned long event)
 		port_cnt = smcibdev->ibdev->phys_port_cnt;
 		for (i = 0; i < min_t(size_t, port_cnt, SMC_MAX_PORTS); i++) {
 			libdev = smcibdev->ibdev;
-			if (!libdev->ops.get_netdev)
-				continue;
-			lndev = libdev->ops.get_netdev(libdev, i + 1);
+			lndev = ib_device_get_netdev(libdev, i + 1);
 			dev_put(lndev);
 			if (lndev != ndev)
 				continue;
diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index a04aa0e882f8..716808f374a8 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -1054,9 +1054,7 @@ static void smc_pnet_find_rdma_dev(struct net_device *netdev,
 		for (i = 1; i <= SMC_MAX_PORTS; i++) {
 			if (!rdma_is_port_valid(ibdev->ibdev, i))
 				continue;
-			if (!ibdev->ibdev->ops.get_netdev)
-				continue;
-			ndev = ibdev->ibdev->ops.get_netdev(ibdev->ibdev, i);
+			ndev = ib_device_get_netdev(ibdev->ibdev, i);
 			if (!ndev)
 				continue;
 			dev_put(ndev);
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net-next] net/smc: use new helper to get the netdev associated to an ibdev

[Wenjia Zhang]

On 24.10.24 07:44, Wen Gu wrote:
> Patch [1] provides common interfaces to store and get net devices
> associated to an IB device port and removes the ops->get_netdev()
> callback of mlx5 driver. So use the new interface in smc.
> 
> [1]: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev functions")
> 
> Reported-by: D. Wythe <alibuda@linux.alibaba.com>
> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
> ---
[...]

We detected the problem as well, and I already sent a patch with the 
same code change in our team internally these. Because some agreement 
issues on the commit message, it is still not sent out externally. Now 
we (our team) have almost an agreement, I'd like to attach it here. 
Please have a look if it is also for you to use:

"
[PATCH net] net/smc: Fix lookup of netdev by using ib_device_get_netdev()

Since/Although commit c2261dd76b54 ("RDMA/device: Add 
ib_device_set_netdev() as an alternative to get_netdev") introduced an 
API ib_device_get_netdev, the SMC-R variant of the SMC protocol 
continued to use the old API ib_device_ops.get_netdev() to lookup 
netdev. As commit 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and 
get_netdev functions") removed the get_netdev callback from 
mlx5_ib_dev_common_roce_ops, calling ib_device_ops.get_netdev didn't 
work any more at least by using a mlx5 device driver. Thus, using 
ib_device_set_netdev() now became mandatory.

Replace ib_device_ops.get_netdev() with ib_device_get_netdev().

Fixes: 54903572c23c ("net/smc: allow pnetid-less configuration")
Fixes: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev 
functions")
"
My main points are:
- This patch should go to net, not net-next. Because it can result in 
malfunction. e.g. if the RoCE devices are used as both handshake device 
and RDMA device without any PNET_ID, it would be failed to find SMC-R 
device, then fallback.
- We need the both fixes, which would help us for the backport


Thanks,
Wenjia

Re: [PATCH net-next] net/smc: use new helper to get the netdev associated to an ibdev

[Wen Gu]

On 2024/10/24 18:00, Wenjia Zhang wrote:
> 
> 
> On 24.10.24 07:44, Wen Gu wrote:
>> Patch [1] provides common interfaces to store and get net devices
>> associated to an IB device port and removes the ops->get_netdev()
>> callback of mlx5 driver. So use the new interface in smc.
>>
>> [1]: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev functions")
>>
>> Reported-by: D. Wythe <alibuda@linux.alibaba.com>
>> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
>> ---
> [...]
> 
> We detected the problem as well, and I already sent a patch with the same code change in our team internally these. Because some agreement issues on the commit message, it is still not sent out externally. Now we (our team) have almost an agreement, I'd like to attach it here. Please have a look if 
> it is also for you to use:
> 
> "
> [PATCH net] net/smc: Fix lookup of netdev by using ib_device_get_netdev()
> 
> Since/Although commit c2261dd76b54 ("RDMA/device: Add ib_device_set_netdev() as an alternative to get_netdev") introduced an API ib_device_get_netdev, the SMC-R variant of the SMC protocol continued to use the old API ib_device_ops.get_netdev() to lookup netdev. As commit 8d159eb2117b ("RDMA/mlx5: 
> Use IB set_netdev and get_netdev functions") removed the get_netdev callback from mlx5_ib_dev_common_roce_ops, calling ib_device_ops.get_netdev didn't work any more at least by using a mlx5 device driver. Thus, using ib_device_set_netdev() now became mandatory.
> 
> Replace ib_device_ops.get_netdev() with ib_device_get_netdev().
> 
> Fixes: 54903572c23c ("net/smc: allow pnetid-less configuration")
> Fixes: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev functions")
> "
> My main points are:
> - This patch should go to net, not net-next. Because it can result in malfunction. e.g. if the RoCE devices are used as both handshake device and RDMA device without any PNET_ID, it would be failed to find SMC-R device, then fallback.
> - We need the both fixes, which would help us for the backport
> 
> 
> Thanks,
> Wenjia

Hi, Wenjia. I see. Since you're ready to post a patch, and this one has some problems,
I think you can supersede this one with yours. It is totally OK for me.

Thanks!
Wen Gu

Re: [PATCH net-next] net/smc: use new helper to get the netdev associated to an ibdev

[Wenjia Zhang]

On 24.10.24 13:06, Wen Gu wrote:
> 
> 
> On 2024/10/24 18:00, Wenjia Zhang wrote:
>>
>>
>> On 24.10.24 07:44, Wen Gu wrote:
>>> Patch [1] provides common interfaces to store and get net devices
>>> associated to an IB device port and removes the ops->get_netdev()
>>> callback of mlx5 driver. So use the new interface in smc.
>>>
>>> [1]: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev 
>>> functions")
>>>
>>> Reported-by: D. Wythe <alibuda@linux.alibaba.com>
>>> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
>>> ---
>> [...]
>>
>> We detected the problem as well, and I already sent a patch with the 
>> same code change in our team internally these. Because some agreement 
>> issues on the commit message, it is still not sent out externally. Now 
>> we (our team) have almost an agreement, I'd like to attach it here. 
>> Please have a look if it is also for you to use:
>>
>> "
>> [PATCH net] net/smc: Fix lookup of netdev by using ib_device_get_netdev()
>>
>> Since/Although commit c2261dd76b54 ("RDMA/device: Add 
>> ib_device_set_netdev() as an alternative to get_netdev") introduced an 
>> API ib_device_get_netdev, the SMC-R variant of the SMC protocol 
>> continued to use the old API ib_device_ops.get_netdev() to lookup 
>> netdev. As commit 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and 
>> get_netdev functions") removed the get_netdev callback from 
>> mlx5_ib_dev_common_roce_ops, calling ib_device_ops.get_netdev didn't 
>> work any more at least by using a mlx5 device driver. Thus, using 
>> ib_device_set_netdev() now became mandatory.
>>
>> Replace ib_device_ops.get_netdev() with ib_device_get_netdev().
>>
>> Fixes: 54903572c23c ("net/smc: allow pnetid-less configuration")
>> Fixes: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev 
>> functions")
>> "
>> My main points are:
>> - This patch should go to net, not net-next. Because it can result in 
>> malfunction. e.g. if the RoCE devices are used as both handshake 
>> device and RDMA device without any PNET_ID, it would be failed to find 
>> SMC-R device, then fallback.
>> - We need the both fixes, which would help us for the backport
>>
>>
>> Thanks,
>> Wenjia
> 
> Hi, Wenjia. I see. Since you're ready to post a patch, and this one has 
> some problems,
> I think you can supersede this one with yours. It is totally OK for me.
> 
> Thanks!
> Wen Gu
> 
Thank you, Wen!
I'll send some fixes this days.

Thanks,
Wenjia