On Tue, Oct 22, 2024 at 8:03 AM Rick Macklem [off-list ref] wrote:

On Tue, Oct 22, 2024 at 5:28 AM Chuck Lever III [off-list ref] wrote:

quoted

quoted

On Oct 21, 2024, at 6:33 PM, Rick Macklem [off-list ref] wrote:

On Mon, Oct 21, 2024 at 7:45 AM Chuck Lever III [off-list ref] wrote:

quoted

quoted

On Oct 20, 2024, at 7:09 PM, Rick Macklem [off-list ref] wrote:

Hi,

As some of you will already know, I have been working on patches
that add support for POSIX draft ACLs to NFSv4.2.
The internet draft can be found here, if you are interested.
https://datatracker.ietf.org/doc/draft-rmacklem-nfsv4-posix-acls/

The patches now basically work, but handling of large POSIX
draft ACLs for the server side is not done yet.

A POSIX draft ACL can have 1024 aces and since a "who" field
can be 128 bytes, a POSIX draft ACL can end up being about 140Kbytes
of XDR. Do both the default and access ACLs for a directory and it
could be 280Kbytes. (Of course, they usually end up much smaller.)

For the client side, to handle large ACLs for SETATTR (which never
sets other attributes in the same SETATTR), I came up with some
simple functions (called nfs_xdr_putpage_bytes(), nfs_xdr_putpage_word()
and nfs_xdr_putpage_cleanup() in the current client.patch) which
fill the large ACL into pages. Then xdr_write_pages() is called to
put them in the xdr stream.
--> Whether this is the right approach is a good question, but at
    least it seems to work.

For the server, the problem is more difficult, since a GETATTR reply
might include encodings of other attributes. (At this time, the proposed
POSIX draft ACL attributes are at the end, since they have the highest
attribute #s, but that will not last long.)

The same technique as for the client could be used, but only if there
are no attributes that come after the POSIX draft ACL ones in the XDR
for GETATTR's reply.

This brings me to one question...
- What do others think w.r.t. restricting the POSIX draft ACLs to only
GETATTR (and not a READDIR reply) and only with a limited set
of other attributes, which will all be lower #s, so they come before
POSIX draft ACL ones?
--> Since it is only a personal draft at this time, this requirement
      could easily be added and may make sense to limit the size
       of most GETATTRs.
This restriction should be ok for both the LInux and FreeBSD clients,
since they only ask for acl attributes when a getfacl(1) command is
done and do not need a lot of other attributes for the GETATTR.

Generally, I don't immediately see why POSIX ACLs need a restriction
that the protocol doesn't already have for NFSv4 ACLs.

quoted

Alternately, there needs to be a way to build 280Kbytes or more
of XDR for an arbitrary GETATTR/READDIR reply.

IIUC, NFSD already handles this for both READDIR and NFSv4 ACLs
(and perhaps also GETXATTR / LISTXATTR).

So I'll have to have a look at your specific implementation,
maybe sometime this week. I can't think of a reason that our
current XDR and NFSD implementation wouldn't handle this, but
haven't thought deeply about it.

It might not be efficient for large ACLs, but does it need
to be?

Nope, it doesn't need to be...
Well, this in embarrassing (blush!).
It turns out it works fine for GETATTR of a large ACL (either the
acl attribute or the new ones added by the patch).

For the client side, I could have sworn I needed to do the
"fill in the page stuff" to get the large one to work for SETATTR,
but for the knfsd, it figures it out.

I tested this further and it does seem to be this way.
For SETATTR done by the client, the xdr_stream_encode_XXX()
functions fail for large ACLs. As such, something needs to be done.
Right now, the client code fills the ACL into pages and then does a
xdr_write_pages() for the large ones.
--> Does anyone know of a better way than filling in pages and then
      doing xdr_write_pages()?

For GETATTR done by the server, it handles large ACLs fine.
(It also allocates a scratch buffer, so the patch I wrote didn't need
to do so.)

rick

quoted

quoted

Btw, I was only able to get to about 60K, because the ext4 fs
I have on the server wouldn't allow an ACL with 1000 entries to
be set (replied with out of space on device).
--> I did get one with 455 entries to work and most of those entries
     were for 110 byte group names.

IIRC the Linux file systems have a 64KB length limit on extended
attributes, where ACLs are stored.

ENOSPC is a little strange.

Well, when I did this, it had nothing to do with NFS. (I did the setfacl
locally on the server.)
I will try a test doing the same setfacl over NFSv4 sometime to-day
to see what happens.

The good news is that the "hard problem" I thought existed doesn't
exist. I will try the client without the "use pages code" and see if
the client really needs the change or not.

If you are interested in trying it, I left two files "groups" and "bigacl"
on the nfsv4-newlap server. All you do is put "groups" in your group
database and then "setfacl -M bigacl <file>".
(Oh, and I usually "echo N > /sys/module/nfsd/parameters/nfs4_disable_idmapping"
so it uses names instead of #s for the groups, but you can try both ways.)
--> I think the ENOSPC failure occurs either way.

Anyhow, thanks for pointing out that big acls already worked
and sorry for the noise, rick

quoted

quoted

So, what can I say, except I should have tried it before I posted.
(One gotcha is that FreeBSD only handles 32 ACE ACLs, but
when you look at the packet trace, you can see they are all in
the GETATTR reply. Not an excuse, but...)

Thanks for the reply Chuck, rick

quoted

quoted

Btw, I have not tested to see what happens if a large POSIX draft
ACL is set for a file (locally on the server, for example) and then
a client does a GETATTR of the acl attribute (which replies with
a NFSv4 ACL created by mapping from the POSIX draft ACL).
--> I have a hunch it will fail, but I need to test to be sure?

Thanks in advance for any comments w.r.t. this issue, rick
ps; In particular, I'd like to know what others think about adding
    the restriction on acquisition of the POSIX draft ACLs by GETATTR.

--
Chuck Lever

--
Chuck Lever