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...
Steve.
