-
Notifications
You must be signed in to change notification settings - Fork 476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance issue with OpennSSL 3 and libsrtp #645
Comments
I am building libsrtp using nodejs bundled OpenSSL and I have seen a performance performance issue with node 18 & 19 which bundles OpenSSL 3.0 both on AES GCM and AES CTR modes compared to node 16 which bundles OpenSSL 1.1 using the following benchmark: https://gist.github.com/Sean-Der/7a42bd70edfe1324ccc6ab399d653c0e The performance hit is quite significative, with AES GCM being more than 2x slower and AES CTR being 1.5x slower (depending on the server types I have tested with): Server 1:
|
Hi, I will try to reproduce with your test and see. It looks like it is encrypting a very small packet so the idea is mostly to measure the overhead of setting up encryption, rather than actually performing encryption, does that sound correct? |
i can try later with bigger payloads |
Tried with different payload sizes and still seeing the performance regression on OpenSSL 3
|
The iv length is preserved inside the EVP_CIPHER_CTX so no need to set more than once. This is especially important with OpenSSL 3 where setting the iv len is a expensive operation due to param lookup code inside of OpenSSL. This should help Performance issue with OpennSSL 3 and libsrtp with cisco#645 in the case of GCM but does not fix all of the performance issues.
Seems it is highly unlikely that openssl performance issues are solved anytime soon. Would you be prone to consider a patch supporting asm gcm functions directly? (like porting the code from go: https://go.dev/src/crypto/aes/) |
@murillo128 do you mean as "built in support" like the existing AES implementation. In general I would prefer not to have to maintain and support such code but it is possible. Would need to talk with others. libsrtp/crypto/include/cipher.h Line 241 in fab73a3
|
I agree that maintaining the code is a burden, that's why i preferred to be in libsrtp itself hehe.. the srtp_replace_cipher_type seems very helpful and would allow me to implement the hw optimized gcm version for the chipsets i care to optimize, but keeping openssl backend for everything else. Thank you for the tip! |
Follow up on this, in case it's useful to someone: we ended up extracting code from OpenSSL into a custom AES-GCM backend for libsrtp. Our conclusion is that OpenSSL is designed for lengthy encryption ops, and not just with regards to the whole OSSL_PARAM API introduced in v3. The particular asm backend we were interested in was also made substantially slower for small messages in order to reduce its state size, so it could fit in the existing We considered upstreaming the backend, but given it only works for AVX-512 capable processors we decided not to. |
Hi, yes AVX-512 only is probably too specific. |
I think it is an OpenSSL issue, but crossposting it here:
openssl/openssl#20625
The text was updated successfully, but these errors were encountered: