Re: [PATCH 2/2] nfsstat01: Check that RPC stats don't leak between net namespaces

[Petr Vorel <pvorel@suse.cz>]

> Hi all,

> > On Fri, Aug 30, 2024 at 04:13:40PM +0200, Martin Doucha wrote:
> > > When the NFS server and client run on the same host in different net
> > > namespaces, check that RPC calls from the client namespace don't
> > > change RPC statistics in the root namespace.

> > > Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> > > ---

> > > I've initially tried to test both NFS and RPC client stats but it appears
> > > that NFS client stats are still shared across all namespaces. Only RPC
> > > client stats are separate for each net namespace. The kernel patchset[1]
> > > which introduced per-NS stats confirms that only RPC stats have been changed.

> Yes, only RPC client stats needed to be fixed in LTP test.

OTOH there is also nfsd stats namespaced [2] as a second part of whole "Make nfs
and nfsd stats visible in network ns" patchset. I suppose we would need to
reverse client and server to detect this (IMHO worth of doing it).

Kind regards,
Petr

[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=author&q=4b14885411f74b2b0ce0eb2b39d0fffe54e5ca0d
[3] https://lore.kernel.org/linux-nfs/cover.1706212207.git.josef@toxicpanda.com/


> Kind regards,
> Petr

> > I believe that is correct, Josef changed only RPC counters. Which
> > counters did you expect also would be containerized, exactly?
> > Perhaps this issue should be raised on linux-nfs@vger, it could be
> > considered to be another information leak.


> > > If NFS client stats should be separate for each namespace as well, let
> > > me know and I'll return the second set of NS checks in patch v2.

> > > Tested on kernel v5.14 with Neil's backports.

> > > [1] https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/

> > >  testcases/network/nfs/nfsstat01/nfsstat01.sh | 18 ++++++++++++++++--
> > >  1 file changed, 16 insertions(+), 2 deletions(-)

> > > diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > > index 8d7202cf3..3379c4d46 100755
> > > --- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > > +++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > > @@ -22,6 +22,7 @@ get_calls()
> > >  	local name=$1
> > >  	local field=$2
> > >  	local nfs_f=$3
> > > +	local netns=${4:-rhost}
> > >  	local type="lhost"
> > >  	local calls opt

> > > @@ -30,7 +31,8 @@ get_calls()

> > >  	if tst_net_use_netns; then
> > >  		# In netns setup, rhost is the client
> > > -		[ "$nfs_f" = "nfs" ] && [ $NS_STAT_RHOST -ne 0 ] && type="rhost"
> > > +		[ "$nfs_f" = "nfs" ] && [ $NS_STAT_RHOST -ne 0 ] && \
> > > +			type="$netns"
> > >  	else
> > >  		[ "$nfs_f" != "nfs" ] && type="rhost"
> > >  	fi
> > > @@ -64,13 +66,14 @@ get_calls()
> > >  do_test()
> > >  {
> > >  	local client_calls server_calls new_server_calls new_client_calls
> > > -	local client_field server_field
> > > +	local client_field server_field root_calls new_root_calls
> > >  	local client_v=$VERSION server_v=$VERSION

> > >  	tst_res TINFO "checking RPC calls for server/client"

> > >  	server_calls="$(get_calls rpc 2 nfsd)"
> > >  	client_calls="$(get_calls rpc 2 nfs)"
> > > +	root_calls="$(get_calls rpc 2 nfs lhost)"

> > >  	tst_res TINFO "calls $server_calls/$client_calls"

> > > @@ -79,6 +82,7 @@ do_test()

> > >  	new_server_calls="$(get_calls rpc 2 nfsd)"
> > >  	new_client_calls="$(get_calls rpc 2 nfs)"
> > > +	new_root_calls="$(get_calls rpc 2 nfs lhost)"
> > >  	tst_res TINFO "new calls $new_server_calls/$new_client_calls"

> > >  	if [ "$new_server_calls" -le "$server_calls" ]; then
> > > @@ -93,6 +97,16 @@ do_test()
> > >  		tst_res TPASS "client RPC calls increased"
> > >  	fi

> > > +	if [ $NS_STAT_RHOST -ne 0 ]; then
> > > +		tst_res TINFO "Root NS client RPC calls: $root_calls => $new_root_calls"
> > > +
> > > +		if [ $root_calls -ne $new_root_calls ]; then
> > > +			tst_res TFAIL "RPC stats leaked between net namespaces"
> > > +		else
> > > +			tst_res TPASS "RPC stats stay within net namespaces"
> > > +		fi
> > > +	fi
> > > +
> > >  	tst_res TINFO "checking NFS calls for server/client"
> > >  	case $VERSION in
> > >  	2) client_field=13 server_field=13
> > > -- 
> > > 2.46.0