All of lore.kernel.org
 help / color / mirror / Atom feed
* [dm-devel] [PATCH] dm-integrity - add the "reset_recalculate" flag
@ 2021-03-23 14:59 Mikulas Patocka
  2021-03-23 15:12 ` [dm-devel] " Mike Snitzer
  0 siblings, 1 reply; 4+ messages in thread
From: Mikulas Patocka @ 2021-03-23 14:59 UTC (permalink / raw)
  To: Milan Broz, Mike Snitzer; +Cc: dm-devel

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@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")) {

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dm-devel] dm-integrity - add the "reset_recalculate" flag
  2021-03-23 14:59 [dm-devel] [PATCH] dm-integrity - add the "reset_recalculate" flag Mikulas Patocka
@ 2021-03-23 15:12 ` Mike Snitzer
  2021-03-23 15:50   ` Milan Broz
  0 siblings, 1 reply; 4+ messages in thread
From: Mike Snitzer @ 2021-03-23 15:12 UTC (permalink / raw)
  To: Mikulas Patocka; +Cc: dm-devel, Milan Broz

On Tue, Mar 23 2021 at 10:59am -0400,
Mikulas Patocka <mpatocka@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@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)?

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dm-devel] dm-integrity - add the "reset_recalculate" flag
  2021-03-23 15:12 ` [dm-devel] " Mike Snitzer
@ 2021-03-23 15:50   ` Milan Broz
  2021-03-23 17:15     ` Mikulas Patocka
  0 siblings, 1 reply; 4+ messages in thread
From: Milan Broz @ 2021-03-23 15:50 UTC (permalink / raw)
  To: Mike Snitzer, Mikulas Patocka; +Cc: dm-devel

On 23/03/2021 16:12, Mike Snitzer wrote:
> On Tue, Mar 23 2021 at 10:59am -0400,
> Mikulas Patocka <mpatocka@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@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

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dm-devel] dm-integrity - add the "reset_recalculate" flag
  2021-03-23 15:50   ` Milan Broz
@ 2021-03-23 17:15     ` Mikulas Patocka
  0 siblings, 0 replies; 4+ messages in thread
From: Mikulas Patocka @ 2021-03-23 17:15 UTC (permalink / raw)
  To: Milan Broz; +Cc: dm-devel, Mike Snitzer



On Tue, 23 Mar 2021, Milan Broz wrote:

> > Do you need to bump the number of feature args supported (from 17 to
> > 18)?

Goo point. I've sent version 2 of the patch.

> 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.

I found out that it is not as hard as it seemed, so we can implement it.

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

Mikulas

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-23 17:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 14:59 [dm-devel] [PATCH] dm-integrity - add the "reset_recalculate" flag Mikulas Patocka
2021-03-23 15:12 ` [dm-devel] " Mike Snitzer
2021-03-23 15:50   ` Milan Broz
2021-03-23 17:15     ` Mikulas Patocka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.