Re: [dm-devel] [PATCH 0/1] t10-pi bio split fix

From: Milan Broz <gmazyland@gmail.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>,
	"Ivanov, Dmitry (HPC)" <dmitry.ivanov2@hpe.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Bart Van Assche <bvanassche@acm.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Dmitry Fomichev <dmitry.fomichev@wdc.com>,
	Mike Snitzer <snitzer@redhat.com>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	device-mapper development <dm-devel@redhat.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	"Lyashkov, Alexey" <alexey.lyashkov@hpe.com>
Subject: Re: [dm-devel] [PATCH 0/1] t10-pi bio split fix
Date: Fri, 4 Feb 2022 08:43:58 +0100	[thread overview]
Message-ID: <e033bbdf-5c07-8085-030d-a9954b321f08@gmail.com> (raw)
In-Reply-To: <yq1tudfz49v.fsf@ca-mkp.ca.oracle.com>

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

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