avbtool: Generate forward-error-correcting codes for hashtree by default.
The overhead of FEC is tiny and data shows that it help a lot. So just
enable it by default to avoid everybody having to manually turn it on.
Technically this is a breaking change (insofar hashtree images will
now have FEC on when avbtool is updated) but since we haven't actually
released 1.0.0 and it's still early days for AVB, do it anyway. Do
warn on stderr that the --generate_fec option is now deprecated.
Bug: None
Test: All unit tests pass
Test: Manually tested on UEFI-based bootloader.
Change-Id: I4eb5cca25bb1ca08f0e80ca033dc037fc4379104
diff --git a/README.md b/README.md
index a6d033e..8421383 100644
--- a/README.md
+++ b/README.md
@@ -331,7 +331,7 @@
[--include_descriptors_from_footer /path/to/image.bin] \
[--setup_rootfs_from_kernel /path/to/image.bin] \
[--output_vbmeta_image OUTPUT_IMAGE] [--do_not_append_vbmeta_image] \
- [--generate_fec] [--fec_num_roots FEC_NUM_ROOTS] \
+ [--do_not_generate_fec] [--fec_num_roots FEC_NUM_ROOTS] \
[--signing_helper /path/to/external/signer] \
[--append_to_release_string STR]
@@ -441,12 +441,14 @@
The variable `BOARD_AVB_SYSTEM_ADD_HASHTREE_FOOTER_ARGS` can be used
to specify additional options passed to `avbtool add_hashtree_footer`
for `system.img`. Typical options to be used here include
-`--hash_algorithm`, `--salt`, `--block_size`, and `--generate_fec`.
+`--hash_algorithm`, `--salt`, `--block_size`, and
+`--do_not_generate_fec`.
The variable `BOARD_AVB_VENDOR_ADD_HASHTREE_FOOTER_ARGS` can be used
to specify additional options passed to `avbtool add_hashtree_footer`
for `vendor.img`. Typical options to be used here include
-`--hash_algorithm`, `--salt`, `--block_size`, and `--generate_fec`.
+`--hash_algorithm`, `--salt`, `--block_size`, and
+`--do_not_generate_fec`.
The variable `BOARD_AVB_DTBO_ADD_HASH_FOOTER_ARGS` can be used to
specify additional options passed to `avbtool add_hash_footer` for
diff --git a/avbtool b/avbtool
index 08876a2..7c18d46 100755
--- a/avbtool
+++ b/avbtool
@@ -3279,8 +3279,16 @@
help='Block size (default: 4096)',
type=parse_number,
default=4096)
+ # TODO(zeuthen): The --generate_fec option was removed when we
+ # moved to generating FEC by default. To avoid breaking existing
+ # users needing to transition we simply just print a warning below
+ # in add_hashtree_footer(). Remove this option and the warning at
+ # some point in the future.
sub_parser.add_argument('--generate_fec',
- help='Add forward-error-correction codes',
+ help=argparse.SUPPRESS,
+ action='store_true')
+ sub_parser.add_argument('--do_not_generate_fec',
+ help='Do not generate forward-error-correction codes',
action='store_true')
sub_parser.add_argument('--fec_num_roots',
help='Number of roots for FEC (default: 2)',
@@ -3485,10 +3493,16 @@
def add_hashtree_footer(self, args):
"""Implements the 'add_hashtree_footer' sub-command."""
args = self._fixup_common_args(args)
+ # TODO(zeuthen): Remove when removing support for the
+ # '--generate_fec' option above.
+ if args.generate_fec:
+ sys.stderr.write('The --generate_fec option is deprecated since FEC '
+ 'is now generated by default. Use the option '
+ '--do_not_generate_fec to not generate FEC.\n')
self.avb.add_hashtree_footer(args.image.name if args.image else None,
args.partition_size,
args.partition_name,
- args.generate_fec, args.fec_num_roots,
+ not args.do_not_generate_fec, args.fec_num_roots,
args.hash_algorithm, args.block_size,
args.salt, args.chain_partition, args.algorithm,
args.key, args.public_key_metadata,
diff --git a/test/avb_ab_flow_unittest.cc b/test/avb_ab_flow_unittest.cc
index 3d16b60..ea79224 100644
--- a/test/avb_ab_flow_unittest.cc
+++ b/test/avb_ab_flow_unittest.cc
@@ -201,7 +201,8 @@
" --partition_size %zd"
" --salt deadbeef"
" --algorithm SHA512_RSA4096 "
- " --key test/data/testkey_rsa4096.pem",
+ " --key test/data/testkey_rsa4096.pem"
+ " --do_not_generate_fec",
odm_path.value().c_str(),
rollback_odm,
odm_partition_size);
diff --git a/test/avb_slot_verify_unittest.cc b/test/avb_slot_verify_unittest.cc
index d8c414a..76ce2f5 100644
--- a/test/avb_slot_verify_unittest.cc
+++ b/test/avb_slot_verify_unittest.cc
@@ -1299,7 +1299,8 @@
"--partition_size %d --partition_name foobar "
"--algorithm SHA256_RSA2048 "
"--key test/data/testkey_rsa2048.pem "
- "--internal_release_string \"\"",
+ "--internal_release_string \"\" "
+ "--do_not_generate_fec",
rootfs_path.value().c_str(),
(int)partition_size);
@@ -1429,7 +1430,8 @@
"--partition_size %d --partition_name system "
"--algorithm SHA256_RSA2048 "
"--key test/data/testkey_rsa2048.pem "
- "--internal_release_string \"\"",
+ "--internal_release_string \"\" "
+ "--do_not_generate_fec",
system_path.value().c_str(),
(int)system_part_size);
@@ -1438,7 +1440,8 @@
"--partition_size %d --partition_name foobar "
"--algorithm SHA256_RSA2048 "
"--key test/data/testkey_rsa2048.pem "
- "--internal_release_string \"\"",
+ "--internal_release_string \"\" "
+ "--do_not_generate_fec",
foobar_path.value().c_str(),
(int)foobar_part_size);
@@ -1447,7 +1450,8 @@
"--partition_size %d --partition_name bazboo "
"--algorithm SHA512_RSA4096 "
"--key test/data/testkey_rsa4096.pem "
- "--internal_release_string \"\"",
+ "--internal_release_string \"\" "
+ "--do_not_generate_fec",
bazboo_path.value().c_str(),
(int)bazboo_part_size);
@@ -1704,7 +1708,8 @@
"--partition_size %d --partition_name system "
"--algorithm SHA256_RSA2048 "
"--key test/data/testkey_rsa2048.pem "
- "--internal_release_string \"\"",
+ "--internal_release_string \"\" "
+ "--do_not_generate_fec",
system_path.value().c_str(),
(int)system_part_size);
diff --git a/test/avbtool_unittest.cc b/test/avbtool_unittest.cc
index b9d058a..6cb9ac0 100644
--- a/test/avbtool_unittest.cc
+++ b/test/avbtool_unittest.cc
@@ -700,7 +700,8 @@
"--algorithm SHA256_RSA2048 "
"--key test/data/testkey_rsa2048.pem "
"--output_vbmeta_image %s "
- "--internal_release_string \"\"",
+ "--internal_release_string \"\" "
+ "--do_not_generate_fec",
rootfs_path.value().c_str(),
(int)partition_size,
ext_vbmeta_path.value().c_str());
@@ -914,7 +915,8 @@
"--algorithm SHA256_RSA2048 "
"--key test/data/testkey_rsa2048.pem "
"--output_vbmeta %s_2nd_run --do_not_append_vbmeta_image "
- "--internal_release_string \"\"",
+ "--internal_release_string \"\" "
+ "--do_not_generate_fec",
rootfs_path.value().c_str(),
(int)partition_size,
ext_vbmeta_path.value().c_str());
@@ -968,7 +970,6 @@
EXPECT_COMMAND(0,
"./avbtool add_hashtree_footer --salt d00df00d --image %s "
"--partition_size %d --partition_name foobar "
- "--generate_fec "
"--algorithm SHA256_RSA2048 "
"--key test/data/testkey_rsa2048.pem "
"--internal_release_string \"\"",
@@ -1149,7 +1150,8 @@
EXPECT_COMMAND(0,
"./avbtool add_hashtree_footer "
- "--partition_size %zd --calc_max_image_size > %s",
+ "--partition_size %zd --calc_max_image_size "
+ "--do_not_generate_fec > %s",
partition_size,
output_path.value().c_str());
std::string max_image_size_data;
@@ -1172,7 +1174,8 @@
" --salt deadbeef"
" --algorithm SHA512_RSA4096 "
" --key test/data/testkey_rsa4096.pem"
- " --internal_release_string \"\"",
+ " --internal_release_string \"\" "
+ "--do_not_generate_fec",
system_path.value().c_str(),
partition_size);
}
@@ -1183,8 +1186,7 @@
EXPECT_COMMAND(0,
"./avbtool add_hashtree_footer "
- "--partition_size %zd --generate_fec "
- "--calc_max_image_size > %s",
+ "--partition_size %zd --calc_max_image_size > %s",
partition_size,
output_path.value().c_str());
std::string max_image_size_data;
@@ -1205,7 +1207,6 @@
" --partition_name system"
" --partition_size %zd"
" --salt deadbeef"
- " --generate_fec "
" --algorithm SHA512_RSA4096 "
" --key test/data/testkey_rsa4096.pem"
" --internal_release_string \"\"",
