Re: [PATCH v5 00/10] Introduce Signature feature

2014-03-09 Thread Sagi Grimberg

On 3/7/2014 9:43 PM, Roland Dreier wrote:

So I went ahead and applied this for 3.15,


Hey Roland,

Thanks for taking the time to take a look and pick this up.


although I suspect the
verbs API is probably the wrong one.


I will be more than happy to here your concerns, share our approach and 
fix what is needed.



   I understand that the mlx5
microarchitecture requires some of this signature binding stuff to go
through a work queue, but conceptually I don't think the
IB_WR_REG_SIG_MR work request makes sense as the right interface.  Why
do I need to do both a synchronous create_mr and an asynchronous work
request for what's conceptually a single operation?


Hmm, well create_mr and REG_SIG_MR work request are not a single operation
even conceptually. create_mr is a memory key allocation while REG_SIG_MR 
is the

actual (fast) memory registration operation.
* create_mr is the equivalent of ib_alloc_fast_reg_mr and other memory 
key allocation
   routines. Note that we proposed a general routine that can 
replace/unite all memory

   allocations routines in the future.
* REG_SIG_MR is the equivalent of fastreg work request - the actual 
registration.

   It will be taken in the fast-path upon each transaction.

Note that for each data-transfer the user (iSER/SRP) will fast register 
signature MR
as this is a fast path operation and that's why it is a work request. 
REG_SIG_MR may

be seen as fastreg extension. Moreover it would be useful for the user to
pipeline WRs by using a post-list of REG_SIG_MR+RDMA and save a HW doorbell.


Similarly the ib_check_mr_status() operation doesn't really make sense
to me as an additional synchronous operation -- why do I not just get
signature information as part of the completion entry for the
transaction that generated it?


We actually thought about it more then once. But we chose this way due 
to a couple of reasons:
- Providing the data-integrity status in the completion introduces an 
asymmetric verbs behavior for the
  target and initiator. While the target is the RDMA requester posting 
RDMA operation, it may get the
  data-integrity info in the work completion of the RDMA work request. 
But the initiator can only get the
  data-integrity information in the receive completion of the SCSI 
response as this is the only indication
  that the transaction completed. As you know post recvs are not 
correlated with transactions be any means.


-  Data-integrity does not really belong to the transport completions, 
it belongs to the data itself.
   Meaning RDMA may have completed successfully but the data is 
corrupted. We aimed to keep this
   layering as most applications will handle each differently and may 
wish to explore each error in a different
   stage (for example iSER needs to construct a specific sense for 
data-integrity errors and needs to have the

   iSCSI task at reach, which is provided in a different processing stage).

So we chose to go with a simple lightweight routine that is easy to 
handle and will keep a symmetrical
implementation on all ends. In case this can be improved, I'm open to 
other ideas.


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 v5 00/10] Introduce Signature feature

2014-03-07 Thread Roland Dreier
So I went ahead and applied this for 3.15, although I suspect the
verbs API is probably the wrong one.  I understand that the mlx5
microarchitecture requires some of this signature binding stuff to go
through a work queue, but conceptually I don't think the
IB_WR_REG_SIG_MR work request makes sense as the right interface.  Why
do I need to do both a synchronous create_mr and an asynchronous work
request for what's conceptually a single operation?

Similarly the ib_check_mr_status() operation doesn't really make sense
to me as an additional synchronous operation -- why do I not just get
signature information as part of the completion entry for the
transaction that generated it?

Anyway, you guys are the ones that will have to clean this up in the
future when it becomes a problem, so I'm not going to push on this.

 - R.
--
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 v5 00/10] Introduce Signature feature

2014-02-24 Thread Sagi Grimberg

On 2/24/2014 9:01 AM, Nicholas A. Bellinger wrote:

On Sun, 2014-02-23 at 14:19 +0200, Sagi Grimberg wrote:

This patchset Introduces Verbs level support for signature handover feature.
Siganture is intended to implement end-to-end data integrity on a transactional
basis in a completely offloaded manner.

There are several end-to-end data integrity methods used today in various
applications and/or upper layer protocols such as T10-DIF defined by SCSI
specifications (SBC), CRC32, XOR8 and more. This patchset adds verbs support
only for T10-DIF. The proposed framework allows adding more signature methods
in the future.

In T10-DIF, when a series of 512-byte data blocks are transferred, each block
is followed by an 8-byte guard (note that other protection intervals may be used
other then 512-bytes). The guard consists of CRC that protects the integrity of
the data in the block, and tag that protects against mis-directed IOs and a free
tag for application use.

Data can be protected when transferred over the wire, but can also be protected
in the memory of the sender/receiver. This allows true end- to-end protection
against bits flipping either over the wire, through gateways, in memory, over 
PCI, etc.

While T10-DIF clearly defines that over the wire protection guards are 
interleaved
into the data stream (each 512-Byte block followed by 8-byte guard), when in 
memory,
the protection guards may reside in a buffer separated from the data. Depending 
on the
application, it is usually easier to handle the data when it is contiguous.
In this case the data buffer will be of size 512xN and the protection buffer 
will
be of size 8xN (where N is the number of blocks in the transaction).

There are 3 kinds of signature handover operation:
1. Take unprotected data (from wire or memory) and ADD protection
guards.
2. Take protetected data (from wire or memory), validate the data
integrity against the protection guards and STRIP the protection
guards.
3. Take protected data (from wire or memory), validate the data
integrity against the protection guards and PASS the data with
the guards as-is.

This translates to defining to the HCA how/if data protection exists in memory 
domain,
and how/if data protection exists is wire domain.

The way that data integrity is performed is by using a new kind of memory
region: signature-enabled MR, and a new kind of work request: REG_SIG_MR.
The REG_SIG_MR WR operates on the signature-enabled MR, and defines all the
needed information for the signature handover (data buffer, protection buffer
if needed and signature attributes). The result is an MR that can be used for
data transfer as usual, that will also add/validate/strip/pass protection 
guards.

When the data transfer is successfully completed, it does not mean that there 
are
no integrity errors. The user must afterwards check the signature status of the
handover operation using a new light-weight verb.

This feature shall be used in storage upper layer protocols iSER/SRP 
implementing
end-to-end data integrity T10-DIF. Following this patchset, ib_iser/ib_isert
will use these verbs for T10-PI offload support.

Patchset summary:
- Intoduce verbs for create/destroy memory regions supporting signature.
- Introduce IB core signature verbs API.
- Implement mr create/destroy verbs in mlx5 driver.
- Preperation patches for signature support in mlx5 driver.
- Implement signature handover work request in mlx5 driver.
- Implement signature error collection and handling in mlx5 driver.

Changes from v4:
- Update to for-next 3.14-rc2

Series applied to target-pending/rdma-dif, minus the missing upstream
check in patch #5.

Thanks Sagi!

--nab


Thanks Nic.
--
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 v5 00/10] Introduce Signature feature

2014-02-23 Thread Sagi Grimberg
This patchset Introduces Verbs level support for signature handover feature.
Siganture is intended to implement end-to-end data integrity on a transactional
basis in a completely offloaded manner.

There are several end-to-end data integrity methods used today in various
applications and/or upper layer protocols such as T10-DIF defined by SCSI
specifications (SBC), CRC32, XOR8 and more. This patchset adds verbs support
only for T10-DIF. The proposed framework allows adding more signature methods
in the future.

In T10-DIF, when a series of 512-byte data blocks are transferred, each block
is followed by an 8-byte guard (note that other protection intervals may be used
other then 512-bytes). The guard consists of CRC that protects the integrity of
the data in the block, and tag that protects against mis-directed IOs and a free
tag for application use.

Data can be protected when transferred over the wire, but can also be protected
in the memory of the sender/receiver. This allows true end- to-end protection
against bits flipping either over the wire, through gateways, in memory, over 
PCI, etc.

While T10-DIF clearly defines that over the wire protection guards are 
interleaved
into the data stream (each 512-Byte block followed by 8-byte guard), when in 
memory,
the protection guards may reside in a buffer separated from the data. Depending 
on the
application, it is usually easier to handle the data when it is contiguous.
In this case the data buffer will be of size 512xN and the protection buffer 
will
be of size 8xN (where N is the number of blocks in the transaction).

There are 3 kinds of signature handover operation:
1. Take unprotected data (from wire or memory) and ADD protection
   guards.
2. Take protetected data (from wire or memory), validate the data
   integrity against the protection guards and STRIP the protection
   guards.
3. Take protected data (from wire or memory), validate the data
   integrity against the protection guards and PASS the data with
   the guards as-is.

This translates to defining to the HCA how/if data protection exists in memory 
domain,
and how/if data protection exists is wire domain.

The way that data integrity is performed is by using a new kind of memory
region: signature-enabled MR, and a new kind of work request: REG_SIG_MR.
The REG_SIG_MR WR operates on the signature-enabled MR, and defines all the
needed information for the signature handover (data buffer, protection buffer
if needed and signature attributes). The result is an MR that can be used for
data transfer as usual, that will also add/validate/strip/pass protection 
guards.

When the data transfer is successfully completed, it does not mean that there 
are
no integrity errors. The user must afterwards check the signature status of the
handover operation using a new light-weight verb.

This feature shall be used in storage upper layer protocols iSER/SRP 
implementing
end-to-end data integrity T10-DIF. Following this patchset, ib_iser/ib_isert
will use these verbs for T10-PI offload support.

Patchset summary:
- Intoduce verbs for create/destroy memory regions supporting signature.
- Introduce IB core signature verbs API.
- Implement mr create/destroy verbs in mlx5 driver.
- Preperation patches for signature support in mlx5 driver.
- Implement signature handover work request in mlx5 driver.
- Implement signature error collection and handling in mlx5 driver.

Changes from v4:
- Update to for-next 3.14-rc2
- IB/mlx5: Fix smatch warning.
- IB/mlx5: Fix signature rules constants.
- IB/mlx5: Allow create_qp to receive sig_enable create flag.
- IB/mlx5: Remove redundant asignment.

Changes from v3 (mostly bug fixes):
- IB/core: Generalized ib_check_sig_status to a general ib_check_mr_status
   for other light-weight status checks that may be used on ib_mr.
- IB/core: Changed ib_sig_err to inform only expected and actual values
   of the corrupted field (block guard, reference tag or application 
tag).
- IB/mlx5: Fail un-supported protection intervals.
- IB/mlx5: Fxied possible SQ corruption in REG_SIG_MR.
- IB/mlx5: Fixed wr iterator wrong incrementation in mlx5_ib_post_send.
- IB/mlx5: Avoid expanding the SQ depth for signature when wqe_size is
   sufficient.

Changes from v2 (mostly CR comments):
- IB/core: Added comment on IB_T10DIF_CRC/CSUM declarations.
- IB/core: Renamed block_size as pi_interval in ib_sig_attrs.
- IB/core: Took t10_dif domain out of sig union (ib_sig_domain).
- IB/mlx5: Fixed memory leak in create_mr
- IB/mlx5: Remove redundant assignment in WQE initialization.
- IB/mlx5: Fixed possible NULL dereference in check_sig_status
   and set_sig_wr.
- IB/mlx5: Added helper function to convert mkey to base key.
- IB/mlx5: Reduced Fencing in compund REG_SIG_MR WR.
- Resolved checkpatch warnings.

Changes from v1:
- IB/core: Reduced sizeof ib_send_wr by using wr-sg_list for data
   and dedicated ib_sge for protection guards buffer.
   Currently 

Re: [PATCH v5 00/10] Introduce Signature feature

2014-02-23 Thread Nicholas A. Bellinger
On Sun, 2014-02-23 at 14:19 +0200, Sagi Grimberg wrote:
 This patchset Introduces Verbs level support for signature handover feature.
 Siganture is intended to implement end-to-end data integrity on a 
 transactional
 basis in a completely offloaded manner.
 
 There are several end-to-end data integrity methods used today in various
 applications and/or upper layer protocols such as T10-DIF defined by SCSI
 specifications (SBC), CRC32, XOR8 and more. This patchset adds verbs support
 only for T10-DIF. The proposed framework allows adding more signature methods
 in the future.
 
 In T10-DIF, when a series of 512-byte data blocks are transferred, each block
 is followed by an 8-byte guard (note that other protection intervals may be 
 used
 other then 512-bytes). The guard consists of CRC that protects the integrity 
 of
 the data in the block, and tag that protects against mis-directed IOs and a 
 free
 tag for application use.
 
 Data can be protected when transferred over the wire, but can also be 
 protected
 in the memory of the sender/receiver. This allows true end- to-end protection
 against bits flipping either over the wire, through gateways, in memory, over 
 PCI, etc.
 
 While T10-DIF clearly defines that over the wire protection guards are 
 interleaved
 into the data stream (each 512-Byte block followed by 8-byte guard), when in 
 memory,
 the protection guards may reside in a buffer separated from the data. 
 Depending on the
 application, it is usually easier to handle the data when it is contiguous.
 In this case the data buffer will be of size 512xN and the protection buffer 
 will
 be of size 8xN (where N is the number of blocks in the transaction).
 
 There are 3 kinds of signature handover operation:
 1. Take unprotected data (from wire or memory) and ADD protection
guards.
 2. Take protetected data (from wire or memory), validate the data
integrity against the protection guards and STRIP the protection
guards.
 3. Take protected data (from wire or memory), validate the data
integrity against the protection guards and PASS the data with
the guards as-is.
 
 This translates to defining to the HCA how/if data protection exists in 
 memory domain,
 and how/if data protection exists is wire domain.
 
 The way that data integrity is performed is by using a new kind of memory
 region: signature-enabled MR, and a new kind of work request: REG_SIG_MR.
 The REG_SIG_MR WR operates on the signature-enabled MR, and defines all the
 needed information for the signature handover (data buffer, protection buffer
 if needed and signature attributes). The result is an MR that can be used for
 data transfer as usual, that will also add/validate/strip/pass protection 
 guards.
 
 When the data transfer is successfully completed, it does not mean that there 
 are
 no integrity errors. The user must afterwards check the signature status of 
 the
 handover operation using a new light-weight verb.
 
 This feature shall be used in storage upper layer protocols iSER/SRP 
 implementing
 end-to-end data integrity T10-DIF. Following this patchset, ib_iser/ib_isert
 will use these verbs for T10-PI offload support.
 
 Patchset summary:
 - Intoduce verbs for create/destroy memory regions supporting signature.
 - Introduce IB core signature verbs API.
 - Implement mr create/destroy verbs in mlx5 driver.
 - Preperation patches for signature support in mlx5 driver.
 - Implement signature handover work request in mlx5 driver.
 - Implement signature error collection and handling in mlx5 driver.
 
 Changes from v4:
 - Update to for-next 3.14-rc2

Series applied to target-pending/rdma-dif, minus the missing upstream
check in patch #5.

Thanks Sagi!

--nab

--
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