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

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

 



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.

It is how it is done in netdev with much more large number of users.
The advantages of having stable names are much more than disadvantage
of what we have now.

Thanks

>
> Ira
>

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