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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 13, 2024 at 8:07 PM Kangjing Huang <huangkangjing@xxxxxxxxx> 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?

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@xxxxxxxxxx> 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@xxxxxxxxxxxxx> wrote:
> > > >
> > > > On Wed, 6 Nov 2024 15:59:10 +0200
> > > > Leon Romanovsky <leon@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx 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





[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux