[PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Petr Vorel]

6.9 moved client RPC calls to namespace in "Make nfs stats visible in
network NS" patchet.

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

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v1->v2:
* Point out whole patchset, not just single commit
* Add a comment about the patchset

Hi all,

could you please ack this so that we have fixed mainline?

FYI Some parts has been backported, e.g.:
d47151b79e322 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
to all stable/LTS: 5.4.276, 5.10.217, 5.15.159, 6.1.91, 6.6.31.

But most of that is not yet (but planned to be backported), e.g.
93483ac5fec62 ("nfsd: expose /proc/net/sunrpc/nfsd in net namespaces")
see Chuck's patchset for 6.6
https://lore.kernel.org/linux-nfs/20240812223604.32592-1-cel@kernel.org/

Once all kernels up to 5.4 fixed we should update the version.

Kind regards,
Petr

 testcases/network/nfs/nfsstat01/nfsstat01.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
index c2856eff1f..1beecbec43 100755
--- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
+++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
@@ -15,7 +15,14 @@ get_calls()
 	local calls opt
 
 	[ "$name" = "rpc" ] && opt="r" || opt="n"
-	! tst_net_use_netns && [ "$nfs_f" != "nfs" ] && type="rhost"
+
+	if tst_net_use_netns; then
+		# "Make nfs stats visible in network NS" patchet
+		# https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
+		tst_kvcmp -ge "6.9" && [ "$nfs_f" = "nfs" ] && type="rhost"
+	else
+		[ "$nfs_f" != "nfs" ] && type="rhost"
+	fi
 
 	if [ "$type" = "lhost" ]; then
 		calls="$(grep $name /proc/net/rpc/$nfs_f | cut -d' ' -f$field)"
-- 
2.45.2

Re: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Chuck Lever]

On Wed, Aug 14, 2024 at 10:57:21AM +0200, Petr Vorel wrote:
> 6.9 moved client RPC calls to namespace in "Make nfs stats visible in
> network NS" patchet.
> 
> https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Changes v1->v2:
> * Point out whole patchset, not just single commit
> * Add a comment about the patchset
> 
> Hi all,
> 
> could you please ack this so that we have fixed mainline?
> 
> FYI Some parts has been backported, e.g.:
> d47151b79e322 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
> to all stable/LTS: 5.4.276, 5.10.217, 5.15.159, 6.1.91, 6.6.31.
> 
> But most of that is not yet (but planned to be backported), e.g.
> 93483ac5fec62 ("nfsd: expose /proc/net/sunrpc/nfsd in net namespaces")
> see Chuck's patchset for 6.6
> https://lore.kernel.org/linux-nfs/20240812223604.32592-1-cel@kernel.org/
> 
> Once all kernels up to 5.4 fixed we should update the version.
> 
> Kind regards,
> Petr
> 
>  testcases/network/nfs/nfsstat01/nfsstat01.sh | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> index c2856eff1f..1beecbec43 100755
> --- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
> +++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> @@ -15,7 +15,14 @@ get_calls()
>  	local calls opt
>  
>  	[ "$name" = "rpc" ] && opt="r" || opt="n"
> -	! tst_net_use_netns && [ "$nfs_f" != "nfs" ] && type="rhost"
> +
> +	if tst_net_use_netns; then
> +		# "Make nfs stats visible in network NS" patchet
> +		# https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> +		tst_kvcmp -ge "6.9" && [ "$nfs_f" = "nfs" ] && type="rhost"

Hello Petr-

My concern with this fix is it targets v6.9 specifically, yet we
know these fixes will appear in LTS/stable kernels as well.

Neil Brown suggested an alternative approach that might not depend
on knowing the specific kernel version:

https://lore.kernel.org/linux-nfs/172078283934.15471.13377048166707693692@noble.neil.brown.name/

HTH


> +	else
> +		[ "$nfs_f" != "nfs" ] && type="rhost"
> +	fi
>  
>  	if [ "$type" = "lhost" ]; then
>  		calls="$(grep $name /proc/net/rpc/$nfs_f | cut -d' ' -f$field)"
> -- 
> 2.45.2
> 
> 

-- 
Chuck Lever

Re: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Petr Vorel]

Hi Chuck, Neil, all,

> On Wed, Aug 14, 2024 at 10:57:21AM +0200, Petr Vorel wrote:
> > 6.9 moved client RPC calls to namespace in "Make nfs stats visible in
> > network NS" patchet.

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

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Changes v1->v2:
> > * Point out whole patchset, not just single commit
> > * Add a comment about the patchset

> > Hi all,

> > could you please ack this so that we have fixed mainline?

> > FYI Some parts has been backported, e.g.:
> > d47151b79e322 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
> > to all stable/LTS: 5.4.276, 5.10.217, 5.15.159, 6.1.91, 6.6.31.

> > But most of that is not yet (but planned to be backported), e.g.
> > 93483ac5fec62 ("nfsd: expose /proc/net/sunrpc/nfsd in net namespaces")
> > see Chuck's patchset for 6.6
> > https://lore.kernel.org/linux-nfs/20240812223604.32592-1-cel@kernel.org/

> > Once all kernels up to 5.4 fixed we should update the version.

> > Kind regards,
> > Petr

> >  testcases/network/nfs/nfsstat01/nfsstat01.sh | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)

> > diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > index c2856eff1f..1beecbec43 100755
> > --- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > +++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > @@ -15,7 +15,14 @@ get_calls()
> >  	local calls opt

> >  	[ "$name" = "rpc" ] && opt="r" || opt="n"
> > -	! tst_net_use_netns && [ "$nfs_f" != "nfs" ] && type="rhost"
> > +
> > +	if tst_net_use_netns; then
> > +		# "Make nfs stats visible in network NS" patchet
> > +		# https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> > +		tst_kvcmp -ge "6.9" && [ "$nfs_f" = "nfs" ] && type="rhost"

> Hello Petr-

> My concern with this fix is it targets v6.9 specifically, yet we
> know these fixes will appear in LTS/stable kernels as well.

Great! I see you already fixed up to 5.15. I suppose the code is really
backportable to the other still active branches (5.10, 5.4, 4.19).

We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
that it works on all kernels. As I note [1]

1) either we give up on checking the new functionality still works (if we
fallback to old behavior)
2) or we need to specify from which kernel we expect new functionality
(so far it's 5.15, I suppose it will be older).

I would prefer 2) to have new functionality always tested.
Or am I missing something obvious?

Kind regards,
Petr

[1] https://lore.kernel.org/linux-nfs/172367387549.6062.7078032983644586462@noble.neil.brown.name/

> Neil Brown suggested an alternative approach that might not depend
> on knowing the specific kernel version:

> https://lore.kernel.org/linux-nfs/172078283934.15471.13377048166707693692@noble.neil.brown.name/

> HTH


> > +	else
> > +		[ "$nfs_f" != "nfs" ] && type="rhost"
> > +	fi

> >  	if [ "$type" = "lhost" ]; then
> >  		calls="$(grep $name /proc/net/rpc/$nfs_f | cut -d' ' -f$field)"
> > -- 
> > 2.45.2

Re: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Chuck Lever III]

> On Aug 23, 2024, at 2:46 AM, Petr Vorel <pvorel@suse.cz> wrote:
> 
> Hi Chuck, Neil, all,
> 
>> On Wed, Aug 14, 2024 at 10:57:21AM +0200, Petr Vorel wrote:
>>> 6.9 moved client RPC calls to namespace in "Make nfs stats visible in
>>> network NS" patchet.
> 
>>> https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> 
>>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>>> ---
>>> Changes v1->v2:
>>> * Point out whole patchset, not just single commit
>>> * Add a comment about the patchset
> 
>>> Hi all,
> 
>>> could you please ack this so that we have fixed mainline?
> 
>>> FYI Some parts has been backported, e.g.:
>>> d47151b79e322 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
>>> to all stable/LTS: 5.4.276, 5.10.217, 5.15.159, 6.1.91, 6.6.31.
> 
>>> But most of that is not yet (but planned to be backported), e.g.
>>> 93483ac5fec62 ("nfsd: expose /proc/net/sunrpc/nfsd in net namespaces")
>>> see Chuck's patchset for 6.6
>>> https://lore.kernel.org/linux-nfs/20240812223604.32592-1-cel@kernel.org/
> 
>>> Once all kernels up to 5.4 fixed we should update the version.
> 
>>> Kind regards,
>>> Petr
> 
>>> testcases/network/nfs/nfsstat01/nfsstat01.sh | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
> 
>>> diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
>>> index c2856eff1f..1beecbec43 100755
>>> --- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
>>> +++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
>>> @@ -15,7 +15,14 @@ get_calls()
>>> local calls opt
> 
>>> [ "$name" = "rpc" ] && opt="r" || opt="n"
>>> - ! tst_net_use_netns && [ "$nfs_f" != "nfs" ] && type="rhost"
>>> +
>>> + if tst_net_use_netns; then
>>> + # "Make nfs stats visible in network NS" patchet
>>> + # https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
>>> + tst_kvcmp -ge "6.9" && [ "$nfs_f" = "nfs" ] && type="rhost"
> 
>> Hello Petr-
> 
>> My concern with this fix is it targets v6.9 specifically, yet we
>> know these fixes will appear in LTS/stable kernels as well.
> 
> Great! I see you already fixed up to 5.15. I suppose the code is really
> backportable to the other still active branches (5.10, 5.4, 4.19).

I plan to work on backporting to v5.10 next week.

I've been asked to look at v5.4, but I'm not sure how difficult
that will be because it's missing a lot of NFSD patches. I will
look into that in a couple of weeks.

I'm very likely to punt on v4.19, though Oracle's stable backport
team might try to tackle it at some point. (pun intended)


> We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> that it works on all kernels. As I note [1]
> 
> 1) either we give up on checking the new functionality still works (if we
> fallback to old behavior)
> 2) or we need to specify from which kernel we expect new functionality
> (so far it's 5.15, I suppose it will be older).
> 
> I would prefer 2) to have new functionality always tested.
> Or am I missing something obvious?

I don't quite understand the question.

The "old functionality" of reporting these statistics globally
is broken, but we're stuck with it in the older kernels. I guess
you might want to confirm that, for a given recent kernel
release, the stats are actually per-namespace -- that's what we
expect in fixed kernels. Is that what you mean?


> Kind regards,
> Petr
> 
> [1] https://lore.kernel.org/linux-nfs/172367387549.6062.7078032983644586462@noble.neil.brown.name/
> 
>> Neil Brown suggested an alternative approach that might not depend
>> on knowing the specific kernel version:
> 
>> https://lore.kernel.org/linux-nfs/172078283934.15471.13377048166707693692@noble.neil.brown.name/
> 
>> HTH
> 
> 
>>> + else
>>> + [ "$nfs_f" != "nfs" ] && type="rhost"
>>> + fi
> 
>>> if [ "$type" = "lhost" ]; then
>>> calls="$(grep $name /proc/net/rpc/$nfs_f | cut -d' ' -f$field)"
>>> -- 
>>> 2.45.2

--
Chuck Lever

Re: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Petr Vorel]

> > On Aug 23, 2024, at 2:46 AM, Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Chuck, Neil, all,

> >> On Wed, Aug 14, 2024 at 10:57:21AM +0200, Petr Vorel wrote:
> >>> 6.9 moved client RPC calls to namespace in "Make nfs stats visible in
> >>> network NS" patchet.

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

> >>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> >>> ---
> >>> Changes v1->v2:
> >>> * Point out whole patchset, not just single commit
> >>> * Add a comment about the patchset

> >>> Hi all,

> >>> could you please ack this so that we have fixed mainline?

> >>> FYI Some parts has been backported, e.g.:
> >>> d47151b79e322 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
> >>> to all stable/LTS: 5.4.276, 5.10.217, 5.15.159, 6.1.91, 6.6.31.

> >>> But most of that is not yet (but planned to be backported), e.g.
> >>> 93483ac5fec62 ("nfsd: expose /proc/net/sunrpc/nfsd in net namespaces")
> >>> see Chuck's patchset for 6.6
> >>> https://lore.kernel.org/linux-nfs/20240812223604.32592-1-cel@kernel.org/

> >>> Once all kernels up to 5.4 fixed we should update the version.

> >>> Kind regards,
> >>> Petr

> >>> testcases/network/nfs/nfsstat01/nfsstat01.sh | 9 ++++++++-
> >>> 1 file changed, 8 insertions(+), 1 deletion(-)

> >>> diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> >>> index c2856eff1f..1beecbec43 100755
> >>> --- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
> >>> +++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> >>> @@ -15,7 +15,14 @@ get_calls()
> >>> local calls opt

> >>> [ "$name" = "rpc" ] && opt="r" || opt="n"
> >>> - ! tst_net_use_netns && [ "$nfs_f" != "nfs" ] && type="rhost"
> >>> +
> >>> + if tst_net_use_netns; then
> >>> + # "Make nfs stats visible in network NS" patchet
> >>> + # https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> >>> + tst_kvcmp -ge "6.9" && [ "$nfs_f" = "nfs" ] && type="rhost"

> >> Hello Petr-

> >> My concern with this fix is it targets v6.9 specifically, yet we
> >> know these fixes will appear in LTS/stable kernels as well.

> > Great! I see you already fixed up to 5.15. I suppose the code is really
> > backportable to the other still active branches (5.10, 5.4, 4.19).

> I plan to work on backporting to v5.10 next week.

> I've been asked to look at v5.4, but I'm not sure how difficult
> that will be because it's missing a lot of NFSD patches. I will
> look into that in a couple of weeks.

> I'm very likely to punt on v4.19, though Oracle's stable backport
> team might try to tackle it at some point. (pun intended)

Thanks a lot for info, we'll see what you / your Oracle backport team will
manage in the end.

> > We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> > that it works on all kernels. As I note [1]

> > 1) either we give up on checking the new functionality still works (if we
> > fallback to old behavior)
> > 2) or we need to specify from which kernel we expect new functionality
> > (so far it's 5.15, I suppose it will be older).

> > I would prefer 2) to have new functionality always tested.
> > Or am I missing something obvious?

> I don't quite understand the question.

> The "old functionality" of reporting these statistics globally
> is broken, but we're stuck with it in the older kernels. I guess
> you might want to confirm that, for a given recent kernel
> release, the stats are actually per-namespace -- that's what we
> expect in fixed kernels. Is that what you mean?

Yes. I'm just trying to say that Neil's proposal "work everywhere without
checking kernel version" will not work. I would like next week, after you send
5.10 patches to expect anything >= 5.10 will have new functionality
and update kernel version if more gets backported.

Kind regards,
Petr

> > Kind regards,
> > Petr

> > [1] https://lore.kernel.org/linux-nfs/172367387549.6062.7078032983644586462@noble.neil.brown.name/

> >> Neil Brown suggested an alternative approach that might not depend
> >> on knowing the specific kernel version:

> >> https://lore.kernel.org/linux-nfs/172078283934.15471.13377048166707693692@noble.neil.brown.name/

> >> HTH


> >>> + else
> >>> + [ "$nfs_f" != "nfs" ] && type="rhost"
> >>> + fi

> >>> if [ "$type" = "lhost" ]; then
> >>> calls="$(grep $name /proc/net/rpc/$nfs_f | cut -d' ' -f$field)"
> >>> -- 
> >>> 2.45.2

Re: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Chuck Lever]

On Fri, Aug 23, 2024 at 08:53:02PM +0200, Petr Vorel wrote:
> 
> 
> > > On Aug 23, 2024, at 2:46 AM, Petr Vorel <pvorel@suse.cz> wrote:
> 
> > > Hi Chuck, Neil, all,
> 
> > >> On Wed, Aug 14, 2024 at 10:57:21AM +0200, Petr Vorel wrote:
> > >>> 6.9 moved client RPC calls to namespace in "Make nfs stats visible in
> > >>> network NS" patchet.
> 
> > >>> https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> 
> > >>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > >>> ---
> > >>> Changes v1->v2:
> > >>> * Point out whole patchset, not just single commit
> > >>> * Add a comment about the patchset
> 
> > >>> Hi all,
> 
> > >>> could you please ack this so that we have fixed mainline?
> 
> > >>> FYI Some parts has been backported, e.g.:
> > >>> d47151b79e322 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
> > >>> to all stable/LTS: 5.4.276, 5.10.217, 5.15.159, 6.1.91, 6.6.31.
> 
> > >>> But most of that is not yet (but planned to be backported), e.g.
> > >>> 93483ac5fec62 ("nfsd: expose /proc/net/sunrpc/nfsd in net namespaces")
> > >>> see Chuck's patchset for 6.6
> > >>> https://lore.kernel.org/linux-nfs/20240812223604.32592-1-cel@kernel.org/
> 
> > >>> Once all kernels up to 5.4 fixed we should update the version.
> 
> > >>> Kind regards,
> > >>> Petr
> 
> > >>> testcases/network/nfs/nfsstat01/nfsstat01.sh | 9 ++++++++-
> > >>> 1 file changed, 8 insertions(+), 1 deletion(-)
> 
> > >>> diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > >>> index c2856eff1f..1beecbec43 100755
> > >>> --- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > >>> +++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > >>> @@ -15,7 +15,14 @@ get_calls()
> > >>> local calls opt
> 
> > >>> [ "$name" = "rpc" ] && opt="r" || opt="n"
> > >>> - ! tst_net_use_netns && [ "$nfs_f" != "nfs" ] && type="rhost"
> > >>> +
> > >>> + if tst_net_use_netns; then
> > >>> + # "Make nfs stats visible in network NS" patchet
> > >>> + # https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> > >>> + tst_kvcmp -ge "6.9" && [ "$nfs_f" = "nfs" ] && type="rhost"
> 
> > >> Hello Petr-
> 
> > >> My concern with this fix is it targets v6.9 specifically, yet we
> > >> know these fixes will appear in LTS/stable kernels as well.
> 
> > > Great! I see you already fixed up to 5.15. I suppose the code is really
> > > backportable to the other still active branches (5.10, 5.4, 4.19).
> 
> > I plan to work on backporting to v5.10 next week.
> 
> > I've been asked to look at v5.4, but I'm not sure how difficult
> > that will be because it's missing a lot of NFSD patches. I will
> > look into that in a couple of weeks.
> 
> > I'm very likely to punt on v4.19, though Oracle's stable backport
> > team might try to tackle it at some point. (pun intended)
> 
> Thanks a lot for info, we'll see what you / your Oracle backport team will
> manage in the end.
> 
> > > We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> > > that it works on all kernels. As I note [1]
> 
> > > 1) either we give up on checking the new functionality still works (if we
> > > fallback to old behavior)
> > > 2) or we need to specify from which kernel we expect new functionality
> > > (so far it's 5.15, I suppose it will be older).
> 
> > > I would prefer 2) to have new functionality always tested.
> > > Or am I missing something obvious?
> 
> > I don't quite understand the question.
> 
> > The "old functionality" of reporting these statistics globally
> > is broken, but we're stuck with it in the older kernels. I guess
> > you might want to confirm that, for a given recent kernel
> > release, the stats are actually per-namespace -- that's what we
> > expect in fixed kernels. Is that what you mean?
> 
> Yes. I'm just trying to say that Neil's proposal "work everywhere without
> checking kernel version" will not work. I would like next week, after you send
> 5.10 patches to expect anything >= 5.10 will have new functionality
> and update kernel version if more gets backported.

I wanted to be sure you were aware of Neil's suggestion, and it
sounds like it isn't workable for you. So, fair enough. I will get
to work on v5.10.y ;-)


> Kind regards,
> Petr
> 
> > > Kind regards,
> > > Petr
> 
> > > [1] https://lore.kernel.org/linux-nfs/172367387549.6062.7078032983644586462@noble.neil.brown.name/
> 
> > >> Neil Brown suggested an alternative approach that might not depend
> > >> on knowing the specific kernel version:
> 
> > >> https://lore.kernel.org/linux-nfs/172078283934.15471.13377048166707693692@noble.neil.brown.name/
> 
> > >> HTH
> 
> 
> > >>> + else
> > >>> + [ "$nfs_f" != "nfs" ] && type="rhost"
> > >>> + fi
> 
> > >>> if [ "$type" = "lhost" ]; then
> > >>> calls="$(grep $name /proc/net/rpc/$nfs_f | cut -d' ' -f$field)"
> > >>> -- 
> > >>> 2.45.2

-- 
Chuck Lever

Re: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[NeilBrown]

On Fri, 23 Aug 2024, Petr Vorel wrote:
> Hi Chuck, Neil, all,
> 
> > On Wed, Aug 14, 2024 at 10:57:21AM +0200, Petr Vorel wrote:
> > > 6.9 moved client RPC calls to namespace in "Make nfs stats visible in
> > > network NS" patchet.
> 
> > > https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> 
> > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > ---
> > > Changes v1->v2:
> > > * Point out whole patchset, not just single commit
> > > * Add a comment about the patchset
> 
> > > Hi all,
> 
> > > could you please ack this so that we have fixed mainline?
> 
> > > FYI Some parts has been backported, e.g.:
> > > d47151b79e322 ("nfs: expose /proc/net/sunrpc/nfs in net namespaces")
> > > to all stable/LTS: 5.4.276, 5.10.217, 5.15.159, 6.1.91, 6.6.31.
> 
> > > But most of that is not yet (but planned to be backported), e.g.
> > > 93483ac5fec62 ("nfsd: expose /proc/net/sunrpc/nfsd in net namespaces")
> > > see Chuck's patchset for 6.6
> > > https://lore.kernel.org/linux-nfs/20240812223604.32592-1-cel@kernel.org/
> 
> > > Once all kernels up to 5.4 fixed we should update the version.
> 
> > > Kind regards,
> > > Petr
> 
> > >  testcases/network/nfs/nfsstat01/nfsstat01.sh | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> > > diff --git a/testcases/network/nfs/nfsstat01/nfsstat01.sh b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > > index c2856eff1f..1beecbec43 100755
> > > --- a/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > > +++ b/testcases/network/nfs/nfsstat01/nfsstat01.sh
> > > @@ -15,7 +15,14 @@ get_calls()
> > >  	local calls opt
> 
> > >  	[ "$name" = "rpc" ] && opt="r" || opt="n"
> > > -	! tst_net_use_netns && [ "$nfs_f" != "nfs" ] && type="rhost"
> > > +
> > > +	if tst_net_use_netns; then
> > > +		# "Make nfs stats visible in network NS" patchet
> > > +		# https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
> > > +		tst_kvcmp -ge "6.9" && [ "$nfs_f" = "nfs" ] && type="rhost"
> 
> > Hello Petr-
> 
> > My concern with this fix is it targets v6.9 specifically, yet we
> > know these fixes will appear in LTS/stable kernels as well.
> 
> Great! I see you already fixed up to 5.15. I suppose the code is really
> backportable to the other still active branches (5.10, 5.4, 4.19).
> 
> We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> that it works on all kernels. As I note [1]
> 
> 1) either we give up on checking the new functionality still works (if we
> fallback to old behavior)

I don't understand.  What exactly do you mean by "the new
functionality".
As I understand it there is no new functionality.  All there was was and
information leak between network namespaces, and we stopped the leak.
Do you consider that to be new functionality?

> 2) or we need to specify from which kernel we expect new functionality
> (so far it's 5.15, I suppose it will be older).
> 
> I would prefer 2) to have new functionality always tested.
> Or am I missing something obvious?

As I understand it the primary purpose of the test is to confirm that
when an NFS request is made, the client sees an increase the the count
of RPC requests in the client statistics.  and the server sees an
increase in the count of RPC requests in the server statistics.
That test, if performed correctly, should always work.

Is there something else you want to test?
If you want to test that the server DOESN'T see and increase in the
CLIENT statistics, then that is a valid thing to test and it won't work
on early kernels.  I think we only need to test that for kernels since
the fix landed in mainline.

I'm sure one of us is missing something obvious because I am CONFIDENT
that a test for correct functionality can be written to work on all
kernels.  We didn't add any new functionality and didn't break any old
functionality.  We just fixed a bug.

NeilBrown


> 
> Kind regards,
> Petr
> 
> [1] https://lore.kernel.org/linux-nfs/172367387549.6062.7078032983644586462@noble.neil.brown.name/
> 
> > Neil Brown suggested an alternative approach that might not depend
> > on knowing the specific kernel version:
> 
> > https://lore.kernel.org/linux-nfs/172078283934.15471.13377048166707693692@noble.neil.brown.name/
> 
> > HTH
> 
> 
> > > +	else
> > > +		[ "$nfs_f" != "nfs" ] && type="rhost"
> > > +	fi
> 
> > >  	if [ "$type" = "lhost" ]; then
> > >  		calls="$(grep $name /proc/net/rpc/$nfs_f | cut -d' ' -f$field)"
> > > -- 
> > > 2.45.2
> 

Re: [LTP] [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Martin Doucha]

On 23. 08. 24 23:59, NeilBrown wrote:
> On Fri, 23 Aug 2024, Petr Vorel wrote:
>> We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
>> that it works on all kernels. As I note [1]
>>
>> 1) either we give up on checking the new functionality still works (if we
>> fallback to old behavior)
> 
> I don't understand.  What exactly do you mean by "the new
> functionality".
> As I understand it there is no new functionality.  All there was was and
> information leak between network namespaces, and we stopped the leak.
> Do you consider that to be new functionality?

The new functionality is that the patches add a new file to network 
namespaces: /proc/net/rpc/nfs. This file did not exist outside the root 
network namespace at least on some of the kernels where we still need to 
run this test. So the question is: How aggressively do we want to 
enforce backporting of these NFS patches into distros with older kernels?

We have 3 options how to fix the test depending on the answer:
1) Don't enforce at all. We'll check whether /proc/net/rpc/nfs exists in 
the client namespace and read it only if it does. Otherwise we'll fall 
back on the global file.
2) Enforce aggressively. We'll hardcode a minimal kernel version into 
the test (e.g. v5.4) and if the procfile doesn't exist on any newer 
kernel, it's a bug.
3) Enforce on new kernels only. We'll set a hard requirement for kernel 
v6.9+ as in option 2) and check for existence of the procfile on any 
older kernels as in option 1).

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

Re: [LTP] [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Petr Vorel]

Hi all,

> On 23. 08. 24 23:59, NeilBrown wrote:
> > On Fri, 23 Aug 2024, Petr Vorel wrote:
> > > We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> > > that it works on all kernels. As I note [1]

> > > 1) either we give up on checking the new functionality still works (if we
> > > fallback to old behavior)

> > I don't understand.  What exactly do you mean by "the new
> > functionality".
> > As I understand it there is no new functionality.  All there was was and
> > information leak between network namespaces, and we stopped the leak.
> > Do you consider that to be new functionality?

Thanks Martin for jumping in. I hoped I was clear, but obviously not.

Following are the questions for kernel maintainers and developers. I put my
opinion, but it's really up to you what you want to have tested.

> The new functionality is that the patches add a new file to network
> namespaces: /proc/net/rpc/nfs. This file did not exist outside the root
> network namespace at least on some of the kernels where we still need to run
> this test. So the question is: How aggressively do we want to enforce
> backporting of these NFS patches into distros with older kernels?

> We have 3 options how to fix the test depending on the answer:
> 1) Don't enforce at all. We'll check whether /proc/net/rpc/nfs exists in the
> client namespace and read it only if it does. Otherwise we'll fall back on
> the global file.

1) is IMHO the worst case because it's not testing patch gets reverted.

> 2) Enforce aggressively. We'll hardcode a minimal kernel version into the
> test (e.g. v5.4) and if the procfile doesn't exist on any newer kernel, it's
> a bug.

I would prefer 2), which is the usual LTP approach (do not hide bugs, we even
fail on upstream kernel WONTFIX [1], why we should refuse the policy here?).

Whichever older LTS upstream kernel gets fixed would drive the line where new
functionality is requested (currently v5.14, I suppose at least 5.10 will also
be fixed). LTP also has a way to specify enterprise distro kernel version if
older enterprise kernel also gets fixed (this should not be needed, but it'd be
possible).

> 3) Enforce on new kernels only. We'll set a hard requirement for kernel
> v6.9+ as in option 2) and check for existence of the procfile on any older
> kernels as in option 1).

Due way to specify enterprise distro kernel version and upstream kernel testing
expecting people update to the latest stable/LTS we should not worry much about
people with older kernels.

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/ustat/ustat01.c#L48-L49

Re: [LTP] [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Chuck Lever III]

> On Aug 27, 2024, at 9:22 AM, Petr Vorel <pvorel@suse.cz> wrote:
> 
> Hi all,
> 
>> On 23. 08. 24 23:59, NeilBrown wrote:
>>> On Fri, 23 Aug 2024, Petr Vorel wrote:
>>>> We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
>>>> that it works on all kernels. As I note [1]
> 
>>>> 1) either we give up on checking the new functionality still works (if we
>>>> fallback to old behavior)
> 
>>> I don't understand.  What exactly do you mean by "the new
>>> functionality".
>>> As I understand it there is no new functionality.  All there was was and
>>> information leak between network namespaces, and we stopped the leak.
>>> Do you consider that to be new functionality?
> 
> Thanks Martin for jumping in. I hoped I was clear, but obviously not.
> 
> Following are the questions for kernel maintainers and developers. I put my
> opinion, but it's really up to you what you want to have tested.
> 
>> The new functionality is that the patches add a new file to network
>> namespaces: /proc/net/rpc/nfs. This file did not exist outside the root
>> network namespace at least on some of the kernels where we still need to run
>> this test. So the question is: How aggressively do we want to enforce
>> backporting of these NFS patches into distros with older kernels?
> 
>> We have 3 options how to fix the test depending on the answer:
>> 1) Don't enforce at all. We'll check whether /proc/net/rpc/nfs exists in the
>> client namespace and read it only if it does. Otherwise we'll fall back on
>> the global file.
> 
> 1) is IMHO the worst case because it's not testing patch gets reverted.
> 
>> 2) Enforce aggressively. We'll hardcode a minimal kernel version into the
>> test (e.g. v5.4) and if the procfile doesn't exist on any newer kernel, it's
>> a bug.
> 
> I would prefer 2), which is the usual LTP approach (do not hide bugs, we even
> fail on upstream kernel WONTFIX [1], why we should refuse the policy here?).

2) makes sense to me.


> Whichever older LTS upstream kernel gets fixed would drive the line where new
> functionality is requested (currently v5.14, I suppose at least 5.10 will also
> be fixed). LTP also has a way to specify enterprise distro kernel version if
> older enterprise kernel also gets fixed (this should not be needed, but it'd be
> possible).
> 
>> 3) Enforce on new kernels only. We'll set a hard requirement for kernel
>> v6.9+ as in option 2) and check for existence of the procfile on any older
>> kernels as in option 1).
> 
> Due way to specify enterprise distro kernel version and upstream kernel testing
> expecting people update to the latest stable/LTS we should not worry much about
> people with older kernels.
> 
> Kind regards,
> Petr
> 
> [1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/ustat/ustat01.c#L48-L49


--
Chuck Lever

Re: [LTP] [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Petr Vorel]

Hi all,

> > On Aug 27, 2024, at 9:22 AM, Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> >> On 23. 08. 24 23:59, NeilBrown wrote:
> >>> On Fri, 23 Aug 2024, Petr Vorel wrote:
> >>>> We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> >>>> that it works on all kernels. As I note [1]

> >>>> 1) either we give up on checking the new functionality still works (if we
> >>>> fallback to old behavior)

> >>> I don't understand.  What exactly do you mean by "the new
> >>> functionality".
> >>> As I understand it there is no new functionality.  All there was was and
> >>> information leak between network namespaces, and we stopped the leak.
> >>> Do you consider that to be new functionality?

> > Thanks Martin for jumping in. I hoped I was clear, but obviously not.

> > Following are the questions for kernel maintainers and developers. I put my
> > opinion, but it's really up to you what you want to have tested.

> >> The new functionality is that the patches add a new file to network
> >> namespaces: /proc/net/rpc/nfs. This file did not exist outside the root
> >> network namespace at least on some of the kernels where we still need to run
> >> this test. So the question is: How aggressively do we want to enforce
> >> backporting of these NFS patches into distros with older kernels?

> >> We have 3 options how to fix the test depending on the answer:
> >> 1) Don't enforce at all. We'll check whether /proc/net/rpc/nfs exists in the
> >> client namespace and read it only if it does. Otherwise we'll fall back on
> >> the global file.

> > 1) is IMHO the worst case because it's not testing patch gets reverted.

> >> 2) Enforce aggressively. We'll hardcode a minimal kernel version into the
> >> test (e.g. v5.4) and if the procfile doesn't exist on any newer kernel, it's
> >> a bug.

> > I would prefer 2), which is the usual LTP approach (do not hide bugs, we even
> > fail on upstream kernel WONTFIX [1], why we should refuse the policy here?).

> 2) makes sense to me.

Thanks for your opinion. I'll send another version (+ still wait for others input).

Kind regards,
Petr

> > Whichever older LTS upstream kernel gets fixed would drive the line where new
> > functionality is requested (currently v5.14, I suppose at least 5.10 will also
> > be fixed). LTP also has a way to specify enterprise distro kernel version if
> > older enterprise kernel also gets fixed (this should not be needed, but it'd be
> > possible).

> >> 3) Enforce on new kernels only. We'll set a hard requirement for kernel
> >> v6.9+ as in option 2) and check for existence of the procfile on any older
> >> kernels as in option 1).

> > Due way to specify enterprise distro kernel version and upstream kernel testing
> > expecting people update to the latest stable/LTS we should not worry much about
> > people with older kernels.

> > Kind regards,
> > Petr

> > [1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/ustat/ustat01.c#L48-L49

Re: [LTP] [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[NeilBrown]

On Tue, 27 Aug 2024, Martin Doucha wrote:
> On 23. 08. 24 23:59, NeilBrown wrote:
> > On Fri, 23 Aug 2024, Petr Vorel wrote:
> >> We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> >> that it works on all kernels. As I note [1]
> >>
> >> 1) either we give up on checking the new functionality still works (if we
> >> fallback to old behavior)
> > 
> > I don't understand.  What exactly do you mean by "the new
> > functionality".
> > As I understand it there is no new functionality.  All there was was and
> > information leak between network namespaces, and we stopped the leak.
> > Do you consider that to be new functionality?
> 
> The new functionality is that the patches add a new file to network 
> namespaces: /proc/net/rpc/nfs. This file did not exist outside the root 
> network namespace at least on some of the kernels where we still need to 
> run this test. So the question is: How aggressively do we want to 
> enforce backporting of these NFS patches into distros with older kernels?

Thanks for explaining that.  I had assumed that the the file was always
visible from all name spaces, but before the fix every namespace saw the
same file.  Clearly I was wrong.

> 
> We have 3 options how to fix the test depending on the answer:
> 1) Don't enforce at all. We'll check whether /proc/net/rpc/nfs exists in 
> the client namespace and read it only if it does. Otherwise we'll fall 
> back on the global file.
> 2) Enforce aggressively. We'll hardcode a minimal kernel version into 
> the test (e.g. v5.4) and if the procfile doesn't exist on any newer 
> kernel, it's a bug.
> 3) Enforce on new kernels only. We'll set a hard requirement for kernel 
> v6.9+ as in option 2) and check for existence of the procfile on any 
> older kernels as in option 1).

I think there are two totally separate questions here.
1/ How to fix the existing test to work on new and old kernels.  The
  existing test checked that the rpc count increased when NFS traffic
  happened.  I think 1 is the correct fix.  I don't think the test
  should check kernel version.

2/ We have discovered a bug and want to add a test to guard against
  regression.  This should be a new test.  That test can simply check if
  the given file exist in a non-init namespace.  I have no particular
  opinion about testing kernel versions.  A credible approach would be
  to choose the oldest kernel which it still maintained at the time that
  that bug was discovered.  Or maybe create a list of kernel versions
  where were maintained at that time and only run the test on versions
  in that list, or after the last in the list.

I really think there is value in having two different tests because we
are testing two different things.

Thanks,
NeilBrown


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

Re: [LTP] [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[Petr Vorel]

Hi Neil,

> On Tue, 27 Aug 2024, Martin Doucha wrote:
> > On 23. 08. 24 23:59, NeilBrown wrote:
> > > On Fri, 23 Aug 2024, Petr Vorel wrote:
> > >> We discussed in v1 how to fix tests.  Neil suggested to fix the test the way so
> > >> that it works on all kernels. As I note [1]

> > >> 1) either we give up on checking the new functionality still works (if we
> > >> fallback to old behavior)

> > > I don't understand.  What exactly do you mean by "the new
> > > functionality".
> > > As I understand it there is no new functionality.  All there was was and
> > > information leak between network namespaces, and we stopped the leak.
> > > Do you consider that to be new functionality?

> > The new functionality is that the patches add a new file to network 
> > namespaces: /proc/net/rpc/nfs. This file did not exist outside the root 
> > network namespace at least on some of the kernels where we still need to 
> > run this test. So the question is: How aggressively do we want to 
> > enforce backporting of these NFS patches into distros with older kernels?

> Thanks for explaining that.  I had assumed that the the file was always
> visible from all name spaces, but before the fix every namespace saw the
> same file.  Clearly I was wrong.


> > We have 3 options how to fix the test depending on the answer:
> > 1) Don't enforce at all. We'll check whether /proc/net/rpc/nfs exists in 
> > the client namespace and read it only if it does. Otherwise we'll fall 
> > back on the global file.
> > 2) Enforce aggressively. We'll hardcode a minimal kernel version into 
> > the test (e.g. v5.4) and if the procfile doesn't exist on any newer 
> > kernel, it's a bug.
> > 3) Enforce on new kernels only. We'll set a hard requirement for kernel 
> > v6.9+ as in option 2) and check for existence of the procfile on any 
> > older kernels as in option 1).

> I think there are two totally separate questions here.
> 1/ How to fix the existing test to work on new and old kernels.  The
>   existing test checked that the rpc count increased when NFS traffic
>   happened.  I think 1 is the correct fix.  I don't think the test
>   should check kernel version.

> 2/ We have discovered a bug and want to add a test to guard against
>   regression.  This should be a new test.  That test can simply check if
>   the given file exist in a non-init namespace.  I have no particular
>   opinion about testing kernel versions.  A credible approach would be
>   to choose the oldest kernel which it still maintained at the time that
>   that bug was discovered.  Or maybe create a list of kernel versions
>   where were maintained at that time and only run the test on versions
>   in that list, or after the last in the list.

> I really think there is value in having two different tests because we
> are testing two different things.

IMHO this is 3), just split in 2 tests (maybe more obvious for a reviewer).
Sure, we can be explicit and split it into 2 tests.

Regards the version, I would really either draw the line for new change for 6.9
or whatever stable will be the last which gets that. I mean, if it's e.g. 5.14,
this test should fail on some old e.g. unsupported 5.15 some companies may test.
When we in LTP test if fix is still working (no regression), usually the same
tests is used to verify if the fix was applied.

Kind regards,
Petr

> Thanks,
> NeilBrown



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

Re: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree.
Subject: [PATCH v2 1/1] nfsstat01: Update client RPC calls for kernel 6.9
Link: https://lore.kernel.org/stable/20240814085721.518800-1-pvorel%40suse.cz

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki