Re: [PATCH v11 07/10] block: introduce BIP_CHECK_GUARD/REFTAG/APPTAG bip_flags

[Anuj Gupta <anuj20.g@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]

On Mon, Feb 03, 2025 at 07:53:31AM +0100, Christoph Hellwig wrote:
> Hi Anuj,
> 
> I just stumbled over this patch when forward porting my XFS PI support
> code over the weekend, which failed badly because it didn't set the
> new BIP_CHECK_GUARD and BIP_CHECK_REFTAG flags.  Now for the XFS side
> that was just me being to lazy to forward port, but when I started
> looking over bio_integrity_add_page users as part of doing this I think
> I found a regression caused by this patch.
> 
> The scsi and nvme targets never sets these new flags when passing on PI,
> so that will probably stop working.  So we'll need to set them and for
> nvmet we could also improve the code to actually pass through the
> individual flags.  Note that this is just by observation, I didn't find
> time to actually set up the SCSI and NVMe target code with PI support.

Hi Christoph,

Thanks for sharing. Right, the target code is not setting these flags.
I tried to reproduce it by creating a target setup. nvme-tcp doesn't
support T-10 PI (it doesn't set the NVMF_METADATA_SUPPORTED flag).
nvme-rdma supports T-10 PI, trying to reproduce it there.

Something like this (compile-tested only) [1] could work for
nvme-fabrics. Will investigate more and test.

[1]

diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index c1f574fe3280..a3152699b7de 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -210,6 +210,10 @@ static int nvmet_bdev_alloc_bip(struct nvmet_req *req, struct bio *bio,
 		return PTR_ERR(bip);
 	}
 
+	if (bi->csum_type == BLK_INTEGRITY_CSUM_IP)
+		bip->bip_flags |= BIP_IP_CHECKSUM;
+	if (bi->flags & BLK_INTEGRITY_REF_TAG)
+		bip->bip_flags |= BIP_CHECK_REFTAG;
 	/* virtual start sector must be in integrity interval units */
 	bip_set_seed(bip, bio->bi_iter.bi_sector >>
 		     (bi->interval_exp - SECTOR_SHIFT));


Thanks,
Anuj Gupta

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]