On 07/04/2021 16:40, Leon Romanovsky wrote: > On Wed, Apr 07, 2021 at 04:30:50PM +0300, Gal Pressman wrote: >> On 07/04/2021 16:23, Leon Romanovsky wrote: >>> On Wed, Apr 07, 2021 at 04:14:46PM +0300, Gal Pressman wrote: >>>> On 07/04/2021 15:39, Leon Romanovsky wrote: >>>>>> @@ -1710,7 +1721,8 @@ static int nldev_set_sys_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh, >>>>>> >>>>>> err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1, >>>>>> nldev_policy, extack); >>>>>> - if (err || !tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]) >>>>>> + if (err || !tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE] || >>>>>> + tb[RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK]) >>>>> >>>>> Why do we fail if user supplies RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK? >>>> >>>> It's a read-only attribute, if someone tries to set its value I assume it's best >>>> to return an error. >>> >>> Not in netlink world, you need to ignore the parameters that >>> you don't "know how to handle" and check for must-to-be input only. >> >> Not sure I understand. >> So you expect the set function to remain unchanged in this patch? Isn't it bad >> that a user can request to change the copy on fork value and get a success >> return value although nothing happened? > > The same goes for all netlink attributes, user can send some *_RES_* > attribute to _set_ and we won't fail. The same goes for rtnetlink too. Thanks, will remove the check in nldev_set_sys_set_doit().
