Hi Leon, > -----Original Message----- > From: Leon Romanovsky <leon@xxxxxxxxxx> > Sent: Monday, March 18, 2019 10:04 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 02:40:18PM +0000, Parav Pandit wrote: > > > > > > > -----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-boo > > > > > > t/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. > > I don't think that anyone else except you was excited to combine SIW and > RXE, especially user visible front ends. So answer is yes, device_type is equal > to link_type. > It's not about combining SIW and RXE. Its about rxe supporting IB ("ib") and RoCE ("roce") fields of this patch. How one shall create a IB link using rxe driver? Can you please describe? RDMA_NLDEV_ATTR_LINK_TYPE=rxe. How to say this new device type is IB or ROCE? > In very extreme case and for backward compatibility, internally in kernel, we > can write that siw == iwarp and rxe == roce and use them as aliases. > > > > > > 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. > > I read. > > Thanks
