On Tue, 2019-06-18 at 22:45 -0400, Doug Ledford wrote:
> 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.,
>
Maybe the best way to do string items if you add them ahead of time is
to give them an input length of 1. Theoretically that's just the null
byte. Then when you add a function to actually read the data, set the
size to something real.
Anyway, pushed to wip/dl-for-next, check it to make sure you're happy
with the final results.
--
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
