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

[David Hildenbrand]

On 05.11.24 19:05, Petr Vorel wrote:
>> 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).

Yes, likely.

> 
> 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.

Agreed.

> 
>> 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.

Ah, yes that's what I meant: this unofficial rule makes sense to me :)

-- 
Cheers,

David / dhildenb