Re: [PATCH 6.1 00/42] 6.1.15-rc1 review

[Greg Kroah-Hartman]

On Fri, Mar 03, 2023 at 11:56:26AM +0100, Matthieu Baerts wrote:
> Hi Greg,
> 
> 3 Mar 2023 11:26:46 Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>:
> 
> > On Fri, Mar 03, 2023 at 10:47:33AM +0100, Matthieu Baerts wrote:
> >> Hi Greg, Naresh, Paolo,
> >>
> >> Thank you for the new version and for having reported the issue and running MPTCP selftests!
> >>
> >> 3 Mar 2023 10:23:06 Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>:
> >>
> >>> On Fri, Mar 03, 2023 at 02:34:05PM +0530, Naresh Kamboju wrote:
> >>>> On Fri, 3 Mar 2023 at 13:34, Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
> >>>>>
> >>>>> Hello,
> >>>>>
> >>>>> On Fri, 2023-03-03 at 01:32 +0530, Naresh Kamboju wrote:
> >>>>>> …
> >>>>>
> >>>>> I read the above as you are running self-tests from 6.2.1 on top of an
> >>>>> older (6.1) kernel. Is that correct?
> >>>>
> >>>> correct.
> >>>>
> >>>>> If so failures are expected;
> >>>
> >>> Shouldn't the test be able to know that "new features" are not present
> >>> and properly skip the test for when that happens?  Otherwise this feels
> >>> like a problem going forward as no one will know if this feature can be
> >>> used or not (assuming it is a new feature and not just a functional
> >>> change.)
> >>
> >> All MPTCP selftests are designed to run on the same kernel version
> >> they are attached to. This allows us to do more checks knowing they
> >> are not supposed to fail on newer kernel versions and not being
> >> skipped if there is an error when trying to use the new feature. If
> >> there are fixes, we make sure the stable team is Cc'ed. If there are
> >> API changes, it would be visible because we would need to adapt
> >> existing selftests.
> >
> > "Features" are not usually limited to specific kernel versions (think
> > about the mess that "enterprise" kernels create by backports).  And if
> > they are, running a userspace test should be able to detect if the
> > feature is present or not by the error returned to it, right?  If not,
> > then the feature is mis-designed.
> 
> Thank you for the explanation. (I didn't know these tests had to support "enterprise" kernels.)

Tests have to run anywhere.

> For features where the userspace explicitly asks to use them, that's easy. For events that are only created from a specific kernel version, that will be harder but it is maybe a sign that something else is missing on our side :)

Think about userspace, how will it even know if a feature is present or
not in the kernel it is running on?  It needs to know "I tried to use
this and it failed because of it not being there or because something
else went wrong".  Userspace has to be able to distinguish between the
two somehow, otherwise no one will be able to use your feature very
well.

So just rewrite the test to handle "not present", like we can handle
ioctls or syscalls not being present vs. "an error happened".

> >> That's how we thought we should design MPTCP selftests. Maybe we need to change this design?
> >
> > Yes, please "skip" tests for features that are just not present, do not
> > fail them.
> 
> It might take a bit of time but we will look at that. I think we don't
> even check MPTCP is available before starting the first test, we just
> assume it is there if someone explicitly starts these tests :-)

That too should probably be fixed up :)

thanks,

greg k-h