Re: [RFC PATCH v2 00/11] NFS/NFSD: nfs4_acl passthru for NFSv4 reexport

[Chuck Lever <cel@kernel.org>]

On 2/25/26 11:53 AM, Mike Snitzer wrote:
> On Tue, Feb 24, 2026 at 04:58:10PM -0500, Chuck Lever wrote:
>> Current NFSD support for DACL/SACL:
>>
>> - FATTR4_DACL (bit 58) and FATTR4_SACL (bit 59) are defined in
>>   `include/uapi/linux/nfs4.h` but not in any supported attrs mask
>> - Dispatch table (`fs/nfsd/nfs4xdr.c`):
>>       [FATTR4_DACL] = nfsd4_encode_fattr4__noop,
>>       [FATTR4_SACL] = nfsd4_encode_fattr4__noop,
>> - Not decoded in nfsd4_decode_fattr4()
>> - Not in NFSD_WRITEABLE_ATTRS_WORD1 (`fs/nfsd/nfsd.h`)
>>
>> Patch sequence might look something like:
>>
>>  1. NFSD: add acl_flags to struct nfs4_acl for nfsacl41
>>  2. NFSD: add DACL/SACL decode at correct fattr4 bit position
>>  3. NFSD: handle nfsacl41 wire format (aclflag4 + aces) in decode
>>  4. NFSD: add nfsacl41 encode for DACL/SACL responses
>>  5. NFSD: clear ACL, DACL, and SACL together in supported_attrs
>>  6. NFSD: add DACL/SACL to supported and writable attribute masks
>>
>>
>> For the API between NFSD and the local NFS client, I might favor an
>> alternative to new export operations: NFSD can set "system.nfs4_acl"
>> xattrs on NFS client inodes. The only issue there is it limits the
>> total number of bytes to 64K. But we can worry about that once the
>> DACL/SACL attributes are implemented.
> 
> This line of work _seems_ outside the scope of what is needed, so I
> won't be pursuing it unless you can help me better understand how this
> stepping stone of NFSD having native support for DACL and SACL will
> begin to offer more capable ACL pass-through support for NFS reexport.

This is not about what narrow use case Hammerspace happens to need.

Your series adds incomplete support for the DACL and SACL attributes.
That incomplete support is not sufficient for a properly compliant
server implementation, and makes that part of your proposal unacceptable
for merge because it adds significant technical debt and is likely to
result in bug reports. That's no skin off your nose, but it's
considerable effort for the community to have to deal with long-term.

Since your patches already do some of this work, I'm asking you to do
the right thing and provide a clean and complete implementation of the
feature.

If you want to look at this transactionally, I'm asking Hammerspace to
pay a small one-time price for the community's long-term support of this
feature. I regard that as very fair.


> Further review of the code provided in this series would be
> appreciated.

Adding .setacl/.getacl operations to the export_operations structure is
simply wrong, architecturally speaking. So NACK to that implementation.
All of that needs to be redone once we have agreed on a proper cross-
system API.

> I do think I've captured the essence of what is needed.

That's exactly what a prototype should do. That does not make a
prototype implementation acceptable in the long run, however.


-- 
Chuck Lever