Re: [PATCH rdma-next 3/3] RDMA/nldev: Return device protocol

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux