Re: [PATCH for-next v2] RDMA/nldev: Add copy-on-fork attribute to get sys command

From: Leon Romanovsky <leon@kernel.org>
To: Gal Pressman <galpress@amazon.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>,
	Doug Ledford <dledford@redhat.com>,
	Yossi Leybovich <sleybo@amazon.com>,
	linux-rdma@vger.kernel.org,
	Alexander Matushevsky <matua@amazon.com>,
	Jason Gunthorpe <jgg@ziepe.ca>, Peter Xu <peterx@redhat.com>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH for-next v2] RDMA/nldev: Add copy-on-fork attribute to get sys command
Date: Wed, 7 Apr 2021 16:40:22 +0300	[thread overview]
Message-ID: <YG22Rm1jcYcKWXdT@unreal> (raw)
In-Reply-To: <010c6b45-a9b0-67c7-82e3-78533a532225@amazon.com>

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