FUSE passthrough: fd lifetime?

FUSE passthrough: fd lifetime?

[Han-Wen Nienhuys]

Hi folks,

I am implementing passthrough support for go-fuse. It seems to be
working now (code:
https://review.gerrithub.io/c/hanwen/go-fuse/+/1199984 and
predecessors), but I am unsure of the correct lifetimes for the file
descriptors.

The passthrough_hp example seems to do:

Open:
  backing_fd = ..
  backing_id = ioctl(fuse_device_fd,
     FUSE_DEV_IOC_BACKING_OPEN, backing_fd)

Release:
  ioctl(fuse_device_fd,
     FUSE_DEV_IOC_BACKING_CLOSE, backing_id)
  close(backing_fd)

Is it necessary to keep backing_fd open while the backing file is
used? In the case of go-fuse, the backing_fd is managed by client
code, so I can't ensure it is kept open long enough. I can work around
this by doing

Open:
   new_backing_fd = ioctl(backing_fd, DUP_FD, 0)
   backing_id = ioctl(fuse_device_fd,
     FUSE_DEV_IOC_BACKING_OPEN, new_backing_fd)

Release:
  ioctl(fuse_device_fd,
     FUSE_DEV_IOC_BACKING_CLOSE, backing_id)
  close(new_backing_fd)

but it would double the number of FDs the process uses.

I tried simply closing the backing FD right after obtaining the
backing ID, and it seems to work. Is this permitted?

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: FUSE passthrough: fd lifetime?

[Amir Goldstein]

On Tue, Aug 27, 2024 at 11:42 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
>
> Hi folks,

Hi Han-Wen,

For future reference, please CC FUSE and libfuse maintainers
and FUSE passthrough developer (me) if you want to make sure
that you got our attention.

>
> I am implementing passthrough support for go-fuse. It seems to be
> working now (code:
> https://review.gerrithub.io/c/hanwen/go-fuse/+/1199984 and
> predecessors), but I am unsure of the correct lifetimes for the file
> descriptors.
>
> The passthrough_hp example seems to do:
>
> Open:
>   backing_fd = ..
>   backing_id = ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_OPEN, backing_fd)
>
> Release:
>   ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_CLOSE, backing_id)
>   close(backing_fd)
>

If you look closer, that is not exactly what passthough_hp does.
What it does is:

Open #1:
   backing_fd1 = ..
   backing_id = ioctl(fuse_device_fd,
      FUSE_DEV_IOC_BACKING_OPEN, backing_fd1)

Open #2 (of the same inode):
   backing_fd2 = ..
   /* No ioctl, reusing existing backing_id for inode */

Release #1:
   /* No ioctl */
   close(backing_fd1)

Release #2:
   ioctl(fuse_device_fd,
      FUSE_DEV_IOC_BACKING_CLOSE, backing_id)
   close(backing_fd2)

Not necessary.

passthrough_hp needs to keep backing_fd open because of
operations that are not passthrough.
It does not do that for keeping the backing_fd object alive.
Only the ioctls manage the lifetime of the backing fd object in the kernel.

> In the case of go-fuse, the backing_fd is managed by client
> code, so I can't ensure it is kept open long enough. I can work around
> this by doing
>
> Open:
>    new_backing_fd = ioctl(backing_fd, DUP_FD, 0)
>    backing_id = ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_OPEN, new_backing_fd)
>
> Release:
>   ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_CLOSE, backing_id)
>   close(new_backing_fd)
>
> but it would double the number of FDs the process uses.
>
> I tried simply closing the backing FD right after obtaining the
> backing ID, and it seems to work. Is this permitted?

Yes.

BTW, since you are one of the first (publicly announced) users of
FUSE passthrough, it would be helpful to get feedback about API,
which could change down the road and about your wish list.

Specifically, I have WIP patches for
- readdir() passthrough
- stat()/getxattr()/listxattr() passthrough

and users feedback could help me decide how to prioritize between
those (and other) FUSE passthrough efforts.

Thanks,
Amir.

Re: FUSE passthrough: fd lifetime?

[Han-Wen Nienhuys]

On Tue, Aug 27, 2024 at 3:48 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Aug 27, 2024 at 11:42 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
> >
> > Hi folks,
>
> Hi Han-Wen,
>
> For future reference, please CC FUSE and libfuse maintainers
> and FUSE passthrough developer (me) if you want to make sure
> that you got our attention.

Sure. Who is the libfuse maintainer these days?

> > I tried simply closing the backing FD right after obtaining the
> > backing ID, and it seems to work. Is this permitted?
>
> Yes.

awesome!

> BTW, since you are one of the first (publicly announced) users of
> FUSE passthrough, it would be helpful to get feedback about API,
> which could change down the road and about your wish list.

For full transparency, I just work on the go-fuse library for fun, I
don't have direct applications for passthrough at the moment. That
said, I have been trying to make it go faster, and obviously bypassing
user-space for reads/writes helps in a big way.

> Specifically, I have WIP patches for
> - readdir() passthrough
> - stat()/getxattr()/listxattr() passthrough
>
> and users feedback could help me decide how to prioritize between
> those (and other) FUSE passthrough efforts.

It's been useful in the past to defer all file operations to an
underlying file, and only change where in the tree a file is surfaced.
In that sense, it would be nice to just say "pass all operations on
this (open) file to this other file". Go-FUSE has a LoopbackFile and
LoopbackNode that lets you do that very easily, but it would be extra
attractive if that would also avoid kernel roundtrips.

For flexibility, it might be useful to pass a bitmask or a list of
FUSE opcodes back to the kernel, so you can select which operations
should be passed through.

The most annoying part of the current functionality is the
CAP_SYS_ADMIN restriction; I am not sure everyone is prepared to run
their file systems as root. Could the ioctl check that the file was
opened as O_RDWR, and stop checking for root?

If you are proposing to do xattr functions, that means that you also
will do passthrough for files that are not opened?

Right now, the backing ID is in the OpenOut structure; how would you
pass back the backing ID so it could be used for stat() ? IMO the most
natural solution would be to extend the lookup response so you can
directly insert a backing ID there.

I made a mistake in how I treat opendir/readdir in the high-level
go-fuse API. Until that is fixed, I cannot use passthrough for
readdir. That said, part of making FUSE filesystems go fast is to
amortize the cost of lookup using readdirplus. That wouldn't work with
readdir passthrough, so it may not be that useful.

So in summary, my wishlist for passthrough (decreasing importance):

1.  get rid of cap_sys_admin requirement
2. allow passthrough for all file operations (fallocate, fsync, flush,
setattr, etc.)

cheers,
-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: FUSE passthrough: fd lifetime?

[Han-Wen Nienhuys]

On Tue, Aug 27, 2024 at 5:32 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:

> Right now, the backing ID is in the OpenOut structure; how would you
> pass back the backing ID so it could be used for stat() ? IMO the most
> natural solution would be to extend the lookup response so you can
> directly insert a backing ID there.

Actually, scratch that: the primary use case for things I've built in
the past are lazily loading file systems, where you only want to fetch
the file if it is opened for reading/writing.  If you have to provide
the backing file before it is opened, it would negate the benefits of
using FUSE altogether.

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: FUSE passthrough: fd lifetime?

[Josef Bacik]

On Tue, Aug 27, 2024 at 11:41:04AM +0200, Han-Wen Nienhuys wrote:
> Hi folks,
> 
> I am implementing passthrough support for go-fuse. It seems to be
> working now (code:
> https://review.gerrithub.io/c/hanwen/go-fuse/+/1199984 and
> predecessors), but I am unsure of the correct lifetimes for the file
> descriptors.
> 
> The passthrough_hp example seems to do:
> 
> Open:
>   backing_fd = ..
>   backing_id = ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_OPEN, backing_fd)
> 
> Release:
>   ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_CLOSE, backing_id)
>   close(backing_fd)
> 
> Is it necessary to keep backing_fd open while the backing file is
> used? In the case of go-fuse, the backing_fd is managed by client
> code, so I can't ensure it is kept open long enough. I can work around
> this by doing
> 
> Open:
>    new_backing_fd = ioctl(backing_fd, DUP_FD, 0)
>    backing_id = ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_OPEN, new_backing_fd)
> 
> Release:
>   ioctl(fuse_device_fd,
>      FUSE_DEV_IOC_BACKING_CLOSE, backing_id)
>   close(new_backing_fd)
> 
> but it would double the number of FDs the process uses.
> 
> I tried simply closing the backing FD right after obtaining the
> backing ID, and it seems to work. Is this permitted?

Yes that's permitted, we're holding a reference to the file in the kernel.  The
dup thing works too, but as you say that's a lot more fd's.  Thanks,

Josef

Re: FUSE passthrough: fd lifetime?

[Amir Goldstein]

On Tue, Aug 27, 2024 at 5:32 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
>
> On Tue, Aug 27, 2024 at 3:48 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Tue, Aug 27, 2024 at 11:42 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
> > >
> > > Hi folks,
> >
> > Hi Han-Wen,
> >
> > For future reference, please CC FUSE and libfuse maintainers
> > and FUSE passthrough developer (me) if you want to make sure
> > that you got our attention.
>
> Sure. Who is the libfuse maintainer these days?

Oops, forgot to CC Bernd.

>
> > > I tried simply closing the backing FD right after obtaining the
> > > backing ID, and it seems to work. Is this permitted?
> >
> > Yes.
>
> awesome!
>
> > BTW, since you are one of the first (publicly announced) users of
> > FUSE passthrough, it would be helpful to get feedback about API,
> > which could change down the road and about your wish list.
>
> For full transparency, I just work on the go-fuse library for fun, I
> don't have direct applications for passthrough at the moment. That
> said, I have been trying to make it go faster, and obviously bypassing
> user-space for reads/writes helps in a big way.
>
> > Specifically, I have WIP patches for
> > - readdir() passthrough
> > - stat()/getxattr()/listxattr() passthrough
> >
> > and users feedback could help me decide how to prioritize between
> > those (and other) FUSE passthrough efforts.
>
> It's been useful in the past to defer all file operations to an
> underlying file, and only change where in the tree a file is surfaced.
> In that sense, it would be nice to just say "pass all operations on
> this (open) file to this other file". Go-FUSE has a LoopbackFile and
> LoopbackNode that lets you do that very easily, but it would be extra
> attractive if that would also avoid kernel roundtrips.
>
> For flexibility, it might be useful to pass a bitmask or a list of
> FUSE opcodes back to the kernel, so you can select which operations
> should be passed through.

There is an ops_mask in my WIP patches:
https://github.com/amir73il/linux/commits/fuse-backing-inode-wip/

>
> The most annoying part of the current functionality is the
> CAP_SYS_ADMIN restriction; I am not sure everyone is prepared to run
> their file systems as root. Could the ioctl check that the file was
> opened as O_RDWR, and stop checking for root?
>

Donno. It's a challenge. Will need to think about it.

> If you are proposing to do xattr functions, that means that you also
> will do passthrough for files that are not opened?
>
> Right now, the backing ID is in the OpenOut structure; how would you
> pass back the backing ID so it could be used for stat() ? IMO the most
> natural solution would be to extend the lookup response so you can
> directly insert a backing ID there.

Yes, that's the idea.
The WIP demonstrates a different lifetime for the backing inode
that could persist from lookup to evict, but the lookup response was
not extended yet.

>
> I made a mistake in how I treat opendir/readdir in the high-level
> go-fuse API. Until that is fixed, I cannot use passthrough for
> readdir. That said, part of making FUSE filesystems go fast is to
> amortize the cost of lookup using readdirplus. That wouldn't work with
> readdir passthrough, so it may not be that useful.
>

This is indeed a challenge.
I'll need to see how to deal with it.

> So in summary, my wishlist for passthrough (decreasing importance):
>
> 1.  get rid of cap_sys_admin requirement
> 2. allow passthrough for all file operations (fallocate, fsync, flush,
> setattr, etc.)
>

Thanks for the feedback!
Amir.

Re: FUSE passthrough: fd lifetime?

[Antonio SJ Musumeci]

On 8/27/24 10:32, Han-Wen Nienhuys wrote:
> Sure. Who is the libfuse maintainer these days?

https://github.com/libfuse/libfuse/blob/master/AUTHORS is up to date


> For full transparency, I just work on the go-fuse library for fun, I
> don't have direct applications for passthrough at the moment. That
> said, I have been trying to make it go faster, and obviously bypassing
> user-space for reads/writes helps in a big way.

Keep in mind that passthrough isn't some simple feature that you enable  
or not. There is a decent amount of subtleness to using it and it must 
be very explicitly managed by the library user. It also will only work 
where underlying FD is a filesystem.

And for anyone doing union filesystem like stuff, such as myself, 
passthrough has limited use outside read/write. Even then the user will 
have to give up certain features to enable since the FUSE server no 
longer has control. Including errors.

> The most annoying part of the current functionality is the
> CAP_SYS_ADMIN restriction; I am not sure everyone is prepared to run
> their file systems as root. Could the ioctl check that the file was
> opened as O_RDWR, and stop checking for root?

This is a security feature ATM. And yes, it is a PITA in cases like mine 
where you have to set{u,g}id to the calling request {u,g}id to ensure 
proper authn/authz. In my prototyping with passthrough feature I'm 
having to change to uid/gid, change back to root to setup passthrough, 
then change back to uid/gid. Not the end of the world but...

> So in summary, my wishlist for passthrough (decreasing importance):
>
> 1.  get rid of cap_sys_admin requirement
> 2. allow passthrough for all file operations (fallocate, fsync, flush,
> setattr, etc.)
I'd generally agree. Having to have a singular backing fd for a node is 
inconvenient in my usecase but I totally understand the reason. If there 
was a way to remove the need to have a global construct to manage the 
resource, it'd be nice, but not sure there is a good way of doing that. 
Perhaps if certain requests, like open, could have a fh object like 
other instructions and the kernel guaranteed there was no race condition 
with setting it then perhaps that could be an option. Like... if lookup 
had a fh that was attached to the node.

Re: FUSE passthrough: fd lifetime?

[Han-Wen Nienhuys]

On Tue, Aug 27, 2024 at 7:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > The most annoying part of the current functionality is the
> > CAP_SYS_ADMIN restriction; I am not sure everyone is prepared to run
> > their file systems as root. Could the ioctl check that the file was
> > opened as O_RDWR, and stop checking for root?
> >
>
> Donno. It's a challenge. Will need to think about it.

It looks like `struct file` has an owner field. Could the passthrough
always be allowed if the owner of the FUSE process is the owner of the
backing file? In most of my FUSE filesystems, the backing file is
typically created by the FUSE process (eg. by downloading something
from the internet).

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: FUSE passthrough: fd lifetime?

[Han-Wen Nienhuys]

On Tue, Aug 27, 2024 at 3:48 PM Amir Goldstein <amir73il@gmail.com> wrote:

> BTW, since you are one of the first (publicly announced) users of
> FUSE passthrough, it would be helpful to get feedback about API,
> which could change down the road and about your wish list.

I guess it is too late to change now, but I noticed that
fuse_backing_map takes the file descriptors and backing IDs as signed
int32. Why int32 and not uint32 ? open(2) is documented as never
returning negative integers.

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: FUSE passthrough: fd lifetime?

[Amir Goldstein]

On Wed, Aug 28, 2024 at 12:00 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
>
> On Tue, Aug 27, 2024 at 3:48 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> > BTW, since you are one of the first (publicly announced) users of
> > FUSE passthrough, it would be helpful to get feedback about API,
> > which could change down the road and about your wish list.
>
> I guess it is too late to change now, but I noticed that
> fuse_backing_map takes the file descriptors and backing IDs as signed
> int32. Why int32 and not uint32 ? open(2) is documented as never
> returning negative integers.
>

It seemed safer this way and allows to extend the API with special
return codes later.
Why? what is to gain from uint32 in this API?

Thanks,
Amir.

Re: FUSE passthrough: fd lifetime?

[Han-Wen Nienhuys]

On Wed, Aug 28, 2024 at 12:06 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Aug 28, 2024 at 12:00 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
> >
> > On Tue, Aug 27, 2024 at 3:48 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > > BTW, since you are one of the first (publicly announced) users of
> > > FUSE passthrough, it would be helpful to get feedback about API,
> > > which could change down the road and about your wish list.
> >
> > I guess it is too late to change now, but I noticed that
> > fuse_backing_map takes the file descriptors and backing IDs as signed
> > int32. Why int32 and not uint32 ? open(2) is documented as never
> > returning negative integers.
> >
>
> It seemed safer this way and allows to extend the API with special
> return codes later.
> Why? what is to gain from uint32 in this API?

Consistency. Almost all fields in the FUSE API are uint64 or uint32.
Having it be different suggests that something special is going on,
and that negative numbers have a valid use case. If they're always
non-negative, that could be documented.

Similarly, it looks like the first backing ID is usually 1. Is it
guaranteed that 0 is never a valid backing ID? I am not sure, and it
would certainly help implementation on my side.

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: FUSE passthrough: fd lifetime?

[Amir Goldstein]

On Wed, Aug 28, 2024 at 12:33 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
>
> On Wed, Aug 28, 2024 at 12:06 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Wed, Aug 28, 2024 at 12:00 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
> > >
> > > On Tue, Aug 27, 2024 at 3:48 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > > BTW, since you are one of the first (publicly announced) users of
> > > > FUSE passthrough, it would be helpful to get feedback about API,
> > > > which could change down the road and about your wish list.
> > >
> > > I guess it is too late to change now, but I noticed that
> > > fuse_backing_map takes the file descriptors and backing IDs as signed
> > > int32. Why int32 and not uint32 ? open(2) is documented as never
> > > returning negative integers.
> > >
> >
> > It seemed safer this way and allows to extend the API with special
> > return codes later.
> > Why? what is to gain from uint32 in this API?
>
> Consistency. Almost all fields in the FUSE API are uint64 or uint32.
> Having it be different suggests that something special is going on,
> and that negative numbers have a valid use case. If they're always
> non-negative, that could be documented.
>
> Similarly, it looks like the first backing ID is usually 1. Is it
> guaranteed that 0 is never a valid backing ID? I am not sure, and it
> would certainly help implementation on my side.

No guarantee.
There was some suggestion about special use for value 0,
but I don't remember what it was right now.

Thanks,
Amir.

Re: FUSE passthrough: fd lifetime?

[Han-Wen Nienhuys]

On Wed, Aug 28, 2024 at 12:48 PM Amir Goldstein <amir73il@gmail.com> wrote:

> > Similarly, it looks like the first backing ID is usually 1. Is it
> > guaranteed that 0 is never a valid backing ID? I am not sure, and it
> > would certainly help implementation on my side.
>
> No guarantee.
> There was some suggestion about special use for value 0,
> but I don't remember what it was right now.

In a file system, not all inodes are backed by a file. If 0 would not
be handed out as an ID, then backing ID=0 could mean: this node is not
backed by a file (and doesn't need to unregister the ID on
forget/release). If 0 is a valid ID, I either have to add another
boolean to the inode, or keep calling the ioctl until I get a non-zero
value.

Reading, the code, the call is

        id = idr_alloc_cyclic(&fc->backing_files_map, fb, 1, 0, GFP_ATOMIC);

ie. start=1. In other words, if the counter wraps, it will start at 1
again, and 0 is not handed out.

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: FUSE passthrough: fd lifetime?

[Amir Goldstein]

On Wed, Aug 28, 2024 at 1:02 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote:
>
> On Wed, Aug 28, 2024 at 12:48 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> > > Similarly, it looks like the first backing ID is usually 1. Is it
> > > guaranteed that 0 is never a valid backing ID? I am not sure, and it
> > > would certainly help implementation on my side.
> >
> > No guarantee.
> > There was some suggestion about special use for value 0,
> > but I don't remember what it was right now.
>
> In a file system, not all inodes are backed by a file. If 0 would not
> be handed out as an ID, then backing ID=0 could mean: this node is not
> backed by a file (and doesn't need to unregister the ID on
> forget/release). If 0 is a valid ID, I either have to add another
> boolean to the inode, or keep calling the ioctl until I get a non-zero
> value.
>
> Reading, the code, the call is
>
>         id = idr_alloc_cyclic(&fc->backing_files_map, fb, 1, 0, GFP_ATOMIC);
>
> ie. start=1. In other words, if the counter wraps, it will start at 1
> again, and 0 is not handed out.

Yes, I understand.
I think the rational for reserving 0 as special value was for the
future use of FOPEN_PASSTHROUGH response to mean
"passthrough to backing inode if one was already mapped during lookup".

So yeah, I am personally ok with the backing id handle being > 0
as it is in the code right now.

Thanks,
Amir.