diff mbox series

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

Message ID 20241024054456.37124-1-guwen@linux.alibaba.com (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net/smc: use new helper to get the netdev associated to an ibdev | expand

Commit Message

Wen Gu Oct. 24, 2024, 5:44 a.m. UTC
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(-)

Comments

Wenjia Zhang Oct. 24, 2024, 10 a.m. UTC | #1
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
Wen Gu Oct. 24, 2024, 11:06 a.m. UTC | #2
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
Wenjia Zhang Oct. 24, 2024, 11:31 a.m. UTC | #3
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
diff mbox series

Patch

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);