[dm-devel] dm-integrity - add the "reset_recalculate" flag

[Milan Broz]

On 23/03/2021 16:12, Mike Snitzer wrote:
> On Tue, Mar 23 2021 at 10:59am -0400,
> Mikulas Patocka <mpatocka at redhat.com> wrote:
> 
>> This patch adds a new flag "reset_recalculate" that will restart
>> recalculating from the beginning of the device. It can be used if we want
>> to change the hash function. Example:
>>
>> #!/bin/sh
>> dmsetup remove_all
>> rmmod brd
>> set -e
>> modprobe brd rd_size=1048576
>> dmsetup create in --table '0 2000000 integrity /dev/ram0 0 16 J 2 internal_hash:sha256 recalculate'
>> sleep 10
>> dmsetup status
>> dmsetup remove in
>> dmsetup create in --table '0 2000000 integrity /dev/ram0 0 16 J 2 internal_hash:sha3-256 reset_recalculate'
>>
>> Signed-off-by: Mikulas Patocka <mpatocka at redhat.com>
>>
>> Index: linux-2.6/drivers/md/dm-integrity.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/md/dm-integrity.c
>> +++ linux-2.6/drivers/md/dm-integrity.c
>> @@ -262,6 +262,7 @@ struct dm_integrity_c {
>>  	bool journal_uptodate;
>>  	bool just_formatted;
>>  	bool recalculate_flag;
>> +	bool reset_recalculate_flag;
>>  	bool discard;
>>  	bool fix_padding;
>>  	bool fix_hmac;
>> @@ -3134,7 +3135,8 @@ static void dm_integrity_resume(struct d
>>  		rw_journal_sectors(ic, REQ_OP_READ, 0, 0,
>>  				   ic->n_bitmap_blocks * (BITMAP_BLOCK_SIZE >> SECTOR_SHIFT), NULL);
>>  		if (ic->mode == 'B') {
>> -			if (ic->sb->log2_blocks_per_bitmap_bit == ic->log2_blocks_per_bitmap_bit) {
>> +			if (ic->sb->log2_blocks_per_bitmap_bit == ic->log2_blocks_per_bitmap_bit &&
>> +			    !ic->reset_recalculate_flag) {
>>  				block_bitmap_copy(ic, ic->recalc_bitmap, ic->journal);
>>  				block_bitmap_copy(ic, ic->may_write_bitmap, ic->journal);
>>  				if (!block_bitmap_op(ic, ic->journal, 0, ic->provided_data_sectors,
>> @@ -3156,7 +3158,8 @@ static void dm_integrity_resume(struct d
>>  			}
>>  		} else {
>>  			if (!(ic->sb->log2_blocks_per_bitmap_bit == ic->log2_blocks_per_bitmap_bit &&
>> -			      block_bitmap_op(ic, ic->journal, 0, ic->provided_data_sectors, BITMAP_OP_TEST_ALL_CLEAR))) {
>> +			      block_bitmap_op(ic, ic->journal, 0, ic->provided_data_sectors, BITMAP_OP_TEST_ALL_CLEAR)) ||
>> +			    ic->reset_recalculate_flag) {
>>  				ic->sb->flags |= cpu_to_le32(SB_FLAG_RECALCULATING);
>>  				ic->sb->recalc_sector = cpu_to_le64(0);
>>  			}
>> @@ -3169,6 +3172,10 @@ static void dm_integrity_resume(struct d
>>  			dm_integrity_io_error(ic, "writing superblock", r);
>>  	} else {
>>  		replay_journal(ic);
>> +		if (ic->reset_recalculate_flag) {
>> +			ic->sb->flags |= cpu_to_le32(SB_FLAG_RECALCULATING);
>> +			ic->sb->recalc_sector = cpu_to_le64(0);
>> +		}
>>  		if (ic->mode == 'B') {
>>  			ic->sb->flags |= cpu_to_le32(SB_FLAG_DIRTY_BITMAP);
>>  			ic->sb->log2_blocks_per_bitmap_bit = ic->log2_blocks_per_bitmap_bit;
>> @@ -3242,6 +3249,7 @@ static void dm_integrity_status(struct d
>>  		arg_count += !!ic->meta_dev;
>>  		arg_count += ic->sectors_per_block != 1;
>>  		arg_count += !!(ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING));
>> +		arg_count += ic->reset_recalculate_flag;
>>  		arg_count += ic->discard;
>>  		arg_count += ic->mode == 'J';
>>  		arg_count += ic->mode == 'J';
>> @@ -3261,6 +3269,8 @@ static void dm_integrity_status(struct d
>>  			DMEMIT(" block_size:%u", ic->sectors_per_block << SECTOR_SHIFT);
>>  		if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))
>>  			DMEMIT(" recalculate");
>> +		if (ic->reset_recalculate_flag)
>> +			DMEMIT(" reset_recalculate");
>>  		if (ic->discard)
>>  			DMEMIT(" allow_discards");
>>  		DMEMIT(" journal_sectors:%u", ic->initial_sectors - SB_SECTORS);
>> @@ -4058,6 +4068,9 @@ static int dm_integrity_ctr(struct dm_ta
>>  				goto bad;
>>  		} else if (!strcmp(opt_string, "recalculate")) {
>>  			ic->recalculate_flag = true;
>> +		} else if (!strcmp(opt_string, "reset_recalculate")) {
>> +			ic->recalculate_flag = true;
>> +			ic->reset_recalculate_flag = true;
>>  		} else if (!strcmp(opt_string, "allow_discards")) {
>>  			ic->discard = true;
>>  		} else if (!strcmp(opt_string, "fix_padding")) {
> 
> Do you need to bump the number of feature args supported (from 17 to
> 18)?

And also update target minor version.

I was just under the impression that we decided not to support such a flag (because we cannot change tag size, so it is not usable in some situations).
But if it is so simple, why not.

For the reference, it was discovered in this report https://gitlab.com/cryptsetup/cryptsetup/-/issues/631

Milan