On Tue, Oct 22, 2024 at 8:03 AM Rick Macklem <rick.macklem@xxxxxxxxx> wrote: > > 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. 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 > > > > > > 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 > > > >
