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

["Leon Romanovsky" <leon@kernel.org>]

On Sat, Dec 14, 2024, at 04:33, Namjae Jeon wrote:
> On Fri, Dec 13, 2024 at 8:07 PM Kangjing Huang <huangkangjing@gmail.com> wrote:
>>
>> Hi there,
>>
>> I am the original author of commit ecce70cf17d9 ("ksmbd: fix missing
>> RDMA-capable flag for IPoIB device in ksmbd_rdma_capable_netdev()"),
>> as mentioned in the thread.
>>
>> I am working on modifying the patch to take care of the layering
>> violation. The original patch was meant to fix an issue with ksmbd,
>> where an IPoIB netdev was not recognized as RDMA-capable. The original
>> version of the capability evaluation tries to match each netdev to
>> ib_device by calling get_netdev in ib verbs. However this only works
>> in cases where the ib_device is the upper layer of netdev (e.g. RoCE),
>> and since with IPoIB it is the other way around (netdev is the upper
>> layer of ib_device), get_netdev won't work anymore.
>>
>> I tried to replicate the behavior of device matching reversely in the
>> original version of my patch using GID, which ended up as the layering
>> violation. However I am unaware of any exported functions from the
>> IPoIB driver that could do the reverse lookup from netdev to the lower
>> layer ib_device. Actually it seems that the IPoIB driver does not have
>> any exported symbols at all.
>>
>> It might be that the device matching in reverse just does not make any
>> sense and does not need to be done at all. As long as it is an IPoIB
>> device (netdev->type == ARPHRD_INFINIBAND) it might be ok to just
>> automatically assume it is RDMA-capable. I am not 100% sure about this
>> though.
> Why can't we assume RDMA-capable if it's ARPHRD_INFINIBAND type?
> How about assuming it's RDMA-capable and allowing users to turn
> RDMA-capable on/off via sysfs?

Any attempt to treat ipoib differently from regular netdevice is wrong by definition.

>
> Thanks!
>>
>> I am uncertain about how to proceed at this point and would like to
>> know your thoughts and opinions on this.
>>
>> Thanks,
>> Kangjing
>>
>> On Fri, Nov 8, 2024 at 5:59 PM Leon Romanovsky <leon@kernel.org> wrote:
>> >
>> > On Fri, Nov 08, 2024 at 08:40:40AM +0900, Namjae Jeon wrote:
>> > > On Thu, Nov 7, 2024 at 9:00 PM Halil Pasic <pasic@linux.ibm.com> wrote:
>> > > >
>> > > > On Wed, 6 Nov 2024 15:59:10 +0200
>> > > > Leon Romanovsky <leon@kernel.org> wrote:
>> > > >
>> > > > > > Does  fs/smb/server/transport_rdma.c qualify as inside of RDMA core code?
>> > > > >
>> > > > > RDMA core code is drivers/infiniband/core/*.
>> > > >
>> > > > Understood. So this is a violation of the no direct access to the
>> > > > callbacks rule.
>> > > >
>> > > > >
>> > > > > > I would guess it is not, and I would not actually mind sending a patch
>> > > > > > but I have trouble figuring out the logic behind  commit ecce70cf17d9
>> > > > > > ("ksmbd: fix missing RDMA-capable flag for IPoIB device in
>> > > > > > ksmbd_rdma_capable_netdev()").
>> > > > >
>> > > > > It is strange version of RDMA-CM. All other ULPs use RDMA-CM to avoid
>> > > > > GID, netdev and fabric complexity.
>> > > >
>> > > > I'm not familiar enough with either of the subsystems. Based on your
>> > > > answer my guess is that it ain't outright bugous but still a layering
>> > > > violation. Copying linux-cifs@vger.kernel.org so that
>> > > > the smb are aware.
>> > > Could you please elaborate what the violation is ?
>> >
>> > There are many, but the most screaming is that ksmbd has logic to
>> > differentiate IPoIB devices. These devices are pure netdev devices
>> > and should be treated like that. ULPs should treat them exactly
>> > as they treat netdev devices.
>> >
>> > > I would also appreciate it if you could suggest to me how to fix this.
>> > >
>> > > Thanks.
>> > > >
>> > > > Thank you very much for all the explanations!
>> > > >
>> > > > Regards,
>> > > > Halil
>> > > >
>>
>>
>>
>> --
>> Kangjing "Chaser" Huang