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
>
