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@xxxxxxx > > SW Quality Engineer > > SUSE LINUX, s.r.o. > > CORSO IIa > > Krizikova 148/34 > > 186 00 Prague 8 > > Czech Republic
