Re: [PATCH 2/2] nfsd: expose /proc/net/sunrpc/nfsd in net namespaces

[Chuck Lever <chuck.lever@oracle.com>]

On Wed, Jan 24, 2024 at 02:37:00PM -0500, Josef Bacik wrote:
> We are running nfsd servers inside of containers with their own network
> namespace, and we want to monitor these services using the stats found
> in /proc.  However these are not exposed in the proc inside of the
> container, so we have to bind mount the host /proc into our containers
> to get at this information.
> 
> Separate out the stat counters init and the proc registration, and move
> the proc registration into the pernet operations entry and exit points
> so that these stats can be exposed inside of network namespaces.

Maybe I missed something, but this looks like it exposes the global
stat counters to all net namespaces...? Is that an information leak?
As an administrator I might be surprised by that behavior.

Seems like this patch needs to make nfsdstats and nfsd_svcstats into
per-namespace objects as well.


> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/nfsd/nfsctl.c |  8 +++++---
>  fs/nfsd/stats.c  | 21 ++++++---------------
>  fs/nfsd/stats.h  |  6 ++++--
>  3 files changed, 15 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index f206ca32e7f5..b57480b50e35 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1679,6 +1679,7 @@ static __net_init int nfsd_net_init(struct net *net)
>  	nfsd4_init_leases_net(nn);
>  	get_random_bytes(&nn->siphash_key, sizeof(nn->siphash_key));
>  	seqlock_init(&nn->writeverf_lock);
> +	nfsd_proc_stat_init(net);
>  
>  	return 0;
>  
> @@ -1699,6 +1700,7 @@ static __net_exit void nfsd_net_exit(struct net *net)
>  {
>  	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>  
> +	nfsd_proc_stat_shutdown(net);
>  	nfsd_net_reply_cache_destroy(nn);
>  	nfsd_idmap_shutdown(net);
>  	nfsd_export_shutdown(net);
> @@ -1722,7 +1724,7 @@ static int __init init_nfsd(void)
>  	retval = nfsd4_init_pnfs();
>  	if (retval)
>  		goto out_free_slabs;
> -	retval = nfsd_stat_init();	/* Statistics */
> +	retval = nfsd_stat_counters_init();	/* Statistics */
>  	if (retval)
>  		goto out_free_pnfs;
>  	retval = nfsd_drc_slab_create();
> @@ -1762,7 +1764,7 @@ static int __init init_nfsd(void)
>  	nfsd_lockd_shutdown();
>  	nfsd_drc_slab_free();
>  out_free_stat:
> -	nfsd_stat_shutdown();
> +	nfsd_stat_counters_destroy();
>  out_free_pnfs:
>  	nfsd4_exit_pnfs();
>  out_free_slabs:
> @@ -1780,7 +1782,7 @@ static void __exit exit_nfsd(void)
>  	nfsd_drc_slab_free();
>  	remove_proc_entry("fs/nfs/exports", NULL);
>  	remove_proc_entry("fs/nfs", NULL);
> -	nfsd_stat_shutdown();
> +	nfsd_stat_counters_destroy();
>  	nfsd_lockd_shutdown();
>  	nfsd4_free_slabs();
>  	nfsd4_exit_pnfs();
> diff --git a/fs/nfsd/stats.c b/fs/nfsd/stats.c
> index 12d79f5d4eb1..394a65a33942 100644
> --- a/fs/nfsd/stats.c
> +++ b/fs/nfsd/stats.c
> @@ -108,31 +108,22 @@ void nfsd_percpu_counters_destroy(struct percpu_counter counters[], int num)
>  		percpu_counter_destroy(&counters[i]);
>  }
>  
> -static int nfsd_stat_counters_init(void)
> +int nfsd_stat_counters_init(void)
>  {
>  	return nfsd_percpu_counters_init(nfsdstats.counter, NFSD_STATS_COUNTERS_NUM);
>  }
>  
> -static void nfsd_stat_counters_destroy(void)
> +void nfsd_stat_counters_destroy(void)
>  {
>  	nfsd_percpu_counters_destroy(nfsdstats.counter, NFSD_STATS_COUNTERS_NUM);
>  }
>  
> -int nfsd_stat_init(void)
> +void nfsd_proc_stat_init(struct net *net)
>  {
> -	int err;
> -
> -	err = nfsd_stat_counters_init();
> -	if (err)
> -		return err;
> -
> -	svc_proc_register(&init_net, &nfsd_svcstats, &nfsd_proc_ops);
> -
> -	return 0;
> +	svc_proc_register(net, &nfsd_svcstats, &nfsd_proc_ops);
>  }
>  
> -void nfsd_stat_shutdown(void)
> +void nfsd_proc_stat_shutdown(struct net *net)
>  {
> -	nfsd_stat_counters_destroy();
> -	svc_proc_unregister(&init_net, "nfsd");
> +	svc_proc_unregister(net, "nfsd");
>  }
> diff --git a/fs/nfsd/stats.h b/fs/nfsd/stats.h
> index 14f50c660b61..5cd6517b52a9 100644
> --- a/fs/nfsd/stats.h
> +++ b/fs/nfsd/stats.h
> @@ -40,8 +40,10 @@ extern struct svc_stat		nfsd_svcstats;
>  int nfsd_percpu_counters_init(struct percpu_counter *counters, int num);
>  void nfsd_percpu_counters_reset(struct percpu_counter *counters, int num);
>  void nfsd_percpu_counters_destroy(struct percpu_counter *counters, int num);
> -int nfsd_stat_init(void);
> -void nfsd_stat_shutdown(void);
> +int nfsd_stat_counters_init(void);
> +void nfsd_stat_counters_destroy(void);
> +void nfsd_proc_stat_init(struct net *net);
> +void nfsd_proc_stat_shutdown(struct net *net);
>  
>  static inline void nfsd_stats_rc_hits_inc(void)
>  {
> -- 
> 2.43.0
> 
> 

-- 
Chuck Lever