Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
MIME-Version: 1.0
References: <20201210121627.GB28441@gondor.apana.org.au> <CAMj1kXE-+35tfO87024xB274ZVOu7HTHqDa8o-hjoxDasd8p7g@mail.gmail.com>
 <CAMj1kXH5LPib2vPgLkdzHX4gSawDSE=ij451s106_xTuT19YmA@mail.gmail.com>
 <20201215091902.GA21455@gondor.apana.org.au> <062a2258-fad4-2c6f-0054-b0f41786ff85@candelatech.com>
 <Y2sj84u/w/nOgKwx@gondor.apana.org.au> <CAMj1kXG3id6ABX=5D4H0XLmVnijHCY6whp09U5pLQr0Ftf5Gzw@mail.gmail.com>
 <6e20b593-393c-9fa1-76aa-b78951b1f2f3@candelatech.com> <CAMj1kXEqcPvb-uLvGLhue=6eME-6WhuPgoG+HgLH0EoZLE9aZA@mail.gmail.com>
 <32a44a29-c5f4-f5fa-496f-a9dc98d8209d@candelatech.com> <20231017031603.GB1907@sol.localdomain>
In-Reply-To: <20231017031603.GB1907@sol.localdomain>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Tue, 17 Oct 2023 08:43:29 +0200
Message-ID: <CAMj1kXFRpbGJ_nkomb88o3F5Eg9ghh+xTZgGgeD7wfC3uwSk0g@mail.gmail.com>
Subject: Re: [PATCH v2] crypto: aesni - add ccm(aes) algorithm implementation
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     Ben Greear <greearb@candelatech.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.