diff mbox series

[5/5] exportfs: support encoding non-decodeable file handles by default

Message ID 20231018100000.2453965-6-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series Support more filesystems with FAN_REPORT_FID | expand

Commit Message

Amir Goldstein Oct. 18, 2023, 10 a.m. UTC
AT_HANDLE_FID was added as an API for name_to_handle_at() that request
the encoding of a file id, which is not intended to be decoded.

This file id is used by fanotify to describe objects in events.

So far, overlayfs is the only filesystem that supports encoding
non-decodeable file ids, by providing export_operations with an
->encode_fh() method and without a ->decode_fh() method.

Add support for encoding non-decodeable file ids to all the filesystems
that do not provide export_operations, by encoding a file id of type
FILEID_INO64_GEN from { i_ino, i_generation }.

A filesystem may that does not support NFS export, can opt-out of
encoding non-decodeable file ids for fanotify by defining an empty
export_operations struct (i.e. with a NULL ->encode_fh() method).

This allows the use of fanotify events with file ids on filesystems
like 9p which do not support NFS export to bring fanotify in feature
parity with inotify on those filesystems.

Note that fanotify also requires that the filesystems report a non-null
fsid.  Currently, many simple filesystems that have support for inotify
(e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
used with fanotify in file id reporting mode.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
 include/linux/exportfs.h | 10 +++++++---
 2 files changed, 34 insertions(+), 6 deletions(-)

Comments

Jeff Layton Oct. 18, 2023, 2:28 p.m. UTC | #1
On Wed, 2023-10-18 at 13:00 +0300, Amir Goldstein wrote:
> AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> the encoding of a file id, which is not intended to be decoded.
> 
> This file id is used by fanotify to describe objects in events.
> 
> So far, overlayfs is the only filesystem that supports encoding
> non-decodeable file ids, by providing export_operations with an
> ->encode_fh() method and without a ->decode_fh() method.
> 
> Add support for encoding non-decodeable file ids to all the filesystems
> that do not provide export_operations, by encoding a file id of type
> FILEID_INO64_GEN from { i_ino, i_generation }.
> 
> A filesystem may that does not support NFS export, can opt-out of
> encoding non-decodeable file ids for fanotify by defining an empty
> export_operations struct (i.e. with a NULL ->encode_fh() method).
> 
> This allows the use of fanotify events with file ids on filesystems
> like 9p which do not support NFS export to bring fanotify in feature
> parity with inotify on those filesystems.
> 
> Note that fanotify also requires that the filesystems report a non-null
> fsid.  Currently, many simple filesystems that have support for inotify
> (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> used with fanotify in file id reporting mode.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
>  include/linux/exportfs.h | 10 +++++++---
>  2 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 30da4539e257..34e7d835d4ef 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>  }
>  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>  
> +/**
> + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> + * @inode:   the object to encode
> + * @fid:     where to store the file handle fragment
> + * @max_len: maximum length to store there
> + *
> + * This generic function is used to encode a non-decodeable file id for
> + * fanotify for filesystems that do not support NFS export.
> + */
> +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> +				     int *max_len)
> +{
> +	if (*max_len < 3) {
> +		*max_len = 3;
> +		return FILEID_INVALID;
> +	}
> +
> +	fid->i64.ino = inode->i_ino;
> +	fid->i64.gen = inode->i_generation;

Note that i_ino is unsigned long and so is a 32-bit value on 32-bit
arches. If the backend storage uses 64-bit inodes, then we usually end
up hashing them down to 32-bits first. e.g. see nfs_fattr_to_ino_t().
ceph has some similar code.

The upshot is that if you're relying on i_ino, the value can change
between different arches, even when they are dealing with the same
backend filesystem.

Since this is expected to be used by filesystems that don't set up
export operations, then that may just be something they have to deal
with. I'm not sure what else you can use in lieu of i_ino in this case.

> +	*max_len = 3;
> +
> +	return FILEID_INO64_GEN;
> +}
> +
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
>   * @inode:   the object to encode
> @@ -401,10 +425,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  	if (!exportfs_can_encode_fh(nop, flags))
>  		return -EOPNOTSUPP;
>  
> -	if (nop && nop->encode_fh)
> -		return nop->encode_fh(inode, fid->raw, max_len, parent);
> +	if (!nop && (flags & EXPORT_FH_FID))
> +		return exportfs_encode_ino64_fid(inode, fid, max_len);
>  
> -	return -EOPNOTSUPP;
> +	return nop->encode_fh(inode, fid->raw, max_len, parent);
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>  
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 21eeb9f6bdbd..6688e457da64 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -134,7 +134,11 @@ struct fid {
>  			u32 parent_ino;
>  			u32 parent_gen;
>  		} i32;
> - 		struct {
> +		struct {
> +			u64 ino;
> +			u32 gen;
> +		} __packed i64;
> +		struct {
>   			u32 block;
>   			u16 partref;
>   			u16 parent_partref;
> @@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -	return nop && nop->encode_fh;
> +	return !nop || nop->encode_fh;
>  }
>  
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
>  {
>  	/*
>  	 * If a non-decodeable file handle was requested, we only need to make
> -	 * sure that filesystem can encode file handles.
> +	 * sure that filesystem did not opt-out of encoding fid.
>  	 */
>  	if (fh_flags & EXPORT_FH_FID)
>  		return exportfs_can_encode_fid(nop);
Amir Goldstein Oct. 18, 2023, 3:11 p.m. UTC | #2
On Wed, Oct 18, 2023 at 5:28 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Wed, 2023-10-18 at 13:00 +0300, Amir Goldstein wrote:
> > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > the encoding of a file id, which is not intended to be decoded.
> >
> > This file id is used by fanotify to describe objects in events.
> >
> > So far, overlayfs is the only filesystem that supports encoding
> > non-decodeable file ids, by providing export_operations with an
> > ->encode_fh() method and without a ->decode_fh() method.
> >
> > Add support for encoding non-decodeable file ids to all the filesystems
> > that do not provide export_operations, by encoding a file id of type
> > FILEID_INO64_GEN from { i_ino, i_generation }.
> >
> > A filesystem may that does not support NFS export, can opt-out of
> > encoding non-decodeable file ids for fanotify by defining an empty
> > export_operations struct (i.e. with a NULL ->encode_fh() method).
> >
> > This allows the use of fanotify events with file ids on filesystems
> > like 9p which do not support NFS export to bring fanotify in feature
> > parity with inotify on those filesystems.
> >
> > Note that fanotify also requires that the filesystems report a non-null
> > fsid.  Currently, many simple filesystems that have support for inotify
> > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > used with fanotify in file id reporting mode.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
> >  include/linux/exportfs.h | 10 +++++++---
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> > index 30da4539e257..34e7d835d4ef 100644
> > --- a/fs/exportfs/expfs.c
> > +++ b/fs/exportfs/expfs.c
> > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> >  }
> >  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
> >
> > +/**
> > + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> > + * @inode:   the object to encode
> > + * @fid:     where to store the file handle fragment
> > + * @max_len: maximum length to store there
> > + *
> > + * This generic function is used to encode a non-decodeable file id for
> > + * fanotify for filesystems that do not support NFS export.
> > + */
> > +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> > +                                  int *max_len)
> > +{
> > +     if (*max_len < 3) {
> > +             *max_len = 3;
> > +             return FILEID_INVALID;
> > +     }
> > +
> > +     fid->i64.ino = inode->i_ino;
> > +     fid->i64.gen = inode->i_generation;
>
> Note that i_ino is unsigned long and so is a 32-bit value on 32-bit
> arches. If the backend storage uses 64-bit inodes, then we usually end
> up hashing them down to 32-bits first. e.g. see nfs_fattr_to_ino_t().
> ceph has some similar code.
>
> The upshot is that if you're relying on i_ino, the value can change
> between different arches, even when they are dealing with the same
> backend filesystem.
>
> Since this is expected to be used by filesystems that don't set up
> export operations, then that may just be something they have to deal
> with. I'm not sure what else you can use in lieu of i_ino in this case.
>

True. That is one more justification for patch [1/5].

If we only support watching inodes when the reported fid is
{i_ino, i_generation}, then the likelihood of collision drops
considerably compared to watching sb/mount.

Because watcher cannot decode fid, watcher must use
name_to_handle_at() when setting up the inode watch and
store it in some index, so it knows how to map from event->fid
to inode later.

This means that even if there is a fid collision between two inodes,
then the watcher can detect the collision when setting up the
inode watch.

Thanks,
Amir.
Chuck Lever Oct. 18, 2023, 3:27 p.m. UTC | #3
On Wed, Oct 18, 2023 at 01:00:00PM +0300, Amir Goldstein wrote:
> AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> the encoding of a file id, which is not intended to be decoded.
> 
> This file id is used by fanotify to describe objects in events.
> 
> So far, overlayfs is the only filesystem that supports encoding
> non-decodeable file ids, by providing export_operations with an
> ->encode_fh() method and without a ->decode_fh() method.
> 
> Add support for encoding non-decodeable file ids to all the filesystems
> that do not provide export_operations, by encoding a file id of type
> FILEID_INO64_GEN from { i_ino, i_generation }.
> 
> A filesystem may that does not support NFS export, can opt-out of
> encoding non-decodeable file ids for fanotify by defining an empty
> export_operations struct (i.e. with a NULL ->encode_fh() method).
> 
> This allows the use of fanotify events with file ids on filesystems
> like 9p which do not support NFS export to bring fanotify in feature
> parity with inotify on those filesystems.
> 
> Note that fanotify also requires that the filesystems report a non-null
> fsid.  Currently, many simple filesystems that have support for inotify
> (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> used with fanotify in file id reporting mode.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
>  include/linux/exportfs.h | 10 +++++++---
>  2 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 30da4539e257..34e7d835d4ef 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>  }
>  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>  
> +/**
> + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> + * @inode:   the object to encode
> + * @fid:     where to store the file handle fragment
> + * @max_len: maximum length to store there

Length in what units? Is the 3 below in units of bytes or
sizeof(__be32) ? I'm guessing it's the latter; if so, it should
be mentioned here. (We have XDR_UNIT for this purpose, btw).

export_encode_fh() isn't exactly clear about that either, sadly.


> + *
> + * This generic function is used to encode a non-decodeable file id for
> + * fanotify for filesystems that do not support NFS export.
> + */
> +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> +				     int *max_len)
> +{
> +	if (*max_len < 3) {
> +		*max_len = 3;

Let's make this a symbolic constant rather than a naked integer.


> +		return FILEID_INVALID;
> +	}
> +
> +	fid->i64.ino = inode->i_ino;
> +	fid->i64.gen = inode->i_generation;
> +	*max_len = 3;
> +
> +	return FILEID_INO64_GEN;
> +}
> +
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
>   * @inode:   the object to encode
> @@ -401,10 +425,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  	if (!exportfs_can_encode_fh(nop, flags))
>  		return -EOPNOTSUPP;
>  
> -	if (nop && nop->encode_fh)
> -		return nop->encode_fh(inode, fid->raw, max_len, parent);
> +	if (!nop && (flags & EXPORT_FH_FID))
> +		return exportfs_encode_ino64_fid(inode, fid, max_len);
>  
> -	return -EOPNOTSUPP;
> +	return nop->encode_fh(inode, fid->raw, max_len, parent);
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>  
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 21eeb9f6bdbd..6688e457da64 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -134,7 +134,11 @@ struct fid {
>  			u32 parent_ino;
>  			u32 parent_gen;
>  		} i32;
> - 		struct {
> +		struct {
> +			u64 ino;
> +			u32 gen;
> +		} __packed i64;
> +		struct {
>   			u32 block;
>   			u16 partref;
>   			u16 parent_partref;
> @@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -	return nop && nop->encode_fh;
> +	return !nop || nop->encode_fh;
>  }
>  
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
>  {
>  	/*
>  	 * If a non-decodeable file handle was requested, we only need to make
> -	 * sure that filesystem can encode file handles.
> +	 * sure that filesystem did not opt-out of encoding fid.
>  	 */
>  	if (fh_flags & EXPORT_FH_FID)
>  		return exportfs_can_encode_fid(nop);
> -- 
> 2.34.1
> 
>
Amir Goldstein Oct. 18, 2023, 5:19 p.m. UTC | #4
On Wed, Oct 18, 2023 at 6:28 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Wed, Oct 18, 2023 at 01:00:00PM +0300, Amir Goldstein wrote:
> > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > the encoding of a file id, which is not intended to be decoded.
> >
> > This file id is used by fanotify to describe objects in events.
> >
> > So far, overlayfs is the only filesystem that supports encoding
> > non-decodeable file ids, by providing export_operations with an
> > ->encode_fh() method and without a ->decode_fh() method.
> >
> > Add support for encoding non-decodeable file ids to all the filesystems
> > that do not provide export_operations, by encoding a file id of type
> > FILEID_INO64_GEN from { i_ino, i_generation }.
> >
> > A filesystem may that does not support NFS export, can opt-out of
> > encoding non-decodeable file ids for fanotify by defining an empty
> > export_operations struct (i.e. with a NULL ->encode_fh() method).
> >
> > This allows the use of fanotify events with file ids on filesystems
> > like 9p which do not support NFS export to bring fanotify in feature
> > parity with inotify on those filesystems.
> >
> > Note that fanotify also requires that the filesystems report a non-null
> > fsid.  Currently, many simple filesystems that have support for inotify
> > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > used with fanotify in file id reporting mode.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
> >  include/linux/exportfs.h | 10 +++++++---
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> > index 30da4539e257..34e7d835d4ef 100644
> > --- a/fs/exportfs/expfs.c
> > +++ b/fs/exportfs/expfs.c
> > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> >  }
> >  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
> >
> > +/**
> > + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> > + * @inode:   the object to encode
> > + * @fid:     where to store the file handle fragment
> > + * @max_len: maximum length to store there
>
> Length in what units? Is the 3 below in units of bytes or
> sizeof(__be32) ? I'm guessing it's the latter; if so, it should
> be mentioned here. (We have XDR_UNIT for this purpose, btw).
>
> export_encode_fh() isn't exactly clear about that either, sadly.
>
>

Yeh, it's the same all over the place including in filesystem
implementations.

> > + *
> > + * This generic function is used to encode a non-decodeable file id for
> > + * fanotify for filesystems that do not support NFS export.
> > + */
> > +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> > +                                  int *max_len)
> > +{
> > +     if (*max_len < 3) {
> > +             *max_len = 3;
>
> Let's make this a symbolic constant rather than a naked integer.
>

Sure, no problem.

Thanks for the review.
Amir.
Amir Goldstein Oct. 23, 2023, 1:55 p.m. UTC | #5
On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> the encoding of a file id, which is not intended to be decoded.
>
> This file id is used by fanotify to describe objects in events.
>
> So far, overlayfs is the only filesystem that supports encoding
> non-decodeable file ids, by providing export_operations with an
> ->encode_fh() method and without a ->decode_fh() method.
>
> Add support for encoding non-decodeable file ids to all the filesystems
> that do not provide export_operations, by encoding a file id of type
> FILEID_INO64_GEN from { i_ino, i_generation }.
>
> A filesystem may that does not support NFS export, can opt-out of
> encoding non-decodeable file ids for fanotify by defining an empty
> export_operations struct (i.e. with a NULL ->encode_fh() method).
>
> This allows the use of fanotify events with file ids on filesystems
> like 9p which do not support NFS export to bring fanotify in feature
> parity with inotify on those filesystems.
>
> Note that fanotify also requires that the filesystems report a non-null
> fsid.  Currently, many simple filesystems that have support for inotify
> (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> used with fanotify in file id reporting mode.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

Hi Jan,

Did you get a chance to look at this patch?
I saw your review comments on the rest of the series, so was waiting
for feedback on this last one before posting v2.

BTW, I am going to post a complementary patch to add fsid support for
the simple filesystems.

Thanks,
Amir.

>  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
>  include/linux/exportfs.h | 10 +++++++---
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 30da4539e257..34e7d835d4ef 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>  }
>  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>
> +/**
> + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> + * @inode:   the object to encode
> + * @fid:     where to store the file handle fragment
> + * @max_len: maximum length to store there
> + *
> + * This generic function is used to encode a non-decodeable file id for
> + * fanotify for filesystems that do not support NFS export.
> + */
> +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> +                                    int *max_len)
> +{
> +       if (*max_len < 3) {
> +               *max_len = 3;
> +               return FILEID_INVALID;
> +       }
> +
> +       fid->i64.ino = inode->i_ino;
> +       fid->i64.gen = inode->i_generation;
> +       *max_len = 3;
> +
> +       return FILEID_INO64_GEN;
> +}
> +
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
>   * @inode:   the object to encode
> @@ -401,10 +425,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>         if (!exportfs_can_encode_fh(nop, flags))
>                 return -EOPNOTSUPP;
>
> -       if (nop && nop->encode_fh)
> -               return nop->encode_fh(inode, fid->raw, max_len, parent);
> +       if (!nop && (flags & EXPORT_FH_FID))
> +               return exportfs_encode_ino64_fid(inode, fid, max_len);
>
> -       return -EOPNOTSUPP;
> +       return nop->encode_fh(inode, fid->raw, max_len, parent);
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 21eeb9f6bdbd..6688e457da64 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -134,7 +134,11 @@ struct fid {
>                         u32 parent_ino;
>                         u32 parent_gen;
>                 } i32;
> -               struct {
> +               struct {
> +                       u64 ino;
> +                       u32 gen;
> +               } __packed i64;
> +               struct {
>                         u32 block;
>                         u16 partref;
>                         u16 parent_partref;
> @@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -       return nop && nop->encode_fh;
> +       return !nop || nop->encode_fh;
>  }
>
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
>  {
>         /*
>          * If a non-decodeable file handle was requested, we only need to make
> -        * sure that filesystem can encode file handles.
> +        * sure that filesystem did not opt-out of encoding fid.
>          */
>         if (fh_flags & EXPORT_FH_FID)
>                 return exportfs_can_encode_fid(nop);
> --
> 2.34.1
>
Jan Kara Oct. 23, 2023, 4:33 p.m. UTC | #6
On Mon 23-10-23 16:55:40, Amir Goldstein wrote:
> On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > the encoding of a file id, which is not intended to be decoded.
> >
> > This file id is used by fanotify to describe objects in events.
> >
> > So far, overlayfs is the only filesystem that supports encoding
> > non-decodeable file ids, by providing export_operations with an
> > ->encode_fh() method and without a ->decode_fh() method.
> >
> > Add support for encoding non-decodeable file ids to all the filesystems
> > that do not provide export_operations, by encoding a file id of type
> > FILEID_INO64_GEN from { i_ino, i_generation }.
> >
> > A filesystem may that does not support NFS export, can opt-out of
> > encoding non-decodeable file ids for fanotify by defining an empty
> > export_operations struct (i.e. with a NULL ->encode_fh() method).
> >
> > This allows the use of fanotify events with file ids on filesystems
> > like 9p which do not support NFS export to bring fanotify in feature
> > parity with inotify on those filesystems.
> >
> > Note that fanotify also requires that the filesystems report a non-null
> > fsid.  Currently, many simple filesystems that have support for inotify
> > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > used with fanotify in file id reporting mode.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> 
> Hi Jan,
> 
> Did you get a chance to look at this patch?
> I saw your review comments on the rest of the series, so was waiting
> for feedback on this last one before posting v2.

Ah, sorry. I don't have any further comments regarding this patch besides
what Chuck already wrote.

								Honza
Amir Goldstein Oct. 23, 2023, 4:44 p.m. UTC | #7
On Mon, Oct 23, 2023 at 7:33 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 23-10-23 16:55:40, Amir Goldstein wrote:
> > On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > > the encoding of a file id, which is not intended to be decoded.
> > >
> > > This file id is used by fanotify to describe objects in events.
> > >
> > > So far, overlayfs is the only filesystem that supports encoding
> > > non-decodeable file ids, by providing export_operations with an
> > > ->encode_fh() method and without a ->decode_fh() method.
> > >
> > > Add support for encoding non-decodeable file ids to all the filesystems
> > > that do not provide export_operations, by encoding a file id of type
> > > FILEID_INO64_GEN from { i_ino, i_generation }.
> > >
> > > A filesystem may that does not support NFS export, can opt-out of
> > > encoding non-decodeable file ids for fanotify by defining an empty
> > > export_operations struct (i.e. with a NULL ->encode_fh() method).
> > >
> > > This allows the use of fanotify events with file ids on filesystems
> > > like 9p which do not support NFS export to bring fanotify in feature
> > > parity with inotify on those filesystems.
> > >
> > > Note that fanotify also requires that the filesystems report a non-null
> > > fsid.  Currently, many simple filesystems that have support for inotify
> > > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > > used with fanotify in file id reporting mode.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> >
> > Hi Jan,
> >
> > Did you get a chance to look at this patch?
> > I saw your review comments on the rest of the series, so was waiting
> > for feedback on this last one before posting v2.
>
> Ah, sorry. I don't have any further comments regarding this patch besides
> what Chuck already wrote.

No worries.
I will post v2 with minor fixes and add your RVB to all patches.

Thanks,
Amir.
diff mbox series

Patch

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 30da4539e257..34e7d835d4ef 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -383,6 +383,30 @@  int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
 }
 EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
 
+/**
+ * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
+ * @inode:   the object to encode
+ * @fid:     where to store the file handle fragment
+ * @max_len: maximum length to store there
+ *
+ * This generic function is used to encode a non-decodeable file id for
+ * fanotify for filesystems that do not support NFS export.
+ */
+static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
+				     int *max_len)
+{
+	if (*max_len < 3) {
+		*max_len = 3;
+		return FILEID_INVALID;
+	}
+
+	fid->i64.ino = inode->i_ino;
+	fid->i64.gen = inode->i_generation;
+	*max_len = 3;
+
+	return FILEID_INO64_GEN;
+}
+
 /**
  * exportfs_encode_inode_fh - encode a file handle from inode
  * @inode:   the object to encode
@@ -401,10 +425,10 @@  int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 	if (!exportfs_can_encode_fh(nop, flags))
 		return -EOPNOTSUPP;
 
-	if (nop && nop->encode_fh)
-		return nop->encode_fh(inode, fid->raw, max_len, parent);
+	if (!nop && (flags & EXPORT_FH_FID))
+		return exportfs_encode_ino64_fid(inode, fid, max_len);
 
-	return -EOPNOTSUPP;
+	return nop->encode_fh(inode, fid->raw, max_len, parent);
 }
 EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
 
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 21eeb9f6bdbd..6688e457da64 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -134,7 +134,11 @@  struct fid {
 			u32 parent_ino;
 			u32 parent_gen;
 		} i32;
- 		struct {
+		struct {
+			u64 ino;
+			u32 gen;
+		} __packed i64;
+		struct {
  			u32 block;
  			u16 partref;
  			u16 parent_partref;
@@ -246,7 +250,7 @@  extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 
 static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
 {
-	return nop && nop->encode_fh;
+	return !nop || nop->encode_fh;
 }
 
 static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
@@ -259,7 +263,7 @@  static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
 {
 	/*
 	 * If a non-decodeable file handle was requested, we only need to make
-	 * sure that filesystem can encode file handles.
+	 * sure that filesystem did not opt-out of encoding fid.
 	 */
 	if (fh_flags & EXPORT_FH_FID)
 		return exportfs_can_encode_fid(nop);