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

[Mike Snitzer <snitzer@kernel.org>]

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.