On 1/11/15, 11:37 PM, Chris Leech wrote:
On Thu, Jan 08, 2015 at 08:40:17PM -0600, Mike Christie wrote:
On 01/08/2015 11:11 AM, Chris Leech wrote:
On Thu, Jan 08, 2015 at 10:36:59AM -0600, Michael Christie wrote:

On Jan 6, 2015, at 6:40 PM, Chris Leech <cle...@redhat.com> wrote:

Hi all,

It looks to me that the changes in "iscsid: retry login for
ISCSI_ERR_HOST_NOT_FOUND" have broken interface binding for iscsi_tcp
(and iser, assuming interface bindin

iser does not do binding.

Thanks for clearing that up, less to worry about then.


Is this a regression with the patch:

commit c0e509e7535372cd5d655bc5a20d3d2bae45df84
Author: Mike Christie <micha...@cs.wisc.edu>
Date:   Wed May 7 14:38:13 2014 -0500

     iscsid: retry login for ISCSI_ERR_HOST_NOT_FOUND

Yes, it looks that way to me.


Didn't you have a patch to add some flag on the iscsi_transport struct
in userspace that indicated if it was a offload driver or something like
bind_ep was required? I think you sent it to me, when I was at my old job.

Doesn't ring a bell, must have been someone else.  It does look a lot
cleaner than trying to push the host lookup further down into the
transport connect call and then make more use of the return codes, which
is where I was headed.


Instead of trying to be cute and detect it, then we could just do your
patch and modify this test:

} else if (*rc == ISCSI_ERR_HOST_NOT_FOUND &&
           t->bind_ep_required) {
                 goto free_session;

Or, I guess maybe it is better to check before, so do:

if (t->bind_ep_required) {
         host_no = iscsi_sysfs_get_host_no_from_hwinfo(iface, &rc);

        ......
}

Yeah, that works.  Following this with a full patch.

But, I'm not sold on the bind_ep_required name.  We've got bnx2i which
tries to handle an unbound connect with a driver internal route lookup,

I know that code you mean, but we have always passed the host. I am not sure if that even works. I think some things are going to break due to some sysfs/device-object tree assumptions.

cxgbi which does the same but also always validates the host against a
route lookup, then qla4xxx and be2iscsi which do require userspace

Same for cxgbi.

specifying a host.

It seems to me that what really needs to be treated differently are
dynamic / per-session host allocating transports like iscsi_tcp and
iser.  Should be flag them as dynamic_host? per_session_host?

We can then update transport.c and remove the "session->conn[0].bind_ep
= 1;" lines.

I'm not sure we can remove bind_ep if we still want to support the
cxgb3i and bnx2i general iface names.

I am not sure what you mean here. Are saying you can use cxgbi and bnx2i but not specify a iface name that is associated with a specific cxgbi/bnx2i nic? That has never been supported. Does it work ok?

Actually, the code might be a leftover from when we wanted to bind more closely with the network layer, but were not allowed. As you can see from the cxgbi code you can see it handles things like local port allocation a lot differently from bnx2i. I think during upstream driver review, we were told to work like how bnx2i did it, but somehow cxgbi got in like how they did it. For another example, cxgbi can share IPs with the normal nic function, but bnx2i cannot, but I think bnx2i was told to not do what cxgbi did.

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

Reply via email to