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)"
On Wed, Aug 14, 2024 at 10:57:21AM +0200, Petr Vorel wrote:
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@no...
HTH
Hi Chuck, Neil, all,
https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
Hi all,
could you please ack this so that we have fixed mainline?
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(-)
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@nobl...
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@no...
HTH
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)
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?
-- Chuck Lever
On Aug 23, 2024, at 2:46 AM, Petr Vorel pvorel@suse.cz wrote:
Hi Chuck, Neil, all,
https://lore.kernel.org/linux-nfs/cover.1708026931.git.josef@toxicpanda.com/
Hi all,
could you please ack this so that we have fixed mainline?
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(-)
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'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]
I would prefer 2) to have new functionality always tested. Or am I missing something obvious?
I don't quite understand the question.
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@nobl...
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@no...
HTH
On Fri, 23 Aug 2024, Petr Vorel wrote:
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?
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
On 23. 08. 24 23:59, NeilBrown wrote:
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).
Hi all,
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.
1) is IMHO the worst case because it's not testing patch gets reverted.
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).
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/sysca...
Hi all,
On Aug 27, 2024, at 9:22 AM, Petr Vorel pvorel@suse.cz wrote:
Hi all,
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.
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?).
Thanks for your opinion. I'll send another version (+ still wait for others input).
Kind regards, Petr
Kind regards, Petr
[1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/sysca...
On Tue, 27 Aug 2024, Martin Doucha wrote:
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.
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
Hi Neil,
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
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#opti...
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
linux-stable-mirror@lists.linaro.org
-
Chuck Lever -
Chuck Lever III
-
kernel test robot -
Martin Doucha -
NeilBrown -
Petr Vorel