Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
MIME-Version: 1.0
References: <20231018100000.2453965-1-amir73il@gmail.com> <20231018100000.2453965-6-amir73il@gmail.com>
 <ZS/5fp4XL4D49q9T@tissot.1015granger.net>
In-Reply-To: <ZS/5fp4XL4D49q9T@tissot.1015granger.net>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Wed, 18 Oct 2023 20:19:44 +0300
Message-ID: <CAOQ4uxhaoup7OCTCcbV-uqZF8SaLWppqN8tM3svZsfAf022Jsw@mail.gmail.com>
Subject: Re: [PATCH 5/5] exportfs: support encoding non-decodeable file
 handles by default
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     Jan Kara <jack@suse.cz>, Jeff Layton <jlayton@kernel.org>,
        Christian Brauner <brauner@kernel.org>,
        linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Oct 18, 2023 at 6:28=E2=80=AFPM 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 fo=
r
> > + * 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 =3D 3;
>
> Let's make this a symbolic constant rather than a naked integer.
>

Sure, no problem.

Thanks for the review.
Amir.