Re: [PATCH 2/2 2.6.29] cxgb3i - accelerating open-iscsi initiator

2008-12-09 Thread Boaz Harrosh

Mike Christie wrote:
 Boaz Harrosh wrote:
 
 +   if (!ddp) {
 +   ddp_log_warn(%s unable to alloc ddp 0x%d, ddp disabled.\n,
 +tdev-name, ppmax);
 +   return 0;
 +   }
 +   ddp-gl_map = (struct cxgb3i_gather_list **)(ddp + 1);
 +   ddp-gl_skb = (struct sk_buff **)(((char *)ddp-gl_map) +
 + ppmax *
 + sizeof(struct cxgb3i_gather_list *));
 +   spin_lock_init(ddp-map_lock);
 +
 +   ddp-tdev = tdev;
 +   ddp-pdev = uinfo.pdev;
 +   ddp-max_txsz = min_t(unsigned int, uinfo.max_txsz, ULP2_MAX_PKT_SIZE);
 +   ddp-max_rxsz = min_t(unsigned int, uinfo.max_rxsz, ULP2_MAX_PKT_SIZE);
 Please note that from what I understand, only the out-going headers can be
 big, like iscsi_cmd header. But the in-coming headers are always 
 size_of(struct iscsi_hdr).
 So there is no symmetry here. Actually only iscsi_cmd can get big, the other 
 out-going 
 data packets are with small headers, but I guess that is an open-iscsi 
 limitation.

 Mike correct me if I'm wrong?
 
 Yeah, correct. Other iscsi pdus like tmfs and scsi data out could have 
 ahs but we do not support it. You did the code, so I assumed you only 
 did what you needed and could test.
 
 On the recv side, I thought scsi cmd response or scsi data-in could have 
 ahses, but libiscsi_tcp/libiscsi just reads as much as it can in and 
 does not process it (actually it only reads in as much buffer space as 
 it has then fails if we overflow). I believe Olaf did this code to be 
 complete as he could with the existing code for the future, and to make 
 sure his abstraction would work for ahs if we needed it.

Yes you are right. I guess none of the targets send AHSs since otherwise
our initiator would fail on that particular packet.

It's time for me to go and look at why would a target send one.
It might be better to make a small enhancement and jump over
iscsi_hdr-hlength just throwing the data away, as a matter of error handling.
Better then failing the command completely.
That is if the AHS is just optional information.

I'll look into it.

Thanks
Boaz

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



RE: [PATCH 2/2 2.6.29] cxgb3i - accelerating open-iscsi initiator

2008-12-09 Thread Karen Xie


-Original Message-
From: Boaz Harrosh [mailto:[EMAIL PROTECTED] 
Subject: Re: [PATCH 2/2 2.6.29] cxgb3i - accelerating open-iscsi initiator

 +3. edit /etc/iscsi/iscsid.conf
 +   The default setting for MaxRecvDataSegmentLength (131072) is too big, 
 search
 +   and replace all occurances of xxx.iscsi.MaxRecvDataSegmentLength to be a
 +   value no bigger than 15360 (for example 8192):
 +
 + discovery.sendtargets.iscsi.MaxRecvDataSegmentLength = 8192
 + node.conn[0].iscsi.MaxRecvDataSegmentLength = 8192
 +

What happens if you don't do this? it will then not use acceleration and fall 
back
to SW tcp? or it will not work at all? Can the driver limit that no matter what 
the user
put?

[Karen] If node.conn[0].iscsi.MaxRecvDataSegmentLength is not changed, the 
login will fail and a message will be printed to dmesg in the format of 
cxgb3i: ERR! MaxRecvSegmentLength X too big. Need to be = Y. The 
discovery session is not accelerated, so it will go through. I will update the 
.txt file.

 +config SCSI_CXGB3_ISCSI
 + tristate Chelsio S3xx iSCSI support
 + select CHELSIO_T3
 + select SCSI_ISCSI_ATTRS
 + select ISCSI_TCP

Looks like you don't need ISCSI_TCP since Mike's last changes. ISCSI_TCP will
give you iscsi_tcp.ko, but all you need is libiscsi_tcp.ko which will be enabled
by default. (I think)

[Karen] Will remove select ISCSI_TCP.

 + ---help---
 + This driver supports iSCSI offload for the Chelsio S3 series devices.
 diff --git a/drivers/scsi/cxgb3i/Makefile b/drivers/scsi/cxgb3i/Makefile

 new file mode 100644
 index 000..ee7d6d2
 --- /dev/null
 +++ b/drivers/scsi/cxgb3i/Makefile
 @@ -0,0 +1,4 @@
 +EXTRA_CFLAGS += -I$(TOPDIR)/drivers/net/cxgb3
 +

+ccflags-y += -I$(TOPDIR)/drivers/net/cxgb3

Make Sam's life easier.

It's better to put this in a Kbuild file. It is not a real Makefile anyway.

[Karen] Will rename Makefile to Kbuild.

 +#define ISCSI_PDU_HEADER_MAX (56 + 256) /* bhs + digests + ahs */
 +

Actually sizeof(iscsi_sw_tcp_hdrbuf), which is ? I'm not sure a bit
less I think like 308. 
or:
+#define ISCSI_PDU_HEADER_MAX \
+   sizeof(struct iscsi_hdr) + ISCSI_MAX_AHS_SIZE + ISCSI_DIGEST_SIZE;

[Karen] Will use defines instead of actual numbers.

 +
 + ddp = cxgb3i_alloc_big_mem(sizeof(struct cxgb3i_ddp_info) +
 +ppmax *
 + (sizeof(struct cxgb3i_gather_list *) +
 + sizeof(struct sk_buff *)),
 +GFP_KERNEL);

From what I understand so far:
DDP is only used for read of PDU, right? so we only need space for the read DDP.
the writes are made by regular SKBs right?

How is then the acceleration works with out-going data? OK I guess that is a 
separate
issue. DDP acceleration for reads, and separately, digest accelerations for 
both.

[Karen] Yes, ddp is for read only, the write will have digest acceleration.

 + if (!ddp) {
 + ddp_log_warn(%s unable to alloc ddp 0x%d, ddp disabled.\n,
 +  tdev-name, ppmax);
 + return 0;
 + }
 + ddp-gl_map = (struct cxgb3i_gather_list **)(ddp + 1);
 + ddp-gl_skb = (struct sk_buff **)(((char *)ddp-gl_map) +
 +   ppmax *
 +   sizeof(struct cxgb3i_gather_list *));
 + spin_lock_init(ddp-map_lock);
 +
 + ddp-tdev = tdev;
 + ddp-pdev = uinfo.pdev;
 + ddp-max_txsz = min_t(unsigned int, uinfo.max_txsz, ULP2_MAX_PKT_SIZE);
 + ddp-max_rxsz = min_t(unsigned int, uinfo.max_rxsz, ULP2_MAX_PKT_SIZE);

Please note that from what I understand, only the out-going headers can be
big, like iscsi_cmd header. But the in-coming headers are always size_of(struct 
iscsi_hdr).
So there is no symmetry here. Actually only iscsi_cmd can get big, the other 
out-going 
data packets are with small headers, but I guess that is an open-iscsi 
limitation.

[Karen] Here the max size is the packet size (i.e., the total pdu size). The 
h/w does support sending and recv AHS. I will make sure the code is consistent 
in handling of AHS.

Mike correct me if I'm wrong?

 +/**
 + * cxgb3i_reserve_itt - generate tag for a give task
 + * Try to set up ddp for a scsi read task.
 + * @task: iscsi task
 + * @hdr_itt: tag, filled in by this function
 + */
 +int cxgb3i_reserve_itt(struct iscsi_task *task, itt_t *hdr_itt)
 +{
 + struct scsi_cmnd *sc = task-sc;
 + struct iscsi_conn *conn = task-conn;
 + struct iscsi_session *sess = conn-session;
 + struct iscsi_tcp_conn *tcp_conn = conn-dd_data;
 + struct cxgb3i_conn *cconn = tcp_conn-dd_data;
 + struct cxgb3i_adapter *snic = cconn-hba-snic;
 + struct cxgb3i_tag_format *tformat = snic-tag_format;
 + u32 sw_tag = (sess-age  cconn-task_idx_bits) | task-itt;
 + u32 tag;
 + int err = -EINVAL;
 +
 + if (sc  (sc-sc_data_direction == DMA_FROM_DEVICE) 
 + cxgb3i_sw_tag_usable(tformat, sw_tag

Re: [PATCH 2/2 2.6.29] cxgb3i - accelerating open-iscsi initiator

2008-12-08 Thread Mike Christie

Karen Xie wrote:
 [PATCH 2/2 2.6.29] cxgb3i - accelerating open-iscsi initiator
 
 From: Karen Xie [EMAIL PROTECTED] 
 
 Add cxgb3i iSCSI driver.
 
 This patch implements the cxgb3i iscsi connection acceleration for the
 open-iscsi initiator.
 
 The cxgb3i driver offers the iscsi PDU based offload:
 - digest insertion and verification
 - payload direct-placement into host memory buffer.
 
 Signed-off-by: Karen Xie [EMAIL PROTECTED]

There are a couple issues with using network functions instead of 
defining your own (I guess some are not widespread because other code is 
not using it yet) and a function that is no longer used (cxgb3 net 
driver function replaced by common iscsi functionality), but they are 
small and can probably be fixed up after the merge with a simple patch 
(I can submit it after it is merged if you guys want).

The iscsi hook in parts look ok.

Acked-by: Mike Christie [EMAIL PROTECTED]

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---