Re: [PATCH v2 03/16] iscsi-target: add int (*iscsit_xmit_datain_pdu)()

2016-04-11 Thread Varun Prakash 
On Sun, Apr 10, 2016 at 08:38:03PM +0300, Sagi Grimberg wrote:
> 
> 
> On 09/04/16 16:11, Varun Prakash wrote:
> >Add int (*iscsit_xmit_datain_pdu)() to
> >struct iscsit_transport, iscsi-target
> >uses this callback to transmit a DATAIN
> >iSCSI PDU.
> >
> >Signed-off-by: Varun Prakash 
> >---
> >  drivers/target/iscsi/iscsi_target.c| 143 
> > +++--
> >  include/target/iscsi/iscsi_transport.h |   3 +
> >  2 files changed, 86 insertions(+), 60 deletions(-)
> >
> >diff --git a/drivers/target/iscsi/iscsi_target.c 
> >b/drivers/target/iscsi/iscsi_target.c
> >index 0e7a481..9e65e5d 100644
> >--- a/drivers/target/iscsi/iscsi_target.c
> >+++ b/drivers/target/iscsi/iscsi_target.c
> >@@ -577,6 +577,84 @@ static int iscsit_xmit_pdu(struct iscsi_conn *conn, 
> >struct iscsi_cmd *cmd,
> > return 0;
> >  }
> >
> >+static int iscsit_map_iovec(struct iscsi_cmd *, struct kvec *, u32, u32);
> >+static void iscsit_unmap_iovec(struct iscsi_cmd *);
> >+static u32 iscsit_do_crypto_hash_sg(struct ahash_request *, struct 
> >iscsi_cmd *,
> >+u32, u32, u32, u8 *);
> >+static int
> >+iscsit_xmit_datain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
> >+   struct iscsi_datain_req *dr, struct iscsi_datain *datain)
> 
> Looks very similar to xmit_pdu(), if we add a datain pointer that
> can be null for normal pdus would that reduce code duplication?

Yes, we can have a common xmit function for all pdus -

int iscsit_xmit_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
struct iscsi_datain_req *dr, const void *buf, u32 buf_len);

dr will be NULL for non datain pdus and buf will point to data buffer.
For datain pdus buf will point to datain(struct iscsi_datain) buffer, buf_len 
will be zero.

I will add this in -v3, thanks.
--
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

Re: [PATCH v2 03/16] iscsi-target: add int (*iscsit_xmit_datain_pdu)()

2016-04-10 Thread Sagi Grimberg 



On 09/04/16 16:11, Varun Prakash wrote:

Add int (*iscsit_xmit_datain_pdu)() to
struct iscsit_transport, iscsi-target
uses this callback to transmit a DATAIN
iSCSI PDU.

Signed-off-by: Varun Prakash 
---
  drivers/target/iscsi/iscsi_target.c| 143 +++--
  include/target/iscsi/iscsi_transport.h |   3 +
  2 files changed, 86 insertions(+), 60 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 0e7a481..9e65e5d 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -577,6 +577,84 @@ static int iscsit_xmit_pdu(struct iscsi_conn *conn, struct 
iscsi_cmd *cmd,
return 0;
  }

+static int iscsit_map_iovec(struct iscsi_cmd *, struct kvec *, u32, u32);
+static void iscsit_unmap_iovec(struct iscsi_cmd *);
+static u32 iscsit_do_crypto_hash_sg(struct ahash_request *, struct iscsi_cmd *,
+   u32, u32, u32, u8 *);
+static int
+iscsit_xmit_datain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
+  struct iscsi_datain_req *dr, struct iscsi_datain *datain)


Looks very similar to xmit_pdu(), if we add a datain pointer that
can be null for normal pdus would that reduce code duplication?
--
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

[PATCH v2 03/16] iscsi-target: add int (*iscsit_xmit_datain_pdu)()

2016-04-09 Thread Varun Prakash 
Add int (*iscsit_xmit_datain_pdu)() to
struct iscsit_transport, iscsi-target
uses this callback to transmit a DATAIN
iSCSI PDU.

Signed-off-by: Varun Prakash 
---
 drivers/target/iscsi/iscsi_target.c| 143 +++--
 include/target/iscsi/iscsi_transport.h |   3 +
 2 files changed, 86 insertions(+), 60 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 0e7a481..9e65e5d 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -577,6 +577,84 @@ static int iscsit_xmit_pdu(struct iscsi_conn *conn, struct 
iscsi_cmd *cmd,
return 0;
 }
 
+static int iscsit_map_iovec(struct iscsi_cmd *, struct kvec *, u32, u32);
+static void iscsit_unmap_iovec(struct iscsi_cmd *);
+static u32 iscsit_do_crypto_hash_sg(struct ahash_request *, struct iscsi_cmd *,
+   u32, u32, u32, u8 *);
+static int
+iscsit_xmit_datain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
+  struct iscsi_datain_req *dr, struct iscsi_datain *datain)
+{
+   struct kvec *iov;
+   u32 iov_count = 0, tx_size = 0;
+   int ret, iov_ret;
+
+   iov = >iov_data[0];
+   iov[iov_count].iov_base = cmd->pdu;
+   iov[iov_count++].iov_len = ISCSI_HDR_LEN;
+   tx_size += ISCSI_HDR_LEN;
+
+   if (conn->conn_ops->HeaderDigest) {
+   u32 *header_digest = (u32 *)>pdu[ISCSI_HDR_LEN];
+
+   iscsit_do_crypto_hash_buf(conn->conn_tx_hash, cmd->pdu,
+ ISCSI_HDR_LEN, 0, NULL,
+ (u8 *)header_digest);
+
+   iov[0].iov_len += ISCSI_CRC_LEN;
+   tx_size += ISCSI_CRC_LEN;
+
+   pr_debug("Attaching CRC32 HeaderDigest for DataIN PDU 0x%08x\n",
+*header_digest);
+   }
+
+   iov_ret = iscsit_map_iovec(cmd, >iov_data[1],
+  datain->offset, datain->length);
+   if (iov_ret < 0)
+   return -1;
+
+   iov_count += iov_ret;
+   tx_size += datain->length;
+
+   cmd->padding = ((-datain->length) & 3);
+   if (cmd->padding) {
+   iov[iov_count].iov_base = cmd->pad_bytes;
+   iov[iov_count++].iov_len= cmd->padding;
+   tx_size += cmd->padding;
+
+   pr_debug("Attaching %u padding bytes\n", cmd->padding);
+   }
+
+   if (conn->conn_ops->DataDigest) {
+   cmd->data_crc = iscsit_do_crypto_hash_sg(conn->conn_tx_hash,
+cmd, datain->offset,
+datain->length,
+cmd->padding,
+cmd->pad_bytes);
+
+   iov[iov_count].iov_base = >data_crc;
+   iov[iov_count++].iov_len = ISCSI_CRC_LEN;
+   tx_size += ISCSI_CRC_LEN;
+
+   pr_debug("Attached CRC32C DataDigest %d bytes, crc 0x%08x\n",
+datain->length + cmd->padding, cmd->data_crc);
+   }
+
+   cmd->iov_data_count = iov_count;
+   cmd->tx_size = tx_size;
+
+   ret = iscsit_fe_sendpage_sg(cmd, conn);
+
+   iscsit_unmap_iovec(cmd);
+
+   if (ret < 0) {
+   iscsit_tx_thread_wait_for_tcp(conn);
+   return ret;
+   }
+
+   return 0;
+}
+
 static enum target_prot_op iscsit_get_sup_prot_ops(struct iscsi_conn *conn)
 {
return TARGET_PROT_NORMAL;
@@ -599,6 +677,7 @@ static struct iscsit_transport iscsi_target_transport = {
.iscsit_aborted_task= iscsit_aborted_task,
.iscsit_alloc_pdu   = iscsit_alloc_pdu,
.iscsit_xmit_pdu= iscsit_xmit_pdu,
+   .iscsit_xmit_datain_pdu = iscsit_xmit_datain_pdu,
.iscsit_get_sup_prot_ops = iscsit_get_sup_prot_ops,
 };
 
@@ -2701,9 +2780,7 @@ static int iscsit_send_datain(struct iscsi_cmd *cmd, 
struct iscsi_conn *conn)
struct iscsi_data_rsp *hdr;
struct iscsi_datain datain;
struct iscsi_datain_req *dr;
-   struct kvec *iov;
-   u32 iov_count = 0, tx_size = 0;
-   int eodr = 0, ret, iov_ret;
+   int eodr = 0, ret;
bool set_statsn = false;
 
memset(, 0, sizeof(struct iscsi_datain));
@@ -2749,64 +2826,10 @@ static int iscsit_send_datain(struct iscsi_cmd *cmd, 
struct iscsi_conn *conn)
 
iscsit_build_datain_pdu(cmd, conn, , hdr, set_statsn);
 
-   iov = >iov_data[0];
-   iov[iov_count].iov_base = cmd->pdu;
-   iov[iov_count++].iov_len= ISCSI_HDR_LEN;
-   tx_size += ISCSI_HDR_LEN;
-
-   if (conn->conn_ops->HeaderDigest) {
-   u32 *header_digest = (u32 *)>pdu[ISCSI_HDR_LEN];
-
-   iscsit_do_crypto_hash_buf(conn->conn_tx_hash, cmd->pdu,
-   ISCSI_HDR_LEN, 0, NULL,