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