On Thu, Aug 29, 2024 at 6:41 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2024-08-29 at 18:32 -0700, Rick Macklem wrote:
> > On Thu, Aug 29, 2024 at 5:33 PM Trond Myklebust
> > <trondmy@xxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, 2024-08-29 at 17:19 -0700, Rick Macklem wrote:
> > > > Hi,
> > > >
> > > > I have a rather crude patch that does the POSIX draft ACL
> > > > attributes
> > > > that my draft is suggesting for NFSv4.2 for the Linux client.
> > > > - It is working ok for small ACLs, but...
> > > >
> > > > The hassle is that the on-the-wire ACEs have a "who" field that
> > > > can
> > > > be up to 128bytes (IDMAP_NAMESZ).
> > > >
> > > > I think I have figured out the SETATTR side, which isn't too bad
> > > > because
> > > > it knows how many ACEs. (It does roughly what the NFSv3 NFSACL
> > > > code
> > > > did, which is allocate some pages for the large ones.)
> > > >
> > > > However, the getfacl side doesn't know how bug the ACL will be in
> > > > the reply. The NFSACL code allocates pages (7 of them) to handle
> > > > the
> > > > largest possible ACL. Unfortunately, for these NFSv4 attributes,
> > > > they
> > > > could be roughly 140Kbytes (140bytes assuming the largest "who"
> > > > times
> > > > 1024 ACEs).
> > > > --> Anyone have a better suggestion than just allocating 35pages
> > > > each
> > > > time
> > > > (when 99.99% of them will fit in a fraction of a page)?
> > > >
> > > > Thanks for any suggestions, rick
> > > >
> > >
> > > See the NFSv3 posix acl client code.
> > >
> > > It allocates the 'pages[]' array of pointers to the page buffers to
> > > be
> > > of length NFSACL_MAXPAGES, but only allocates the first entry, and
> > > leaves the rest NULL.
> > > Then in the XDR encoder "nfs3_xdr_enc_getacl3args()" where it
> > > declares
> > > the length of that array, it sets the flag XDRBUF_SPARSE_PAGES on
> > > the
> > > reply buffer.
> > >
> > > That tells the RPC layer that if the incoming RPC reply needs to
> > > fill
> > > in more data than will fit into that single page, then it should
> > > allocate extra pages and add them to the 'pages' array.
> > Oh, ok thanks for the explanation.
> > It doesn't sound like a problem then.
> >
> > I'll just code things the same way.
> >
> > Maybe I can ask one more question??
> > There are a large # of XXX_decode_XXX functions. Are there any that
> > should/should not be used for the above case?
> > For example, there are:
> > - Ones that take a "struct xdr_stream *xdr" (usually with _stream_ in
> > the name)
> > vs
> > - Ones that take a "struct xdr_buf *buf" argument.
> > (I ended up using these for the encode side and this looks like
> > what
> > nfsacl_decode() uses, as well.)
> >
> > (I'll admit I have been wading around in the code, but haven't really
> > gotten to the point of understanding which ones should be used.)
> >
>
> The "struct xdr_stream' based code is the 'newer' way of doing things,
> and allows you to write code that abstracts away some of the ugliness
> in the RPC layer, particularly when you need to mix regular buffers and
> page data.
> That said, there is definitely legacy code out there that works quite
> well and is not worth a lot of effort to convert.
>
> I'd recommend trying to use the xdr_stream if possible, just because of
> the better abstraction, but if you have to fall back to manipulating
> the xdr_buf directly, then that API is there, and is still supported.
I haven't been able to figure out how to use the xdr_stream... stuff
when the attributes get large enough that they need pages.
What I currently have (that seems to work) is...
For GETATTR, XDRBUF_SPARSE_PAGES is set, and then, once it
gets to the actual ACL (which can be pretty big), I use
xdr_decode_word() and read_bytes_from_xdr_buf() to decode it
(The stream calls worked until the ACL got too big for the non-page
part, which I think is 2Kbytes?)
- Maybe there is some trick I don't know to get the big ones to work
with the xdr_stream_XXX() decode calls?
For SETATTR, it seemed to be similar. I actually have two ways that
seem to work:
(A) - Allocates the max # of pages (assuming 128byte who fields) and
then in the enc function specified in PROC(xxx), it does a
- xdr_write_pages()
- fills the data in with xdr_encode_word() and write_bytes_to_xdr_buf()
- trims the size to the correct (not maximum) length by setting
xdr->buf->len and xdr->buf->page_len
--> I don't like this one because it typically allocates more pages
than needed and requires the len to be trimmed.
(B) - I wrote a couple of simple functions (I couldn't find ones that already
did this) which encode the ACLs before the RPC call
(nfs4_do_call_sync())
allocating page(s) as required. Then all the enc function in PROC(...)
does is a xdr_write_pages().
I prefer (B), but they both seem to work. Small ACLs don't do (A) or (B) and
are just encoded with the xdr_stream_XXX() functions.
Anyhow, if there are better ways to handle the big (up to 2 * 140Kbytes) ACLs,
please let me know.
In the meantime, the above seems to be working.
Thanks for your comments, rick
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx
>
>
