Re: [PATCH v7 14/20] nfsd: implement server support for NFS_LOCALIO_PROGRAM

Mike Snitzer
2024-06-26T16:27:32Z

On Wed, Jun 26, 2024 at 09:23:30AM +1000, NeilBrown wrote:
> On Tue, 25 Jun 2024, Mike Snitzer wrote:
> > LOCALIOPROC_GETUUID encodes the server's uuid_t in terms of the fixed
> > UUID_SIZE (16). The fixed size opaque encode and decode XDR methods
> > are used instead of the less efficient variable sized methods.
> >
> > Aside from a bit of code in nfssvc.c, all the knowledge of the LOCALIO
> > RPC protocol is in fs/nfsd/localio.c which implements just a single
> > version (1) that is used independently of what NFS version is used.
> >
> > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > [neilb: factored out and simplified single localio protocol]
> > Co-developed-by: NeilBrown <neilb@suse.de>
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> > fs/nfsd/localio.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
> > fs/nfsd/nfssvc.c | 29 ++++++++++++++++++-
> > 2 files changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
> > index f6df66b1d523..aaa5293eb352 100644
> > --- a/fs/nfsd/localio.c
> > +++ b/fs/nfsd/localio.c
> > @@ -11,12 +11,15 @@
> > #include <linux/sunrpc/svcauth_gss.h>
> > #include <linux/sunrpc/clnt.h>
> > #include <linux/nfs.h>
> > +#include <linux/nfs_fs.h>
> > +#include <linux/nfs_xdr.h>
> > #include <linux/string.h>
> >
> > #include "nfsd.h"
> > #include "vfs.h"
> > #include "netns.h"
> > #include "filecache.h"
> > +#include "cache.h"
> >
> > #define NFSDDBG_FACILITY NFSDDBG_FH
> >
> > @@ -249,3 +252,74 @@ EXPORT_SYMBOL_GPL(nfsd_open_local_fh);
> >
> > /* Compile time type checking, not used by anything */
> > static nfs_to_nfsd_open_t __maybe_unused nfsd_open_local_fh_typecheck = nfsd_open_local_fh;
> > +
> > +/*
> > + * GETUUID XDR encode functions
> > + */
> > +
> > +static __be32 nfsd_proc_null(struct svc_rqst *rqstp)
> > +{
> > + return rpc_success;
> > +}
> > +
> > +struct nfsd_getuuidres {
> > + uuid_t uuid;
> > +};
> > +
> > +static __be32 nfsd_proc_getuuid(struct svc_rqst *rqstp)
> > +{
> > + struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> > + struct nfsd_getuuidres *resp = rqstp->rq_resp;
> > +
> > + uuid_copy(&resp->uuid, &nn->nfsd_uuid.uuid);
> > +
> > + return rpc_success;
> > +}
> > +
> > +static bool nfslocalio_encode_getuuidres(struct svc_rqst *rqstp,
> > + struct xdr_stream *xdr)
> > +{
> > + struct nfsd_getuuidres *resp = rqstp->rq_resp;
> > + u8 uuid[UUID_SIZE];
> > +
> > + export_uuid(uuid, &resp->uuid);
> > + encode_opaque_fixed(xdr, uuid, UUID_SIZE);
> > + dprintk("%s: uuid=%pU\n", __func__, uuid);
> > +
> > + return true;
> > +}
> > +
> > +static const struct svc_procedure nfsd_localio_procedures[2] = {
>
> Including a '2' here is unnecessary.
>
> > + [LOCALIOPROC_NULL] = {
> > + .pc_func = nfsd_proc_null,
> > + .pc_decode = nfssvc_decode_voidarg,
> > + .pc_encode = nfssvc_encode_voidres,
> > + .pc_argsize = sizeof(struct nfsd_voidargs),
> > + .pc_ressize = sizeof(struct nfsd_voidres),
> > + .pc_cachetype = RC_NOCACHE,
> > + .pc_xdrressize = 0,
> > + .pc_name = "NULL",
> > + },
> > + [LOCALIOPROC_GETUUID] = {
> > + .pc_func = nfsd_proc_getuuid,
> > + .pc_decode = nfssvc_decode_voidarg,
> > + .pc_encode = nfslocalio_encode_getuuidres,
> > + .pc_argsize = sizeof(struct nfsd_voidargs),
> > + .pc_ressize = sizeof(struct nfsd_getuuidres),
> > + .pc_cachetype = RC_NOCACHE,
> > + .pc_xdrressize = XDR_QUADLEN(UUID_SIZE),
> > + .pc_name = "GETUUID",
> > + },
> > +};
> > +
> > +static DEFINE_PER_CPU_ALIGNED(unsigned long,
> > + nfsd_localio_count[ARRAY_SIZE(nfsd_localio_procedures)]);
>
> We have ARRAY_SIZE above and "= 2" below, both referring to the same
> value.
>
> Maybe #define LOCALIO_NR_PROCEEDURES ARRAY_SIZE(nfsd_localio_procedures)
> ??

Yeap, sounds good, fixed for v8.