[LTP] [PATCH 1/2] Add test for per-NS NFS client statistics

[LTP] [PATCH 1/2] Add test for per-NS NFS client statistics

[Martin Doucha]

Check that /proc/net/rpc/nfs file exists in nested network namespaces.

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

Only do the minimal check here. If the file exists in namespaces,
nfsstat01.sh will take care of functional testing.

 runtest/net.nfs                              |  2 ++
 testcases/network/nfs/nfsstat01/nfsstat02.sh | 23 ++++++++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100755 testcases/network/nfs/nfsstat01/nfsstat02.sh

diff --git a/runtest/net.nfs b/runtest/net.nfs
index 929868a7b..7f84457bc 100644
--- a/runtest/net.nfs
+++ b/runtest/net.nfs
@@ -116,6 +116,8 @@ nfsstat01_v40_ip6t nfsstat01.sh -6 -v 4 -t tcp
 nfsstat01_v41_ip6t nfsstat01.sh -6 -v 4.1 -t tcp
 nfsstat01_v42_ip6t nfsstat01.sh -6 -v 4.2 -t tcp
 
+nfsstat02 nfsstat02.sh
+
 fsx_v30_ip4u fsx.sh -v 3 -t udp
 fsx_v30_ip4t fsx.sh -v 3 -t tcp
 fsx_v40_ip4t fsx.sh -v 4 -t tcp
diff --git a/testcases/network/nfs/nfsstat01/nfsstat02.sh b/testcases/network/nfs/nfsstat01/nfsstat02.sh
new file mode 100755
index 000000000..1e1bebe97
--- /dev/null
+++ b/testcases/network/nfs/nfsstat01/nfsstat02.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2024 SUSE LLC <mdoucha@suse.cz>
+
+TST_TESTFUNC="do_test"
+
+# PURPOSE:  Check that /proc/net/rpc/nfs exists in nested network namespaces
+do_test()
+{
+	local procfile="/proc/net/rpc/nfs"
+
+	if tst_rhost_run -c "test -e '$procfile'"; then
+		tst_res TPASS "$procfile exists in net namespaces"
+	else
+		tst_res TFAIL "$procfile missing in net namespaces"
+	fi
+}
+
+# Force use of nested net namespace
+unset RHOST
+
+. nfs_lib.sh
+tst_run
-- 
2.46.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Martin Doucha]

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.

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


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Chuck Lever via ltp]

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.

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
> 

-- 
Chuck Lever

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

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.

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

> 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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Martin Doucha]

On 30. 08. 24 20:10, Chuck Lever wrote:
> 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.
> 
> 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.

I tried to test the NFS client call counters, fields 13, 15 or 24 
(depending on NFS version) in the "procX" line of /proc/net/rpc/nfs. 
These are the counters that the test already checks after RPC.

Although when I think about it some more, I'm not sure whether the 
NFS/RPC client statistics should be attached to network namespaces in 
the first place. AFAICT, processes from any network namespace can 
trigger client calls for both RPC and NFS as long as they can access the 
NFS mountpoint. Perhaps a mount namespace would be the more logical 
domain for counting per-NS statistics instead?

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Chuck Lever III via ltp]

> On Sep 2, 2024, at 7:49 AM, Martin Doucha <mdoucha@suse.cz> wrote:
> 
> On 30. 08. 24 20:10, Chuck Lever wrote:
>> 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.
>> 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.
> 
> I tried to test the NFS client call counters, fields 13, 15 or 24 (depending on NFS version) in the "procX" line of /proc/net/rpc/nfs. These are the counters that the test already checks after RPC.
> 
> Although when I think about it some more, I'm not sure whether the NFS/RPC client statistics should be attached to network namespaces in the first place. AFAICT, processes from any network namespace can trigger client calls for both RPC and NFS as long as they can access the NFS mountpoint. Perhaps a mount namespace would be the more logical domain for counting per-NS statistics instead?

Disclaimer: I'm not one of the NFS client maintainers, but only
a very long time contributor to the Linux NFS implementation,
so I can offer only a somewhat-educated opinion on this topic.

IIRC in a container, the RPC client is bound to the network
namespace.

The statistics in /proc/self/mountstats are accrued to an
individual mount. I think those are associated with the mount
namespace. I could very well be wrong about that.

This is another topic that would be appropriate to bring to
linux-nfs@ .

--
Chuck Lever



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Martin Doucha]

On 02. 09. 24 20:13, Chuck Lever III wrote:
> Disclaimer: I'm not one of the NFS client maintainers, but only
> a very long time contributor to the Linux NFS implementation,
> so I can offer only a somewhat-educated opinion on this topic.
> 
> IIRC in a container, the RPC client is bound to the network
> namespace.

I'm not a NFS expert either. Just thinking out loud because I've noticed 
that the nfsstat01 test triggers RPC calls from outside the client net 
namespace.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

> 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.

> 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

Maybe also add TCONF message? (can be added before merge)

    else
		tst_res TCONF "Not testing leak between root NS and net NS due old kernel"

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Thanks for this test!

Kind regards,
Petr

> +	fi
> +
>  	tst_res TINFO "checking NFS calls for server/client"
>  	case $VERSION in
>  	2) client_field=13 server_field=13

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Martin Doucha]

Hi,

On 30. 08. 24 22:15, Petr Vorel wrote:
>> @@ -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
> 
> Maybe also add TCONF message? (can be added before merge)
> 
>      else
> 		tst_res TCONF "Not testing leak between root NS and net NS due old kernel"

I think the TCONF message doesn't make sense here. There are several 
cases where the new check will be skipped:
1) the NFS server runs on another machine ($RHOST is not empty)
2) the test is configure to ignore namespaces ($LTP_NFS_NETNS_USE_LO is 
not empty)
3) /proc/net/rpc/nfs doesn't exist in nested net namespaces

You want to print the TCONF message only in case #3. Let's keep the 
condition above simple.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Martin,

> Hi,

> On 30. 08. 24 22:15, Petr Vorel wrote:
> > > @@ -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

> > Maybe also add TCONF message? (can be added before merge)

> >      else
> > 		tst_res TCONF "Not testing leak between root NS and net NS due old kernel"

> I think the TCONF message doesn't make sense here. There are several cases
> where the new check will be skipped:
> 1) the NFS server runs on another machine ($RHOST is not empty)
> 2) the test is configure to ignore namespaces ($LTP_NFS_NETNS_USE_LO is not
> empty)
> 3) /proc/net/rpc/nfs doesn't exist in nested net namespaces

> You want to print the TCONF message only in case #3. Let's keep the
> condition above simple.

Fair enough. I merged just with adding kernel commit and patchset link in the
first commit. Thanks!

NOTE: I noticed your point about mount namespaces (interesting I wonder if there
will be some change), but merged as it reflects the current kernel code.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] Add test for per-NS NFS client statistics

[Petr Vorel]

Hi Martin,

> Check that /proc/net/rpc/nfs file exists in nested network namespaces.

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

> Only do the minimal check here. If the file exists in namespaces,
> nfsstat01.sh will take care of functional testing.

>  runtest/net.nfs                              |  2 ++
>  testcases/network/nfs/nfsstat01/nfsstat02.sh | 23 ++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100755 testcases/network/nfs/nfsstat01/nfsstat02.sh

> diff --git a/runtest/net.nfs b/runtest/net.nfs
> index 929868a7b..7f84457bc 100644
> --- a/runtest/net.nfs
> +++ b/runtest/net.nfs
> @@ -116,6 +116,8 @@ nfsstat01_v40_ip6t nfsstat01.sh -6 -v 4 -t tcp
>  nfsstat01_v41_ip6t nfsstat01.sh -6 -v 4.1 -t tcp
>  nfsstat01_v42_ip6t nfsstat01.sh -6 -v 4.2 -t tcp

> +nfsstat02 nfsstat02.sh
> +
>  fsx_v30_ip4u fsx.sh -v 3 -t udp
>  fsx_v30_ip4t fsx.sh -v 3 -t tcp
>  fsx_v40_ip4t fsx.sh -v 4 -t tcp
> diff --git a/testcases/network/nfs/nfsstat01/nfsstat02.sh b/testcases/network/nfs/nfsstat01/nfsstat02.sh
> new file mode 100755
> index 000000000..1e1bebe97
> --- /dev/null
> +++ b/testcases/network/nfs/nfsstat01/nfsstat02.sh
> @@ -0,0 +1,23 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2024 SUSE LLC <mdoucha@suse.cz>
> +
> +TST_TESTFUNC="do_test"
> +
> +# PURPOSE:  Check that /proc/net/rpc/nfs exists in nested network namespaces
I would point here a commit which added it or a patchset.

Shell API does not have functionality to point out missing kernel git commit,
but that might change. But even without it a comment is useful.

Maybe point out d47151b79e32 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
and whole patchset
https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested on 6.8 and 6.11 mainline
Tested-by: Petr Vorel <pvorel@suse.cz>

Thanks for this test!

Kind regards,
Petr

> +do_test()
> +{
> +	local procfile="/proc/net/rpc/nfs"
> +
> +	if tst_rhost_run -c "test -e '$procfile'"; then
> +		tst_res TPASS "$procfile exists in net namespaces"
> +	else
> +		tst_res TFAIL "$procfile missing in net namespaces"
> +	fi
> +}
> +
> +# Force use of nested net namespace
> +unset RHOST
> +
> +. nfs_lib.sh
> +tst_run

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] Add test for per-NS NFS client statistics

[Martin Doucha]

Hi,

On 30. 08. 24 22:09, Petr Vorel wrote:
> Hi Martin,
> 
>> +# PURPOSE:  Check that /proc/net/rpc/nfs exists in nested network namespaces
> I would point here a commit which added it or a patchset.
> 
> Shell API does not have functionality to point out missing kernel git commit,
> but that might change. But even without it a comment is useful.
> 
> Maybe point out d47151b79e32 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
> and whole patchset
> https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/

Feel free to add the comment during merge.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp