Hi Herbert,

On 2/20/23 09:41, Herbert Xu wrote:

On Wed, Feb 15, 2023 at 06:33:15PM +0000, Dmitry Safonov wrote:

quoted

TCP-AO similarly to TCP-MD5 needs to allocate tfms on a slow-path, which
is setsockopt() and use crypto ahash requests on fast paths, which are
RX/TX softirqs. It as well needs a temporary/scratch buffer for
preparing the hashing request.

Extend tcp_md5sig_pool to support other hashing algorithms than MD5.
Move it in a separate file.

This patch was previously submitted as more generic crypto_pool [1],
but Herbert nacked making it generic crypto API. His view is that crypto
requests should be atomically allocated on fast-paths. So, in this
version I don't move this pool anywhere outside TCP, only extending it
for TCP-AO use-case. It can be converted once there will be per-request
hashing crypto keys.

[1]: https://lore.kernel.org/all/20230118214111.394416-1-dima@arista.com/T/#u (local)
Signed-off-by: Dmitry Safonov <redacted>
---
 include/net/tcp.h        |  48 ++++--
 net/ipv4/Kconfig         |   4 +
 net/ipv4/Makefile        |   1 +
 net/ipv4/tcp.c           | 103 +++---------
 net/ipv4/tcp_ipv4.c      |  97 +++++++-----
 net/ipv4/tcp_minisocks.c |  21 ++-
 net/ipv4/tcp_sigpool.c   | 333 +++++++++++++++++++++++++++++++++++++++
 net/ipv6/tcp_ipv6.c      |  58 +++----
 8 files changed, 493 insertions(+), 172 deletions(-)
 create mode 100644 net/ipv4/tcp_sigpool.c

Please wait for my per-request hash work before you resubmit this.

Do you have a timeline for that work?
And if you don't mind I keep re-iterating, as I'm trying to address TCP
reviews and missed functionality/selftests.

Once that's in place all you need is a single tfm for the whole
system.

Unfortunately, not really: RFC5926 prescribes the mandatory-to-implement
MAC algorithms for TCP-AO: HMAC-SHA-1-96 and AES-128-CMAC-96. But since
the RFC was written sha1 is now more eligible for attacks as well as
RFC5925 has:

The option should support algorithms other than the default, to
allow agility over time.
TCP-AO allows any desired algorithm, subject to TCP option
space limitations, as noted in Section 2.2. The use of a set
of MKTs allows separate connections to use different
algorithms, both for the MAC and the KDF.

As well as from a customer's request we need to support more than two
required algorithms. So, this implementation let the user choose the
algorithm that is supported by crypto/ layer (more or less like xfrm does).

Which means, that it still has to support multiple tfms. I guess that
pool of tfms can be converted to use per-request keys quite easily.

As to request pools what exactly is the point of that? Just kmalloc
them on demand.

1) before your per-request key patches - it's not possible.
2) after your patches - my question would be: "is it better to
kmalloc(GFP_ATOMIC) in RX/TX for every signed TCP segment, rather than
pre-allocate it?"

The price of (2) may just well be negligible, but worth measuring before
switching.

Thanks,
          Dmitry