Re: [PATCH 12/16] BNX2I: Added feature to silently drop NOPOUT request

2010-11-19 Thread Mike Christie

On 11/18/2010 03:59 PM, Eddie Wai wrote:

As for the NOPOUT requests, since the NOPOUT send request failed, the
last_ping jiffies count would not get updated.  Then the
iscsi_check_transport_timeouts callback will just keeps getting called
and stalling the system.

Perhaps a better way to do this is to allow the last_ping jiffies to be
updated but use a different ping_timeout value for failed nopout ping
conditions.



If this is a hard error then we can just have xmit_task return a error, 
and then have iscsi_send_nopout call iscsi_conn_failure for the driver.


Is there any issue with starting the eh process at this time? Would the 
quicker swing around to ep_disconnect and ep_connect cause problems?


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



Re: [PATCH 12/16] BNX2I: Added feature to silently drop NOPOUT request

2010-11-18 Thread Eddie Wai

On Thu, 2010-11-18 at 12:04 -0800, Mike Christie wrote:
> On 11/18/2010 01:55 PM, Mike Christie wrote:
> > On 11/18/2010 01:25 PM, Eddie Wai wrote:
> >>
> >> On Wed, 2010-11-17 at 19:40 -0800, Mike Christie wrote:
> >>> On 11/10/2010 05:04 PM, Eddie Wai wrote:
>  In the case the chip is undergoing different invasive operation
>  which requires a chip reset, all NOPOUT request during this period
> >>>
> >>> For these invasive operations that reset the chip, do we always end up
> >>> having to relogin the connection/session or once the reset is done are
> >>> we able to just go on happily like nothing ever happened?
> >> Operations like mtu change/ifupdown/etc will require the chip to undergo
> >> reset. Prior to this, the connections will be cleaned up via the
> >> conn_failure->ep_disconnect path and eventually put into the reopen
> >> recovery path. During this period, we must disallow any send pdu
> >> requests to be queued to the chip for a more immediately connection tear
> >> down time (so we don't have to wait for the pdu's completion).
> >>
> >> We had to treat NOPOUT requests differently as the routine in libiscsi
> >> would continuously loop until the NOPOUT send request returns with
> >> success. This is the why we added the NOPOUT workaround.
> >
> > At this time, have you already called iscsi_conn or session failure? If
> > so then I think it sounds like there is bug in iscsi_send_nopout or
> > __iscsi_conn_send_pdu. If the conn/session has been failed, I think we
> > want to add a check in __iscsi_conn_send_pdu where if the conn/session
> > is down then we do not send NOPs. There is no point iSCSI RFC wise and
> > it screws up drivers.
> 
> We actually have a check in __iscsi_conn_send_pdu. There is the 
> session->state == ISCSI_STATE_LOGGED_IN, so I guess you have not called 
> one of the iscsi failure functions.
> 
The check is correct, but its just that the conn failures were not being
called prior to these inflight send_pdu calls.  We want to terminate
these calls immediately before they get queued up to the chip.

As for the NOPOUT requests, since the NOPOUT send request failed, the
last_ping jiffies count would not get updated.  Then the
iscsi_check_transport_timeouts callback will just keeps getting called
and stalling the system.

Perhaps a better way to do this is to allow the last_ping jiffies to be
updated but use a different ping_timeout value for failed nopout ping
conditions.

> At this time, is just the apdater_state getting changed? What code path 
> is that?
> 
> Maybe related... For bnx2i_get_link_state ADAPTER_STATE_LINK_DOWN, I 
> think you will want to call the iscsi_suspend_queue function discussed 
> in the other mail. When the link state comes back up though, do we 
> always have to reconnect and relogin to the target or are their cases 
> where we can just restart the queues?
> 


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



Re: [PATCH 12/16] BNX2I: Added feature to silently drop NOPOUT request

2010-11-18 Thread Mike Christie

On 11/18/2010 01:55 PM, Mike Christie wrote:

On 11/18/2010 01:25 PM, Eddie Wai wrote:


On Wed, 2010-11-17 at 19:40 -0800, Mike Christie wrote:

On 11/10/2010 05:04 PM, Eddie Wai wrote:

In the case the chip is undergoing different invasive operation
which requires a chip reset, all NOPOUT request during this period


For these invasive operations that reset the chip, do we always end up
having to relogin the connection/session or once the reset is done are
we able to just go on happily like nothing ever happened?

Operations like mtu change/ifupdown/etc will require the chip to undergo
reset. Prior to this, the connections will be cleaned up via the
conn_failure->ep_disconnect path and eventually put into the reopen
recovery path. During this period, we must disallow any send pdu
requests to be queued to the chip for a more immediately connection tear
down time (so we don't have to wait for the pdu's completion).

We had to treat NOPOUT requests differently as the routine in libiscsi
would continuously loop until the NOPOUT send request returns with
success. This is the why we added the NOPOUT workaround.


At this time, have you already called iscsi_conn or session failure? If
so then I think it sounds like there is bug in iscsi_send_nopout or
__iscsi_conn_send_pdu. If the conn/session has been failed, I think we
want to add a check in __iscsi_conn_send_pdu where if the conn/session
is down then we do not send NOPs. There is no point iSCSI RFC wise and
it screws up drivers.


We actually have a check in __iscsi_conn_send_pdu. There is the 
session->state == ISCSI_STATE_LOGGED_IN, so I guess you have not called 
one of the iscsi failure functions.


At this time, is just the apdater_state getting changed? What code path 
is that?


Maybe related... For bnx2i_get_link_state ADAPTER_STATE_LINK_DOWN, I 
think you will want to call the iscsi_suspend_queue function discussed 
in the other mail. When the link state comes back up though, do we 
always have to reconnect and relogin to the target or are their cases 
where we can just restart the queues?


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



Re: [PATCH 12/16] BNX2I: Added feature to silently drop NOPOUT request

2010-11-18 Thread Mike Christie

On 11/18/2010 01:25 PM, Eddie Wai wrote:


On Wed, 2010-11-17 at 19:40 -0800, Mike Christie wrote:

On 11/10/2010 05:04 PM, Eddie Wai wrote:

In the case the chip is undergoing different invasive operation
which requires a chip reset, all NOPOUT request during this period


For these invasive operations that reset the chip, do we always end up
having to relogin the connection/session or once the reset is done are
we able to just go on happily like nothing ever happened?

Operations like mtu change/ifupdown/etc will require the chip to undergo
reset.  Prior to this, the connections will be cleaned up via the
conn_failure->ep_disconnect path and eventually put into the reopen
recovery path.  During this period, we must disallow any send pdu
requests to be queued to the chip for a more immediately connection tear
down time (so we don't have to wait for the pdu's completion).

We had to treat NOPOUT requests differently as the routine in libiscsi
would continuously loop until the NOPOUT send request returns with
success.  This is the why we added the NOPOUT workaround.


At this time, have you already called iscsi_conn or session failure? If 
so then I think it sounds like there is bug in iscsi_send_nopout or 
__iscsi_conn_send_pdu. If the conn/session has been failed, I think we 
want to add a check in __iscsi_conn_send_pdu where if the conn/session 
is down then we do not send NOPs. There is no point iSCSI RFC wise and 
it screws up drivers.


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



Re: [PATCH 12/16] BNX2I: Added feature to silently drop NOPOUT request

2010-11-18 Thread Eddie Wai

On Wed, 2010-11-17 at 19:40 -0800, Mike Christie wrote:
> On 11/10/2010 05:04 PM, Eddie Wai wrote:
> > In the case the chip is undergoing different invasive operation
> > which requires a chip reset, all NOPOUT request during this period
> 
> For these invasive operations that reset the chip, do we always end up 
> having to relogin the connection/session or once the reset is done are 
> we able to just go on happily like nothing ever happened?
Operations like mtu change/ifupdown/etc will require the chip to undergo
reset.  Prior to this, the connections will be cleaned up via the
conn_failure->ep_disconnect path and eventually put into the reopen
recovery path.  During this period, we must disallow any send pdu
requests to be queued to the chip for a more immediately connection tear
down time (so we don't have to wait for the pdu's completion).

We had to treat NOPOUT requests differently as the routine in libiscsi
would continuously loop until the NOPOUT send request returns with
success.  This is the why we added the NOPOUT workaround.

Once the chip reset is completed, the ep_connect requests from the
recovery path will be honored and the pre-existed connections will get
re-established.  
> 
> > must be silently dropped from queuing to the hardware.  This patch
> > will respond to libiscsi immediately with sent success.  Since the
> > request was not actually sent, the NOPIN wait timeout will get
> > triggered and another NOPOUT request will commence through the
> > proper channel.
> >
> > Signed-off-by: Eddie Wai
> > Acked-by: Anil Veerabhadrappa
> > ---
> >   drivers/scsi/bnx2i/bnx2i_iscsi.c |   16 
> >   1 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
> > b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> > index b32baf8..507cd25 100644
> > --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
> > +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> > @@ -1068,6 +1068,22 @@ static int bnx2i_iscsi_send_generic_request(struct 
> > iscsi_task *task)
> > char *buf;
> > int data_len;
> >
> > +   /*
> > +* Forcefully terminate all in progress connection recovery at the
> > +* earliest, either in bind(), send_pdu(LOGIN), or conn_start()
> > +*/
> > +   if (bnx2i_adapter_ready(bnx2i_conn->ep->hba)) {
> > +   if ((task->hdr->opcode&  ISCSI_OPCODE_MASK) ==
> > +   ISCSI_OP_NOOP_OUT)
> > +   /* This is a WA to indicate to libiscsi that the nopout
> > +* request was sent successfully without actually
> > +* submitting to the hardware.
> > +* Just silently drop the nopout request
> > +*/
> > +   return 0;
> > +   else
> > +   return -EIO;
> > +   }
> > bnx2i_iscsi_prep_generic_pdu_bd(bnx2i_conn);
> > switch (task->hdr->opcode&  ISCSI_OPCODE_MASK) {
> > case ISCSI_OP_LOGIN:
> 
> 


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



Re: [PATCH 12/16] BNX2I: Added feature to silently drop NOPOUT request

2010-11-17 Thread Mike Christie

On 11/10/2010 05:04 PM, Eddie Wai wrote:

In the case the chip is undergoing different invasive operation
which requires a chip reset, all NOPOUT request during this period


For these invasive operations that reset the chip, do we always end up 
having to relogin the connection/session or once the reset is done are 
we able to just go on happily like nothing ever happened?



must be silently dropped from queuing to the hardware.  This patch
will respond to libiscsi immediately with sent success.  Since the
request was not actually sent, the NOPIN wait timeout will get
triggered and another NOPOUT request will commence through the
proper channel.

Signed-off-by: Eddie Wai
Acked-by: Anil Veerabhadrappa
---
  drivers/scsi/bnx2i/bnx2i_iscsi.c |   16 
  1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index b32baf8..507cd25 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -1068,6 +1068,22 @@ static int bnx2i_iscsi_send_generic_request(struct 
iscsi_task *task)
char *buf;
int data_len;

+   /*
+* Forcefully terminate all in progress connection recovery at the
+* earliest, either in bind(), send_pdu(LOGIN), or conn_start()
+*/
+   if (bnx2i_adapter_ready(bnx2i_conn->ep->hba)) {
+   if ((task->hdr->opcode&  ISCSI_OPCODE_MASK) ==
+   ISCSI_OP_NOOP_OUT)
+   /* This is a WA to indicate to libiscsi that the nopout
+* request was sent successfully without actually
+* submitting to the hardware.
+* Just silently drop the nopout request
+*/
+   return 0;
+   else
+   return -EIO;
+   }
bnx2i_iscsi_prep_generic_pdu_bd(bnx2i_conn);
switch (task->hdr->opcode&  ISCSI_OPCODE_MASK) {
case ISCSI_OP_LOGIN:


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