Re: [PATCH v2 2/3] RDMA: Add NLDEV_GET_CHARDEV to allow char dev discovery and autoload

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2019-06-18 at 16:58 -0400, Doug Ledford wrote:
> On Tue, 2019-06-18 at 15:46 -0300, Jason Gunthorpe wrote:
> > On Tue, Jun 18, 2019 at 07:53:38PM +0300, Leon Romanovsky wrote:
> > > I have a very strong opinion about it.
> > 
> > Then Doug should add the policies, here are the output values from
> > the
> > userspace:
> > 
> >         [RDMA_NLDEV_ATTR_CHARDEV] = { .type = NLA_U64 },
> >         [RDMA_NLDEV_ATTR_CHARDEV_ABI] = { .type = NLA_U64 },
> >         [RDMA_NLDEV_ATTR_DEV_INDEX] = { .type = NLA_U32 },
> >         [RDMA_NLDEV_ATTR_DEV_NODE_TYPE] = { .type = NLA_U8 },
> >         [RDMA_NLDEV_ATTR_NODE_GUID] = { .type = NLA_U64 },
> >         [RDMA_NLDEV_ATTR_UVERBS_DRIVER_ID] = { .type = NLA_U32 },
> >         [RDMA_NLDEV_ATTR_CHARDEV_NAME] = { .type = NLA_NUL_STRING },
> >         [RDMA_NLDEV_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING },
> >         [RDMA_NLDEV_ATTR_DEV_PROTOCOL] = { .type = NLA_NUL_STRING },
> >         [RDMA_NLDEV_ATTR_FW_VERSION] = { .type = NLA_NUL_STRING },
> 
> Most of those were already in the policies.  Only the four that you
> added to enum rdma_nldev_attr needed added to the policies, and two of
> them your patch already added.  The only question I had is what the
> string length should be on ATTR_CHARDEV_NAME?  I throw in the default
> of
> .len = RDMA_NLDEV_ATTR_ENTRY_STRLEN, but I wasn't sure if that was
> right
> for this entry?

First of all, let me say that this is a PITA.  I'm thinking we need to
order all of the attributes in the policy array in alphabetical order so
it's easier to tell what's there and what's missing.

Also, I'm starting to not like adding items to the policy array before
they are actually acceptable as inputs.  Sure, there's the argument that
they won't get missed.  But there's the counter argument that until you
define them as an input in some netlink function that reads them, at
least for all of the string items, you can't actually set the real
string length limit.  By adding them to the policy now, you make it a
guarantee that you'll end up changing the API under userspace later when
they become a legitimate input.  Not sure if that's actually wise.  The
u32 and u64, sure, that's fine.  But the strings, that can cause
problems later.,

-- 
Doug Ledford <dledford@xxxxxxxxxx>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux