On Tue, Apr 04, 2023 at 06:50:08PM -0700, Daniel Rosenberg wrote: > On Tue, Apr 4, 2023 at 3:58 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > I'm pretty sure we can make bpf_dynptr_data() support readonly dynptrs. > > Should be easy to add in the verifier. > > But could you pseudo code what you're trying to do first? > > > > I'm trying to do something like this: > > bpf_fuse_get_ro_dynptr(name, &name_ptr); so the idea that bpf prog will see opaque ctx == name and a set of kfuncs will extract different things from ctx into local dynptrs ? Have you considered passing dynptr-s directly into bpf progs as arguments of struct_ops callbacks? That would be faster or slower performance wise? > name_buf = bpf_dynptr_data(&name_ptr, 4); > if (!bpf_strncmp(name_buf, 4, "test")) > return 42; > return 0; > > Really I just want to work with the data in the dynptrs in as > convenient a way as possible. > I'd like to avoid copying the data if it isn't necessary. of course. > At the moment I'm using bpf_dynptr_slice and declaring an empty and > unused buffer. I'm then hitting an issue with bpf_strncmp not > expecting mem that's tagged as being associated with a dynptr. I'm > currently working around that by adjusting check_reg_type to be less > picky, stripping away DYNPTR_TYPE_LOCAL if we're looking at an > ARG_PTR_TO_MEM. I suspect that would also be the case for other dynptr > types. > > I guess for dynptr_data to support R/O, the dynptr in question would > just need to be tagged as read-only or read/write by the verifier > previously, and then it could just pass along that tag to the mem it > returns. yep. Don't be shy from improving the verifier to your needs. > > > > Do you expect bpf prog to see both ro and rw dynptrs on input? > > And you want bpf prog to use bpf_dynptr_data() to access these buffers > > wrapped as dynptr-s? > > The string manipulation questions muddy the picture here. > > If bpf progs deals with file system block data why strings? > > Is that a separate set of bpf prog hooks that receive strings on > > input wrapped as dynptrs? > > What are those strings? file path? > > We need more info to help you design the interface, so it's easy to > > use from bpf prog pov. > > I have a few usecases for them. I've restructured fuse-bpf to use > struct ops. Each fuse operation has an associated prefilter and > postfilter op. > At the moment I'm using dynptrs for any variable length block of data > that these ops need. For many operations, this includes the file name. > In some, a path. Other times, it's file data, or xattr names/data. > They can all have different sizes, and may be backed by data that may > not be changed, like the dentry name field. I have a pair of kfuncs > for requesting a dynptr from an opaque storage type, so you can avoid > having to make unnecessary copies if you're not planning on making > modifications. The r/w version of the kfunc will allocate new space > and copy data as needed. They're a direct analog of the helper > functions from the initial patch. The opaque structure includes > information about the current and max size of the argument, whether > the current backing is read-only, and other flags for cleanup if the > helper allocates memory. > > The Fuse bpf programs may alter those fields, and then returns whether > it was able to handle the request, or if the request must go to > userspace. > > While the fields have a max size, their actual size isn't available > until runtime. That makes the current set of dynptr functions largely > unusable, since I believe they all require a known constant size to > access, and I don't think the helper to return the length of a dynptr > counts for that as far as the verifier is concerned. Worst case, I > suppose more of those interactions could happen behind kfuncs, where > they can perform runtime checks. That may end up being overly > restrictive though. Overall makes sense, but a bit too abstract for concrete suggestions. I think it would be best if you just send your rough patches to the list with comments where things still need work and we will go from there.
