On 18/08/2021 18:44, Anton Ivanov wrote:
On 18/08/2021 17:40, Ilya Maximets wrote:
On 8/18/21 4:30 PM, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov <anton.iva...@cambridgegreys.com>
This allows to leverage the openssl implementation which can use
hardware crypto on supported platforms.
UUID generation speed is improved by ~ 12% on an AMD Ryzen with
support for AES instructions.
Signed-off-by: Anton Ivanov <anton.iva...@cambridgegreys.com>
Hi, Anton.
Thanks for working on this. The problem with this implementation
is that we're loosing unit-tests for our own AES library.
I think, we still need to test it (i.e. by tests/aes128.at).
They work for me here (Debian buster). I do not understand why they
failed in the automated tests by the robot.
Sorry, misunderstood your point.
I see your point now.
One option may be to reorganize it in the following manner.
1. Rename the original functions.
2. Have the API functions call the newly renamed ones for the non
openssl case.
3. Change the test to call the renamed ones directly instead of using
the wrappers.
This way we retain the test cases. but we do not test that we have
integrated openssl correctly.
And we
need to test that we're invoking openssl correctly in the same
testsuite. Would be great to avoid extra CI job for --disable-ssl
case.
It is a compile level choice, it will be difficult to test them at the
same time.
Few comments inline.
Best regards, Ilya Maximets.
---
lib/aes128.c | 34 ++++++++++++++++++++++++++++++++++
lib/aes128.h | 16 ++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/lib/aes128.c b/lib/aes128.c
index 98447d14b..207925b58 100644
--- a/lib/aes128.c
+++ b/lib/aes128.c
@@ -28,6 +28,39 @@
#include "util.h"
+#ifdef HAVE_OPENSSL
+
+
+
Too much empty space.
+#include <openssl/conf.h>
+#include <openssl/evp.h>
+#include <openssl/err.h>
+#include <string.h>
+#include "entropy.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(aes);
+
+void aes128_schedule(struct aes128 *aes, const uint8_t key[16])
+{
+ uint8_t iv[16];
+ aes->ctx = EVP_CIPHER_CTX_new();
+ memset(iv, 0, sizeof iv);
+ if (EVP_EncryptInit_ex(aes->ctx, EVP_aes_128_cbc(), NULL, key,
iv) != 1) {
+ VLOG_FATAL("Encryption init failed");
Would be great to have a better error message here, explaining
what happened in a bit more details.
Ugh... Openssl error handling...
OK. Fine, I will add it.
+ }
+}
+
+void aes128_encrypt(const struct aes128 *aes, const void *plain,
void *cipher)
+{
+ int len;
+ if (1 != EVP_EncryptUpdate(aes->ctx, cipher, &len, plain, 16)) {
+ VLOG_FATAL("Encryption failed");
Same here.
Ack.
+ }
+}
+
+#else
+
static const uint32_t Te0[256] = {
0xc66363a5U, 0xf87c7c84U, 0xee777799U, 0xf67b7b8dU,
0xfff2f20dU, 0xd66b6bbdU, 0xde6f6fb1U, 0x91c5c554U,
@@ -507,3 +540,4 @@ aes128_encrypt(const struct aes128 *aes, const
void *input_, void *output_)
^ rk[3]);
put_u32(output + 12, s3);
}
+#endif
diff --git a/lib/aes128.h b/lib/aes128.h
index f0f55d7cf..efa71c764 100644
--- a/lib/aes128.h
+++ b/lib/aes128.h
@@ -25,12 +25,28 @@
#ifndef AES128_H
#define AES128_H
+#include <config.h>
Header files should not include config.h.
We need to know if OPENSSL is used or not because the contents of the
aes structure depend on it.
On the other hand, IIRC aes is opaque to users, so it may work if it
is not declared explicitly. I will try that tomorrow.
#include <stdint.h>
+#ifdef HAVE_OPENSSL
+
+#include <openssl/conf.h>
+#include <openssl/evp.h>
+#include <openssl/err.h>
+#include <string.h>
+
+struct aes128 {
+ EVP_CIPHER_CTX *ctx;
+};
+
+#else
+
struct aes128 {
uint32_t rk[128/8 + 28];
};
+#endif
+
void aes128_schedule(struct aes128 *, const uint8_t key[16]);
void aes128_encrypt(const struct aes128 *, const void *, void *);
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
