On Tue, Oct 22, 2024 at 5:28 AM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: > > > > > On Oct 21, 2024, at 6:33 PM, Rick Macklem <rick.macklem@xxxxxxxxx> wrote: > > > > On Mon, Oct 21, 2024 at 7:45 AM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: > >> > >> > >> > >>> On Oct 20, 2024, at 7:09 PM, Rick Macklem <rick.macklem@xxxxxxxxx> 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. > >> > >> > >>> 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. > > > > 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 > > > > 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 > > > >> > >> > >>> 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 > >