Re: [PATCH 10/14] nfsd: add new netlink spec for svc_export upcall

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2026-03-20 at 11:17 -0400, Chuck Lever wrote:
> On 3/16/26 11:14 AM, Jeff Layton wrote:
> > ...and generate the headers.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  Documentation/netlink/specs/nfsd.yaml | 172 ++++++++++++++++++++++++++++++++++
> >  fs/nfsd/netlink.c                     |  61 ++++++++++++
> >  fs/nfsd/netlink.h                     |  13 +++
> >  include/uapi/linux/nfsd_netlink.h     | 102 ++++++++++++++++++++
> >  net/sunrpc/netlink.c                  |  49 ++--------
> >  net/sunrpc/netlink.h                  |   6 +-
> >  6 files changed, 357 insertions(+), 46 deletions(-)
> > 
> 
> 
> This is the last patch in this series that applied cleanly to the
> current nfsd-testing branch, so I'm stopping with this one.
> 
> I'm going to whine only a little about the commit message being not even
> a full sentence. </whine>
> 
> review-prompts seems to think that removing sunrpc_cache_notify in this
> patch will break the build (temporarily). I mention that only for due
> diligence -- my earlier request to move sunrpc_cache_notify out of
> tool-generated files will probably make this moot.
> 

Fixed in my current tree.

> The svc-export-req attribute set isn't used in this patch; perhaps
> its introduction should be deferred to a patch where it is used.
> NFSD_CMD_CACHE_NOTIFY looks like the same: dead code for now.
> 

Ok. I'll see about moving those bits.

> 0x3ffff and 0x7 in the NLA policy masks correspond to NFSEXP_ALLFLAGS
> and NFSEXP_XPRTSEC_ALL from include/uapi/linux/nfsd/export.h, but the
> policy uses raw hex. Adding a new export flag requires updating three
> independent locations with no compile-time check. Using the named
> constants directly in the policy would make the link more explicit. I
> don't have a good suggestion about the list of flags in the YAML spec.
> 

+	[NFSD_A_SVC_EXPORT_XPRTSEC] = NLA_POLICY_MASK(NLA_U32, 0x7),
+	[NFSD_A_SVC_EXPORT_FLAGS] = NLA_POLICY_MASK(NLA_U32, 0x3ffff),

These are generated by ynl from the provided flags in the spec. I'm not
sure there is much we can do here. ynl just has no idea that those
constants exist. All it cares about is filtering out flags that it
doesn't understand.

Ease of adding new flags is a valid concern though. That's one of the
main reasons for doing this. I'll think about this some more.


> Several AI reviewers noted that GENLMSG_DEFAULT_SIZE is 8KB, yet the
> request and reply attributes for some of the commands added in this
> series are no larger than sizeof(u32).
> 
> Recommend you add Jakub to the cc: for the series for closer human
> inspection of the YAML / netlink protocol aspects.
> 

I'll do that on the next posting.

Thanks for all the review so far! I'm working on addressing your other
comments too.
-- 
Jeff Layton <jlayton@kernel.org>