On Wed, Sep 4, 2024 at 8:28 AM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: > > > > > On Sep 2, 2024, at 8:16 PM, Rick Macklem <rick.macklem@xxxxxxxxx> wrote: > > > > 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? > > See xdr_set_scratch_buffer() -- xdr_stream wants a scratch > buffer for managing the transitions between the xdr_buf's > head/page/tail components. Thanks Chuck. That seems to work fine, rick > > > -- > Chuck Lever > >
