Hi Leon, > -----Original Message----- > From: Leon Romanovsky <leon@xxxxxxxxxx> > Sent: Friday, March 15, 2019 6:25 AM > To: Parav Pandit <parav@xxxxxxxxxxxx> > Cc: Ira Weiny <ira.weiny@xxxxxxxxx>; Doug Ledford <dledford@xxxxxxxxxx>; > Jason Gunthorpe <jgg@xxxxxxxxxxxx>; RDMA mailing list <linux- > rdma@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH rdma-next 3/3] RDMA/nldev: Return device protocol > > On Fri, Mar 15, 2019 at 06:13:17AM +0000, Parav Pandit wrote: > > > > > > > -----Original Message----- > > > From: Leon Romanovsky <leon@xxxxxxxxxx> > > > Sent: Friday, March 15, 2019 12:58 AM > > > To: Ira Weiny <ira.weiny@xxxxxxxxx> > > > Cc: Parav Pandit <parav@xxxxxxxxxxxx>; Doug Ledford > > > <dledford@xxxxxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxxxxxx>; RDMA > > > mailing list <linux-rdma@xxxxxxxxxxxxxxx> > > > Subject: Re: [PATCH rdma-next 3/3] RDMA/nldev: Return device > > > protocol > > > > > > On Thu, Mar 14, 2019 at 02:34:34AM -0700, Ira Weiny wrote: > > > > On Sun, Mar 10, 2019 at 05:02:45PM +0000, Parav Pandit wrote: > > > > > > > > > > > > On Sun, Mar 10, 2019 at 04:45:30PM +0000, Parav Pandit wrote: > > > > > > > > > > > > > > > > On Sun, Mar 10, 2019 at 04:18:37PM +0000, Parav Pandit wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > Reuse existing RDMA_NLDEV_ATTR_LINK_TYPE to give > > > > > > > > > > ability for stable names UDEV rule create Ib device > > > > > > > > > > stable names based on link type > > > > > > > > protocol. > > > > > > > > > > The assumption that devices like mlx4 with duality in > > > > > > > > > > their link type under one IB device struct won't be > > > > > > > > > > allowed in > > > the future. > > > > > > > > > > > > > > > > > > > I was under impression that it qedr or cavium driver has > > > > > > > > > iwarp and roce > > > > > > > > ports on same hca. > > > > > > > > > Any reason to not have the link type on per port basis? > > > > > > > > > > > > > > > > Not really, they don't mix link types in one IB device, I > > > > > > > > remember that Jason ensured that during code review. > > > > > > > > > > > > > > > > > If it already exist at port level, than at device level > > > > > > > > > addition is > > > > > > confusing. > > > > > > > > > It is like having port_num in ah_attr and also in qp_attr. > > > > > > > > > > > > > > > > It is just a name with already existing index and proper values. > > > > > > > > What name do you think more appropriate? I'll add alias > > > > > > > > for that, something like > "RDMA_NLDEV_ATTR_NEW_COOL_NAME = > > > > > > > > RDMA_NLDEV_ATTR_LINK_TYPE" > > > > > > > Why can't we keep it as port attribute? > > > > > > > > > > > > I didn't find any reason to expose it as port attribute, > > > > > > especially after Jason's "request" to do "technology" property > > > > > > per- > > > device. > > > > > It is at port level in verbs, so it is not harmful to keep it as > > > > > port level, > > > instead of duplicating it at device level. > > > > > > > > > > > > > I agree. When we went to the "port_imutable" patch set years ago > > > > we started a move toward having attributes be port based as much > > > > as > > > possible. > > > > > > Ira, Parav, > > > > > > The fact that standard describes that link type is per-port has > > > nothing to do with this patch. The Linux implementation of IB > > > devices (exclude > > > mlx4) is one type per-device for all their ports. The HW device > > > which needs to expose different protocols on its ports will create > > > separate IB devices for each protocol. > > > > > > > > > > > I'm failing to understand why link type needs to be part of an > > > > immutable device instance name? This is where "mlx4_0" worked > > > > because it was a "mlx4" device -- device instance number 0. > > > > > > Mainly because we want to be nice to our users, so they won't need > > > to update their scripts each time they change one RoCE adapter to > another. > > > All those adapters will have common and well understandable name > > > "roce....". > > > > > > > > > > > I know that we are moving toward a single driver supporting more > > > > device types so having the driver name is probably not the right > > > > name but perhaps we should just name the devices. Since we are > > > > already cryptic should we use the PCI device ID? But using driver > > > > name could still > > > work. Couldn't it? > > > > > > I'm not sure that I understand your worries here. Kernel names are > > > not going to be changed after this patch and it is userspace "job" > > > to rename them to something more stable, based on PCI or GUID. > > > > > > I tried to document how it is going to be and it includes an option > > > to disable such renaming. > > > https://github.com/linux-rdma/rdma- > > > core/pull/487/commits/03ba0496c78d9418f8bbe82eb4828f16b8b0ecf9 > > > > > > > > > > > I still think this is going to be hard for users. But eventually > > > > I think it will be better once the tools figure out how to "translate" > > > > and/or users figure out how to assign names. > > > > > I do not understand udev framework a lot. Hence the below dumb > question: > > Why link type of first port cannot be ready by the user space to build the > stable name? > > This is exactly what this patch is doing - providing such information. > Prior to this patch, we simply didn't have any way to understand protocol > during device creation. > > > (because of which link type per device should be provided by kernel) > > After this patch, we will be able to fix hardcoded mapping between driver > module name and protocol supported. > https://github.com/linux-rdma/rdma-core/blob/master/kernel-boot/rdma- > description.rules#L10 > > Thanks I looked at existing code. I got confused with your commit log - " Reuse existing RDMA_NLDEV_ATTR_LINK_TYPE" This attribute today is not exposed in port info via netlink. I misunderstood that 'reuse this port info now at device level too'. So your patch looks fine because this is not a duplicated field between port and device level.
