Milan,
I and Dmitriy tries to fix a problem with BIO split with integrity data attached.
This is case, integrity generated/attached before bio_submit over raid device (md stack in my case) and bio is subject of bio_integrity_advance.
But bio_integrity_advance incorrectly split an integrity data, as it's assume integrity iterator in the 512b blocks.
( bip->bip_iter.bi_sector += bytes_done >> 9 )
But t10 generate function (t10_pi_generate) increase a iterator by 1 for each integrity interval in the t10_pi_generate function.
( for (i = 0 ; i < iter->data_size ; i += iter->interval) {
iter->seed++;
}
)
It's good for the scsi with 512b blocks, but bad for the nvme with 4k block size.
So two solutions exist
1) my solution - lets fix an t10_pi_generate / t10_pi_verify / t10_pi_type1_prepare to increase iterator by value related to real integrity block in the 512b blocks.
2) Martin solution, just change an bio_integrity_advance function to make iterator shit in this integrity data blocks units instead of 512b blocks.
Alex
On 04/02/2022, 10:44, "Milan Broz" <gmazyland@xxxxxxxxx> wrote:
On 04/02/2022 04:44, Martin K. Petersen wrote:
>
> Dmitry,
>
>> My only concern is dm_crypt target which operates on bip_iter directly
>> with the code copy-pasted from bio_integrity_advance:
>>
>> static int dm_crypt_integrity_io_alloc(struct dm_crypt_io *io, struct bio *bio)
>> {
>> struct bio_integrity_payload *bip;
>> unsigned int tag_len;
>> int ret;
>>
>> if (!bio_sectors(bio) || !io->cc->on_disk_tag_size)
>> return 0;
>>
>> bip = bio_integrity_alloc(bio, GFP_NOIO, 1);
>> if (IS_ERR(bip))
>> return PTR_ERR(bip);
>>
>> tag_len = io->cc->on_disk_tag_size * (bio_sectors(bio) >> io->cc->sector_shift);
>>
>> bip->bip_iter.bi_size = tag_len;
>> bip->bip_iter.bi_sector = io->cc->start + io->sector;
>> ^^^
>>
>> ret = bio_integrity_add_page(bio, virt_to_page(io->integrity_metadata),
>> tag_len, offset_in_page(io->integrity_metadata));
>> ...
>> }
>
> I copied Milan and Mike who are more familiar with the dm-drypt internals.
Hi,
What's the problem here you are trying to fix?
Even if I read linux-block posts, I do not understand the context...
Anyway, cc to Mikulas and dm-devel, as dm-integrity/dm-crypt is
the major user of bio_integrity here.
If you touch the code, please be sure you run cryptsetup testsuite
with the integrity tests.
(IOW integritysetup tests and LUKS2 with authenticated encryption
that uses dm-crypt over dm-integrity.)
All we need is that dm-integrity can process bio integrity data
directly. (I know some people do not like it, but this was
the most "elegant" solution here.)
Here dm-crypt uses the data for authenticated encryption (additional
auth tag in bio field), so because dm-crypt owns bio, integrity data
must be allocated in dm-crypt (stacked over dm-integrity).
Milan
