Re: Problem setting host net params via iscsiuio on first try: bad error message

2015-03-16 Thread The Lee-Man
Oops. I almost forgot. The patch did not apply cleanly because of a recent
update you did. Please make sure I modified it correctly (attached).


On Monday, March 16, 2015 at 11:56:48 AM UTC-7, The Lee-Man wrote:
Hi Mike:

I verified this patch works. Feel free to add a "Tested-by:".

By "works", I mean that it eliminated the retry that I was seeing when
connecting the first time to the iscsiuio daemon.

On Tuesday, March 10, 2015 at 8:00:54 AM UTC-7, Mike Christie wrote:
On 03/09/2015 08:54 PM, Lee Duncan wrote: 
> Hi Mike: 
> 
> I am *finally* getting to this bug, previously buried under a mountain of 
more pressing matters. 
> 
>> On Jan 12, 2015, at 12:50 PM, Mike Christie  wrote: 
>> 
>> Thanks. I merged and pushed these patches. 
>> 
>> When you get some time, reply to that mail I sent offlist about how long 
>> it is taking for the set host net params to succeed, so I can fix my 
>> login/init patch as needed. 
> 
> I have looked at this problem again, and I will reply in a new email 
tomorrow. Fair warning. :) 

:) 

Hey, 

Attached is the patch that was modifying iscsid's code path which from 
what I understand handled the same problem you were handling in the 
iscsiadm's discovery code path. 

The patch is removing the iscsiuio specific calls in iscsid and using 
the generic initial login/connect retry code. It is currently retrying 
for up to login_timeout seconds instead of the hard coded 5 seconds. 

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
>From 31c26a00bbfa6cff83040eb3780a7e1753409aaa Mon Sep 17 00:00:00 2001
From: Mike Christie 
Date: Mon, 16 Mar 2015 11:52:50 -0700
Subject: [PATCH] iscsid: make sure actor is delayed before rescheduling

iscsi_conn_connect() be called from a login_timer that is not deleted.
This causes it to be scheduled multiple times. This patch adds a new
function actor_timer_mod to handle both deletion and rescheduling.

This patch also then removed the uio poll code which was using the
login_timer to schedule itself. This uio retry, is now just done in
the generic initial login retry.
---
 usr/actor.c|   8 
 usr/actor.h|   3 +-
 usr/initiator.c| 120 +
 usr/initiator.h|   1 -
 usr/initiator_common.c |   8 ++--
 5 files changed, 25 insertions(+), 115 deletions(-)

diff --git a/usr/actor.c b/usr/actor.c
index 67ae0e036ce4..017d013fb596 100644
--- a/usr/actor.c
+++ b/usr/actor.c
@@ -195,6 +195,14 @@ actor_timer(actor_t *thread, uint32_t timeout_secs, void (*callback)(void *),
 	actor_schedule_private(thread, timeout_secs, 0);
 }
 
+void
+actor_timer_mod(actor_t *thread, uint32_t new_timeout_secs, void *data)
+{
+	actor_delete(thread);
+	thread->data = data;
+	actor_schedule_private(thread, new_timeout_secs, 0);
+}
+
 /*
  * Execute all items that have expired.
  *
diff --git a/usr/actor.h b/usr/actor.h
index 7283dcebfe1e..f572f2e32af3 100644
--- a/usr/actor.h
+++ b/usr/actor.h
@@ -43,7 +43,8 @@ extern void actor_schedule_head(actor_t *thread);
 extern void actor_schedule(actor_t *thread);
 extern void actor_timer(actor_t *thread, uint32_t delay_secs,
 			void (*callback)(void *), void *data);
-extern int actor_timer_mod(actor_t *thread, uint32_t new_delay_secs, void *data);
+extern void actor_timer_mod(actor_t *thread, uint32_t new_delay_secs,
+			void *data);
 extern void actor_poll(void);
 
 #endif /* ACTOR_H */
diff --git a/usr/initiator.c b/usr/initiator.c
index b25ded81c9d7..542ce0d3179f 100644
--- a/usr/initiator.c
+++ b/usr/initiator.c
@@ -263,6 +263,7 @@ __session_conn_create(iscsi_session_t *session, int cid)
 
 	conn->state = ISCSI_CONN_STATE_FREE;
 	conn->session = session;
+	actor_init(&conn->login_timer, iscsi_login_timedout, NULL);
 	/*
 	 * TODO: we must export the socket_fd/transport_eph from sysfs
 	 * so if iscsid is resyncing up we can pick that up and cleanup up
@@ -529,9 +530,7 @@ queue_delayed_reopen(queue_task_t *qtask, int delay)
  	 * iscsi_login_eh can handle the login resched as
  	 * if it were login time out
  	 */
-	actor_delete(&conn->login_timer);
-	actor_timer(&conn->login_timer, delay,
-		iscsi_login_timedout, qtask);
+	actor_timer_mod(&conn->login_timer, delay, qtask);
 }
 
 static int iscsi_conn_connect(struct iscsi_conn *conn, queue_task_t *qtask)
@@ -566,53 +565,10 @@ static int iscsi_conn_connect(struct iscsi_conn *conn, queue_task_t *qtask)
 	iscsi_sched_ev_context(ev_context, conn, 0, EV_CONN_POLL);
 	log_debug(3, "Setting login timer %p timeout %d", &conn->login_timer,
 		  conn->login_timeout);
-	actor_timer(&conn->login_timer, conn->login_timeout,
-		iscsi_login_

Re: Problem setting host net params via iscsiuio on first try: bad error message

2015-03-16 Thread The Lee-Man
Hi Mike:

I verified this patch works. Feel free to add a "Tested-by:".

By "works", I mean that it eliminated the retry that I was seeing when
connecting the first time to the iscsiuio daemon.

On Tuesday, March 10, 2015 at 8:00:54 AM UTC-7, Mike Christie wrote:
>
> On 03/09/2015 08:54 PM, Lee Duncan wrote: 
> > Hi Mike: 
> > 
> > I am *finally* getting to this bug, previously buried under a mountain 
> of more pressing matters. 
> > 
> >> On Jan 12, 2015, at 12:50 PM, Mike Christie  > wrote: 
> >> 
> >> Thanks. I merged and pushed these patches. 
> >> 
> >> When you get some time, reply to that mail I sent offlist about how 
> long 
> >> it is taking for the set host net params to succeed, so I can fix my 
> >> login/init patch as needed. 
> > 
> > I have looked at this problem again, and I will reply in a new email 
> tomorrow. Fair warning. :) 
>
> :) 
>
> Hey, 
>
> Attached is the patch that was modifying iscsid's code path which from 
> what I understand handled the same problem you were handling in the 
> iscsiadm's discovery code path. 
>
> The patch is removing the iscsiuio specific calls in iscsid and using 
> the generic initial login/connect retry code. It is currently retrying 
> for up to login_timeout seconds instead of the hard coded 5 seconds. 
>

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


RE: Problem setting host net params via iscsiuio on first try: bad error message

2015-03-13 Thread wei, xiaoyan
Hi Mike,

I sent out an email regarding iscsiadm's discovery problem (attached).
Looks like Lee is referring the same problem and the patch will fix it. Could 
you please confirm?

Thanks!


-Original Message-
From: open-iscsi@googlegroups.com [mailto:open-iscsi@googlegroups.com] On 
Behalf Of Mike Christie
Sent: Tuesday, March 10, 2015 11:01 AM
To: open-iscsi@googlegroups.com
Subject: Re: Problem setting host net params via iscsiuio on first try: bad 
error message

On 03/09/2015 08:54 PM, Lee Duncan wrote:
> Hi Mike:
> 
> I am *finally* getting to this bug, previously buried under a mountain of 
> more pressing matters.
> 
>> On Jan 12, 2015, at 12:50 PM, Mike Christie  wrote:
>>
>> Thanks. I merged and pushed these patches.
>>
>> When you get some time, reply to that mail I sent offlist about how 
>> long it is taking for the set host net params to succeed, so I can 
>> fix my login/init patch as needed.
> 
> I have looked at this problem again, and I will reply in a new email 
> tomorrow. Fair warning. :)

:)

Hey,

Attached is the patch that was modifying iscsid's code path which from what I 
understand handled the same problem you were handling in the iscsiadm's 
discovery code path.

The patch is removing the iscsiuio specific calls in iscsid and using the 
generic initial login/connect retry code. It is currently retrying for up to 
login_timeout seconds instead of the hard coded 5 seconds.

--
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
--- Begin Message ---
Hi,

Open-iscsi README specified a bug in iscsiadm discovery:

"Be aware that iscsiadm will use the default route to do discovery. It will
not use the iface specified. So if you are using a offload card, you will need 
a separate network connection to the target for discovery purposes.
*This will be fixed in the next version of open-iscsi*"

Is there already a fix for this?  When the fix is planned to be released?

Thanks.
--- End Message ---


Re: Problem setting host net params via iscsiuio on first try: bad error message

2015-03-10 Thread Mike Christie
On 03/10/2015 11:52 AM, wei, xiaoyan wrote:
> Hi Mike,
> 
> I sent out an email regarding iscsiadm's discovery problem (attached).
> Looks like Lee is referring the same problem and the patch will fix it. Could 
> you please confirm?
> 

My patch only cleans up the code that handles the problem when doing
normal session login. I cleaned it up when fixing another bug in that
code, and was only bygging Lee about the issue because I was looking for
more info on how to hit it so I could test my patch. Lee - someone at RH
ended up having a system btw.

Ideally, we would want to have these type of issues fixed in one place.
Due to how iscsiadm does not use iscsid though, I do not think that is
possible though.


> Thanks!
> 
> 
> -Original Message-
> From: open-iscsi@googlegroups.com [mailto:open-iscsi@googlegroups.com] On 
> Behalf Of Mike Christie
> Sent: Tuesday, March 10, 2015 11:01 AM
> To: open-iscsi@googlegroups.com
> Subject: Re: Problem setting host net params via iscsiuio on first try: bad 
> error message
> 
> On 03/09/2015 08:54 PM, Lee Duncan wrote:
>> Hi Mike:
>>
>> I am *finally* getting to this bug, previously buried under a mountain of 
>> more pressing matters.
>>
>>> On Jan 12, 2015, at 12:50 PM, Mike Christie  wrote:
>>>
>>> Thanks. I merged and pushed these patches.
>>>
>>> When you get some time, reply to that mail I sent offlist about how 
>>> long it is taking for the set host net params to succeed, so I can 
>>> fix my login/init patch as needed.
>>
>> I have looked at this problem again, and I will reply in a new email 
>> tomorrow. Fair warning. :)
> 
> :)
> 
> Hey,
> 
> Attached is the patch that was modifying iscsid's code path which from what I 
> understand handled the same problem you were handling in the iscsiadm's 
> discovery code path.
> 
> The patch is removing the iscsiuio specific calls in iscsid and using the 
> generic initial login/connect retry code. It is currently retrying for up to 
> login_timeout seconds instead of the hard coded 5 seconds.
> 
> --
> You received this message because you are subscribed to the Google Groups 
> "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-iscsi@googlegroups.com.
> Visit this group at http://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.
> 

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Problem setting host net params via iscsiuio on first try: bad error message

2015-03-10 Thread Mike Christie
On 03/09/2015 08:54 PM, Lee Duncan wrote:
> Hi Mike:
> 
> I am *finally* getting to this bug, previously buried under a mountain of 
> more pressing matters.
> 
>> On Jan 12, 2015, at 12:50 PM, Mike Christie  wrote:
>>
>> Thanks. I merged and pushed these patches.
>>
>> When you get some time, reply to that mail I sent offlist about how long
>> it is taking for the set host net params to succeed, so I can fix my
>> login/init patch as needed.
> 
> I have looked at this problem again, and I will reply in a new email 
> tomorrow. Fair warning. :)

:)

Hey,

Attached is the patch that was modifying iscsid's code path which from
what I understand handled the same problem you were handling in the
iscsiadm's discovery code path.

The patch is removing the iscsiuio specific calls in iscsid and using
the generic initial login/connect retry code. It is currently retrying
for up to login_timeout seconds instead of the hard coded 5 seconds.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
>From 05abcf6947f32186e540a102b20cd7d768a29d08 Mon Sep 17 00:00:00 2001
From: Mike Christie 
Date: Thu, 5 Feb 2015 02:11:16 -0600
Subject: [PATCH 1/1] iscsid: make sure actor is delated before rescheduling

iscsi_conn_connect() be called from a login_timer that is not deleted.
This causes it to be scheduled multiple times. This patch adds a new
function actor_timer_mod to handle both deletion and rescheduling.

This patch also then removed the uio poll code which was using the
login_timer to schedule itself. This uio retry, is now just done in
the generic initial login retry.
---
 usr/actor.c|   8 
 usr/actor.h|   3 +-
 usr/initiator.c| 122 +
 usr/initiator.h|   1 -
 usr/initiator_common.c |   8 ++--
 5 files changed, 26 insertions(+), 116 deletions(-)

diff --git a/usr/actor.c b/usr/actor.c
index 37b5024..21cd819 100644
--- a/usr/actor.c
+++ b/usr/actor.c
@@ -182,6 +182,14 @@ actor_timer(actor_t *thread, uint32_t timeout_secs, void (*callback)(void *),
 	actor_schedule_private(thread, timeout_secs, 0);
 }
 
+void
+actor_timer_mod(actor_t *thread, uint32_t new_timeout_secs, void *data)
+{
+	actor_delete(thread);
+	thread->data = data;
+	actor_schedule_private(thread, new_timeout_secs, 0);
+}
+
 /*
  * Execute all items that have expired.
  *
diff --git a/usr/actor.h b/usr/actor.h
index 7283dce..f572f2e 100644
--- a/usr/actor.h
+++ b/usr/actor.h
@@ -43,7 +43,8 @@ extern void actor_schedule_head(actor_t *thread);
 extern void actor_schedule(actor_t *thread);
 extern void actor_timer(actor_t *thread, uint32_t delay_secs,
 			void (*callback)(void *), void *data);
-extern int actor_timer_mod(actor_t *thread, uint32_t new_delay_secs, void *data);
+extern void actor_timer_mod(actor_t *thread, uint32_t new_delay_secs,
+			void *data);
 extern void actor_poll(void);
 
 #endif /* ACTOR_H */
diff --git a/usr/initiator.c b/usr/initiator.c
index 1aadc9b..f70db4d 100644
--- a/usr/initiator.c
+++ b/usr/initiator.c
@@ -262,6 +262,7 @@ __session_conn_create(iscsi_session_t *session, int cid)
 
 	conn->state = ISCSI_CONN_STATE_FREE;
 	conn->session = session;
+	actor_init(&conn->login_timer, iscsi_login_timedout, NULL);
 	/*
 	 * TODO: we must export the socket_fd/transport_eph from sysfs
 	 * so if iscsid is resyncing up we can pick that up and cleanup up
@@ -528,9 +529,7 @@ queue_delayed_reopen(queue_task_t *qtask, int delay)
  	 * iscsi_login_eh can handle the login resched as
  	 * if it were login time out
  	 */
-	actor_delete(&conn->login_timer);
-	actor_timer(&conn->login_timer, delay,
-		iscsi_login_timedout, qtask);
+	actor_timer_mod(&conn->login_timer, delay, qtask);
 }
 
 static int iscsi_conn_connect(struct iscsi_conn *conn, queue_task_t *qtask)
@@ -565,53 +564,10 @@ static int iscsi_conn_connect(struct iscsi_conn *conn, queue_task_t *qtask)
 	iscsi_sched_ev_context(ev_context, conn, 0, EV_CONN_POLL);
 	log_debug(3, "Setting login timer %p timeout %d", &conn->login_timer,
 		  conn->login_timeout);
-	actor_timer(&conn->login_timer, conn->login_timeout,
-		iscsi_login_timedout, qtask);
+	actor_timer_mod(&conn->login_timer, conn->login_timeout, qtask);
 	return 0;
 }
 
-static void iscsi_uio_poll_login_timedout(void *data)
-{
-	struct queue_task *qtask = data;
-	struct iscsi_conn *conn = qtask->conn;
-	iscsi_session_t *session = conn->session;
-
-	log_debug(3, "timeout waiting for UIO ...\n");
-	mgmt_ipc_write_rsp(qtask, ISCSI_ERR_TRANS_TIMEOUT);
-	conn_delete_timers(conn);
-	__session_destroy(session);
-}
-
-static int iscsi_sched_uio_poll(queue_task_t *qtask)
-{
-	struct iscsi

Re: Problem setting host net params via iscsiuio on first try: bad error message

2015-03-09 Thread Lee Duncan
Hi Mike:

I am *finally* getting to this bug, previously buried under a mountain of more 
pressing matters.

> On Jan 12, 2015, at 12:50 PM, Mike Christie  wrote:
> 
> Thanks. I merged and pushed these patches.
> 
> When you get some time, reply to that mail I sent offlist about how long
> it is taking for the set host net params to succeed, so I can fix my
> login/init patch as needed.

I have looked at this problem again, and I will reply in a new email tomorrow. 
Fair warning. :)

> 
> On 12/11/2014 01:53 PM, The Lee-Man wrote:
>> Hi Mike:
>> 
>> I have started working more with iscsiuio.
>> 
>> I have discovered what I consider a bug in an error message printed
>> during normal operation of iscsiadm that makes it seem like something
>> bad happened.
>> 
>> As you know, when performing discovery via the bnx2i transport and the
>> iscsiuio daemon, the iscsiadm command tries to set up host parameters
>> when a target is discovered. The iscsiadm command does this by sending a
>> message to the iscsiuio daemon, who actually does the work in this case
>> and then normally returns that information.
>> 
>> But it looks like the design of iscsiuio is that it does not even try to
>> set up the NIC it is using very first time its called. Instead, it
>> initializes the NIC only when the first attempt to use it is received,
>> while it returns EAGAIN to the caller. The protocol evidently is that
>> the caller knows to retry. In this case, the iscsiadm code retries
>> several times no matter what error is returned. In fact, the iscsiadm
>> code being used translates all errors received from this attempt to talk
>> to iscsiuio into an "INTERNAL_ERROR", and the communication is retried
>> anyway.
>> 
>> This translation is the only problem, IMHO, and resulted in these
>> messages from iscsiadm, which are misleading (and poorly formatted):
>> 
>> iscsiadm: Could not set host net params (err 29)
>> 
>> iscsiadm: Connection to discovery portal 10.125.128.120 failed:
>> internal error
>> ...
>> 
>> which becomes, with my patches:
>> 
>> iscsiadm: Could not set host net params (err 29)
>> iscsiadm: Connection to discovery portal 192.168.30.1 failed: operation
>> failed but retry may succeed
>> ...
>> 
>> The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it
>> through. (And fix the error message printing so we don't core dump, of
>> course.)
>> 
>> I thought about a more aggressive fix, i.e. letting all return values
>> from the communications attempt through, but I worried about fixing
>> something that is not really broken and perhaps breaking something else.
>> So, in the spirit of fixing just what is broken, I submit two patches:
>> 
>> 0001-Supply-strings-for-newly-added-error-numbers.patch: This patch is
>> needed by the second patch, though it stands alone. This patch fixes the
>> problem that a couple of new error codes have been added to the
>> open-iscsi package but the array of error strings used to convert those
>> errors into printed messages has not kept pace. So two missing error
>> messages are added.
>> 
>> 0002-Allow-setting-host-params-to-return-EAGAIN-errors.patch - This is
>> the simple patch that allows EAGAIN errors through when trying to talk
>> to iscsiuio. If you want the more aggressive version please let me know.
>> 
>> -- 
>> You received this message because you are subscribed to the Google
>> Groups "open-iscsi" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to open-iscsi+unsubscr...@googlegroups.com
>> .
>> To post to this group, send email to open-iscsi@googlegroups.com
>> .
>> Visit this group at http://groups.google.com/group/open-iscsi.
>> For more options, visit https://groups.google.com/d/optout.
> 
> -- 
> You received this message because you are subscribed to a topic in the Google 
> Groups "open-iscsi" group.
> To unsubscribe from this topic, visit 
> https://groups.google.com/d/topic/open-iscsi/PWbP_a2HR-g/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to 
> open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-iscsi@googlegroups.com.
> Visit this group at http://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.

-- 
Lee Duncan

Love is a snowmobile racing across the tundra and then suddenly it flips over, 
pinning you underneath. At night, the ice weasels come. – Matt Groening





-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Problem setting host net params via iscsiuio on first try: bad error message

2015-01-12 Thread Mike Christie
Thanks. I merged and pushed these patches.

When you get some time, reply to that mail I sent offlist about how long
it is taking for the set host net params to succeed, so I can fix my
login/init patch as needed.

On 12/11/2014 01:53 PM, The Lee-Man wrote:
> Hi Mike:
> 
> I have started working more with iscsiuio.
> 
> I have discovered what I consider a bug in an error message printed
> during normal operation of iscsiadm that makes it seem like something
> bad happened.
> 
> As you know, when performing discovery via the bnx2i transport and the
> iscsiuio daemon, the iscsiadm command tries to set up host parameters
> when a target is discovered. The iscsiadm command does this by sending a
> message to the iscsiuio daemon, who actually does the work in this case
> and then normally returns that information.
> 
> But it looks like the design of iscsiuio is that it does not even try to
> set up the NIC it is using very first time its called. Instead, it
> initializes the NIC only when the first attempt to use it is received,
> while it returns EAGAIN to the caller. The protocol evidently is that
> the caller knows to retry. In this case, the iscsiadm code retries
> several times no matter what error is returned. In fact, the iscsiadm
> code being used translates all errors received from this attempt to talk
> to iscsiuio into an "INTERNAL_ERROR", and the communication is retried
> anyway.
> 
> This translation is the only problem, IMHO, and resulted in these
> messages from iscsiadm, which are misleading (and poorly formatted):
> 
>  iscsiadm: Could not set host net params (err 29)
> 
>  iscsiadm: Connection to discovery portal 10.125.128.120 failed:
> internal error
>  ...
> 
> which becomes, with my patches:
> 
>  iscsiadm: Could not set host net params (err 29)
>  iscsiadm: Connection to discovery portal 192.168.30.1 failed: operation
> failed but retry may succeed
>  ...
> 
> The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it
> through. (And fix the error message printing so we don't core dump, of
> course.)
> 
> I thought about a more aggressive fix, i.e. letting all return values
> from the communications attempt through, but I worried about fixing
> something that is not really broken and perhaps breaking something else.
> So, in the spirit of fixing just what is broken, I submit two patches:
> 
> 0001-Supply-strings-for-newly-added-error-numbers.patch: This patch is
> needed by the second patch, though it stands alone. This patch fixes the
> problem that a couple of new error codes have been added to the
> open-iscsi package but the array of error strings used to convert those
> errors into printed messages has not kept pace. So two missing error
> messages are added.
> 
> 0002-Allow-setting-host-params-to-return-EAGAIN-errors.patch - This is
> the simple patch that allows EAGAIN errors through when trying to talk
> to iscsiuio. If you want the more aggressive version please let me know.
> 
> -- 
> You received this message because you are subscribed to the Google
> Groups "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to open-iscsi+unsubscr...@googlegroups.com
> .
> To post to this group, send email to open-iscsi@googlegroups.com
> .
> Visit this group at http://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Problem setting host net params via iscsiuio on first try: bad error message

2014-12-14 Thread Mike Christie
On 12/12/2014 04:11 PM, Lee Duncan wrote:
> On Dec 12, 2014, at 12:08 PM, Mike Christie  wrote:
>>
>> In that other thread, I said I wanted to find out why we are
>> adding/removing sessions. That should not be happening and if you
>> figured that out, I was saying that I thought you would not hit the
>> sysfs scanning issue. Also I said in that other thread I found that bug
>> with the discoveryd daemon caching when it should not so that also
>> causes a bug that might be affecting your testing.
>>
> 
> You have valid points, of course, but let's discuss that issue on that thread.
> 
>>

 Please just fix this properly. It is really silly that the user would
 have to run this command twice to actually do discovery.

>>>
>>> Ouch. Okay, interesting approach. I guess I stand rebuffed.
>>>
>>> "Fixing this" would really mean getting rid of iscsiuio and having BRCM 
>>> devices use the network stack, wouldn't it? I'm not sure where you draw the 
>>> line between cosmetic hacks and fixing it "properly".
>>
>> Upstream already rejected the net stack approach so that is not an option.
> 
> I understood as much from Eddie. I shouldn't have suggested it, as I know 
> it's not an option. I was trying (poorly) to make a point about how slippery 
> the "right thing" slope can be. I withdraw the point.
> 
>>
>>>
>>> Why do you think the user has to "run this command twice"? That is 
>>> incorrect. The iscsiadm command retries the communication 10 times, I 
>>> believe, or until iscsiuio replies. Do you consider that silly? Do you mean 
>>> that the iscsiadm code should not retry?
>>>
>>
>> If the user/caller has retries off then they would have to retry. In the
>> command output you put in the mail, it was not clear if you allowed
>> retries or not.
>>
>> iscsid and/or iscsiadm should not have to retry. It seems like a bogus
>> design that it does not work the first time. That is what should be fixed.
> 
> I am surprised to hear you say that. That seems a little like saying device 
> drivers shouldn't have to retry I/O commands: they should just work the first 
> time.
> 

That is not what I am saying at all. Above, I am saying that if some
code knows how to execute the operation and can, then I think it should
and should not rely on every caller to know what is right and drive retries.

We have retry code in iscsid. This is for cases like where the
driver/net init might race with iscsi init or for cases where the
network is down. To balance the needs of different callers, we want to
be able to allow iscsid to retry but we do not want to indefinitely
block iscsiadm calls that might be probing or during init time.


> The code in open-iscsi that does this retry loop is in uip_broadcast() in 
> iscsid_req.c:
> 
> ...
> +#define MAX_UIP_BROADCAST_READ_TRIES 3
> +   for (count = 0; count < MAX_UIP_BROADCAST_READ_TRIES; count++) {
> +   /*  Wait for the response */
> +   err = read(fd, &rsp, sizeof(rsp));
> +   if (err == sizeof(rsp)) {
> +   log_debug(3, "Broadcasted to uIP with length: %ld "
> +"cmd: 0x%x rsp: 0x%x\n", buf_len,
> +rsp.command, rsp.err);
> +   err = 0;
> +   break;
> +   } else if ((err == -1) && (errno == EAGAIN)) {
> +   usleep(25);
> +   continue;
> +   } else {
> +   log_error("Could not read response (%d/%d), daemon "
> + "died?", err, errno);
> +   err = ISCSI_ERR;
> +   break;
> +   }
> +   } 
> ...
> 
> This seems to have been added last year when Eddie sent you a patch (see 
> commit d571cdfc0b3c539310f43dc11d54d9308299efdc). That commit was quite 
> extensive, introducing the EAGAIN error to iscsi processing.
> 

This is for when iscsiuio and iscsid init race. To try and detect that
race vs cases where iscsiio never comes we have limited retries there,
so we retry but do not block forever.

I think the code you are interested in is a little lower in that code
(the ISCSID_UIP_MGMT_IPC_DEVICE_UP chunk) in session_conn_uio_poll ->
iscsi_set_net_config. Are you saying below that the retries you have
been talking about are the same ones that for iscsid
session_conn_uio_poll handles? If so I misunderstood you the first time.
Your original program description made me think you were talking about a
different issue.


> And it looks like there are only 3 retries in this case, not 10 as I guessed 
> before. And only in the case of EAGAIN.
> 
> If you look at iscsiuio (as I have now for a few days), it's clear that it 
> was designed as a minimalist state machine. The choice of uIP for the network 
> stack reenforces that impression. The design is all about assuming nothing 
> comes from the operating system, and is often used in firmware.
> 
> Becau

Re: Problem setting host net params via iscsiuio on first try: bad error message

2014-12-12 Thread Lee Duncan
On Dec 12, 2014, at 12:08 PM, Mike Christie  wrote:
> 
> In that other thread, I said I wanted to find out why we are
> adding/removing sessions. That should not be happening and if you
> figured that out, I was saying that I thought you would not hit the
> sysfs scanning issue. Also I said in that other thread I found that bug
> with the discoveryd daemon caching when it should not so that also
> causes a bug that might be affecting your testing.
> 

You have valid points, of course, but let's discuss that issue on that thread.

> 
>>> 
>>> Please just fix this properly. It is really silly that the user would
>>> have to run this command twice to actually do discovery.
>>> 
>> 
>> Ouch. Okay, interesting approach. I guess I stand rebuffed.
>> 
>> "Fixing this" would really mean getting rid of iscsiuio and having BRCM 
>> devices use the network stack, wouldn't it? I'm not sure where you draw the 
>> line between cosmetic hacks and fixing it "properly".
> 
> Upstream already rejected the net stack approach so that is not an option.

I understood as much from Eddie. I shouldn't have suggested it, as I know it's 
not an option. I was trying (poorly) to make a point about how slippery the 
"right thing" slope can be. I withdraw the point.

> 
>> 
>> Why do you think the user has to "run this command twice"? That is 
>> incorrect. The iscsiadm command retries the communication 10 times, I 
>> believe, or until iscsiuio replies. Do you consider that silly? Do you mean 
>> that the iscsiadm code should not retry?
>> 
> 
> If the user/caller has retries off then they would have to retry. In the
> command output you put in the mail, it was not clear if you allowed
> retries or not.
> 
> iscsid and/or iscsiadm should not have to retry. It seems like a bogus
> design that it does not work the first time. That is what should be fixed.

I am surprised to hear you say that. That seems a little like saying device 
drivers shouldn't have to retry I/O commands: they should just work the first 
time.

The code in open-iscsi that does this retry loop is in uip_broadcast() in 
iscsid_req.c:

...
+#define MAX_UIP_BROADCAST_READ_TRIES 3
+   for (count = 0; count < MAX_UIP_BROADCAST_READ_TRIES; count++) {
+   /*  Wait for the response */
+   err = read(fd, &rsp, sizeof(rsp));
+   if (err == sizeof(rsp)) {
+   log_debug(3, "Broadcasted to uIP with length: %ld "
+"cmd: 0x%x rsp: 0x%x\n", buf_len,
+rsp.command, rsp.err);
+   err = 0;
+   break;
+   } else if ((err == -1) && (errno == EAGAIN)) {
+   usleep(25);
+   continue;
+   } else {
+   log_error("Could not read response (%d/%d), daemon "
+ "died?", err, errno);
+   err = ISCSI_ERR;
+   break;
+   }
+   } 
...

This seems to have been added last year when Eddie sent you a patch (see commit 
d571cdfc0b3c539310f43dc11d54d9308299efdc). That commit was quite extensive, 
introducing the EAGAIN error to iscsi processing.

And it looks like there are only 3 retries in this case, not 10 as I guessed 
before. And only in the case of EAGAIN.

If you look at iscsiuio (as I have now for a few days), it's clear that it was 
designed as a minimalist state machine. The choice of uIP for the network stack 
reenforces that impression. The design is all about assuming nothing comes from 
the operating system, and is often used in firmware.

Because of this, I hesitate to modify or replace this state machine, no matter 
how much fun or how pure such an exercise might be.

Instead, I believe the current design works just fine, given all of the design 
considerations (of which there are too many to list here). I therefore still 
think a simple tweak to this current design is all that is needed. The current 
design either works well enough or isn't used, since few bug reports seem to 
have surfaced.

I believe the best approach here is one of two:

- simplest: just allow the "EAGAIN" to percolate up so the error message is 
more correct, or
- better: change the code to not print an error message when EAGAIN is received 
and we still have retries left

I know you think there is a 3rd approach, but I see problems with "fixing" 
this, and here's why ...

The problem: iscsiuio needs to initialize an ethernet NIC/adapter before it can 
use it for bnx2i traffic (e.g. set up a VLAN, etc), but it does not know which 
adapter might be used until it gets a message from iscsid saying it has a new 
host connection. So there is no way for iscsiuio to prepare an adapter until it 
gets a request. And preparing an adapter takes a non-trivial amount of time. 
This is the same problems device drivers have with disc drives: the response 
will take too long to wait for.

One p

Re: Problem setting host net params via iscsiuio on first try: bad error message

2014-12-12 Thread The Lee-Man
On Friday, December 12, 2014 12:26:00 PM UTC-8, Mike Christie wrote:
>
> On 12/12/2014 12:30 PM, Mike Christie wrote: 
> > On 12/11/2014 01:53 PM, The Lee-Man wrote: 
> >> Hi Mike: 
> >> 
> >> I have started working more with iscsiuio. 
> >> 
> >> I have discovered what I consider a bug in an error message printed 
> >> during normal operation of iscsiadm that makes it seem like something 
> >> bad happened. 
> >> 
> >> As you know, when performing discovery via the bnx2i transport and the 
> >> iscsiuio daemon, the iscsiadm command tries to set up host parameters 
> >> when a target is discovered. The iscsiadm command does this by sending 
> a 
> >> message to the iscsiuio daemon, who actually does the work in this case 
> >> and then normally returns that information. 
> >> 
> >> But it looks like the design of iscsiuio is that it does not even try 
> to 
> >> set up the NIC it is using very first time its called. Instead, it 
> >> initializes the NIC only when the first attempt to use it is received, 
> >> while it returns EAGAIN to the caller. The protocol evidently is that 
> >> the caller knows to retry. In this case, the iscsiadm code retries 
> >> several times no matter what error is returned. In fact, the iscsiadm 
> >> code being used translates all errors received from this attempt to 
> talk 
> >> to iscsiuio into an "INTERNAL_ERROR", and the communication is retried 
> >> anyway. 
> >> 
> >> This translation is the only problem, IMHO, and resulted in these 
> >> messages from iscsiadm, which are misleading (and poorly formatted): 
> >> 
> >>  iscsiadm: Could not set host net params (err 29) 
> >> 
> >>  iscsiadm: Connection to discovery portal 10.125.128.120 failed: 
> >> internal error 
> >>  ... 
> >> 
> >> which becomes, with my patches: 
> >> 
> >>  iscsiadm: Could not set host net params (err 29) 
> >>  iscsiadm: Connection to discovery portal 192.168.30.1 failed: 
> operation 
> >> failed but retry may succeed 
> >>  ... 
> >> 
> >> The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it 
> >> through. (And fix the error message printing so we don't core dump, of 
> >> course.) 
> >> 
> >> I thought about a more aggressive fix, i.e. letting all return values 
> >> from the communications attempt through, but I worried about fixing 
> >> something that is not really broken and perhaps breaking something 
> else. 
> >> So, in the spirit of fixing just what is broken, I submit two patches: 
> > 
> > That is not how we do it. Just like in that other thread about the sysfs 
> > scanning. We want to fix the root cause and also any issues that result 
> > from it. 
> > 
> > Please just fix this properly. It is really silly that the user would 
> > have to run this command twice to actually do discovery. 
>
> Hi Lee, 
>
> I want to apologize for being a jerk above. The email is unprofessional. 
> I value your submissions and help on this project. I have no excuse, and 
> request that you please continue to help out and be part of this project 
>
> Mike 
>

Mike: I apologize for taking it personally. No harm no foul. 

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Problem setting host net params via iscsiuio on first try: bad error message

2014-12-12 Thread Mike Christie
On 12/12/2014 12:30 PM, Mike Christie wrote:
> On 12/11/2014 01:53 PM, The Lee-Man wrote:
>> Hi Mike:
>>
>> I have started working more with iscsiuio.
>>
>> I have discovered what I consider a bug in an error message printed
>> during normal operation of iscsiadm that makes it seem like something
>> bad happened.
>>
>> As you know, when performing discovery via the bnx2i transport and the
>> iscsiuio daemon, the iscsiadm command tries to set up host parameters
>> when a target is discovered. The iscsiadm command does this by sending a
>> message to the iscsiuio daemon, who actually does the work in this case
>> and then normally returns that information.
>>
>> But it looks like the design of iscsiuio is that it does not even try to
>> set up the NIC it is using very first time its called. Instead, it
>> initializes the NIC only when the first attempt to use it is received,
>> while it returns EAGAIN to the caller. The protocol evidently is that
>> the caller knows to retry. In this case, the iscsiadm code retries
>> several times no matter what error is returned. In fact, the iscsiadm
>> code being used translates all errors received from this attempt to talk
>> to iscsiuio into an "INTERNAL_ERROR", and the communication is retried
>> anyway.
>>
>> This translation is the only problem, IMHO, and resulted in these
>> messages from iscsiadm, which are misleading (and poorly formatted):
>>
>>  iscsiadm: Could not set host net params (err 29)
>>
>>  iscsiadm: Connection to discovery portal 10.125.128.120 failed:
>> internal error
>>  ...
>>
>> which becomes, with my patches:
>>
>>  iscsiadm: Could not set host net params (err 29)
>>  iscsiadm: Connection to discovery portal 192.168.30.1 failed: operation
>> failed but retry may succeed
>>  ...
>>
>> The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it
>> through. (And fix the error message printing so we don't core dump, of
>> course.)
>>
>> I thought about a more aggressive fix, i.e. letting all return values
>> from the communications attempt through, but I worried about fixing
>> something that is not really broken and perhaps breaking something else.
>> So, in the spirit of fixing just what is broken, I submit two patches:
> 
> That is not how we do it. Just like in that other thread about the sysfs
> scanning. We want to fix the root cause and also any issues that result
> from it.
> 
> Please just fix this properly. It is really silly that the user would
> have to run this command twice to actually do discovery.

Hi Lee,

I want to apologize for being a jerk above. The email is unprofessional.
I value your submissions and help on this project. I have no excuse, and
request that you please continue to help out and be part of this project

Mike

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Problem setting host net params via iscsiuio on first try: bad error message

2014-12-12 Thread Mike Christie
On 12/12/2014 01:13 PM, Lee Duncan wrote:
> On Dec 12, 2014, at 10:30 AM, Mike Christie  wrote:
>>
>> On 12/11/2014 01:53 PM, The Lee-Man wrote:
>>> Hi Mike:
>>>
>>> I have started working more with iscsiuio.
>>>
>>> I have discovered what I consider a bug in an error message printed
>>> during normal operation of iscsiadm that makes it seem like something
>>> bad happened.
>>>
>>> As you know, when performing discovery via the bnx2i transport and the
>>> iscsiuio daemon, the iscsiadm command tries to set up host parameters
>>> when a target is discovered. The iscsiadm command does this by sending a
>>> message to the iscsiuio daemon, who actually does the work in this case
>>> and then normally returns that information.
>>>
>>> But it looks like the design of iscsiuio is that it does not even try to
>>> set up the NIC it is using very first time its called. Instead, it
>>> initializes the NIC only when the first attempt to use it is received,
>>> while it returns EAGAIN to the caller. The protocol evidently is that
>>> the caller knows to retry. In this case, the iscsiadm code retries
>>> several times no matter what error is returned. In fact, the iscsiadm
>>> code being used translates all errors received from this attempt to talk
>>> to iscsiuio into an "INTERNAL_ERROR", and the communication is retried
>>> anyway.
>>>
>>> This translation is the only problem, IMHO, and resulted in these
>>> messages from iscsiadm, which are misleading (and poorly formatted):
>>>
>>> iscsiadm: Could not set host net params (err 29)
>>>
>>> iscsiadm: Connection to discovery portal 10.125.128.120 failed:
>>> internal error
>>> ...
>>>
>>> which becomes, with my patches:
>>>
>>> iscsiadm: Could not set host net params (err 29)
>>> iscsiadm: Connection to discovery portal 192.168.30.1 failed: operation
>>> failed but retry may succeed
>>> ...
>>>
>>> The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it
>>> through. (And fix the error message printing so we don't core dump, of
>>> course.)
>>>
>>> I thought about a more aggressive fix, i.e. letting all return values
>>> from the communications attempt through, but I worried about fixing
>>> something that is not really broken and perhaps breaking something else.
>>> So, in the spirit of fixing just what is broken, I submit two patches:
>>
>> That is not how we do it. Just like in that other thread about the sysfs
>> scanning. We want to fix the root cause and also any issues that result
>> from it.
> 
> Interesting that you would reference that other thread. I don't think there's 
> any absolute root cause in either case, so it's a judgement call.
> 

In that other thread, I said I wanted to find out why we are
adding/removing sessions. That should not be happening and if you
figured that out, I was saying that I thought you would not hit the
sysfs scanning issue. Also I said in that other thread I found that bug
with the discoveryd daemon caching when it should not so that also
causes a bug that might be affecting your testing.


>>
>> Please just fix this properly. It is really silly that the user would
>> have to run this command twice to actually do discovery.
>>
> 
> Ouch. Okay, interesting approach. I guess I stand rebuffed.
> 
> "Fixing this" would really mean getting rid of iscsiuio and having BRCM 
> devices use the network stack, wouldn't it? I'm not sure where you draw the 
> line between cosmetic hacks and fixing it "properly".

Upstream already rejected the net stack approach so that is not an option.

> 
> Why do you think the user has to "run this command twice"? That is incorrect. 
> The iscsiadm command retries the communication 10 times, I believe, or until 
> iscsiuio replies. Do you consider that silly? Do you mean that the iscsiadm 
> code should not retry?
> 

If the user/caller has retries off then they would have to retry. In the
command output you put in the mail, it was not clear if you allowed
retries or not.

iscsid and/or iscsiadm should not have to retry. It seems like a bogus
design that it does not work the first time. That is what should be fixed.


> Or do you mean the iscsiuio code should be redesigned? This old code uses the 
> uIP stack to perform the functions of a network driver. I am hesitant to 
> redesign this multi-threaded beast when I have only one BRCM card, and it 
> mostly just works now.
> 

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Problem setting host net params via iscsiuio on first try: bad error message

2014-12-12 Thread Lee Duncan
On Dec 12, 2014, at 10:30 AM, Mike Christie  wrote:
> 
> On 12/11/2014 01:53 PM, The Lee-Man wrote:
>> Hi Mike:
>> 
>> I have started working more with iscsiuio.
>> 
>> I have discovered what I consider a bug in an error message printed
>> during normal operation of iscsiadm that makes it seem like something
>> bad happened.
>> 
>> As you know, when performing discovery via the bnx2i transport and the
>> iscsiuio daemon, the iscsiadm command tries to set up host parameters
>> when a target is discovered. The iscsiadm command does this by sending a
>> message to the iscsiuio daemon, who actually does the work in this case
>> and then normally returns that information.
>> 
>> But it looks like the design of iscsiuio is that it does not even try to
>> set up the NIC it is using very first time its called. Instead, it
>> initializes the NIC only when the first attempt to use it is received,
>> while it returns EAGAIN to the caller. The protocol evidently is that
>> the caller knows to retry. In this case, the iscsiadm code retries
>> several times no matter what error is returned. In fact, the iscsiadm
>> code being used translates all errors received from this attempt to talk
>> to iscsiuio into an "INTERNAL_ERROR", and the communication is retried
>> anyway.
>> 
>> This translation is the only problem, IMHO, and resulted in these
>> messages from iscsiadm, which are misleading (and poorly formatted):
>> 
>> iscsiadm: Could not set host net params (err 29)
>> 
>> iscsiadm: Connection to discovery portal 10.125.128.120 failed:
>> internal error
>> ...
>> 
>> which becomes, with my patches:
>> 
>> iscsiadm: Could not set host net params (err 29)
>> iscsiadm: Connection to discovery portal 192.168.30.1 failed: operation
>> failed but retry may succeed
>> ...
>> 
>> The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it
>> through. (And fix the error message printing so we don't core dump, of
>> course.)
>> 
>> I thought about a more aggressive fix, i.e. letting all return values
>> from the communications attempt through, but I worried about fixing
>> something that is not really broken and perhaps breaking something else.
>> So, in the spirit of fixing just what is broken, I submit two patches:
> 
> That is not how we do it. Just like in that other thread about the sysfs
> scanning. We want to fix the root cause and also any issues that result
> from it.

Interesting that you would reference that other thread. I don't think there's 
any absolute root cause in either case, so it's a judgement call.

> 
> Please just fix this properly. It is really silly that the user would
> have to run this command twice to actually do discovery.
> 

Ouch. Okay, interesting approach. I guess I stand rebuffed.

"Fixing this" would really mean getting rid of iscsiuio and having BRCM devices 
use the network stack, wouldn't it? I'm not sure where you draw the line 
between cosmetic hacks and fixing it "properly".

Why do you think the user has to "run this command twice"? That is incorrect. 
The iscsiadm command retries the communication 10 times, I believe, or until 
iscsiuio replies. Do you consider that silly? Do you mean that the iscsiadm 
code should not retry?

Or do you mean the iscsiuio code should be redesigned? This old code uses the 
uIP stack to perform the functions of a network driver. I am hesitant to 
redesign this multi-threaded beast when I have only one BRCM card, and it 
mostly just works now.
-- 
The Lee-Man Rides Again

"...And maddest of all, to see life as it is and not as it should be!" — 
Cervantes


-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Problem setting host net params via iscsiuio on first try: bad error message

2014-12-12 Thread Mike Christie
On 12/11/2014 01:53 PM, The Lee-Man wrote:
> Hi Mike:
> 
> I have started working more with iscsiuio.
> 
> I have discovered what I consider a bug in an error message printed
> during normal operation of iscsiadm that makes it seem like something
> bad happened.
> 
> As you know, when performing discovery via the bnx2i transport and the
> iscsiuio daemon, the iscsiadm command tries to set up host parameters
> when a target is discovered. The iscsiadm command does this by sending a
> message to the iscsiuio daemon, who actually does the work in this case
> and then normally returns that information.
> 
> But it looks like the design of iscsiuio is that it does not even try to
> set up the NIC it is using very first time its called. Instead, it
> initializes the NIC only when the first attempt to use it is received,
> while it returns EAGAIN to the caller. The protocol evidently is that
> the caller knows to retry. In this case, the iscsiadm code retries
> several times no matter what error is returned. In fact, the iscsiadm
> code being used translates all errors received from this attempt to talk
> to iscsiuio into an "INTERNAL_ERROR", and the communication is retried
> anyway.
> 
> This translation is the only problem, IMHO, and resulted in these
> messages from iscsiadm, which are misleading (and poorly formatted):
> 
>  iscsiadm: Could not set host net params (err 29)
> 
>  iscsiadm: Connection to discovery portal 10.125.128.120 failed:
> internal error
>  ...
> 
> which becomes, with my patches:
> 
>  iscsiadm: Could not set host net params (err 29)
>  iscsiadm: Connection to discovery portal 192.168.30.1 failed: operation
> failed but retry may succeed
>  ...
> 
> The "fix" is simple: in the case where iscsiuio returns EAGAIN, let it
> through. (And fix the error message printing so we don't core dump, of
> course.)
> 
> I thought about a more aggressive fix, i.e. letting all return values
> from the communications attempt through, but I worried about fixing
> something that is not really broken and perhaps breaking something else.
> So, in the spirit of fixing just what is broken, I submit two patches:

That is not how we do it. Just like in that other thread about the sysfs
scanning. We want to fix the root cause and also any issues that result
from it.

Please just fix this properly. It is really silly that the user would
have to run this command twice to actually do discovery.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.