Re: [PATCH v2 08/16] iscsi-target: add void (*iscsit_get_r2t_ttt)()

[Varun Prakash]

On Wed, Apr 13, 2016 at 12:52:11PM +0300, Sagi Grimberg wrote:
> 
> >>>Add void (*iscsit_get_r2t_ttt)() to
> >>>struct iscsit_transport, iscsi-target
> >>>uses this callback to get
> >>>r2t->targ_xfer_tag.
> >>
> >>Your driver allocates ttt's? That looks like bad
> >>layering to me. This definitely deserves an explanation...
> >
> >cxgbit.ko allocates ttt only for r2t pdus to do Direct Data
> >Placement of Data Out pdus, adapter uses the ttt value in
> >Data Out pdus to place data directly in the host buffers.
> 
> How do you guarantee that the core doesn't conflict with
> your internal ttt's?

- iscsi-target does not make any decision based on  
  r2t->targ_xfer_tag.

- From iscsi RFC https://tools.ietf.org/html/rfc7143#section-11.8.5
  There is no protocol rule about the Target Transfer Tag
  except that the value 0x is reserved and MUST NOT
  be sent by a target in an R2T.

- T5 adapter checks ttt only for Data Out pdus, so even
  if it is same for other pdus it does not matter.
--
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 08/16] iscsi-target: add void (*iscsit_get_r2t_ttt)()

[Sagi Grimberg]

Add void (*iscsit_get_r2t_ttt)() to
struct iscsit_transport, iscsi-target
uses this callback to get
r2t->targ_xfer_tag.


Your driver allocates ttt's? That looks like bad
layering to me. This definitely deserves an explanation...


cxgbit.ko allocates ttt only for r2t pdus to do Direct Data
Placement of Data Out pdus, adapter uses the ttt value in
Data Out pdus to place data directly in the host buffers.


How do you guarantee that the core doesn't conflict with
your internal ttt's?
--
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 08/16] iscsi-target: add void (*iscsit_get_r2t_ttt)()

[Varun Prakash]

On Sun, Apr 10, 2016 at 08:51:44PM +0300, Sagi Grimberg wrote:
> 
> >Add void (*iscsit_get_r2t_ttt)() to
> >struct iscsit_transport, iscsi-target
> >uses this callback to get
> >r2t->targ_xfer_tag.
> 
> Your driver allocates ttt's? That looks like bad
> layering to me. This definitely deserves an explanation...

cxgbit.ko allocates ttt only for r2t pdus to do Direct Data
Placement of Data Out pdus, adapter uses the ttt value in
Data Out pdus to place data directly in the host buffers.
--
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 08/16] iscsi-target: add void (*iscsit_get_r2t_ttt)()

[Sagi Grimberg]

Add void (*iscsit_get_r2t_ttt)() to
struct iscsit_transport, iscsi-target
uses this callback to get
r2t->targ_xfer_tag.


Your driver allocates ttt's? That looks like bad
layering to me. This definitely deserves an explanation...
--
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 08/16] iscsi-target: add void (*iscsit_get_r2t_ttt)()

[Varun Prakash]

Add void (*iscsit_get_r2t_ttt)() to
struct iscsit_transport, iscsi-target
uses this callback to get
r2t->targ_xfer_tag.

Signed-off-by: Varun Prakash 
---
 drivers/target/iscsi/iscsi_target.c| 5 -
 include/target/iscsi/iscsi_transport.h | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 98d96d4..ec785c8 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3060,7 +3060,10 @@ static int iscsit_send_r2t(
int_to_scsilun(cmd->se_cmd.orig_fe_lun,
(struct scsi_lun *)>lun);
hdr->itt= cmd->init_task_tag;
-   r2t->targ_xfer_tag  = session_get_next_ttt(conn->sess);
+   if (conn->conn_transport->iscsit_get_r2t_ttt)
+   conn->conn_transport->iscsit_get_r2t_ttt(conn, cmd, r2t);
+   else
+   r2t->targ_xfer_tag = session_get_next_ttt(conn->sess);
hdr->ttt= cpu_to_be32(r2t->targ_xfer_tag);
hdr->statsn = cpu_to_be32(conn->stat_sn);
hdr->exp_cmdsn  = cpu_to_be32(conn->sess->exp_cmd_sn);
diff --git a/include/target/iscsi/iscsi_transport.h 
b/include/target/iscsi/iscsi_transport.h
index dbef892..f949e70 100644
--- a/include/target/iscsi/iscsi_transport.h
+++ b/include/target/iscsi/iscsi_transport.h
@@ -32,6 +32,8 @@ struct iscsit_transport {
void (*iscsit_release_cmd)(struct iscsi_conn *, struct iscsi_cmd *);
void (*iscsit_get_rx_pdu)(struct iscsi_conn *);
int (*iscsit_validate_params)(struct iscsi_conn *);
+   void (*iscsit_get_r2t_ttt)(struct iscsi_conn *, struct iscsi_cmd *,
+  struct iscsi_r2t *);
enum target_prot_op (*iscsit_get_sup_prot_ops)(struct iscsi_conn *);
 };
 
-- 
2.0.2

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