On Tue, Apr 28, 2020 at 12:39:49PM +0000, liweihang wrote:
> On 2020/4/28 19:19, Leon Romanovsky wrote:
> > On Tue, Apr 28, 2020 at 08:00:29AM +0000, liweihang wrote:
> >> On 2020/4/27 20:03, Leon Romanovsky wrote:
> >>>>>> /**
> >>>>>> * _ib_alloc_device - allocate an IB device struct
> >>>>>> * @size:size of structure to allocate
> >>>>>> + * @name: unique string device name. This may include a '%' which will
> >>>>> It looks like all drivers are setting "%" in their name and "name" can
> >>>>> be changed to be "prefix".
> >>>> Does hfi? I thought the name was forced there for some port swapped
> >>>> reason?
> >>> This patch doesn't touch HFI, nothing prohibits from us to make this
> >>> conversion work for all drivers except HFI and for the HFI add some
> >>> different callback. There is no need to make API harder just because
> >>> one driver needs it.
> >>>
> >>> Thanks
> >>>
> >>>> Jason
> >>
> >> Hi Jason and Leon,
> >>
> >> I missed some codes related to assign_name() in this series including
> >> hfi/qib as Shiraz pointed. And I found a "name" without a "%" in following
> >> funtions in core/nldev.c, and ibdev_name will be used for rxe/siw later.
> >>
> >> static int nldev_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> >> struct netlink_ext_ack *extack)
> >> {
> >> ...
> >>
> >> nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
> >> sizeof(ibdev_name));
> >> if (strchr(ibdev_name, '%') || strlen(ibdev_name) == 0)
> >> return -EINVAL;
> >>
> >> ...
> >> }
> >>
> >> I'm not familiar with these codes, but I think the judgment in assign_name()
> >> is for the situaion like above.
> >>
> >> if (strchr(name, '%'))
> >> ret = alloc_name(device, name);
> >> else
> >> ret = dev_set_name(&device->dev, name);
> >>
> >> So is it a better idea to keep using "name" instead of "prefix"?
> >
> > nldev_newlink() doesn't call to ib_alloc_device() and alloc_name(). The
> > check pointed by you is for the user input.
> >
>
> Hi Leon,
>
> nldev_newlink() will call "ops->newlink(ibdev_name, ndev)", and it point to
> siw_newlink() in siw_main.c. And then it will call ib_alloc_device() and
> ib_register_device().
>
> According to the code I pointed before, it seems that nldev_newlink()
> expects users to input a name without '%', and then passes this name
> to assign_name(). I think siw/rxe have to call ib_alloc_device() with
> a name without '%', so we can't treat it as a prefix and add "_%d" to
> it like for other drivers.
The opposite is actually true.
The reason why newlink checks for % is due to the expectation in
alloc_name() to have a name with % for numbered devices, which is
nice, but the better API will be to provide "prefix" and a flag
if to append an index or not.
Thanks
>
> >>
> >> Thanks
> >> Weihang
> >
>
