[ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow

[Smith, Stan]

________________________________
From: Tzachi Dar [mailto:tzachid at mellanox.co.il]
Sent: Thursday, October 07, 2010 8:58 AM
To: Smith, Stan; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow

I'm fine with the ipoib forwarding checkin. (if no one else has objection).

OK, will make it part of winOFED 2.3 RC4; meaning I will go ahead and commit the patch to winOFED SVN.

As for the shutter checkin, we have been running with it for a few days, but I don't know if shutdown is really tested.

Will include this patch in 2.3 RC4 as I have not witnessed any shutdown problems so far in pre-RC4 testing.
I'll hold back on committing the patch until we have more shutdown testing on both sides of the pond.

thanks,

stan.


Thanks
Tzachi

From: Smith, Stan [mailto:stan.smith at intel.com]
Sent: Thursday, October 07, 2010 5:50 PM
To: Tzachi Dar; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow

Hello,
  If shutter_shut() is called twice an ASSERT() in shutter.h::shutter_shut() will fire.
I do not see the ASSERT() fire so we are OK; although code inspection looked as if shutter_shut() 'could' be called twice:
  1) ex_shutdown path - power down case
  2) PNP port down path

What are your plans for SVN committing this patch and the patch for fixing forwarding of ipoib ?


________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
Sent: Thursday, October 07, 2010 3:22 AM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: Re: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow
Hi Stan,

Shuter_shut should not be called twice. In the case that it will than it will probably stay shut "for ever".

The real question is whether it can be called twice. I believe that there is an assumption in the code that once reset is called shut down cannot be called, so the shutter will not be called twice.

Thanks
Tzachi

From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Wednesday, October 06, 2010 1:13 PM
To: ofw at lists.openfabrics.org
Subject: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow

The patch below prevents race with BSOD when NDIS call receive callback of ipoib while
some ipoib global objects are under destruction (like p_adapter ->p_port).
It was tested by night regression run on IPoIB driver
---
Adding shutter_shut for Adapter Reset flow
Signed-off by: Alexander Naslednikov (xalex at mellanox.co.il)

Index: kernel/ipoib_adapter.cpp
===================================================================
--- kernel/ipoib_adapter.cpp      (revision 2938)
+++ kernel/ipoib_adapter.cpp   (working copy)
@@ -988,10 +988,13 @@
                p_adapter->reset = TRUE;

                if( p_adapter->h_pnp )
-              {
+             {
+
                                h_pnp = p_adapter->h_pnp;
                                p_adapter->h_pnp  = NULL;
                                status = p_adapter->p_ifc->dereg_pnp( h_pnp, __ipoib_pnp_dereg );
+                             // Wait until NDIS will return all indicated NBLs that were received
+                             shutter_shut( &p_adapter->recv_shutter );
                                if( status == IB_SUCCESS )
                                                status = IB_NOT_DONE;
                }
Index: kernel/ipoib_port.cpp
===================================================================
--- kernel/ipoib_port.cpp             (revision 2946)
+++ kernel/ipoib_port.cpp          (working copy)
@@ -830,11 +830,15 @@
                IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_OBJ,
                                ("ref type %d ref_cnt %d\n", ref_init, p_port->obj.ref_cnt) );
 #endif
-              // The port is started as paused and NDIS calls latter to ipoib_restart. We
-              // shut the recv_shuter for now and alive it on ipoib_restart. We did
-              // it in this way since MpInitializeInternal also calls in reset and than
-              // we need to set the rec ref count to 1 //TODO !!!!!!!!!!1
-              //
+             /* The steps of the initialization are as depicted below:
+             I.            adapter_init() calls shutter_init(), the shutter counter is set to 0
+             II.            ipoib_pnp_cb() calls to ipoib_port_init() that calls to __port_init() that SHOULD set shutter counter to -MAX_OPERATIONS
+                             That is, the code below should call to shutter_shut()
+             III.NDIS calls to ipoib_restart() that calls to shutter_alive. Now, shutter_counter is again 0.
+             IV.          NDIS call to ipoib_pause() that sets the adapter to pause state, calls to shutter_shut.
+                             Now, the counter is again equals to -MAX_OPERATIONS
+             V.            NDIS calls to ipoib_restart that calls to shutter_alive. Shutter counter is 0 and we can start working
+             */

                if ( p_adapter->ipoib_state == IPOIB_INIT) {
                                IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,

Alexander (XaleX) Naslednikov
SW Networking Team
Mellanox Technologies

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20101007/2ffb572b/attachment.html>