benchmark: Use benchmark from Mbed TLS #298
Conversation
|
Depends on ARMmbed/mbed-os#14756 |
Patater
force-pushed
the
vanilla-benchmark
branch
from
3f96c4d to
66053ea
Compare
It was merged. We got Travis blocked until we sort out the plan (later today). |
Patater
force-pushed
the
vanilla-benchmark
branch
from
66053ea to
3366156
Compare
|
Travis resumed. Let's get this in as soon as we can to unblock Mbed OS, we still need to update CI and test. |
There was a problem hiding this comment.
@Patater Thanks for the fix.
One comment on removing main.cpp.
Also, there's a build error with the Arm Compiler:
In file included from ../../../../benchmark.c:46:
In file included from ../../../../mbed-os/connectivity/mbedtls/./include/mbedtls/timing.h:60:
../../../../mbed-os/connectivity/mbedtls/./platform/inc/timing_alt.h:31:20: error: field has incomplete type 'struct timeval'
struct timeval start;
^
../../../../mbed-os/connectivity/mbedtls/./platform/inc/timing_alt.h:31:12: note: forward declaration of 'struct timeval'
struct timeval start;
^
1 error generated.
ninja: build stopped: subcommand failed.
ERROR: CMake invocation failed!
I'm looking into this now.
And, we'll need to update benchmark.c when we upgrade Mbed TLS in mbed-os. I wonder what's the best way to ensure this happens.
When we move examples into the mbed-os repo itself, updating Mbed TLS and associated examples will be more straightfoward (can be done in one PR) |
We previously used an older copy of benchmark.c and modified it. This made it difficult to keep up with improvements and bug fixes from Mbed TLS. For easier maintenance, use the benchmark.c application as-is (no modifications) from Mbed TLS. The version imported in this commit is from Mbed TLS v2.25.0. Update the expected log output to reflect the new benchmarking output style. The benchmark application from Mbed TLS has a dependency on MBEDTLS_TIMING_C, so add that to our application-specific Mbed TLS configuration file. This will use the alternate implementation of the Mbed TLS timing module that Mbed OS provides. As the benchmark application was designed to run on a PC, its stack usage is pretty heavy. Configure the main stack to be 32 KiB to accommodate this. Fixes ARMmbed#297
The current Mbed TLS benchmark application requires MBEDTLS_GENPRIME (RSA key generation) to be enabled in order to benchmark RSA. Mbed OS doesn't enable MBEDTLS_GENPRIME by default, so add it to our application-specific Mbed TLS configuration file. Update the expected log output to reflect the freshly enabled RSA benchmarks.
Patater
force-pushed
the
vanilla-benchmark
branch
from
3366156 to
d70ca8e
Compare
|
Deleted the old |
Sounds good. Then we'll be able to extend our import scripts to automate this. |
There was a problem hiding this comment.
@Patater LGTM. The test output matches the log file using mbedhtrun.
The Arm Compiler issue mentioned previously is fixed by ARMmbed/mbed-os#14772, but it doesn't prevent us from merging this PR first which points to the master branch of Mbed OS. Just both of them should be part of the upcoming release.
|
Can we merge this now? |
We previously used an older copy of benchmark.c and modified it. This
made it difficult to keep up with improvements and bug fixes from Mbed
TLS. For easier maintenance, use the benchmark.c application as-is (no
modifications) from Mbed TLS. The version imported in this commit is
from Mbed TLS v2.25.0.
Update the expected log output to reflect the new benchmarking output
style.
The benchmark application from Mbed TLS has a dependency on
MBEDTLS_TIMING_C, so add that to our application-specific Mbed TLS
configuration file. This will use the alternate implementation of the
Mbed TLS timing module that Mbed OS provides.
As the benchmark application was designed to run on a PC, its stack
usage is pretty heavy. Configure the main stack to be 32 KiB to
accommodate this.
Fixes #297