On Thu, May 29, 2025 at 12:42:45PM +0530, Anuj Gupta/Anuj Gupta wrote:
> On 5/29/2025 8:32 AM, Martin K. Petersen wrote:
> >
> > Hi Anuj!
> >
> > Thanks for working on this!
> >
> Hi Martin,
> Thanks for the feedback!
>
> >> 4. tuple_size: size (in bytes) of the protection information tuple.
> >> 6. pi_offset: offset of protection info within the tuple.
> >
> > I find this a little confusing. The T10 PI tuple is <guard, app, ref>.
> >
> > I acknowledge things currently are a bit muddy in the block layer since
> > tuple_size has been transmogrified to hold the NVMe metadata size.
> >
> > But for a new user-visible interface I think we should make the
> > terminology clear. The tuple is the PI and not the rest of the metadata.
> >
> > So I think you'd want:
> >
> > 4. metadata_size: size (in bytes) of the metadata associated with each interval.
> > 6. pi_offset: offset of protection information tuple within the metadata.
> >
>
> Yes, this representation looks better. Will make this change.
>
> >> +#define FILE_PI_CAP_INTEGRITY (1 << 0)
> >> +#define FILE_PI_CAP_REFTAG (1 << 1)
> >
> > You'll also need to have corresponding uapi defines for:
> >
> > enum blk_integrity_checksum {
> > BLK_INTEGRITY_CSUM_NONE = 0,
> > BLK_INTEGRITY_CSUM_IP = 1,
> > BLK_INTEGRITY_CSUM_CRC = 2,
> > BLK_INTEGRITY_CSUM_CRC64 = 3,
> > } __packed ;
> >
>
> Right, I'll add these definitions to the UAPI.
Would it make sense to give the CRCs clearer names? For example CRC16_T10DIF
and CRC64_NVME.
- Eric
