Re: open-iscsi init script on suse

2008-10-06 Thread Eli Dorfman

> For discovery though it seems like you can just do discovery and tell
> iscsiadm not to overwrite the existing db (just add new ones) and that
> would solve some of the issues with iser records getting wacked.
>
How do we tell iscsiadm not to overwrite the existing db?
Also maybe i missed something but how does re-discover (in the initd.suse) help
when login returns EHOSTNOTREACH.

Eli.

--~--~-~--~~~---~--~~
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: open-iscsi init script on suse

2008-09-24 Thread Eli Dorfman

On Tue, Sep 23, 2008 at 8:04 PM, Mike Christie <[EMAIL PROTECTED]> wrote:
>
> Doron Shoham wrote:
>> Hi,
>>
>> Why does the init script on suse re-discovers all iscsi targets which were 
>> set
>> to automatic login?
>> To avoid deadlocks on the root fs there is patch which limits the number of 
>> retries on first login.
>> When doing so, it sets back all the default parameters (overriding any user 
>> definitions).
>> I think it should be like in redhat - just login to all the targets which 
>> are automatic.
>>
>> Another issue is that the script logouts only from automatic nodes (not from 
>> all nodes as in redhat).
>> This causes a bug, when iscsi is stopped while manual node is still 
>> logged-in (session is active).
>> The result is that iscsid is down but session is still alive - iscsiadm -m 
>> session shows this stale session.
>> I suggest that we do the same as redhat, any objections?
>>
>>
>> Also, what is the purpose of "node.startup" parameter?
>> When is it in use?
>>
>
> node.startup should be renamed record.startup. The possible values are
> automatic, manual and onboot. When the init scripts start they can run
> over the the db and check which records that the users has requested
> autoatmic startup for and login at that time.
>
> onboot is used to for the session used for boot/root. It just signals
> the tools to handle it differently. During shutdown for example we
> cannot kill that session when the init script stop is done, because it
> is still needed for root.
>
> manual is used because a lot of targets will return all the portals on
> the target. Some of these portals may be disabled or not even connected
> to the network. Instead of iscsiadm/iscsid wasting time trying to log in
> admins can mark them as manual and the init scripts will not auto start
> them. Why not just delete them of they cannot be used? I do not know.
>
The question is why there are two node.startup fields and what is the
difference between them (if any):
node.startup
AND
node.conn[0].startup

Thanks,
Eli

--~--~-~--~~~---~--~~
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: open-iscsi init script on suse

2008-09-22 Thread Eli Dorfman

On Mon, Sep 22, 2008 at 9:39 AM, Hannes Reinecke <[EMAIL PROTECTED]> wrote:
>
> Hi Doron,
>
> Doron Shoham wrote:
>> Doron Shoham wrote:
>>> Hi,
>>>
>>> Why does the init script on suse re-discovers all iscsi targets which were 
>>> set
>>> to automatic login?
>>> To avoid deadlocks on the root fs there is patch which limits the number of 
>>> retries on first login.
>>> When doing so, it sets back all the default parameters (overriding any user 
>>> definitions).
>>> I think it should be like in redhat - just login to all the targets which 
>>> are automatic.
>>>
> That's what we tried initially. However, certain switches take quite a bit of 
> time for the Spanning-Tree
> Protocol to work out the route, during which time any connect() attempt 
> returns with -EHOSTUNREACH.
> If we do an automatic login, the login request is sent from the kernel 
> directly. And any connect()
> failure from the kernel is taken as a terminal error, hence the login fails.
> The best we can do here is to make this re-discovery conditional, which would 
> allow customers not
> suffering from STP failures to get a faster booting time.

Current implementation only partially solves the issue, but creates
another problem instead - node parameters are changed.
What if first login will ignore this error and and retry anyway - this
is not the cleanest solution but it will satisfy both requirements.

>
>>> Another issue is that the script logouts only from automatic nodes (not 
>>> from all nodes as in redhat).
>>> This causes a bug, when iscsi is stopped while manual node is still 
>>> logged-in (session is active).
>>> The result is that iscsid is down but session is still alive - iscsiadm -m 
>>> session shows this stale session.
>>> I suggest that we do the same as redhat, any objections?
>>>
> Ouch. You touched a very complicated topic. I've had long discussions and 
> patches with NetApp on
> how to get iscsi shutdown right. It's not only that we have stale nodes 
> (which would be ok, given
> that we're shutting down anyway), but it's also well possible that some 
> crucial filesystem bits
> are in fact served by iSCSI, so we definitely shouldn'd be shutting them 
> down, regardless of any
> automatic settings.

Having stale nodes is not ok, since we may use "iscsi stop" not only
when machine shutdowns
but also to change node parameters (e.g. node_transport set to iser).
The dependency of filesystem with iscsi should be resolved
independently by the user.
This applies both for automatic and manual sessions.
What we suggest is to logout all nodes (and not only automatic).

>
> There's a bugzilla open to get this right (Novell bug#392080), you're welcome 
> to join and get
> this sorted out.
I could not find this bug, please send a link.


Thanks,
Eli

--~--~-~--~~~---~--~~
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: Design Questions

2008-07-02 Thread Eli Dorfman

> open-iscsi does not really support link local ipv6 adders yet. There is
> a way around this by setting up a iface for the net inerface (ethX) that
> it is local to then binding it, but it is a little complicated to do.
>
Can you explain in details how to do that.
When do you think ipv6 will be supported? what needs to be done?

Thanks,
Eli

--~--~-~--~~~---~--~~
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] change iscsi discovery on suse's initd

2008-06-01 Thread Eli Dorfman

> Hi Amir,
>
> Mike Christie wrote:
>> Amir Mehler wrote:
>>> Amir Mehler wrote:
>>>
 Hello,

 On open-iscsi, from April 2008 and on, when performing:

>>> (on suse)
/etc/init.d/open-iscsi start
 a discovery is done in line 126:
iscsiadm -m discovery -t st -p "$TARGET_ADDR" > /dev/null 2>&1
 This line, and others, were added by Hannes Reinecke on Apr 09 2008.

 The problem with this discovery is that it modifies old session
 parameters of the nodes.
> Yes, it does with the stock open-iscsi distribution.
> For SUSE I've added a patch to change the default.

Why do we have to run discovery on every open-iscsi start?
Discovery process should be initiated by the user on demand.


Eli.

--~--~-~--~~~---~--~~
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] Divide node parameters into 3 categories

2008-06-01 Thread Eli Dorfman

>> Doron Shoham wrote:
 Hi Mike,
 Are you ok with this patch?

>>> I think so. There was just some minor things I was going to fix when I
>>> merged it . For exmaples the timer values are not picked up right away
>>> so they need to be attr deffered.
>>>
>>> I am not going to merge it for the next release (the one that is in rc
>>> now), because some apps (installers and mkinitrd type of tools) are not
>>> ready for the -f flag and it is too late to change them.
>>>
>>> So I will do a branch for the next release or merge this first when I
>>> make the new release. I was just waiting to see if Hannes fixes the
>>> Makefile patch he had.
>>
>> Hi,
>>
>> Did you meant open-iscsi-2.0-869.1 as the next release?
>
> The dot releases are just bug fixes.
>
>> If not, can you estimate in what release it will be merged?
>>
>
> 870.

Isn't this patch a bug fix as well? For example, changing transport
name while session is active, leaves stale session entry.
Can't you merge it to the nearest release?
If not, when do you plan to release the 870? We would like to see if
we can make it for ofed 1.4.

Thanks,
Eli

--~--~-~--~~~---~--~~
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 1/2] IB/iSER: Do not add unsolicited data offset to VA in iSER header

2008-05-01 Thread Eli Dorfman

On Sun, Apr 27, 2008 at 3:53 PM, Eli Dorfman <[EMAIL PROTECTED]> wrote:
> iSER initiator sends a VA (in the iSER header) which includes
>  an offset for the unsolicited data (which is wrong according to the spec).
>
>  Signed-off-by: Eli Dorfman <[EMAIL PROTECTED]>
>  Signed-off-by: Erez Zilber <[EMAIL PROTECTED]>
>  ---
>   drivers/infiniband/ulp/iser/iser_initiator.c |6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)
>
>  diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c
>  b/drivers/infiniband/ulp/iser/iser_initiator.c
>  index 08dc81c..5c2bbc6 100644
>  --- a/drivers/infiniband/ulp/iser/iser_initiator.c
>  +++ b/drivers/infiniband/ulp/iser/iser_initiator.c
>  @@ -154,12 +154,12 @@ iser_prepare_write_cmd(struct iscsi_cmd_task *ctask,
> if (unsol_sz < edtl) {
> hdr->flags |= ISER_WSV;
> hdr->write_stag = cpu_to_be32(regd_buf->reg.rkey);
>  -   hdr->write_va   = cpu_to_be64(regd_buf->reg.va + unsol_sz);
>  +   hdr->write_va   = cpu_to_be64(regd_buf->reg.va);
>
> iser_dbg("Cmd itt:%d, WRITE tags, RKEY:%#.4X "
>  -"VA:%#llX + unsol:%d\n",
>  +"VA:%#llX\n",
>  ctask->itt, regd_buf->reg.rkey,
>  -(unsigned long long)regd_buf->reg.va, unsol_sz);
>  +(unsigned long long)regd_buf->reg.va);
> }
>
> if (imm_sz > 0) {
>  --
>  1.5.5
>

Please do not apply this patch until we decide how to sync this with
the target side.

Thanks,
Eli

--~--~-~--~~~---~--~~
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] IB/iSER: Use offset from r2t header for rdma

2008-05-01 Thread Eli Dorfman

On Sun, Apr 27, 2008 at 3:55 PM, Eli Dorfman <[EMAIL PROTECTED]> wrote:
> Use offset from r2t header for rdma instead of using
>  internal offset counter.
>
>  Signed-off-by: Eli Dorfman <[EMAIL PROTECTED]>
>  ---
>   usr/iscsi/iscsi_rdma.c |   16 +---
>   1 files changed, 5 insertions(+), 11 deletions(-)
>
>  diff --git a/usr/iscsi/iscsi_rdma.c b/usr/iscsi/iscsi_rdma.c
>  index d46ddff..84f5949 100644
>  --- a/usr/iscsi/iscsi_rdma.c
>  +++ b/usr/iscsi/iscsi_rdma.c
>  @@ -1447,28 +1447,22 @@ static int iscsi_rdma_rdma_read(struct
>  iscsi_connection *conn)
> struct iscsi_r2t_rsp *r2t = (struct iscsi_r2t_rsp *) &conn->rsp.bhs;
> uint8_t *buf;
> uint32_t len;
>  +   uint32_t offset;
> int ret;
>
> buf = (uint8_t *) task->data + task->offset;
> len = be32_to_cpu(r2t->data_length);
>  +   offset = be32_to_cpu(r2t->data_offset);
>
>  -   dprintf("len %u stag %x va %llx\n",
>  +   dprintf("len %u stag %x va %llx offset %x\n",
> len, itask->rem_write_stag,
>  -   (unsigned long long) itask->rem_write_va);
>  +   (unsigned long long) itask->rem_write_va, offset);
>
> ret = iser_post_rdma_wr(ci, task, buf, len, IBV_WR_RDMA_READ,
>  -   itask->rem_write_va, itask->rem_write_stag);
>  +   itask->rem_write_va + offset, 
> itask->rem_write_stag);
> if (ret < 0)
> return ret;
>
>  -   /*
>  -* Initiator registers the entire buffer, but gives us a VA that
>  -* is advanced by immediate + unsolicited data amounts.  Advance
>  -* rem_va as we read, knowing that the target always grabs segments
>  -* in order.
>  -*/
>  -   itask->rem_write_va += len;
>  -
> return 0;
>   }
>
>  --
>  1.5.5
>
Please do not apply this patch until we decide how to sync this with
the initiator side.
See the following discussion for details:
http://www.ietf.org/mail-archive/web/ips/current/msg02506.html

I tend to agree with Pete's option (3) implementing iSER HELLO message
in the initiator and target.
Then adding this patch and the corresponding initiator patch so that we have:
Old initiator working with old target, AND
New initiator working with new target.

Eli

--~--~-~--~~~---~--~~
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: [Ips] [Stgt-devel] [ofa-general] Re: Calculating the VA iniSER header

2008-05-01 Thread Eli Dorfman

On Thu, May 1, 2008 at 11:24 AM, Ken Sandars <[EMAIL PROTECTED]> wrote:
>
> >> [Ken] It appears the current Linux iSER initiator does not send the HELLO
> message when
>
> >> the connection transits to full feature phase. The stgt target also
> ignores this
> >> message (if it were to appear).
> [Ken] The IBTA document does not mention the HELLO/HELLOREPLY messages.
> Implementing this message exchange gives a distinction between the current
> implementations
> and those that will correctly calculate the write_va (as per Pete Wyckoff's
> option 3).

I agree.

>
> >> [Ken] Both of these implementations use a non-conformant iSER header
> (they add
>
> >> write_va and read_va fields, which incidentally do not appear to be
> used). Are
> >> these changes documented anywhere in the IB domain, or are these
> variations
> >> needed for another reason?
> >
> > [Erez] Take a look at the iSER for IB annex:
>
> > http://www.infinibandta.org/members/spec/Annex_iSER.PDF
>
> [Ken] Ouch. That link requires a username/password. Looks like it is only
> available to members
> of the Infiniband Trade Association. Fortunately I gained access to it with
> username "open" and
> password "standard". ;-)
>
> [Ken] Neither of these implementations send or examine the iSER CM REQ/REP
> message private data.
> The document doesn't define what action to take when this message is
> absent. Interestingly,
> when the target reports that "ZBVA shall be used for this connection" and
> "the target shall issue
> Send with Invalidate as needed" then it appears the iSER header specified in
> RFC5046 should be
> used for control-type PDUs. Is there any plan to conform with the list of
> requirements for IBTA
> compliance?

At the moment these capabilities (ZBVA, Send Invalidate) are not
supported in the driver,
though they seem to be supported by the ConnectX HCA.
Hence, iSER implementation do not send/examine them.
This may be added to the CM REQ/REP with the current defaults but in
order to use these capabilities a code
should be added to the HCA driver and iser.

>
> Cheers,
> Ken
>
> 
> Hotmail on your mobile. Never miss another e-mail with

--~--~-~--~~~---~--~~
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: [ofa-general] Re: [Ips] Calculating the VA in iSER header

2008-05-01 Thread Eli Dorfman

On Tue, Apr 29, 2008 at 8:05 PM, Pete Wyckoff <[EMAIL PROTECTED]> wrote:
> [EMAIL PROTECTED] wrote on Thu, 17 Apr 2008 14:13 +0300:
>
> > On Wed, Apr 16, 2008 at 6:46 PM, Roland Dreier <[EMAIL PROTECTED]> wrote:
>  > >  > Agree with the interpretation of the spec, and it's probably a bit
>  > >   > clearer that way too.  But we have working initiators and targets
>  > >   > that do it the "wrong" way.
>  > >
>  > >  Yes... I guess the key question is whether there are any initiators that
>  > >  do things the "right" way.
>  > >
>  > >
>  > >   > 1. Flag day: all initiators and targets change at the same time.
>  > >   > Will see data corruption if someone unluckily runs one or the other
>  > >   > using old non-fixed code.
>  > >
>  > >  Seems unacceptable to me... it doesn't make sense at all to break every
>  > >  setup in the world just to be "right" according to the spec.
>  >
>  > This will break only when both initiator and target will use
>  > InitialR2T=No, which means allow unsolicited data.
>  > As far as I know, STGT is not very common (and its version in RHEL5.1
>  > is considered experimental). Its default is also InitialR2T=Yes.
>  > Voltaire's iSCSI over iSER target also uses default InitialR2T=Yes.
>  > So it seems that nothing will break.
>
>  I finally got a chance to look at this just now.  I think you mean
>  default is InitialR2T=No above, which means no unsolicited data.
>  That is the default case, and true, the two different meanings
>  of the initiator-supplied VA coincide.

InitialR2T=Yes means that R2T is required, hence no unsolicited data.
Only is both sides, initiator and target agree on InitialR2T=No then
first data burst is unsolicited.

>
>  But you missed the impact of immediate data.  We run with the
>  defaults (I think) that say the first write request packet should be
>  filled with a bit of the coming data stream.  From iscsid.conf:
>
> # To enable immediate data (i.e., the initiator sends unsolicited data
> # with the iSCSI command packet), uncomment the following line:
> #
> # The default is Yes
> node.session.iscsi.ImmediateData = Yes
>
>  Looking at the offset printed out by your patch, it is indeed
>  non-zero for the first RDMA read.  Please correct me if I am
>  mistaken about this---you must have tested all four variations of
>  with and without the patches on initiator and target side, but I did
>  not.

You are right about the ImmediateData=Yes.
I really missed that, so after all this patch will break current
target implementation and
cause data corruption.
I suggest to postpone this patch till we implement the iSER HELLO
message and then add this patch with the corresponding target patch.
This will allow current initiator to work with current target and new
initiator work with new target.
I still think we should do that since future iser implementation will
probably rely on the spec.

>
>  Hence I am still a bit unhappy about having to deal with the
>  fallout, with no way to detect it.  For our local use, I'll keep an
>  older version of stgt in use until we switch to a new kernel, then
>  merge up the target side change.  It is a bother, but I can deal
>  with it.  For other institutions, this lockstep upgrade requirement
>  will not be obvious until they debug the resulting data corruption.
>
>  Still, I do understand why it would be nice to conform to the spec,
>  and it is maybe a bit cleaner that way too.  Maybe you can help with
>  the bug reports on stgt-devel during the transition, and maintain
>  and publish a patch to let it work with old kernels.
>
> -- Pete
>

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



[PATCH] IB/iSER: Count fmr alignment violations per session

2008-04-29 Thread Eli Dorfman

Count fmr alignment violations per session
as part of the iscsi statistics.

Signed-off-by: Eli Dorfman <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c  |4 +++-
 drivers/infiniband/ulp/iser/iser_memory.c |2 ++
 include/scsi/libiscsi.h   |1 +
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 451e601..df44fa7 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -472,13 +472,15 @@ iscsi_iser_conn_get_stats(struct iscsi_cls_conn
*cls_conn, struct iscsi_stats *s
stats->r2t_pdus = conn->r2t_pdus_cnt; /* always 0 */
stats->tmfcmd_pdus = conn->tmfcmd_pdus_cnt;
stats->tmfrsp_pdus = conn->tmfrsp_pdus_cnt;
-   stats->custom_length = 3;
+   stats->custom_length = 4;
strcpy(stats->custom[0].desc, "qp_tx_queue_full");
stats->custom[0].value = 0; /* TB iser_conn->qp_tx_queue_full; */
strcpy(stats->custom[1].desc, "fmr_map_not_avail");
stats->custom[1].value = 0; /* TB iser_conn->fmr_map_not_avail */;
strcpy(stats->custom[2].desc, "eh_abort_cnt");
stats->custom[2].value = conn->eh_abort_cnt;
+   strcpy(stats->custom[3].desc, "fmr_unalign_cnt");
+   stats->custom[3].value = conn->fmr_unalign_cnt;
 }

 static int
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c
b/drivers/infiniband/ulp/iser/iser_memory.c
index ee58199..cac50c4 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -423,6 +423,7 @@ void iser_dma_unmap_task_data(struct
iscsi_iser_cmd_task *iser_ctask)
 int iser_reg_rdma_mem(struct iscsi_iser_cmd_task *iser_ctask,
  enum   iser_data_dircmd_dir)
 {
+   struct iscsi_conn*iscsi_conn = iser_ctask->iser_conn->iscsi_conn;
struct iser_conn *ib_conn = iser_ctask->iser_conn->ib_conn;
struct iser_device   *device = ib_conn->device;
struct ib_device *ibdev = device->ib_device;
@@ -437,6 +438,7 @@ int iser_reg_rdma_mem(struct iscsi_iser_cmd_task
*iser_ctask,

aligned_len = iser_data_buf_aligned_len(mem, ibdev);
if (aligned_len != mem->dma_nents) {
+   iscsi_conn->fmr_unalign_cnt++;
iser_warn("rdma alignment violation %d/%d aligned\n",
 aligned_len, mem->size);
iser_data_buf_dump(mem, ibdev);
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 7b90b63..cd3ca63 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -225,6 +225,7 @@ struct iscsi_conn {

/* custom statistics */
uint32_teh_abort_cnt;
+   uint32_tfmr_unalign_cnt;
 };

 struct iscsi_pool {
-- 
1.5.5

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



[PATCH] IB/iSER: Add module param to count alignment violations

2008-04-28 Thread Eli Dorfman

Add read only module param to count alignment violations.
In case of unaligned pages iSER allocates memory and copies
the data to the new memory.

Signed-off-by: Eli Dorfman <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c  |3 +++
 drivers/infiniband/ulp/iser/iscsi_iser.h  |1 +
 drivers/infiniband/ulp/iser/iser_memory.c |1 +
 3 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 451e601..5181a1e 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -77,6 +77,9 @@
 static unsigned int iscsi_max_lun = 512;
 module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);

+unsigned int iser_unaligned_cnt = 0;
+module_param_named(unaligned_cnt, iser_unaligned_cnt, uint, S_IRUGO);
+
 int iser_debug_level = 0;
 module_param_named(debug_level, iser_debug_level, int,
S_IRUGO|S_IWUSR|S_IWGRP);
 MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0
(default:disabled)");
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index a8c1b30..4a39a38 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -294,6 +294,7 @@ struct iser_global {

 extern struct iser_global ig;
 extern int iser_debug_level;
+extern unsigned int iser_unaligned_cnt;

 /* allocate connection resources needed for rdma functionality */
 int iser_conn_set_full_featured_mode(struct iscsi_conn *conn);
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c
b/drivers/infiniband/ulp/iser/iser_memory.c
index ee58199..0f0fcb3 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -437,6 +437,7 @@ int iser_reg_rdma_mem(struct iscsi_iser_cmd_task
*iser_ctask,

aligned_len = iser_data_buf_aligned_len(mem, ibdev);
if (aligned_len != mem->dma_nents) {
+   iser_unaligned_cnt++;
iser_warn("rdma alignment violation %d/%d aligned\n",
 aligned_len, mem->size);
iser_data_buf_dump(mem, ibdev);
-- 
1.5.5

This patch was made against 2.6.26 branch.
Since it includes minor changes please try to push it to 2.6.26.
Otherwise this can go to 2.6.27.

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



[PATCH} IB/iSER: Move high-volume debug output to higher debug levels

2008-04-28 Thread Eli Dorfman

Add more levels for debug.

Signed-off-by: Eli Dorfman <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c  |5 ++---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |7 +++
 drivers/infiniband/ulp/iser/iser_memory.c |7 +--
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index be1b9fb..451e601 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -78,15 +78,14 @@ static unsigned int iscsi_max_lun = 512;
 module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);

 int iser_debug_level = 0;
+module_param_named(debug_level, iser_debug_level, int,
S_IRUGO|S_IWUSR|S_IWGRP);
+MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0
(default:disabled)");

 MODULE_DESCRIPTION("iSER (iSCSI Extensions for RDMA) Datamover "
   "v" DRV_VER " (" DRV_DATE ")");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_AUTHOR("Alex Nezhinsky, Dan Bar Dov, Or Gerlitz");

-module_param_named(debug_level, iser_debug_level, int, 0644);
-MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0
(default:disabled)");
-
 struct iser_global ig;

 void
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 1ee867b..a8c1b30 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -71,6 +71,13 @@

 #define iser_dbg(fmt, arg...)  \
do {\
+   if (iser_debug_level > 1)   \
+   printk(KERN_DEBUG PFX "%s:" fmt,\
+   __func__ , ## arg); \
+   } while (0)
+
+#define iser_warn(fmt, arg...) \
+   do {\
if (iser_debug_level > 0)   \
printk(KERN_DEBUG PFX "%s:" fmt,\
__func__ , ## arg); \
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c
b/drivers/infiniband/ulp/iser/iser_memory.c
index 4a17743..ee58199 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -334,8 +334,11 @@ static void iser_data_buf_dump(struct iser_data_buf *data,
struct scatterlist *sg;
int i;

+   if (iser_debug_level == 0)
+   return;
+
for_each_sg(sgl, sg, data->dma_nents, i)
-   iser_err("sg[%d] dma_addr:0x%lX page:0x%p "
+   iser_warn("sg[%d] dma_addr:0x%lX page:0x%p "
 "off:0x%x sz:0x%x dma_len:0x%x\n",
 i, (unsigned long)ib_sg_dma_address(ibdev, sg),
 sg_page(sg), sg->offset,
@@ -434,7 +437,7 @@ int iser_reg_rdma_mem(struct iscsi_iser_cmd_task
*iser_ctask,

aligned_len = iser_data_buf_aligned_len(mem, ibdev);
if (aligned_len != mem->dma_nents) {
-   iser_err("rdma alignment violation %d/%d aligned\n",
+   iser_warn("rdma alignment violation %d/%d aligned\n",
 aligned_len, mem->size);
iser_data_buf_dump(mem, ibdev);

-- 
1.5.5

This patch was made against 2.6.26 branch.
Since this includes minor changes please try to push it to 2.6.26.
Otherwise this can go to 2.6.27.

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



[PATCH 2/2] IB/iSER: Use offset from r2t header for rdma

2008-04-27 Thread Eli Dorfman

Use offset from r2t header for rdma instead of using
internal offset counter.

Signed-off-by: Eli Dorfman <[EMAIL PROTECTED]>
---
 usr/iscsi/iscsi_rdma.c |   16 +---
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/usr/iscsi/iscsi_rdma.c b/usr/iscsi/iscsi_rdma.c
index d46ddff..84f5949 100644
--- a/usr/iscsi/iscsi_rdma.c
+++ b/usr/iscsi/iscsi_rdma.c
@@ -1447,28 +1447,22 @@ static int iscsi_rdma_rdma_read(struct
iscsi_connection *conn)
struct iscsi_r2t_rsp *r2t = (struct iscsi_r2t_rsp *) &conn->rsp.bhs;
uint8_t *buf;
uint32_t len;
+   uint32_t offset;
int ret;

buf = (uint8_t *) task->data + task->offset;
len = be32_to_cpu(r2t->data_length);
+   offset = be32_to_cpu(r2t->data_offset);

-   dprintf("len %u stag %x va %llx\n",
+   dprintf("len %u stag %x va %llx offset %x\n",
len, itask->rem_write_stag,
-   (unsigned long long) itask->rem_write_va);
+   (unsigned long long) itask->rem_write_va, offset);

ret = iser_post_rdma_wr(ci, task, buf, len, IBV_WR_RDMA_READ,
-   itask->rem_write_va, itask->rem_write_stag);
+   itask->rem_write_va + offset, 
itask->rem_write_stag);
if (ret < 0)
return ret;

-   /*
-* Initiator registers the entire buffer, but gives us a VA that
-* is advanced by immediate + unsolicited data amounts.  Advance
-* rem_va as we read, knowing that the target always grabs segments
-* in order.
-*/
-   itask->rem_write_va += len;
-
return 0;
 }

-- 
1.5.5

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



[PATCH 1/2] IB/iSER: Do not add unsolicited data offset to VA in iSER header

2008-04-27 Thread Eli Dorfman

iSER initiator sends a VA (in the iSER header) which includes
an offset for the unsolicited data (which is wrong according to the spec).

Signed-off-by: Eli Dorfman <[EMAIL PROTECTED]>
Signed-off-by: Erez Zilber <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iser_initiator.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c
b/drivers/infiniband/ulp/iser/iser_initiator.c
index 08dc81c..5c2bbc6 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -154,12 +154,12 @@ iser_prepare_write_cmd(struct iscsi_cmd_task *ctask,
if (unsol_sz < edtl) {
hdr->flags |= ISER_WSV;
hdr->write_stag = cpu_to_be32(regd_buf->reg.rkey);
-   hdr->write_va   = cpu_to_be64(regd_buf->reg.va + unsol_sz);
+   hdr->write_va   = cpu_to_be64(regd_buf->reg.va);

iser_dbg("Cmd itt:%d, WRITE tags, RKEY:%#.4X "
-"VA:%#llX + unsol:%d\n",
+"VA:%#llX\n",
 ctask->itt, regd_buf->reg.rkey,
-(unsigned long long)regd_buf->reg.va, unsol_sz);
+(unsigned long long)regd_buf->reg.va);
}

if (imm_sz > 0) {
-- 
1.5.5

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



[PATCH 0/2] IB/iSER: Calculating the VA in iSER header

2008-04-27 Thread Eli Dorfman
The following patch set includes a bug fix for the VA value in the iSER
header.
The current value is incorrect according to the iSER spec.
This patch set includes a bug fix for the initiator code that was made
against the 2.6.26 branch
and a fix for the iSER code in STGT.

--~--~-~--~~~---~--~~
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: [ofa-general] Re: [Ips] Calculating the VA in iSER header

2008-04-17 Thread Eli Dorfman

On Wed, Apr 16, 2008 at 6:46 PM, Roland Dreier <[EMAIL PROTECTED]> wrote:
>  > Agree with the interpretation of the spec, and it's probably a bit
>   > clearer that way too.  But we have working initiators and targets
>   > that do it the "wrong" way.
>
>  Yes... I guess the key question is whether there are any initiators that
>  do things the "right" way.
>
>
>   > 1. Flag day: all initiators and targets change at the same time.
>   > Will see data corruption if someone unluckily runs one or the other
>   > using old non-fixed code.
>
>  Seems unacceptable to me... it doesn't make sense at all to break every
>  setup in the world just to be "right" according to the spec.

This will break only when both initiator and target will use
InitialR2T=No, which means allow unsolicited data.
As far as I know, STGT is not very common (and its version in RHEL5.1
is considered experimental). Its default is also InitialR2T=Yes.
Voltaire's iSCSI over iSER target also uses default InitialR2T=Yes.
So it seems that nothing will break.

>
>
>   > 2. Rewrite the IB Annex to codify what's done in practice, and don't
>   > "fix" any code.
>
>  If existing practice is universally to do things "wrong" then this seems
>  to me by far the best way to proceed.

Assuming there aren't many iSER installation that currently work with
unsolicited data, then it is the right time to do it right.
Future implementation will rely on the spec and unless you modify the
spec this will lead to greater confusion.

--~--~-~--~~~---~--~~
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: [Ips] Calculating the VA in iSER header

2008-04-16 Thread Eli Dorfman
According to Mike's explanation below it seems that we have a bug in
iSER initiator.
Fixing this bug will require a fix in the stgt iSER code.

The problem is that the initiator send a VA which already includes an
offset for the unsolicited data (which is wrong).
In iser_initiator.c::iser_prepare_write_cmd the code looks like this:
hdr->write_va   = cpu_to_be64(regd_buf->reg.va + unsol_sz);

we think that it should be modified to:
hdr->write_va   = cpu_to_be64(regd_buf->reg.va);

Let's discuss this and verify we interpret the spec correctly.
If agreed we will send a patch.

Eli

2008/4/15 Mike Ko <[EMAIL PROTECTED]>:
>
> VA is a concept introduced in an Infiniband annex to support iSER.  It
> appears in the expanded iSER header for Infiniband use only to support the
> non-Zero Based Virtual Address (non-ZBVA) used in Infiniband vs the ZBVA
> used in IETF.
>
> "The DataDescriptorOut describes the I/O buffer starting with the immediate
> unsolicited data (if any), followed by the non-immediate unsolicited data
> (if any) and solicited data."  If non-ZBVA mode is used, then VA points to
> the beginning of this buffer.  So in your example, the VA field in the
> expanded iSER header will be zero.  Note that for IETF, ZBVA is assumed and
> there is no provision to specify a different VA in the iSER header.
>
> Tagged offset (TO) refers to the offset within a tagged buffer in RDMA Write
> and RDMA Read Request Messages.  When sending non-immediate unsolicited
> data, Send Message types are used and the TO field is not present.  Instead,
> the buffer offset is appropriately represented by the Buffer Offset field in
> the SCSI Data-Out PDU.  Note that Tagged Offset is not the same as write VA
> and it does not appear in the iSER header.
>
> Mike
>
>
>
>  Erez Zilber <[EMAIL PROTECTED]>
> Sent by: [EMAIL PROTECTED]
>
> 04/15/2008 06:40 AM
>
> To [EMAIL PROTECTED]
>
> cc
>
> Subject [Ips] Calculating the VA in iSER header
>
>
>
>
>
>
> We're trying to understand what should be the write VA (tagged offset)
>  in the iSER header for WRITE commands. If unsolicited data is to be
>  sent, should the VA be the original VA or should it be original VA +
>  FirstBurstLength?
>
>
>  Example:
>
>
>  InitialR2T=No
>
>  FirstBurstLength = 1000
>
>
>  Base address of the registered buffer = 0
>
>
>  Now, what should be the VA in the iSER header? 0 or 1000?
>
>
>  We read the following paragraph in the iSER spec, but didn't get an
>  answer from there:
>
>
>  * If there is solicited data to be transferred for the SCSI write or
>  bidirectional command, as indicated by the Expected Data Transfer
>  Length in the SCSI Command PDU exceeding the value of
>  UnsolicitedDataSize, the iSER layer at the initiator MUST do the
>  following:
>
>  a. It MUST allocate a Write STag for the I/O Buffer defined by
>  the qualifier DataDescriptorOut. The DataDescriptorOut
>  describes the I/O buffer starting with the immediate
>  unsolicited data (if any), followed by the non-immediate
>  unsolicited data (if any) and solicited data. This means
>  that the BufferOffset for the SCSI Data-out for this
>  command is equal to the TO. This implies that a zero TO
>  for this STag points to the beginning of this I/O Buffer.
>
>
>  Thanks,
>
>  --
>
>  
>
>  Erez Zilber | 972-9-971-7689
>
>  Software Engineer, Storage Solutions
>
>  Voltaire - _The Grid Backbone_
>
>  __
>
>  www.voltaire.com 
>
>
>
>  ___
>  Ips mailing list
>  [EMAIL PROTECTED]
>  https://www.ietf.org/mailman/listinfo/ips
>
>
> ___
>  Ips mailing list
>  [EMAIL PROTECTED]
>  https://www.ietf.org/mailman/listinfo/ips
>
>

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