On Mon, Apr 17, 2023 at 10:33 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > Which brings me to my biggest concern. > I still do not see how these patches replace Allesio's > FUSE_DEV_IOC_PASSTHROUGH_OPEN patches. > > Is the idea here that ioctl needs to be done at FUSE_LOOKUP > instead or in addition to the ioctl on FUSE_OPEN to setup the > read/write passthrough on the backing file? > In these patches, the fuse daemon responds to the lookup request via an ioctl, essentially in the same way it would have to the /dev/fuse node. It just flags the write as coming from an ioctl and calls fuse_dev_do_write. An additional block in the lookup response gives the backing file and what bpf_ops to use. The main difference is that fuse-bpf uses backing inodes, while passthrough uses a file. Fuse-bpf's read/write support currently isn't complete, but it does allow for direct passthrough. You could set ops to default to userspace in every case that Allesio's passthrough code does and it should have about the same effect. With the struct_op change, I did notice that doing something like that is more annoying, and am planning to add a default op which only takes the meta info and runs if the opcode specific op is not present. > I am missing things like the FILESYSTEM_MAX_STACK_DEPTH check that > was added as a result of review on Allesio's patches. > I'd definitely want to fix any issues that were fixed there. There's a lot of common code between fuse-bpf and fuse passthrough, so many of the suggestions there will apply here. > The reason I am concerned about this is that we are using the > FUSE_DEV_IOC_PASSTHROUGH_OPEN patches and I would like > to upstream their functionality sooner rather than later. > These patches have already been running in production for a while > I believe that they are running in Android as well and there is value > in upsteaming well tested patches. > > The API does not need to stay FUSE_DEV_IOC_PASSTHROUGH_OPEN > it should be an API that is extendable to FUSE-BPF, but it would be > useful if the read/write passthrough could be the goal for first merge. > > Does any of this make sense to you? > Can you draw a roadmap for merging FUSE-BPF that starts with > a first (hopefully short term) phase that adds the read/write passthrough > functionality? > > I can help with review and testing of that part if needed. > I was planning to discuss this with you on LSFMM anyway, > but better start the discussion beforehand. > > Thanks, > Amir. We've been using an earlier version of fuse-bpf on Android, closer to the V1 patches. They fit our current needs but don't cover everything we intend to. The V3 patches switch to a new style of bpf program, which I'm hoping to get some feedback on before I spend too much time fixing up the details. The backing calls themselves can be reviewed separately from that though. Without bpf, we're essentially enabling complete passthrough at a directory or file. By default, once you set a backing file fuse-bpf calls by the backing filesystem by default, with no additional userspace interaction apart from if an installed bpf program says otherwise. If we had some commands without others, we'd have behavior changes as we introduce support for additional calls. We'd need a way to set default behavior. Perhaps something like a u64 flag field extension in FUSE_INIT for indicating which opcodes support backing, and a response for what those should default to doing. If there's a bpf_op present for a given opcode, it would be able to override that default. If we had something like that, we'd be able to add support for a subset of opcodes in a sensible way.
