On Dec 12, 2014, at 12:08 PM, Mike Christie <[email protected]> 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(250000);
+ 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 possible "fix" would be to have iscsiuio actually wait to respond to its
first request until the adapter is prepared and UP, but that would be no better
than having the caller retry: it still blocks success and makes the end user
wait until it is complete.
Perhaps a cleaner fix would be to have iscsiuio accept the request to handle a
new host by detaching and then replying later, asynchronously, when the NIC is
set up and the host is registered. Even though this seems to me to be a cleaner
design, the problem is that the poor end user that has typed "iscsiadm -m
discovery -t st -I <bnx2i-interface> -t <some-target>" still has to wait! The
are still going to have to wait, at least the first time, for the NIC to be set
up. And discovery kind of has to be synchronous based on the current design.
The only way I can see that might fix this iscsiuio delay-on-first-use problem
is to have iscsiuio aggressively configure any and all bnx2i interfaces it sees
when it first starts up. Or perhaps it should have a configuration file that
says "I'm going to use eth0, so get it ready"? I'm not sure this approach would
be considered cleaner, though.
--
Lee-Man Duncan
"Beer is proof that God loves us and wants us to be happy."
-- Ben Franklin
--
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 [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
