Mike,

From: Mike Christie  on Monday, January 31, 2011 4:01 PM
>
> On 01/31/2011 11:31 AM, Mark Rustad wrote:
> > +#ifdef Linux
> 
> We only support linux now and this file was used for all OSs so you can
> just remove the ifdefs.

Excellent. I saw the FreeBSD references in the Makefile, so I added these.
More than happy to remove them!

> > +static int select_priority(iscsi_conn_t *conn, int pri_mask)
> 
> I have been trying to move away from the iscsi_conn_t and just use
> struct iscsi_conn.

Again, happy to change that. You might notice that the parameter is not
used. I assume that it would be used if the TODO in that function were
to be addressed. If you prefer, I could remove the parameter entirely.

> > +
> > +#ifdef Linux
> > +   int pri_mask = 0;
> > +
> > +   if (conn->session->netdev[0])
> > +           pri_mask = get_dcb_app_pri_by_port(conn->session->netdev,
> > +                                           ISCSI_DEFAULT_PORT);
> 
> The problem with doing this here is that we have 2 modes:
> 
> 1. iscsi layer uses socket layer like a normal boring application where
> it just uses the default routing. In this mode we netdev is not set to a
> valid interface here, because we do not know what interface the routing
> is going to take us through at this point. We are just a dumb app and do
> not care, and rely on the network admin to set things up correctly.
> 
> Your patch will not work with this mode.

Yes, I should have mentioned this deficiency. I was aware of it, but forgot
to mention it.

> Solution:?
> 
> - Can we do the SO_PRIORITY sockopt after we are connected?

Yes, with the proviso that any packets sent before the priority is set will
be sent with default priority.

> If so then
> after we connect can we do something like getsockname() to figure out
> what netdevice the socket using. Would we go getsockname to get the addr
> then do if_nameindex or getifaddrs to match the address with a nic?

I think that getifaddrs would have to be used. Are there cases where the stack
could change the association dynamically to another interface?

> Would we do this in iscsi_io_tcp_poll then poll indicates we are logged in?

I'm really not that familiar with open-iscsi yet. Is that where you think that
it would go?

> If we have to do SO_PRIORITY before connect() then would we have to look
> through the routing table and figure out what is going to be used?

That would be nasty.

> 2. iscsi layer will bind the socket to a specific interface. In this
> case the netdev is set to a valid interface and bind_conn_to_iface()
> calls setsockopt SO_BINDTODEVICE to tell network layer to forget routing
> and use what we tell it (if it wrong and there is not valid route then
> we just fail, so the user has to know what they are doing).
> 
> Your patch should work ok with this mode.

Yes, this was the only case I was intending to address with these patches.

I will make the simple changes you suggest. Should I send updated patches at
that point and then look into handling sessions that are not bound to a
net device in a separate series?

-- 
Mark Rustad, LAN Access Division, Intel Corporation.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/open-iscsi?hl=en.

Reply via email to