On 9/26/2018 3:24 AM, 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>
>> ---
>> drivers/infiniband/core/nldev.c | 113 +++++++++++++++++++++++++++++++++++++++
>> include/rdma/rdma_netlink.h | 11 ++++
>> include/uapi/rdma/rdma_netlink.h | 9 ++++
>> 3 files changed, 133 insertions(+)
>>
>> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
>> index 0385ab438320..d107b982c210 100644
>> --- a/drivers/infiniband/core/nldev.c
>> +++ b/drivers/infiniband/core/nldev.c
>> @@ -33,6 +33,7 @@
>> #include <linux/module.h>
>> #include <linux/pid.h>
>> #include <linux/pid_namespace.h>
>> +#include <linux/mutex.h>
>> #include <net/netlink.h>
>> #include <rdma/rdma_cm.h>
>> #include <rdma/rdma_netlink.h>
>> @@ -107,6 +108,8 @@
>> [RDMA_NLDEV_ATTR_DRIVER_U32] = { .type = NLA_U32 },
>> [RDMA_NLDEV_ATTR_DRIVER_S64] = { .type = NLA_S64 },
>> [RDMA_NLDEV_ATTR_DRIVER_U64] = { .type = NLA_U64 },
>> + [RDMA_NLDEV_ATTR_LINK_TYPE] = { .type = NLA_NUL_STRING,
>> + .len = IFNAMSIZ },
>
> In case, we are continuing this path, better to use our define RDMA_NLDEV_ATTR_ENTRY_STRLEN.
>
>> };
>>
>> static int put_driver_name_print_type(struct sk_buff *msg, const char *name,
>> @@ -1072,6 +1075,110 @@ static int nldev_res_get_pd_dumpit(struct sk_buff *skb,
>> return res_get_common_dumpit(skb, cb, RDMA_RESTRACK_PD);
>> }
>>
>> +static LIST_HEAD(link_ops);
>> +static DEFINE_MUTEX(link_ops_mutex);
>> +
>> +void rdma_link_register(struct rdma_link_ops *ops)
>> +{
>> + mutex_lock(&link_ops_mutex);
>> + list_add(&ops->list, &link_ops);
>> + mutex_unlock(&link_ops_mutex);
>> +}
>> +EXPORT_SYMBOL(rdma_link_register);
>> +
>> +void rdma_link_unregister(struct rdma_link_ops *ops)
>> +{
>> + mutex_lock(&link_ops_mutex);
>> + list_del_init(&ops->list);
>
> Why did you use list_del_init and not list_del?
>
> And in case we will use device_type, we won't need manage list at all :)
>
>> + mutex_unlock(&link_ops_mutex);
>> +}
>> +EXPORT_SYMBOL(rdma_link_unregister);
>> +
>> +static const struct rdma_link_ops *link_ops_get(const char *type)
>> +{
>> + const struct rdma_link_ops *ops;
>> +
>> + mutex_lock(&link_ops_mutex);
>> + list_for_each_entry(ops, &link_ops, list) {
>> + if (!strncmp(ops->type, type, IFNAMSIZ))
>> + goto out;
>> + }
>> + ops = NULL;
>> +out:
>> + mutex_unlock(&link_ops_mutex);
>> + return ops;
>> +}
>> +
>> +static int nldev_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nlattr *tb[RDMA_NLDEV_ATTR_MAX];
>> + char ibdev_name[IB_DEVICE_NAME_MAX];
>> + const struct rdma_link_ops *ops;
>> + char ndev_name[IFNAMSIZ];
>> + char type[IFNAMSIZ];
>> + int err;
>> +
>> + err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
>> + nldev_policy, extack);
>> + if (err || !tb[RDMA_NLDEV_ATTR_DEV_NAME] ||
>> + !tb[RDMA_NLDEV_ATTR_LINK_TYPE] || !tb[RDMA_NLDEV_ATTR_NDEV_NAME]) {
>> + err = -EINVAL;
>> + goto err_out;
>> + }
>> +
>> + nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
>> + sizeof(ibdev_name));
>
> Query from userspace should be done RDMA_NLDEV_ATTR_DEV_INDEX, because
> it is our handle and not device name.
I don't understand. There is no ib_device yet, so there is no device
index yet.
>
>> + nla_strlcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
>
> Will it mean that type should be unique?
The type will have to be unique; only one driver can register link ops
for a given type string.
Steve
