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