Re: [PATCH v4 11/11] scsi: add support for user-meta interface

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

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

On Mon, Oct 21, 2024 at 09:58:57PM -0400, Martin K. Petersen wrote:
> 
> Anuj,
> 
> > +/*
> > + * Can't check reftag alone or apptag alone
> > + */
> > +static bool sd_prot_flags_valid(struct scsi_cmnd *scmd)
> > +{
> > +	struct request *rq = scsi_cmd_to_rq(scmd);
> > +	struct bio *bio = rq->bio;
> > +
> > +	if (bio_integrity_flagged(bio, BIP_CHECK_REFTAG) &&
> > +	    !bio_integrity_flagged(bio, BIP_CHECK_APPTAG))
> > +		return false;
> > +	if (!bio_integrity_flagged(bio, BIP_CHECK_REFTAG) &&
> > +	    bio_integrity_flagged(bio, BIP_CHECK_APPTAG))
> > +		return false;
> > +	return true;
> > +}
> 
> This breaks reading the partition table.
> 
> The BIP_CHECK_* flags should really only control DIX in the SCSI case.
> Filling out *PROTECT is left as an exercise for the SCSI disk driver.
> It's the only way we can sanely deal with this. Especially given ATO,
> GRD_CHK, REF_CHK, and APP_CHK. It just gets too complicated.
> 
> You should just drop sd_prot_flags_valid() and things work fine. And
> then with BIP_CHECK_* introduced we can drop BIP_CTRL_NOCHECK.

So I will keep the fine grained userspace/bip flags (which we have in
this version). And drop the sd_prot_flags_valid() and BIP_CTRL_NOCHECK
like below [1]. Hope that looks fine

[1]

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index ca4bc0ac76ad..0913bd43f48a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -814,14 +814,14 @@ static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
 		if (bio_integrity_flagged(bio, BIP_IP_CHECKSUM))
 			scmd->prot_flags |= SCSI_PROT_IP_CHECKSUM;
 
-		if (bio_integrity_flagged(bio, BIP_CTRL_NOCHECK) == false)
+		if (bio_integrity_flagged(bio, BIP_CHECK_GUARD)
 			scmd->prot_flags |= SCSI_PROT_GUARD_CHECK;
 	}
 
 	if (dif != T10_PI_TYPE3_PROTECTION) {	/* DIX/DIF Type 0, 1, 2 */
 		scmd->prot_flags |= SCSI_PROT_REF_INCREMENT;
 
-		if (bio_integrity_flagged(bio, BIP_CTRL_NOCHECK) == false)
+		if (bio_integrity_flagged(bio, BIP_CHECK_REFTAG))
 			scmd->prot_flags |= SCSI_PROT_REF_CHECK;
 	}
 
-- 
2.25.1
> 
> -- 
> Martin K. Petersen	Oracle Linux Engineering
> 

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