Re: [PATCH v2] crypto: aesni - add ccm(aes) algorithm implementation

[Ard Biesheuvel <ardb@kernel.org>]

On Sat, 3 Aug 2024 at 19:20, Ben Greear <greearb@candelatech.com> wrote:
>
> On 10/16/23 23:43, Ard Biesheuvel wrote:
> > On Tue, 17 Oct 2023 at 05:16, Eric Biggers <ebiggers@kernel.org> wrote:
> >>
> >> On Mon, Oct 16, 2023 at 01:50:05PM -0700, Ben Greear wrote:
> >>> On 11/12/22 06:59, Ard Biesheuvel wrote:
> >>>> On Fri, 11 Nov 2022 at 23:29, Ben Greear <greearb@candelatech.com> wrote:
> >>>>>
> >>>>> On 11/9/22 2:05 AM, Ard Biesheuvel wrote:
> >>>>>> On Wed, 9 Nov 2022 at 04:52, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >>>>>>>
> >>>>>>> On Tue, Nov 08, 2022 at 10:50:48AM -0800, Ben Greear wrote:
> >>>>>>>>
> >>>>>>>> While rebasing my patches onto 6.1-rc4, I noticed my aesni for ccm(aes) patch didn't apply cleanly,
> >>>>>>>> and I found this patch described below is applied now.  Does this upstream patch mean that aesni is already
> >>>>>>>> supported upstream now?  Or is it specific to whatever xctr is?  If so,
> >>>>>>>> any chance the patch is wanted upstream now?
> >>>>>>>
> >>>>>>> AFAICS the xctr patch has nothing to do with what you were trying
> >>>>>>> to achieve with wireless.  My objection still stands with regards
> >>>>>>> to wireless, we should patch wireless to use the async crypto
> >>>>>>> interface and not hack around it in the Crypto API.
> >>>>>>>
> >>>>>>
> >>>>>> Indeed. Those are just add/add conflicts because both patches
> >>>>>> introduce new code into the same set of files. The resolution is
> >>>>>> generally to keep both sides.
> >>>>>>
> >>>>>> As for Herbert's objection: I will note here that in the meantime,
> >>>>>> arm64 now has gotten rid of the scalar fallbacks entirely in AEAD and
> >>>>>> skipcher implementations, because those are only callable in task or
> >>>>>> softirq context, and the arm64 SIMD wrappers now disable softirq
> >>>>>> processing. This means that the condition that results in the fallback
> >>>>>> being needed can no longer occur, making the SIMD helper dead code on
> >>>>>> arm64.
> >>>>>>
> >>>>>> I suppose we might do the same thing on x86, but since the kernel mode
> >>>>>> SIMD handling is highly arch specific, you'd really need to raise this
> >>>>>> with the x86 maintainers.
> >>>>>>
> >>>>>
> >>>>> Hello Ard,
> >>>>>
> >>>>> Could you please review the attached patch to make sure I merged it properly?  My concern
> >>>>> is the cleanup section and/or some problems I might have introduced related to the similarly
> >>>>> named code that was added upstream.
> >>>>>
> >>>>
> >>>> I don't think the logic is quite right, although it rarely matter.
> >>>>
> >>>> I've pushed my version here - it invokes the static call for CTR so it
> >>>> will use the faster AVX version if the CPU supports it.
> >>>>
> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=aesni-ccm-v6.1
> >>>
> >>> Hello Ard,
> >>>
> >>> It looks like something changed again in the intel-aesni logic for 6.6 kernel.  I was able to do a small
> >>> change to the patch to get it to compile, but the kernel crashes when I bring up a wlan port in
> >>> 6.6.  When I remove the aesni patch, the station comes up without crashing.  The aesni patch worked
> >>> fine in 6.5 as far as I can tell.
> >>>
> >>> I'm attaching my slightly modified version of the patch you sent previous.  If you have time to
> >>> investigate this it would be much appreciated.
> >>>
> >>> Thanks,
> >>> Ben
> >>
> >> If this patch is useful, shouldn't it be upstreamed?
> >>
> >
> > It was rejected by Herbert on the basis that the wireless stack should
> > be converted to use the async API instead.
>
> Hello,
>
> I'm still dragging this patch along (see attached).  I notice that there was a big re-write of
> the aesni logic in 6.11.  Anyone know if this patch would need to be (or should be)
> modified to work well with the new upstream code?
>

Those changes should be mostly orthogonal, so beyond resolving lexical
conflicts, I wouldn't expect any additional work to be needed to
forward port this.