Re: [PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-11-03 Thread Sagi Grimberg

On 11/1/2013 5:13 PM, Bart Van Assche wrote:

On 31/10/2013 5:24, Sagi Grimberg wrote:

+/**
+ * struct ib_sig_domain - Parameters specific for T10-DIF
+ * domain.
+ * @sig_type: specific signauture type
+ * @sig: union of all signature domain attributes that may
+ *  be used to set domain layout.
+ *  @dif:
+ * @type: T10-DIF type (0|1|2|3)
+ * @bg_type: T10-DIF block guard type (CRC|CSUM)
+ * @block_size: block size in signature domain.
+ * @app_tag: if app_tag is owned be the user,
+ * HCA will take this value to be app_tag.
+ * @ref_tag: initial ref_tag of signature handover.
+ * @type3_inc_reftag: T10-DIF type 3 does not state
+ *about the reference tag, it is the user
+ *choice to increment it or not.
+ */
+struct ib_sig_domain {
+enum ib_signature_type sig_type;
+union {
+struct {
+enum ib_t10_dif_typetype;
+enum ib_t10_dif_bg_type bg_type;
+u16block_size;
+u16bg;
+u16app_tag;
+u32ref_tag;
+booltype3_inc_reftag;
+} dif;
+} sig;
+};


My understanding from SPC-4 is that in that when using protection 
information such information is inserted after every protection 
interval. A protection interval can be smaller than a logical block. 
Shouldn't the name block_size be changed into something like 
pi_interval to avoid confusion with the logical block size ?


Bart.



True, for DIF types 2,3 protection interval is not restricted to be 
logical block length and may be smaller.

I agree with pi_interval naming.

Note that pi_intervals smaller than 512 bytes are not supported at the 
moment.


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


Re: [PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-11-03 Thread Sagi Grimberg

On 11/2/2013 12:23 AM, Bart Van Assche wrote:

On 31/10/2013 5:24, Sagi Grimberg wrote:

+ * @type3_inc_reftag: T10-DIF type 3 does not state
+ *about the reference tag, it is the user
+ *choice to increment it or not.


Can you explain this further ? Does this mean that the HCA can check 
whether the reference tags are increasing when receiving data for TYPE 
3 protection mode ? My understanding of SPC-4 is that the application 
is free to use the reference tag in any way when using TYPE 3 
protection and hence that the HCA must not check whether the reference 
tag is increasing for TYPE 3 protection. See e.g. 
sd_dif_type3_get_tag() in drivers/scsi/sd_dif.c.


Bart.


As I understand TYPE 3, the reference tag is free for the application to 
use - which may choose to inc it each PI or not. This option allows the 
application to inc ref_tag in type 3.
The DIF check is determined via check_mask. As I see it, correct use in 
case of DIF TYPE 3 is not to validate reference tag i.e. set REF_TAG 
bits in check_mask to zero.


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


Re: [PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-11-03 Thread Sagi Grimberg

On 11/1/2013 8:46 PM, Bart Van Assche wrote:

On 31/10/2013 5:24, Sagi Grimberg wrote:

+/**
+ * Signature T10-DIF block-guard types
+ */
+enum ib_t10_dif_bg_type {
+IB_T10DIF_CRC,
+IB_T10DIF_CSUM
+};


In SPC-4 paragraph 4.22.4 I found that the T10-PI guard is the CRC 
computed from the generator polynomial x^16 + x^15 + x^11 + x^9 + x^8 
+ x^7 + x^5 + x^4 + x^2 + x + 1. Could you tell me where I can find 
which guard computation method IB_T10DIF_CSUM corresponds to ?


Bart.


The IB_T10DIF_CSUM  computation method corresponds to IP checksum rules. 
this is aligned with SHOST_DIX_GUARD_IP guard type.


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


Re: [PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-11-03 Thread Bart Van Assche

On 3/11/2013 4:15, Sagi Grimberg wrote:

On 11/1/2013 8:46 PM, Bart Van Assche wrote:

On 31/10/2013 5:24, Sagi Grimberg wrote:

+/**
+ * Signature T10-DIF block-guard types
+ */
+enum ib_t10_dif_bg_type {
+IB_T10DIF_CRC,
+IB_T10DIF_CSUM
+};


In SPC-4 paragraph 4.22.4 I found that the T10-PI guard is the CRC
computed from the generator polynomial x^16 + x^15 + x^11 + x^9 + x^8
+ x^7 + x^5 + x^4 + x^2 + x + 1. Could you tell me where I can find
which guard computation method IB_T10DIF_CSUM corresponds to ?

Bart.


The IB_T10DIF_CSUM  computation method corresponds to IP checksum rules.
this is aligned with SHOST_DIX_GUARD_IP guard type.


Since the declarations added in rdma/ib_verbs.h constitute an 
interface definition I think it would help if it would be made more 
clear what these two symbols stand for. How about mentioning the names 
of the standards these two guard computation methods come from ? An 
alternative is to add a comment like the one above scsi_host_guard_type 
in scsi/scsi_host.h which explains the two guard computation methods well:


/*
 * All DIX-capable initiators must support the T10-mandated CRC
 * checksum.  Controllers can optionally implement the IP checksum
 * scheme which has much lower impact on system performance.  Note
 * that the main rationale for the checksum is to match integrity
 * metadata with data.  Detecting bit errors are a job for ECC memory
 * and buses.
 */

Bart.

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


Re: [PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-11-03 Thread Sagi Grimberg

On 11/3/2013 4:41 PM, Bart Van Assche wrote:

On 3/11/2013 4:15, Sagi Grimberg wrote:

On 11/1/2013 8:46 PM, Bart Van Assche wrote:

On 31/10/2013 5:24, Sagi Grimberg wrote:

+/**
+ * Signature T10-DIF block-guard types
+ */
+enum ib_t10_dif_bg_type {
+IB_T10DIF_CRC,
+IB_T10DIF_CSUM
+};


In SPC-4 paragraph 4.22.4 I found that the T10-PI guard is the CRC
computed from the generator polynomial x^16 + x^15 + x^11 + x^9 + x^8
+ x^7 + x^5 + x^4 + x^2 + x + 1. Could you tell me where I can find
which guard computation method IB_T10DIF_CSUM corresponds to ?

Bart.


The IB_T10DIF_CSUM  computation method corresponds to IP checksum rules.
this is aligned with SHOST_DIX_GUARD_IP guard type.


Since the declarations added in rdma/ib_verbs.h constitute an 
interface definition I think it would help if it would be made more 
clear what these two symbols stand for. How about mentioning the names 
of the standards these two guard computation methods come from ? An 
alternative is to add a comment like the one above 
scsi_host_guard_type in scsi/scsi_host.h which explains the two 
guard computation methods well:


/*
 * All DIX-capable initiators must support the T10-mandated CRC
 * checksum.  Controllers can optionally implement the IP checksum
 * scheme which has much lower impact on system performance.  Note
 * that the main rationale for the checksum is to match integrity
 * metadata with data.  Detecting bit errors are a job for ECC memory
 * and buses.
 */

Bart.



Agreed,

I'll comment on each type correspondence (T10-DIF CRC checksum and IP 
checksum).


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


Re: [PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-11-01 Thread Bart Van Assche

On 31/10/2013 5:24, Sagi Grimberg wrote:

+/**
+ * struct ib_sig_domain - Parameters specific for T10-DIF
+ * domain.
+ * @sig_type: specific signauture type
+ * @sig: union of all signature domain attributes that may
+ *  be used to set domain layout.
+ *  @dif:
+ * @type: T10-DIF type (0|1|2|3)
+ * @bg_type: T10-DIF block guard type (CRC|CSUM)
+ * @block_size: block size in signature domain.
+ * @app_tag: if app_tag is owned be the user,
+ * HCA will take this value to be app_tag.
+ * @ref_tag: initial ref_tag of signature handover.
+ * @type3_inc_reftag: T10-DIF type 3 does not state
+ * about the reference tag, it is the user
+ * choice to increment it or not.
+ */
+struct ib_sig_domain {
+   enum ib_signature_type  sig_type;
+   union {
+   struct {
+   enum ib_t10_dif_typetype;
+   enum ib_t10_dif_bg_type bg_type;
+   u16 block_size;
+   u16 bg;
+   u16 app_tag;
+   u32 ref_tag;
+   booltype3_inc_reftag;
+   } dif;
+   } sig;
+};


My understanding from SPC-4 is that in that when using protection 
information such information is inserted after every protection 
interval. A protection interval can be smaller than a logical block. 
Shouldn't the name block_size be changed into something like 
pi_interval to avoid confusion with the logical block size ?


Bart.

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


Re: [PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-11-01 Thread Bart Van Assche

On 31/10/2013 5:24, Sagi Grimberg wrote:

+ * @type3_inc_reftag: T10-DIF type 3 does not state
+ * about the reference tag, it is the user
+ * choice to increment it or not.


Can you explain this further ? Does this mean that the HCA can check 
whether the reference tags are increasing when receiving data for TYPE 3 
protection mode ? My understanding of SPC-4 is that the application is 
free to use the reference tag in any way when using TYPE 3 protection 
and hence that the HCA must not check whether the reference tag is 
increasing for TYPE 3 protection. See e.g. sd_dif_type3_get_tag() in 
drivers/scsi/sd_dif.c.


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


[PATCH RFC v2 02/10] IB/core: Introduce Signature Verbs API

2013-10-31 Thread Sagi Grimberg
This commit Introduces the Verbs Interface for signature related
operations. A signature handover operation shall configure the
layouts of data and protection attributes both in memory and wire
domains.

Signature operations are:
- INSERT
  Generate and insert protection information when handing over
  data from input space to output space.
- vaildate and STRIP:
  Validate protection information and remove it when handing over
  data from input space to output space.
- validate and PASS:
  Validate protection information and pass it when handing over
  data from input space to output space.

Once the signature handover opration is done, the HCA will
offload data integrity generation/validation while performing
the actual data transfer.

Additions:
1. HCA signature capabilities in device attributes
Verbs provider supporting Signature handover operations shall
fill relevant fields in device attributes structure returned
by ib_query_device.

2. QP creation flag IB_QP_CREATE_SIGNATURE_EN
Creating QP that will carry signature handover operations
may require some special preperations from the verbs provider.
So we add QP creation flag IB_QP_CREATE_SIGNATURE_EN to declare
that the created QP may carry out signature handover operations.
Expose signature support to verbs layer (no support for now)

3. New send work request IB_WR_REG_SIG_MR
Signature handover work request. This WR will define the signature
handover properties of the memory/wire domains as well as the domains
layout. The purpose of this work request is to bind all the needed
information for the signature operation:
- data to be transferred:  wr-sg_list.
  * The raw data, pre-registered to a single MR (normally, before
signature, this MR would have been used directly for the data
transfer). the user will pass the data sge via sg_list exsisting
member.
- data protection guards: sig_handover.prot.
  * The data protection buffer, pre-registered to a single MR, which
contains the data integrity guards of the raw data blocks.
Note that it may not always exist, only in cases where the user is
interested in storing protection guards in memory.
- signature operation attributes: sig_handover.sig_attrs.
  * Tells the HCA how to validate/generate the protection information.

Once the work request is executed, the memory region which
will describe the signature transaction will be the sig_mr. The
application can now go ahead and send the sig_mr.rkey or use the
sig_mr.lkey for data transfer.

4. New Verb ib_check_sig_status
check_sig_status Verb shall check if any signature errors
are pending for a specific signature-enabled ib_mr.
This Verb is a lightwight check and is allowed to be taken
from interrupt context. Application must call this verb after
it is known that the actual data transfer has finished.

Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
 drivers/infiniband/core/verbs.c |8 +++
 include/rdma/ib_verbs.h |  127 ++-
 2 files changed, 134 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 1d94a5c..5636d65 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1293,3 +1293,11 @@ int ib_dealloc_xrcd(struct ib_xrcd *xrcd)
return xrcd-device-dealloc_xrcd(xrcd);
 }
 EXPORT_SYMBOL(ib_dealloc_xrcd);
+
+int ib_check_sig_status(struct ib_mr *sig_mr,
+   struct ib_sig_err *sig_err)
+{
+   return sig_mr-device-check_sig_status ?
+   sig_mr-device-check_sig_status(sig_mr, sig_err) : -ENOSYS;
+}
+EXPORT_SYMBOL(ib_check_sig_status);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 53f065d..19b37eb 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -116,7 +116,19 @@ enum ib_device_cap_flags {
IB_DEVICE_MEM_MGT_EXTENSIONS= (121),
IB_DEVICE_BLOCK_MULTICAST_LOOPBACK = (122),
IB_DEVICE_MEM_WINDOW_TYPE_2A= (123),
-   IB_DEVICE_MEM_WINDOW_TYPE_2B= (124)
+   IB_DEVICE_MEM_WINDOW_TYPE_2B= (124),
+   IB_DEVICE_SIGNATURE_HANDOVER= (125),
+};
+
+enum ib_signature_prot_cap {
+   IB_PROT_T10DIF_TYPE_1 = 1,
+   IB_PROT_T10DIF_TYPE_2 = 1  1,
+   IB_PROT_T10DIF_TYPE_3 = 1  2,
+};
+
+enum ib_signature_guard_cap {
+   IB_GUARD_T10DIF_CRC = 1,
+   IB_GUARD_T10DIF_CSUM= 1  1,
 };
 
 enum ib_atomic_cap {
@@ -166,6 +178,8 @@ struct ib_device_attr {
unsigned intmax_fast_reg_page_list_len;
u16 max_pkeys;
u8  local_ca_ack_delay;
+   int sig_prot_cap;
+   int sig_guard_cap;
 };
 
 enum ib_mtu {
@@ -630,6 +644,7 @@ enum ib_qp_type {
 enum ib_qp_create_flags {
IB_QP_CREATE_IPOIB_UD_LSO   =