netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Scott Dial <scott@scottdial.com>
To: Ryan Cox <ryan_cox@byu.edu>,
	netdev@vger.kernel.org, davem@davemloft.net, sd@queasysnail.net
Cc: Antoine Tenart <antoine.tenart@bootlin.com>
Subject: Re: Severe performance regression in "net: macsec: preserve ingress frame ordering"
Date: Thu, 6 Aug 2020 23:48:16 -0400	[thread overview]
Message-ID: <a335c8eb-0450-1274-d1bf-3908dcd9b251@scottdial.com> (raw)
In-Reply-To: <1b0cec71-d084-8153-2ba4-72ce71abeb65@byu.edu>

On 8/6/2020 5:11 PM, Ryan Cox wrote:
> With 5.7 I get:
> * 9.90 Gb/s with no macsec at all
> * 1.80 Gb/s with macsec WITHOUT encryption
> * 1.00 Gb/s (sometimes, but often less) with macsec WITH encryption
> 
> With 5.7 but with ab046a5d4be4c90a3952a0eae75617b49c0cb01b reverted, I get:
> * 9.90 Gb/s with no macsec at all
> * 7.33 Gb/s with macsec WITHOUT encryption
> * 9.83 Gb/s with macsec WITH encryption
> 
> On tests where performance is bad (including macsec without encryption),
> iperf3 is at 100% CPU usage.  I was able to run it under `perf record`on
> iperf3 in a number of the tests but, unfortunately, I have had trouble
> compiling perf for my own 5.7 compilations (definitely PEBKAC).  If it
> would be useful I can work on fixing the perf compilation issues.

For certain, you are measuring the difference between AES-NI doing
gcm(aes) and gcm_base(ctr(aes-aesni),ghash-generic). Specifically, the
hotspot is ghash-generic's implementation of ghash_update() function.
I appreciate your testing because I was limited in my ability to test
beyond 1Gb/s.

The aes-aesni driver is smart enough to use the FPU if it's not busy and
fallback to the CPU otherwise. Unfortunately, the ghash-clmulni driver
does not have that kind of logic in it and only provides an async version,
so we are forced to use the ghash-generic implementation, which is a pure
CPU implementation. The ideal would be for aesni_intel to provide a
synchronous version of gcm(aes) that fell back to the CPU if the FPU is
busy.
I don't know if the crypto maintainers would be open to such a change, but
if the choice was between reverting and patching the crypto code, then I
would work on patching the crypto code.

In any case, you didn't report how many packets arrived out of order, which
was the issue being addressed by my change. It would be helpful to get
the output of "ip -s macsec show" and specifically the InPktsDelayed
counter. Did iperf3 report out-of-order packets with the patch reverted?
Otherwise, if this is the only process running on your test servers,
then you may not be generating any contention for the FPU, which is the
source of the out-of-order issue. Maybe you could run prime95 to busy
the FPU to see the issue that I was seeing.

I have a product that is a secure router with a half-dozen MACsec
interfaces, boots from a LUKS-encrypted disk, and has a number of TLS
control and status interfaces for local devices attached to product.
Without this patch, the system was completely unusable due to the
out-of-order issue causing TCP retries and UDP out-of-order issues. I
have not seen any examples of this MACsec driver in the wild, so I
assumed nobody had noticed the out-of-order issue because of synthetic
testing.
-- 
Scott Dial
scott@scottdial.com

  reply	other threads:[~2020-08-07  3:54 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-06 21:11 Severe performance regression in "net: macsec: preserve ingress frame ordering" Ryan Cox
2020-08-07  3:48 ` Scott Dial [this message]
2020-08-07 23:21   ` Ryan Cox
2020-08-10 13:34   ` Sabrina Dubroca
2020-08-10 16:09     ` Scott Dial
2020-08-12 10:04       ` Sabrina Dubroca
2020-08-12 10:45         ` Van Leeuwen, Pascal
2020-08-12 12:42           ` Andrew Lunn
2020-08-24  9:07             ` Van Leeuwen, Pascal
2020-08-24 13:01               ` Andrew Lunn
2020-08-25 13:09                 ` Van Leeuwen, Pascal
2020-08-25 13:33                   ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a335c8eb-0450-1274-d1bf-3908dcd9b251@scottdial.com \
    --to=scott@scottdial.com \
    --cc=antoine.tenart@bootlin.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=ryan_cox@byu.edu \
    --cc=sd@queasysnail.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).