Re: [PATCH] gitlab: enable afalg tests in fedora system test

[Thomas Huth <thuth@redhat.com>]

On 22/10/2024 10.03, Daniel P. Berrangé wrote:
> On Tue, Oct 22, 2024 at 07:31:49AM +0200, Thomas Huth wrote:
>> On 21/10/2024 19.02, Daniel P. Berrangé wrote:
>>> The AF_ALG crypto integration for Linux is not being tested in
>>> any CI scenario. It always requires an explicit configure time
>>> flag to be passed to turn it on. The Fedora system test is
>>> arbitrarily picked as the place to test it.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>>>    .gitlab-ci.d/buildtest.yml | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> This is to detect the situation Markus found here:
>>>
>>>     https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg03040.html
>>>
>>> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
>>> index 01e8470a69..f0cbdf1992 100644
>>> --- a/.gitlab-ci.d/buildtest.yml
>>> +++ b/.gitlab-ci.d/buildtest.yml
>>> @@ -115,7 +115,7 @@ build-system-fedora:
>>>        job: amd64-fedora-container
>>>      variables:
>>>        IMAGE: fedora
>>> -    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
>>> +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs --enable-crypto-afalg
>>>        TARGETS: microblaze-softmmu mips-softmmu
>>>          xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
>>>        MAKE_CHECK_ARGS: check-build
>>
>> Is there a reason for this feature being disabled by default? Who is using
>> it if it is disabled by default? Should we maybe rather enable it by default
>> instead?
> 
> afalg isn't something you want to use in general. It performs worse than
> the userspace crypto in many cases, especially for small buffers where the
> context switch overhead kills.
> 
> It is potentially interesting if you have hardware crypto accelerator and
> your guest workload also wants virtio-crypto.
> 
> Having it as a compile time option is not ideal, but it was the least
> effort tradeoff.

Ok, makes sense, thanks for the explanation.

Reviewed-by: Thomas Huth <thuth@redhat.com>