On 8/28/2014 10:31 PM, Martin K. Petersen wrote:
A set of flags introduced in the block layer enable better control over
how protection information is handled. These flags are useful for both
error injection and data recovery purposes. Checking can be enabled and
disabled for controller and disk, and the guard tag format is now a
per-I/O property.


Hi Martin,

Not sure why I didn't post a review for this one...
Anyway sorry, I hope it's not too late...
just two comments below.

Update sd_protect_op to communicate the relevant information to the
low-level device driver via a set of flags in scsi_cmnd.

Signed-off-by: Martin K. Petersen <martin.peter...@oracle.com>
---
  drivers/scsi/sd.c        | 73 ++++++++++++++++++++++++++++--------------------
  drivers/scsi/sd.h        | 66 +++++++++++++++++++++++++++++++++++++++++--
  drivers/scsi/sd_dif.c    | 23 ++++++---------
  include/scsi/scsi_cmnd.h | 36 +++++++++++++++++-------
  4 files changed, 142 insertions(+), 56 deletions(-)


<SNIP>

+/*
+ * Returns a mask of the protection flags that are valid for a given DIX
+ * operation.
+ */
+static inline unsigned int sd_prot_flag_mask(unsigned int prot_op)
+{
+       const unsigned int flag_mask[] = {
+               [SCSI_PROT_NORMAL]              = 0,
+
+               [SCSI_PROT_READ_STRIP]          = SCSI_PROT_TRANSFER_PI |
+                                                 SCSI_PROT_GUARD_CHECK |
+                                                 SCSI_PROT_REF_CHECK |
+                                                 SCSI_PROT_REF_INCREMENT,
+
+               [SCSI_PROT_READ_INSERT]         = SCSI_PROT_REF_INCREMENT |
+                                                 SCSI_PROT_IP_CHECKSUM,
+
+               [SCSI_PROT_READ_PASS]           = SCSI_PROT_TRANSFER_PI |
+                                                 SCSI_PROT_GUARD_CHECK |
+                                                 SCSI_PROT_REF_CHECK |
+                                                 SCSI_PROT_REF_INCREMENT |
+                                                 SCSI_PROT_IP_CHECKSUM,
+
+               [SCSI_PROT_WRITE_INSERT]        = SCSI_PROT_TRANSFER_PI |
+                                                 SCSI_PROT_REF_INCREMENT,
+
+               [SCSI_PROT_WRITE_STRIP]         = SCSI_PROT_GUARD_CHECK |
+                                                 SCSI_PROT_REF_CHECK |
+                                                 SCSI_PROT_REF_INCREMENT |
+                                                 SCSI_PROT_IP_CHECKSUM,
+
+               [SCSI_PROT_WRITE_PASS]          = SCSI_PROT_TRANSFER_PI |
+                                                 SCSI_PROT_GUARD_CHECK |
+                                                 SCSI_PROT_REF_CHECK |
+                                                 SCSI_PROT_REF_INCREMENT |
+                                                 SCSI_PROT_IP_CHECKSUM,

A bit strange to me that you put REF_CHECK & REF_INCREMENT flag
depending on the prot_op while it really depends on the prot_type.
It is &'nd with preset scmnd->prot_flags anyways.. so no worries...

+       };
+
+       return flag_mask[prot_op];
+}
+

<SNIP>

+/*
+enum scsi_prot_flags {
+       SCSI_PROT_TRANSFER_PI           = 1 << 0,
+       SCSI_PROT_GUARD_CHECK           = 1 << 1,
+       SCSI_PROT_REF_CHECK             = 1 << 2,
+       SCSI_PROT_REF_INCREMENT         = 1 << 3,
+       SCSI_PROT_IP_CHECKSUM           = 1 << 4,

I have a question here,
What are your plans with explicit ref/app escape flags?
Were these just left behind on this series?


Other than that, looks good.

Reviewed-by: Sagi Grimberg <sa...@mellanox.com>

Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to