From fb2ed58064e8014bfab5be416a1ae9b4cc289f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Jul 2022 11:04:52 +0200 Subject: [PATCH] Add notes on steps and testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also add example/template script to check for coverage regressions. Signed-off-by: Manuel Pégourié-Gonnard --- .../psa-migration/outcome-analysis.sh | 96 +++++++++++++++++++ docs/architecture/psa-migration/strategy.md | 31 ++++++ 2 files changed, 127 insertions(+) create mode 100755 docs/architecture/psa-migration/outcome-analysis.sh diff --git a/docs/architecture/psa-migration/outcome-analysis.sh b/docs/architecture/psa-migration/outcome-analysis.sh new file mode 100755 index 0000000000..2ed3ca94b4 --- /dev/null +++ b/docs/architecture/psa-migration/outcome-analysis.sh @@ -0,0 +1,96 @@ +#!/bin/sh + +# This is only an example/template script, you should make a copy and edit it +# to suit your needs. The part that needs editing is at the top. +# +# Also, you can comment out parts that don't need to be re-done when +# re-running this script (for example "get numbers before this PR"). + +# ----- BEGIN edit this ----- +DRIVER_COMPONENT=test_psa_crypto_config_accel_hash_use_psa +reference_config () { + scripts/config.py set MBEDTLS_USE_PSA_CRYPTO + scripts/config.py unset MBEDTLS_PKCS1_V21 + scripts/config.py unset MBEDTLS_X509_RSASSA_PSS_SUPPORT + scripts/config.py unset MBEDTLS_PKCS5_C + scripts/config.py unset MBEDTLS_PKCS12_C + scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC +} +SUITES="rsa pkcs1_v15 pk pkparse pkwrite" +# ----- END edit this ----- + +set -eu + +cleanup() { + make clean + git checkout -- include/mbedtls/mbedtls_config.h include/psa/crypto_config.h +} + +record() { + export MBEDTLS_TEST_OUTCOME_FILE="$PWD/outcome-$1.csv" + rm -f $MBEDTLS_TEST_OUTCOME_FILE + make check +} + +# save current HEAD +HEAD=$(git branch --show-current) + +# get the numbers before this PR for default and full +cleanup +git checkout $(git merge-base HEAD development) +record "before-default" + +cleanup +scripts/config.py full +record "before-full" + +# get the numbers now for default and full +cleanup +git checkout $HEAD +record "after-default" + +cleanup +scripts/config.py full +record "after-full" + +# get the numbers now for driver-only and reference +cleanup +reference_config +record "reference" + +cleanup +export MBEDTLS_TEST_OUTCOME_FILE="$PWD/outcome-drivers.csv" +tests/scripts/all.sh -k test_psa_crypto_config_accel_hash_use_psa + +# analysis + +compare_suite () { + ref="outcome-$1.csv" + new="outcome-$2.csv" + suite="$3" + + pattern_suite=";test_suite_$suite;" + total=$(grep -c "$pattern_suite" "$ref") + sed_cmd="s/^.*$pattern_suite\(.*\);SKIP.*/\1/p" + sed -n "$sed_cmd" "$ref" > skipped-ref + sed -n "$sed_cmd" "$new" > skipped-new + nb_ref=$(wc -l %3d\n" \ + $suite $total $nb_ref $nb_new + diff skipped-ref skipped-new | grep '^> ' || true + rm skipped-ref skipped-new +} + +compare_builds () { + printf "\n*** Comparing $1 -> $2 ***\n" + for suite in $SUITES; do + compare_suite "$1" "$2" "$suite" + done +} + +compare_builds before-default after-default +compare_builds before-full after-full +compare_builds reference drivers + diff --git a/docs/architecture/psa-migration/strategy.md b/docs/architecture/psa-migration/strategy.md index 74dd27a910..6c3378b028 100644 --- a/docs/architecture/psa-migration/strategy.md +++ b/docs/architecture/psa-migration/strategy.md @@ -256,6 +256,12 @@ configuration", to allow working around internal crypto dependencies when working on other parts such as X.509 and TLS - for example, a configuration without RSA PKCS#1 v2.1 still allows reasonable use of X.509 and TLS. +Note: this is a conceptual division that will sometimes translate to how the +work is divided into PRs, sometimes not. For example, in situations where it's +not possible to achieve good test coverage at the end of step 1 or step 2, it +is preferable to group with the next step(s) in the same PR until good test +coverage can be reached. + **Status as of Mbed TLS 3.2:** - Step 0 is achieved for most algorithms, with only a few gaps remaining. @@ -355,6 +361,31 @@ the new macros in the hope of avoiding confusion. Executing step 3 will mostly consist of using the right dependency macros in the right places (once the previous steps are done). +**Note on testing** + +Since supporting driver-only builds is not about adding features, but also +supporting existing features in new types of builds, the existing tests should +be enough to cover everything, as long as they're run in the newly supported +configurations. An all.sh component needs to be present each main type of +configutation we support (for example, with this (family of) algorithm +driver-only, with or without `USE_PSA`). + +There is however a risk, especially in step 3 where we change how dependencies +are expressed (sometimes in bulk), to get things wrong in a way that would +result in more tests being skipped, which is easy to miss. Care must be +taken to ensure this does not happen. The following criteria can be used: + +- the sets of tests skipped in the default config and the full config must be + the same before and after the PR that implements step 3; +- the set of tests skipped in the driver-only build is the same as in an + equivalent software-based configuration, or the difference is small enough, +justified, and a github issue is created to track it. + +Note that the favourable case is when the number of tests skipped is 0 in the +driver-only build. In other cases, analysis of the outcome files is needed, +see the example script `outcome-analysis.sh` in the same directory. + + Migrating away from the legacy API ==================================