[tvla] Enable passing lists of selected rounds/bytes for byte-specific AES, add new CI job #289
Conversation
vogelpi
requested review from
andreaskurth,
nasahlpa and
vrozic
as code owners
| @@ -261,14 +261,14 @@ def run_tvla(ctx: typer.Context): | |||
| rnd_list = [0] | |||
| byte_list = [0] | |||
| else: | |||
| if cfg["round_select"] is None: | |||
| if not cfg["round_select"]: | |||
There was a problem hiding this comment.
Is there a reason for that change?
In a new PR (#294), I've made exactly the opposite change. If cfg["round_select"] could be None (i.e., the entry is not in the cfg dictionary), then I think the is None check should be better?
There was a problem hiding this comment.
As discussed offline, this change is necessary because round_select is now a list instead of an int. To find out if the list is empty (no round_select) specified, we have to use not.
analysis/tvla.py
Outdated
| @@ -261,14 +261,14 @@ def run_tvla(ctx: typer.Context): | |||
| rnd_list = [0] | |||
| byte_list = [0] | |||
| else: | |||
| if cfg["round_select"] is None: | |||
| if not cfg["round_select"]: | |||
| rnd_list = list(range(11)) | |||
There was a problem hiding this comment.
Not related to your PR, but maybe we could have a define for this 11 and the 16 below? It would increase the readability.
There was a problem hiding this comment.
Good idea, that's now done!
| @@ -261,14 +261,14 @@ def run_tvla(ctx: typer.Context): | |||
| rnd_list = [0] | |||
| byte_list = [0] | |||
| else: | |||
| if cfg["round_select"] is None: | |||
| if not cfg["round_select"]: | |||
There was a problem hiding this comment.
I think this would be best practice:
| if not cfg["round_select"]: | |
| if cfg.get("round_select") is not None: |
if the key (i.e., round_select) does not exist, the get() returns None.
There was a problem hiding this comment.
Thanks for the suggestion. As answered above, the not tells us if the list round_select is empty. Previously, we just needed to check if it was defined or not.
Previously, we could only: - compute all rounds and all bytes - select individual rounds but all bytes and vice versa - select one round and one byte This commit enables passing a list of bytes and rounds. All combinations of the combined lists are then evaluated, e.g. --round-select 0 --round-select 1 --byte-select 0 --byte-select 15 will evaluate Bytes 0 and 15 in both Round 0 and Round 1, i.e., there will be for tests. This is helps reducing the memory consumption and compute load if only few rounds or bytes need to be analyzed as well as for CI. Signed-off-by: Pirmin Vogel <[email protected]>
This is related to lowRISC#287. Signed-off-by: Pirmin Vogel <[email protected]>
vogelpi
force-pushed
the
aes-specific-ci
branch
from
f77eced to
50b30b2
Compare
|
Thanks for your review @nasahlpa . I'll merge this once CI passes. |
This PR is related to #287 and contains two commits: