> -----Original Message----- > From: Leon Romanovsky <leon@xxxxxxxxxx> > Sent: Monday, March 18, 2019 1:11 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 Mon, Mar 18, 2019 at 04:55:19AM +0000, Parav Pandit wrote: > > 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/rd > > > > ma- > > > > 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. > > As we described before, we won't allow mixing of different link types in the > same IB devices, it means that users won't need to provide link type at all. > driver_type != link_type. If you want to create two devices with different link type in rxe (by one driver), you need a way to differentiate it. > In case, they would like to create extra iwarp link, they are encouraged to > use devlink port functionality. > http://man7.org/linux/man-pages/man8/devlink-port.8.html > I am not sure if you actually read my technical points (specifically #4) before replying in this thread [1]. [1] https://www.spinics.net/lists/linux-rdma/msg76877.html Hence a new attribute should be defined regardless of whether we go loopback route or rxe.
