Re: [PATCH net-next v2 2/5] net/tcp-ao: Use crypto library API instead of crypto_ahash

[David Laight <david.laight.linux@gmail.com>]

On Tue, 28 Apr 2026 08:34:47 +0200
"Ard Biesheuvel" <ardb@kernel.org> wrote:

> On Tue, 28 Apr 2026, at 03:24, David Laight wrote:
> > On Mon, 27 Apr 2026 10:27:24 -0700
> > Eric Biggers <ebiggers@kernel.org> wrote:
> >  
> >> Currently the kernel's TCP-AO implementation does the MAC and KDF
> >> computations using the crypto_ahash API.  This API is inefficient and
> >> difficult to use, and it has required extensive workarounds in the form
> >> of per-CPU preallocated objects (tcp_sigpool) to work at all.
> >> 
> >> Let's use lib/crypto/ instead.  This means switching to straightforward
> >> stack-allocated structures, virtually addressed buffers, and direct
> >> function calls.  It also means removing quite a bit of error handling.
> >> This makes TCP-AO quite a bit faster.
> >> 
> >> This also enables many additional cleanups, which later commits will
> >> handle: removing tcp-sigpool, removing support for crypto_tfm cloning,
> >> removing more error handling, and replacing more dynamically-allocated
> >> buffers with stack buffers based on the now-statically-known limits.
> >> 
> >> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> >> Signed-off-by: Eric Biggers <ebiggers@kernel.org>  
> > ...  
> >> @@ -344,33 +444,26 @@ static int tcp_v4_ao_calc_key(struct tcp_ao_key *mkt, u8 *key,
> >>  	struct kdf_input_block {
> >>  		u8                      counter;
> >>  		u8                      label[6];
> >>  		struct tcp4_ao_context	ctx;
> >>  		__be16                  outlen;
> >> -	} __packed * tmp;  
> >
> > That looks a bit horrid.
> > I also had a feeling that the compiler sometimes rejects non-packed structures
> > inside packed ones.
> > Perhaps nest the whole thing inside another structure that has an initial
> > u8 pad and is marked __packed __aligned(4).
> > Then the assignments to the fields of 'ctx' will be known to be aligned
> > even when tcp4_ao_context is also __packed.
> >  
> 
> Agree with Eric that this has no bearing on this patch,

true - just the in the same code.

> but I'm not sure
> I see the problem here. 'ctx' will not be packed, and appear misaligned
> in struct kdf_input_block, but that would only matter if the address of
> the ctx field were taken and passed to a function taking a pointer to
> struct tcp4_ao_context (which would expect it to appear naturally
> aligned).
> 
> Having a feeling about what the compiler sometimes rejects is not
> actionable feedback - could you be more specific about which problem
> you think needs to be solved here? Are you concerned about unaligned
> accesses when populating the struct?

(It was 2am and the side effects of a cold were stopping me sleeping...)

I tend to double-check __packed because it gets misused in places
where you really want the compiler to error implicit padding rather
than generate expensive misaligned access code.

But I am sure I remember some build warning that needed __packed added
to the definition of a structure embedded in a __packed structure.
I don't think it was only the arm OABI (which pads structures to 2 bytes).
Historically this has never mattered (even the 'address of packed member'
error is moderately recent - well sometime in the last 20 years).

In this case (and the ipv6 code) 'struct tcp4_ao_context' can just be
marked __packed.
Or, since this is the only place it is used, possibly just inlined
into 'struct kdf_input_block' - which may not even need to be named.

	David