Re: [LTP] [PATCH] crypto/af_alg0[13]: update tests for additional possible errno case

[Petr Vorel <pvorel@suse.cz>]

> On 05.11.24 11:40, Petr Vorel wrote:
> > Hi Cyril, Jan, David,

> > > kernel behaviour wrt checking invalid algorithms has changed [1] [2]
> > > updating the tests to address the additional errno case.
> > > Related discussion on the mailing list [3]

> > Looking at 57ab2160c0 ("move_pages04: remove special-casing for kernels < 4.3") [4]
> > recently reverting errnos for 4.3 d539a004dd ("move_pages04: fix zero page
> > status code for kernels >= 4.3") [5] please double check this already merged
> > change. I still believe it's a different case thus merging this is correct.
> > Also Eric is suggesting this (I should have added Suggested-by: tag for him).

> > Maybe we need some rules to clarify when we are ok with different errno and when not.


> Right.

> Regarding d539a004dd, we pretty much hid kernel bugs: behaving differently
> than expected+documented.

> If the kernel starts reporting a different errno it might be a bug: user
> space might not be prepared to handle that. Or it might be expected, because
> nobody really cares about the exact error code.

I don't remember last time anybody noticed, but sure it's important.

FYI an example where errno change was not important enough was for bind, where
commit 0fb44559ffd6 ("af_unix: move unix_mknod() out of bindlock") did some
fixes and a side effect was that errno on an attempt to bind a unix socket to
the same path twice changed from -EINVAL to -EADDRINUSE [1]. Bug which that
kernel commit fixed was way more important than changing errno [2] (and
therefore backported to all stable/LTS mainline kernels [3]).

The attempt to to fix errno [1] was just considered as not needed. But this
might be a special case, because both errnos were listed in the man page, it was
just matter of priority which of the error checks was handled first.

First attempt to fix the affected bind03.c was to detect kernel version [4],
which was wrong (some versions in between did not get the fix). In the end it
was possible to adjust test to get always expected errno regardless kernel got
0fb44559ffd6 backported or not [5].

Back to move_pages04. Jan wrote [6]:
   If people find it too noisy now for older kernels, we could add .min_kver = ""
   to the test.

I guess nothing happen, because 4.3 is way too old (the oldest mainline LTS kernel is 4.19, IMHO nobody tests 4.3 kernel with current LTP).

But having a written rules 1) when we care about errno change and when not and
what to do in both cases would be useful.

> So if a test starts failing, it's definitely concerning and needs a closer
> look.

> > I also thought there would be some rule "don't hide kernel bugs", but I can't
> > find it in the docs.

> That rule makes sense to me.

I was not clear. We follow "don't hide kernel bugs" (at least Cyril mentioned
this several times), but I haven't found if we document it somewhere in doc/
folder.

Kind regards,
Petr

[1] https://lore.kernel.org/netdev/20170630073448.GA9546@unicorn.suse.cz/
[2] https://lore.kernel.org/netdev/CAK8P3a1q32spcF445Zhw-KMXG2VwFZuMw5C1sYFk3qLXz3HB5w@mail.gmail.com/
[3] https://lore.kernel.org/netdev/20181129123650.GI3149@kroah.com/
[4] https://lore.kernel.org/ltp/20180831132453.6461-1-junchi.chen@intel.com/
[5] https://lore.kernel.org/ltp/20180911102403.7094-1-junchi.chen@intel.com/
[6] https://lore.kernel.org/ltp/CAASaF6yLa6F9Bz9Adck=Y5RJePzYmboSAizYWJP_BHmy12cQ=g@mail.gmail.com/

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