public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <cel@kernel.org>, NeilBrown <neilb@ownmail.net>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH v7 2/2] NFSD: convert callback RPC program to per-net namespace
Date: Sat, 14 Mar 2026 08:04:42 -0400	[thread overview]
Message-ID: <5df3076a22438f5be6b070e62c0d3fc567341b20.camel@kernel.org> (raw)
In-Reply-To: <20260313163148.281676-3-cel@kernel.org>

On Fri, 2026-03-13 at 12:31 -0400, Chuck Lever wrote:
> From: Dai Ngo <dai.ngo@oracle.com>
> 
> The callback channel's rpc_program, rpc_version, rpc_stat,
> and per-procedure counts are declared as file-scope statics in
> nfs4callback.c, shared across all network namespaces.
> Forechannel RPC statistics are already maintained per-netns
> (via nfsd_svcstats in struct nfsd_net); the backchannel
> has no such separation. When backchannel statistics are
> eventually surfaced to userspace, the global counters would
> expose cross-namespace data.
> 
> Allocate per-netns copies of these structures through a new
> opaque struct nfsd_net_cb, managed by nfsd_net_cb_init()
> and nfsd_net_cb_shutdown(). The struct definition is private
> to nfs4callback.c; struct nfsd_net holds only a pointer.
> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/netns.h        |   3 ++
>  fs/nfsd/nfs4callback.c | 111 ++++++++++++++++++++++++++++-------------
>  fs/nfsd/nfsctl.c       |   5 ++
>  fs/nfsd/state.h        |   9 ++++
>  4 files changed, 94 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 6ad3fe5d7e12..27da1a3edacb 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -25,6 +25,7 @@
>  #define SESSION_HASH_SIZE	512
>  
>  struct cld_net;
> +struct nfsd_net_cb;
>  struct nfsd4_client_tracking_ops;
>  
>  enum {
> @@ -228,6 +229,8 @@ struct nfsd_net {
>  	struct list_head	local_clients;
>  #endif
>  	siphash_key_t		*fh_key;
> +
> +	struct nfsd_net_cb	*nfsd_cb;

Given that there will only ever be one nfsd_cb per net, why not just
embed this struct inside the nfsd_net instead of doing a separate
allocation?

>  };
>  
>  /* Simple check to find out if a given net was properly initialized */
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 74effafdd0dc..50827405468d 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1032,39 +1032,14 @@ static const struct rpc_procinfo nfs4_cb_procedures[] = {
>  	PROC(CB_GETATTR,	COMPOUND,	cb_getattr,	cb_getattr),
>  };
>  
> -static unsigned int nfs4_cb_counts[ARRAY_SIZE(nfs4_cb_procedures)];
> -static const struct rpc_version nfs_cb_version4 = {
> -/*
> - * Note on the callback rpc program version number: despite language in rfc
> - * 5661 section 18.36.3 requiring servers to use 4 in this field, the
> - * official xdr descriptions for both 4.0 and 4.1 specify version 1, and
> - * in practice that appears to be what implementations use.  The section
> - * 18.36.3 language is expected to be fixed in an erratum.
> - */
> -	.number			= 1,
> -	.nrprocs		= ARRAY_SIZE(nfs4_cb_procedures),
> -	.procs			= nfs4_cb_procedures,
> -	.counts			= nfs4_cb_counts,
> -};
> +#define NFS4_CB_PROGRAM	0x40000000
> +#define NFS4_CB_VERSION	1
>  
> -static const struct rpc_version *nfs_cb_version[2] = {
> -	[1] = &nfs_cb_version4,
> -};
> -
> -static const struct rpc_program cb_program;
> -
> -static struct rpc_stat cb_stats = {
> -	.program		= &cb_program
> -};
> -
> -#define NFS4_CALLBACK 0x40000000
> -static const struct rpc_program cb_program = {
> -	.name			= "nfs4_cb",
> -	.number			= NFS4_CALLBACK,
> -	.nrvers			= ARRAY_SIZE(nfs_cb_version),
> -	.version		= nfs_cb_version,
> -	.stats			= &cb_stats,
> -	.pipe_dir_name		= "nfsd4_cb",
> +struct nfsd_net_cb {
> +	struct rpc_version	version4;
> +	const struct rpc_version *versions[NFS4_CB_VERSION + 1];
> +	struct rpc_program	program;
> +	struct rpc_stat		stat;
>  };
>  
>  static int max_cb_time(struct net *net)
> @@ -1140,6 +1115,7 @@ static const struct cred *get_backchannel_cred(struct nfs4_client *clp, struct r
>  
>  static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *conn, struct nfsd4_session *ses)
>  {
> +	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
>  	int maxtime = max_cb_time(clp->net);
>  	struct rpc_timeout	timeparms = {
>  		.to_initval	= maxtime,
> @@ -1152,14 +1128,14 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c
>  		.addrsize	= conn->cb_addrlen,
>  		.saddress	= (struct sockaddr *) &conn->cb_saddr,
>  		.timeout	= &timeparms,
> -		.program	= &cb_program,
> -		.version	= 1,
> +		.version	= NFS4_CB_VERSION,
>  		.flags		= (RPC_CLNT_CREATE_NOPING | RPC_CLNT_CREATE_QUIET),
>  		.cred		= current_cred(),
>  	};
>  	struct rpc_clnt *client;
>  	const struct cred *cred;
>  
> +	args.program = &nn->nfsd_cb->program;
>  	if (clp->cl_minorversion == 0) {
>  		if (!clp->cl_cred.cr_principal &&
>  		    (clp->cl_cred.cr_flavor >= RPC_AUTH_GSS_KRB5)) {
> @@ -1786,3 +1762,70 @@ bool nfsd4_run_cb(struct nfsd4_callback *cb)
>  		nfsd41_cb_inflight_end(clp);
>  	return queued;
>  }
> +
> +/**
> + * nfsd_net_cb_shutdown - release per-netns callback RPC program resources
> + * @nn: NFS server network namespace
> + *
> + * Frees resources allocated by nfsd_net_cb_init().
> + */
> +void nfsd_net_cb_shutdown(struct nfsd_net *nn)
> +{
> +	struct nfsd_net_cb *cb = nn->nfsd_cb;
> +
> +	if (cb) {
> +		kfree(cb->version4.counts);
> +		kfree(cb);
> +		nn->nfsd_cb = NULL;
> +	}
> +}
> +
> +/**
> + * nfsd_net_cb_init - initialize per-netns callback RPC program
> + * @nn: NFS server network namespace
> + *
> + * Sets up the callback RPC program, version table, procedure
> + * counts, and statistics structure for @nn. Caller must release
> + * these resources using nfsd_net_cb_shutdown().
> + *
> + * Return: 0 on success, or -ENOMEM if allocation fails.
> + */
> +int nfsd_net_cb_init(struct nfsd_net *nn)
> +{
> +	struct nfsd_net_cb *cb;
> +
> +	cb = kzalloc(sizeof(*cb), GFP_KERNEL);
> +	if (!cb)
> +		return -ENOMEM;
> +
> +	cb->version4.counts = kzalloc_objs(unsigned int,
> +			ARRAY_SIZE(nfs4_cb_procedures), GFP_KERNEL);
> +	if (!cb->version4.counts) {
> +		kfree(cb);
> +		return -ENOMEM;
> +	}
> +	/*
> +	 * Note on the callback rpc program version number: despite language
> +	 * in rfc 5661 section 18.36.3 requiring servers to use 4 in this
> +	 * field, the official xdr descriptions for both 4.0 and 4.1 specify
> +	 * version 1, and in practice that appears to be what implementations
> +	 * use. The section 18.36.3 language is expected to be fixed in an
> +	 * erratum.
> +	 */
> +	cb->version4.number = NFS4_CB_VERSION;
> +	cb->version4.nrprocs = ARRAY_SIZE(nfs4_cb_procedures);
> +	cb->version4.procs = nfs4_cb_procedures;
> +	cb->versions[NFS4_CB_VERSION] = &cb->version4;
> +
> +	cb->program.name = "nfs4_cb";
> +	cb->program.number = NFS4_CB_PROGRAM;
> +	cb->program.nrvers = ARRAY_SIZE(cb->versions);
> +	cb->program.version = &cb->versions[0];
> +	cb->program.pipe_dir_name = "nfsd4_cb";
> +	cb->program.stats = &cb->stat;
> +	cb->stat.program = &cb->program;
> +
> +	nn->nfsd_cb = cb;
> +
> +	return 0;
> +}
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 14d9458aeff0..988a79ec4a79 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -2216,6 +2216,9 @@ static __net_init int nfsd_net_init(struct net *net)
>  	int retval;
>  	int i;
>  
> +	retval = nfsd_net_cb_init(nn);
> +	if (retval)
> +		return retval;
>  	retval = nfsd_export_init(net);
>  	if (retval)
>  		goto out_export_error;
> @@ -2256,6 +2259,7 @@ static __net_init int nfsd_net_init(struct net *net)
>  out_idmap_error:
>  	nfsd_export_shutdown(net);
>  out_export_error:
> +	nfsd_net_cb_shutdown(nn);
>  	return retval;
>  }
>  
> @@ -2286,6 +2290,7 @@ static __net_exit void nfsd_net_exit(struct net *net)
>  	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>  
>  	kfree_sensitive(nn->fh_key);
> +	nfsd_net_cb_shutdown(nn);
>  	nfsd_proc_stat_shutdown(net);
>  	percpu_counter_destroy_many(nn->counter, NFSD_STATS_COUNTERS_NUM);
>  	nfsd_idmap_shutdown(net);
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 9b05462da4cc..953675eba5c3 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -862,6 +862,8 @@ struct nfsd_file *find_any_file(struct nfs4_file *f);
>  #ifdef CONFIG_NFSD_V4
>  void nfsd4_revoke_states(struct nfsd_net *nn, struct super_block *sb);
>  void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb);
> +int nfsd_net_cb_init(struct nfsd_net *nn);
> +void nfsd_net_cb_shutdown(struct nfsd_net *nn);
>  #else
>  static inline void nfsd4_revoke_states(struct nfsd_net *nn, struct super_block *sb)
>  {
> @@ -869,6 +871,13 @@ static inline void nfsd4_revoke_states(struct nfsd_net *nn, struct super_block *
>  static inline void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
>  {
>  }
> +static inline int nfsd_net_cb_init(struct nfsd_net *nn)
> +{
> +	return 0;
> +}
> +static inline void nfsd_net_cb_shutdown(struct nfsd_net *nn)
> +{
> +}
>  #endif
>  
>  /* grace period management */

-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-03-14 12:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-13 16:31 [PATCH v7 0/2] NFSD: move accumulated callback ops to per-net namespace Chuck Lever
2026-03-13 16:31 ` [PATCH v7 1/2] NFSD: use per-operation statidx for callback procedures Chuck Lever
2026-03-14 11:55   ` Jeff Layton
2026-03-13 16:31 ` [PATCH v7 2/2] NFSD: convert callback RPC program to per-net namespace Chuck Lever
2026-03-14 12:04   ` Jeff Layton [this message]
2026-03-14 15:47     ` Chuck Lever
2026-03-14 23:01       ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5df3076a22438f5be6b070e62c0d3fc567341b20.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=cel@kernel.org \
    --cc=dai.ngo@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@ownmail.net \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox