On Wed, Sep 26, 2018 at 10:44:56AM -0500, Steve Wise wrote:
>
>
> On 9/26/2018 3:05 AM, Leon Romanovsky wrote:
> > On Tue, Sep 25, 2018 at 02:11:18PM -0600, Jason Gunthorpe wrote:
> >> On Tue, Sep 25, 2018 at 08:55:29PM +0300, Leon Romanovsky wrote:
> >>> On Thu, Sep 13, 2018 at 12:16:20PM -0700, Steve Wise wrote:
> >>>> Add support for new LINK messages to allow adding and deleting rdma
> >>>> interfaces. This will be used initially for soft rdma drivers which
> >>>> instantiate device instances dynamically by the admin specifying a netdev
> >>>> device to use. The rdma_rxe module will be the first user of these
> >>>> messages.
> >>>>
> >>>> The design is modeled after RTNL_NEWLINK/DELLINK: rdma drivers
> >>>> register with the rdma core if they provide link add/delete functions.
> >>>> Each driver registers with a unique "type" string, that is used to
> >>>> dispatch messages coming from user space. A new RDMA_NLDEV_ATTR is
> >>>> defined for the "type" string. User mode will pass 3 attributes in a
> >>>> NEWLINK message: RDMA_NLDEV_ATTR_IBDEV_NAME for the desired rdma device
> >>>> name to be created, RDMA_NLDEV_ATTR_LINK_TYPE for the "type" of link
> >>>> being added, and RDMA_NLDEV_ATTR_NDEV_NAME for the net_device interface
> >>>> to use for this link. The DELLINK message will contain the IBDEV_NAME
> >>>> and LINK_TYPE attributes.
> >>>>
> >>>> Signed-off-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> >>>
> >>> <...>
> >>>>
> >>>> + RDMA_NLDEV_CMD_NEWLINK,
> >>>> +
> >>>> + RDMA_NLDEV_CMD_DELLINK,
> >>>> +
> >>>> RDMA_NLDEV_NUM_OPS
> >>>> };
> >>>>
> >>>> @@ -427,6 +431,11 @@ enum rdma_nldev_attr {
> >>>> RDMA_NLDEV_ATTR_DRIVER_U64, /* u64 */
> >>>>
> >>>> /*
> >>>> + * Identifies the rdma driver. eg: "rxe" or "siw"
> >>>> + */
> >>>> + RDMA_NLDEV_ATTR_LINK_TYPE, /* string */
> >>>
> >>> I'll review it more deeply tomorrow, but two things caught my attention:
> >>> 1. You need RDMA_NL_ADMIN_PERM for your netlink commands
> >>> 2. LINK_TYPE should be index and not string, because it is why we are
> >>> using using netlink :)
> >>
> >> netdev uses strings. See IFLA_INFO_KIND
> >>
> >> strings are not bad in netlink
> >
> > It is not the best example, because IFLA_INFO_KIND is used to forward
> > filtering information and one day we will do the same with various
> > rdmatool dumping options. However in case of ibdevice type, it is
> > declared as number and doesn't seem to me right to convert it to string
> > and limit our KABI to name.
> >
>
> Are you suggesting rdmatool uses enum rdma_driver_id from
> rdma/rdma_user_ioctl_cmds.h? So I add code in rdmatool to map the
> user's string, eg "mlx5" or "rxe" into the enum and send that down?
>
> That works for me. If rdma_driver_id is part of the KABI, then I guess
> its fine for rdmatool to use it and map names to enum values...
I think this is a bad idea, we don't want to tie the kernel to iproute
so tightly, flowing the string from the command line is the right way
to handle this, just like netdev does.
Also, remember, the DRIVER_ID specifies the driver *NOT* the link
type. It would be perfectly resonable for a driver like RXE to have
multiple link types using the same user space driver. I can't imagine
a use case for this, but is wrong to mix these two different concepts.
Jason
