Hi Leon, > -----Original Message----- > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- > owner@xxxxxxxxxxxxxxx> On Behalf Of Parav Pandit > Sent: Saturday, March 16, 2019 11:44 AM > To: Leon Romanovsky <leon@xxxxxxxxxx> > 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 > > 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. Thinking little more, before your patch RDMA_NLDEV_ATTR_LINK_TYPE is used to refer a module such as rxe. And in this patch same RDMA_NLDEV_ATTR_LINK_TYPE refers to ib/iw/roce/opa etc. So if tomorrow if one wants to create IB link type using rxe, or iwarp link type using qedr how one shall describe such command? We probably shouldn't overload this field which is currently describes which kernel module to load as link.
