2024-01-29 11:17:16

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH v4 0/3] Make nfs stats visible in network NS

On Fri, 2024-01-26 at 16:48 -0500, Josef Bacik wrote:
> v1: https://lore.kernel.org/linux-nfs/[email protected]/
> v2: https://lore.kernel.org/linux-nfs/[email protected]/
> v3: https://lore.kernel.org/linux-nfs/[email protected]/
>
> v3->v4:
> - Fix a weird formatting thing that snuck into 1/3.
>
> v2->v3:
> - Split out the nfs and nfsd related changes into their own patches.
> - Dropped the change adding sv_stats throuch svc_create()
> - Changed the th_cnt to be global, re-arranged it's location.
>
> v1->v2:
> - rework the sunprc service creation to take a pointer to the sv_stats.
> - dropped ->pg_stats from the svc_program.
> - converted all of the nfsd global stats to per-network namespace.
> - added the ability to point at a specific rpc_stat for rpc program creation.
> - converted the rpc stats for nfs to per-network namespace.
>
> -- Original email --
> Hello,
>
> We're currently deploying NFS internally and have run into some oddities with
> our usage of containers. All of the services that mount and export NFS volumes
> run inside of containers, specifically all the namespaces including network
> namespaces. Our monitoring is done on a per-container basis, so we need access
> to the nfs and nfsd stats that are under /proc/net/sunrpc. However these are
> only tied to the init_net, which makes them invisible to containers in a
> different network namespace.
>
> Fix this so that these files are tied to the network namespace. This allows us
> to avoid the hack of bind mounting the hosts /proc into the container in order
> to do proper monitoring. Thanks,
>
> Josef
>
> Josef Bacik (3):
> sunrpc: add a struct rpc_stats arg to rpc_create_args
> nfs: expose /proc/net/sunrpc/nfs in net namespaces
> nfs: make the rpc_stat per net namespace
>
> fs/nfs/client.c | 5 ++++-
> fs/nfs/inode.c | 8 ++++----
> fs/nfs/internal.h | 2 --
> fs/nfs/netns.h | 2 ++
> include/linux/sunrpc/clnt.h | 1 +
> net/sunrpc/clnt.c | 2 +-
> 6 files changed, 12 insertions(+), 8 deletions(-)
>

This all looks good to me. You can add:

Reviewed-by: Jeff Layton <[email protected]>