> -----Original Message-----
> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Saturday, December 1, 2018 2:00 AM
> To: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> Cc: dledford@xxxxxxxxxx; jgg@xxxxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx;
> BMT@xxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 1/2] RDMA/Core: add
> RDMA_NLDEV_CMD_NEWLINK/DELLINK support
>
> On Fri, Nov 30, 2018 at 01:58:03PM -0800, 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_DEV_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
> > RDMA_NLDEV_ATTR_DEV_INDEX of the device to delete.
> >
> > Signed-off-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > ---
> > drivers/infiniband/core/nldev.c | 139
> +++++++++++++++++++++++++++++++++++++++
> > include/rdma/ib_verbs.h | 2 +
> > include/rdma/rdma_netlink.h | 13 ++++
> > include/uapi/rdma/rdma_netlink.h | 11 +++-
> > 4 files changed, 163 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/nldev.c
> b/drivers/infiniband/core/nldev.c
> > index 63cc74483188..6df4c98da365 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 = RDMA_NLDEV_ATTR_ENTRY_STRLEN
> },
> > };
> >
> > static int put_driver_name_print_type(struct sk_buff *msg, const char
> *name,
> > @@ -1103,6 +1106,134 @@ 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 DECLARE_RWSEM(link_ops_rwsem);
> > +
> > +static const struct rdma_link_ops *link_ops_get(const char *type)
> > +{
> > + const struct rdma_link_ops *ops;
> > +
> > + list_for_each_entry(ops, &link_ops, list) {
> > + if (!strcmp(ops->type, type))
> > + goto out;
> > + }
> > + ops = NULL;
> > +out:
> > + return ops;
> > +}
> > +
> > +void rdma_link_register(struct rdma_link_ops *ops)
> > +{
> > + down_write(&link_ops_rwsem);
> > + if (link_ops_get(ops->type)) {
> > + WARN_ONCE("Duplicate rdma_link_ops! %s\n", ops->type);
> > + goto out;
> > + }
> > + list_add(&ops->list, &link_ops);
> > +out:
> > + up_write(&link_ops_rwsem);
> > +}
> > +EXPORT_SYMBOL(rdma_link_register);
> > +
> > +void rdma_link_unregister(struct rdma_link_ops *ops)
> > +{
> > + down_write(&link_ops_rwsem);
> > + list_del(&ops->list);
> > + up_write(&link_ops_rwsem);
> > +}
> > +EXPORT_SYMBOL(rdma_link_unregister);
> > +
> > +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;
> > + struct ib_device *device;
> > + 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])
> > + return -EINVAL;
> > +
> > + if (nla_len(tb[RDMA_NLDEV_ATTR_DEV_NAME]) >
> sizeof(ibdev_name) ||
> > + nla_len(tb[RDMA_NLDEV_ATTR_LINK_TYPE]) > sizeof(type) ||
> > + nla_len(tb[RDMA_NLDEV_ATTR_NDEV_NAME]) >
> sizeof(ndev_name))
> > + return -EINVAL;
>
> Why is that? It is supposed to be tested by nldev_policy in nlmsg_parse
> stage.
>
If the policy does it, then I'll remove these. I believe Jason requested
adding this. But I didn't think about the nldev_policy...
Thanks,
Steve.
