> 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. -- Chuck Lever
