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