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

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

On Fri, Dec 13, 2024, at 13:07, Kangjing Huang 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.

This is exactly the purpose and design of IPoIB, to present regular netdev to the users and hide IB layer from them.

> 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.
>
> I am uncertain about how to proceed at this point and would like to
> know your thoughts and opinions on this.

Delete this code completely and make sure that ksmbd has two paths only. One for netdevs and one for ib_devices. It is upto users to decide on which interface to run.

Thanks 

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