lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 27 Apr 2020 19:14:15 -0400 From: Scott Dial <scott@...ttdial.com> To: David Miller <davem@...emloft.net> Cc: netdev@...r.kernel.org Subject: Re: [PATCH net] net: macsec: preserve ingress frame ordering On 4/27/2020 2:12 PM, David Miller wrote: > It's a real shame that instead of somehow fixing the most performant > setup to be actually usable, we are just throwing our hands up in > the air and simply avoiding to use it. > > I feel _really_ bad for the person trying to figure out why they > aren't getting the macsec performance they expect, scratching their > heads for hours trying to figure out why the AES-NI x86 code isn't > being used, and after days finding a commit like this. Like most things, there are competing interests. I was the person scratching my head for hours trying to figure out why my packets were arriving out of of order causing packet drops and breaking UDP streams. In the end, the only solution that I could ship without modifying the kernel was blacklisting aesni_intel and ghash_clmulni_intel. To be clear, the sync version of gcm(aes) on an AES-NI will still use AES-NI for the block cipher if the FPU is available (and otherwise falls back to non-FPU code). Unfortunately, there is not a synchronous version of gcm(aes) implemented by AES-NI, but that would be a logical extension of the pattern to provide maximum performance and correctness. With regards to correctness, you can see that same decision being made in the mac80211 code for handling the AES-GCMP and BIP-GMAC encryption modes. I don't know if the crypto maintainers would entertain adding a sync aes(gcm) implementation to aesni_intel, but it seems like it would be straightforward to implement. Otherwise, I entertained a module option (simple and covers my use case) or even an attribute on the RXSA (a considerably more invasive change). However, I didn't think this would be that controversial since there are many places in the kernel that use AEAD algorithms in synchronous mode for the same reason, and therefore do not get the complete benefits of AES-NI acceleration. >> - tfm = crypto_alloc_aead("gcm(aes)", 0, 0); >> + /* Pick a sync gcm(aes) cipher to ensure order is preserved. */ >> + tfm = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC); > > How does this mask argument passed to crypto_alloc_aead() work? You > are specifying async, does this mean you're asking for async or > non-async? See crypto_requires_sync(type, mask) in include/crypto/algapi.h for the logic of evaluating the mask. In short, the mask is what bits are not allowed to be set, so this masks out any async algorithms. Thanks for taking the time to evaluate my change! -- Scott Dial scott@...ttdial.com
Powered by blists - more mailing lists