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 >