[ofw] [IPoIB_NDIS6_CM][Patch] Improving shutter mechanism for recv flow

Smith, Stan stan.smith at intel.com
Mon Nov 1 17:49:58 PDT 2010


committed.

Revision: 2977
Author: stansmith
Date: 4:09:41 PM, Monday, November 01, 2010
Message:
[IPoiB_NDIS6_CM]
1. Main fix: handling a situation when NDIS sends 'pause' command before the 'reset' command was completed
2. Debug prints improvements
3. Time notification was added to IPOIB_DBG_SHUTTER printouts, because of timestamp importance in this case.
   These debug prints are not in data path, so it's safe to use them here.

signed-off-by: Alex Naslednikov [xalex at mellanox.co.il], stan.smith <stan.smith at intel.com<mailto:stan.smith at intel.com>>

----
Modified : /gen1/branches/WOF2-3/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp
Modified : /gen1/branches/WOF2-3/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h
Modified : /gen1/branches/WOF2-3/ulp/ipoib_NDIS6_CM/kernel/ipoib_debug.h
Modified : /gen1/branches/WOF2-3/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
Modified : /gen1/trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp
Modified : /gen1/trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h
Modified : /gen1/trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_debug.h
Modified : /gen1/trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp

________________________________
From: Alex Naslednikov [mailto:xalex at mellanox.co.il]
Sent: Thursday, October 28, 2010 7:06 AM
To: Smith, Stan; ofw at lists.openfabrics.org
Subject: RE: [ofw] [IPoIB_NDIS6_CM][Patch] Improving shutter mechanism for recv flow

Please, see the revisited version of the patch, which includes:

1.       Main fix: handling a situation when NDIS sends 'pause' command before the 'reset' command was completed

2.       Debug prints improvements

3.       Time notification was added to IPOIB_DBG_SHUTTER printouts, because of timestamp importance in this case. These debug prints are not in data path, so it's safe to use them here.
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp         (revision 2975)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp      (working copy)
@@ -111,6 +111,40 @@
                OUT                                                       UINT                                                                                      *p_len);


+static  void
+             __ipoib_complete_reset(
+             IN                                                           ipoib_adapter_t* const                                p_adapter,
+             IN                                                           NDIS_STATUS                                                                    Status )
+{
+             KLOCK_QUEUE_HANDLE              hdl;
+
+             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+             // Check if Reset flag is set or if adapter is in Paused state
+             p_adapter->ipoib_state =  p_adapter->ipoib_state & ~IPOIB_RESET_OR_DOWN;
+
+             if  ( p_adapter->ipoib_state == IPOIB_RUNNING )
+             {
+                             shutter_alive( &p_adapter->recv_shutter );
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                             ("ipoib_state changed to IPOIB_RUNNING after the IPOIB_RESET_OR_DOWN flag was cleared\n") );
+
+             }
+             else
+             {
+                             // This sittuation happened in real life:
+                             // NDIS called to reset and right after to pause, before the reset was completed
+                             // Thus, when ipoib will try to complete reset, state may be already changed to IPOIB_PAUSE
+                             ASSERT(  p_adapter->ipoib_state == IPOIB_PAUSED );
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                             ("ipoib_state remained IPOIB_PAUSED and will be changed at ipoib_restart()\n") );
+             }
+             KeReleaseInStackQueuedSpinLock( &hdl );
+
+             p_adapter->reset = FALSE;
+             NdisMResetComplete(
+                             p_adapter->h_adapter, Status, TRUE );
+}
+
 /* Implementation */
 ib_api_status_t
 ipoib_create_adapter(
@@ -800,9 +834,7 @@

                                if( p_adapter->reset && p_adapter->state != IB_PNP_PORT_INIT )
                                {
-                                              p_adapter->reset = FALSE;
-                                              NdisMResetComplete(
-                                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                                }
                                status = IB_SUCCESS;
                                break;
@@ -978,6 +1010,7 @@
 {
                ib_api_status_t                                status;
                ib_pnp_handle_t                             h_pnp;
+             KLOCK_QUEUE_HANDLE              hdl;

                IPOIB_ENTER( IPOIB_DBG_INIT );

@@ -992,8 +1025,19 @@
                                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 );
+                             // Avoid shutting the shutter twice
+                             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+                             if ( p_adapter->ipoib_state == IPOIB_RUNNING ) {
+                                             shutter_shut( &p_adapter->recv_shutter );
+                                             // Notify that reset started and disable other shutter_shut
+                                             p_adapter->ipoib_state |= IPOIB_RESET_OR_DOWN;
+                                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                                             ("[%I64u] ipoib_state was IPOIB_RUNNING and IPOIB_RESET_OR_DOWN flag was set \n", cl_get_time_stamp()) );
+                             }
+                             KeReleaseInStackQueuedSpinLock( &hdl );
+
                                if( status == IB_SUCCESS )
                                                status = IB_NOT_DONE;
                }
@@ -1007,7 +1051,31 @@
                                p_adapter->reset = TRUE;
                }
                else {
-                              p_adapter->reset = FALSE;
+                             //do not call to  __ipoib_complete_reset, because we return completion status directly from here
+                             p_adapter->reset = FALSE;
+
+                             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+                             // Check if Reset flag is set or if adapter is in Paused state
+                             p_adapter->ipoib_state =  p_adapter->ipoib_state & ~IPOIB_RESET_OR_DOWN;
+
+                             if  ( p_adapter->ipoib_state == IPOIB_RUNNING )
+                             {
+                                             shutter_alive( &p_adapter->recv_shutter );
+                                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                                             ("ipoib_state changed to IPOIB_RUNNING after the IPOIB_RESET_OR_DOWN flag was cleared\n") );
+
+                             }
+                             else
+                             {
+                                             // This sittuation happened in real life:
+                                             // NDIS called to reset and right after to pause, before the reset was completed
+                                             // Thus, when ipoib will try to complete reset, state may be already changed to IPOIB_PAUSE
+                                             ASSERT(  p_adapter->ipoib_state == IPOIB_PAUSED );
+                                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                                             ("ipoib_state remained IPOIB_PAUSED and will be changed at ipoib_restart()\n") );
+                             }
+                             KeReleaseInStackQueuedSpinLock( &hdl );
+
                }
                IPOIB_EXIT( IPOIB_DBG_INIT );
                return status;
@@ -1042,7 +1110,9 @@

                IPOIB_ENTER( IPOIB_DBG_INIT );
                IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
-                                                              ("Got RESET\n") );
+                                                             ("[%I64u] Got RESET\n", cl_get_time_stamp()) );
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_SHUTTER,
+                                                             ("[%I64u] Got RESET\n", cl_get_time_stamp()) );

                p_adapter = (ipoib_adapter_t*)context;

@@ -1074,19 +1144,15 @@
                                status = __ipoib_pnp_reg( p_adapter, IB_PNP_FLAG_REG_COMPLETE );
                                if( status != IB_SUCCESS )
                                {
-                                              p_adapter->reset = FALSE;
                                                IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
                                                                ("__ipoib_pnp_reg returned %s\n",
                                                                p_adapter->p_ifc->get_err_str( status )) );
-                                              NdisMResetComplete(
-                                                              p_adapter->h_adapter, NDIS_STATUS_HARD_ERRORS, TRUE );
+                                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_HARD_ERRORS );
                                }
                }
                else
                {
-                              p_adapter->reset = FALSE;
-                              NdisMResetComplete(
-                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                                status = IB_SUCCESS;
                }

@@ -1261,10 +1327,7 @@

                if( p_adapter->reset )
                {
-                              //ASSERT(FALSE);
-                              p_adapter->reset = FALSE;
-                              NdisMResetComplete(
-                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                }

                IPOIB_EXIT( IPOIB_DBG_INIT );
@@ -1323,9 +1386,7 @@

                if( p_adapter->reset )
                {
-                              p_adapter->reset = FALSE;
-                              NdisMResetComplete(
-                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                }

                IPOIB_EXIT( IPOIB_DBG_INIT );
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h             (revision 2975)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h          (working copy)
@@ -70,14 +70,14 @@
                CSUM_BYPASS
 } csum_flag_t;

-typedef enum _ipoib_state
-{
-              IPOIB_INIT = -1,
-    IPOIB_PAUSED,
-    IPOIB_PAUSING,
-    IPOIB_RUNNING
-} ipoib_state_t;
+typedef             uint32_t ipoib_state_t;
+#define                             IPOIB_INIT                                          1
+#define             IPOIB_PAUSED                                 2
+#define                             IPOIB_PAUSING                               4
+#define                 IPOIB_RUNNING                         8
+#define                 IPOIB_RESET_OR_DOWN 16

+
 typedef struct _ipoib_offloads_cap_ {
                boolean_t           lso;
                boolean_t           send_chksum_offload;
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_debug.h
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_debug.h                (revision 2975)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_debug.h             (working copy)
@@ -73,7 +73,8 @@
                WPP_DEFINE_BIT(IPOIB_DBG_OID) \
                WPP_DEFINE_BIT(IPOIB_DBG_IOCTL) \
                WPP_DEFINE_BIT(IPOIB_DBG_STAT) \
-              WPP_DEFINE_BIT(IPOIB_DBG_OBJ))
+             WPP_DEFINE_BIT(IPOIB_DBG_OBJ) \
+             WPP_DEFINE_BIT(IPOIB_DBG_SHUTTER))



@@ -103,20 +104,21 @@
 /*
  * Debug macros
  */
-#define IPOIB_DBG_ERR            (1 << 0)
-#define IPOIB_DBG_INIT           (1 << 1)
-#define IPOIB_DBG_PNP           (1 << 2)
-#define IPOIB_DBG_SEND         (1 << 3)
-#define IPOIB_DBG_RECV         (1 << 4)
-#define IPOIB_DBG_ENDPT      (1 << 5)
-#define IPOIB_DBG_IB                (1 << 6)
-#define IPOIB_DBG_BUF           (1 << 7)
-#define IPOIB_DBG_MCAST     (1 << 8)
-#define IPOIB_DBG_ALLOC       (1 << 9)
-#define IPOIB_DBG_OID            (1 << 10)
-#define IPOIB_DBG_IOCTL        (1 << 11)
-#define IPOIB_DBG_STAT          (1 << 12)
-#define IPOIB_DBG_OBJ            (1 << 13)
+#define IPOIB_DBG_ERR                           (1 << 0)
+#define IPOIB_DBG_INIT                          (1 << 1)
+#define IPOIB_DBG_PNP                          (1 << 2)
+#define IPOIB_DBG_SEND                        (1 << 3)
+#define IPOIB_DBG_RECV                        (1 << 4)
+#define IPOIB_DBG_ENDPT                     (1 << 5)
+#define IPOIB_DBG_IB                               (1 << 6)
+#define IPOIB_DBG_BUF                          (1 << 7)
+#define IPOIB_DBG_MCAST                    (1 << 8)
+#define IPOIB_DBG_ALLOC                      (1 << 9)
+#define IPOIB_DBG_OID                           (1 << 10)
+#define IPOIB_DBG_IOCTL                       (1 << 11)
+#define IPOIB_DBG_STAT                         (1 << 12)
+#define IPOIB_DBG_OBJ                           (1 << 13)
+#define IPOIB_DBG_SHUTTER                (1 << 14)

 #define IPOIB_DBG_ERROR       (CL_DBG_ERROR | IPOIB_DBG_ERR)
 #define IPOIB_DBG_ALL             CL_DBG_ALL
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp             (revision 2975)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp          (working copy)
@@ -1877,6 +1877,8 @@
                                                return NDIS_STATUS_FAILURE;
                                }
                                p_adapter->ipoib_state = IPOIB_INIT;
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                             ("ipoib_state changed to IPOIB_INIT\n") );

                                status    = SetAttributes(p_adapter, h_adapter);
                                if (status != NDIS_STATUS_SUCCESS) {
@@ -3155,8 +3157,8 @@
                p_adapter = (ipoib_adapter_t*)adapter_context;

                cl_obj_lock( &p_adapter->obj );
-              if( p_adapter->ipoib_state == IPOIB_PAUSING ||
-                              p_adapter->ipoib_state == IPOIB_PAUSED)
+             if( p_adapter->ipoib_state & IPOIB_PAUSING ||
+                             p_adapter->ipoib_state & IPOIB_PAUSED)
                {
                                status = NDIS_STATUS_PAUSED;
                                IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
@@ -3275,6 +3277,8 @@
                IN NDIS_HANDLE             adapter_context,
                IN NDIS_SHUTDOWN_ACTION  shutdown_action)
 {
+             KLOCK_QUEUE_HANDLE              hdl;
+
                IPOIB_ENTER( IPOIB_DBG_INIT ) ;
                UNUSED_PARAM( shutdown_action );

@@ -3287,7 +3291,19 @@
                                                ("Shutter shut, state = %d\n", p_adapter->ipoib_state));
                                IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
                                                ("Shutter shut, state = %d\n", p_adapter->ipoib_state));
-                              shutter_shut ( &p_adapter->recv_shutter );
+                             // Wait until NDIS will return all indicated NBLs that were received
+                             // Avoid shutting the shutter twice
+                             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+                             ASSERT ( !(p_adapter->ipoib_state & IPOIB_RESET_OR_DOWN) );
+                             if ( p_adapter->ipoib_state == IPOIB_RUNNING ) { //ensure that there was no active reset
+                                             shutter_shut( &p_adapter->recv_shutter );
+                                             // Notify that shutter was already shut
+                                             p_adapter->ipoib_state |= IPOIB_RESET_OR_DOWN;
+                                                             ASSERT( p_adapter->ipoib_state == IPOIB_RUNNING );
+                                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                                                             ("ipoib_state was IPOIB_RUNNING and IPOIB_RESET_OR_DOWN flag was set\n") );
+                             }
+                             KeReleaseInStackQueuedSpinLock( &hdl );
                }

                IPOIB_EXIT( IPOIB_DBG_INIT );
@@ -3965,6 +3981,7 @@
                cl_free( context );
 }

+
 static NDIS_STATUS
 ipoib_pause(
                IN           NDIS_HANDLE                                                                                   adapter_context,
@@ -3972,30 +3989,42 @@
 {
                ipoib_adapter_t                               *p_adapter;
                KLOCK_QUEUE_HANDLE              hdl;
+             ipoib_state_t                     prev_state;

-              UNREFERENCED_PARAMETER(pause_parameters);
+             UNREFERENCED_PARAMETER( pause_parameters );
                IPOIB_ENTER( IPOIB_DBG_INIT );

-              CL_ASSERT(adapter_context);
+             CL_ASSERT( adapter_context );
                p_adapter = (ipoib_adapter_t*)adapter_context;
-              CL_ASSERT(p_adapter->ipoib_state == IPOIB_RUNNING);
-
+
                KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+             prev_state = p_adapter->ipoib_state;
+             CL_ASSERT ( prev_state & IPOIB_RUNNING );
                p_adapter->ipoib_state = IPOIB_PAUSING;
+             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                             ("[%I64u]ipoib_state changed to IPOIB_PAUSING\n", cl_get_time_stamp()) );
                KeReleaseInStackQueuedSpinLock( &hdl );

-              if (p_adapter->p_port) {
+             if ( p_adapter->p_port ) {
                                cl_spinlock_acquire( &p_adapter->p_port->send_lock );
-                              ipoib_port_resume(p_adapter->p_port,FALSE);
+                             ipoib_port_resume( p_adapter->p_port,FALSE );
                                cl_spinlock_release( &p_adapter->p_port->send_lock );
                }
+             if ( prev_state == IPOIB_RUNNING ) { //i.e there was no active reset
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
+                                             ("Shutter shut, state = %d\n", p_adapter->ipoib_state) );
+                             shutter_shut ( &p_adapter->recv_shutter );
+             }
+             else {
+                             ASSERT ( prev_state & IPOIB_RESET_OR_DOWN);
+                             IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
+                             ("[%I64u]Got pause during reset or got reset when IPoIB port wasn't active\n", cl_get_time_stamp()) );
+             }

-              IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
-                              ("Shutter shut, state = %d\n", p_adapter->ipoib_state));
-              shutter_shut ( &p_adapter->recv_shutter );
-
                KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
                p_adapter->ipoib_state = IPOIB_PAUSED;
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                             ("[%I64u]ipoib_state changed to IPOIB_PAUSED\n", cl_get_time_stamp()) );
                KeReleaseInStackQueuedSpinLock( &hdl );

                IPOIB_EXIT( IPOIB_DBG_INIT );
@@ -4025,18 +4054,20 @@
                                // Check to see if we need to change any attributes
                }

-              if ( (p_adapter->ipoib_state == IPOIB_PAUSED) || (p_adapter->ipoib_state == IPOIB_INIT) ) {
+             if ( p_adapter->ipoib_state & (IPOIB_PAUSED | IPOIB_INIT) ) {
                                IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
-                                              ("Shutter Alive, ipoib_state = %d\n", p_adapter->ipoib_state));
+                                             ("[%I64u]Shutter Alive, ipoib_state = %d\n", cl_get_time_stamp(), p_adapter->ipoib_state));
                                shutter_alive( &p_adapter->recv_shutter );
                }
                else {
-                              IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_RECV,
+                             IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_SHUTTER,
                                ("*****Shutter Was not \"Alived\", state = %d*****\n", p_adapter->ipoib_state));
                }

                KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
                p_adapter->ipoib_state = IPOIB_RUNNING;
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION,  IPOIB_DBG_SHUTTER,
+                             ("[%I64u]ipoib_state changed to IPOIB_RUNNING\n",cl_get_time_stamp()) );
                KeReleaseInStackQueuedSpinLock( &hdl );



From: Smith, Stan [mailto:stan.smith at intel.com]
Sent: Wednesday, October 27, 2010 5:26 PM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: RE: [ofw] [IPoIB_NDIS6_CM][Patch] Improving shutter mechanism for recv flow

Hello,
  Patch tests out with no problems - expected behavior.
Would you like for me to svn commit to trunk and WOF2-3 branch?

stan.

________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Tuesday, October 26, 2010 2:59 AM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: Re: [ofw] [IPoIB_NDIS6_CM][Patch] Improving shutter mechanism for recv flow
Now with the patch attached by file

From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Tuesday, October 26, 2010 11:57 AM
To: ofw at lists.openfabrics.org
Subject: [ofw] [IPoIB_NDIS6_CM][Patch] Improving shutter mechanism for recv flow

This patch avoids the situation when the recv shutter can be shut twice.
It was tested with regression run that failed beforehand along with manual debug
(forcing NDIS to call to ipoib_reset() )

Signed-off by: Alexander Naslednikov (xalex at mellanox.co.il)

Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp         (revision 2975)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp      (working copy)
@@ -111,6 +111,28 @@
                OUT                                                       UINT                                                                                      *p_len);


+static inline void
+             __ipoib_complete_reset(
+             IN                                                           ipoib_adapter_t* const                                p_adapter,
+             IN                                                           NDIS_STATUS                                                                    Status )
+{
+             KLOCK_QUEUE_HANDLE              hdl;
+
+             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+             if ( p_adapter->ipoib_state & ~IPOIB_RESET_OR_DOWN )
+             {
+                             //Alive the shutter even if the status was not NDIS_STATUS_SUCCESS
+                             shutter_alive( &p_adapter->recv_shutter );
+                             p_adapter->ipoib_state = p_adapter->ipoib_state & ~IPOIB_RESET_OR_DOWN;
+                             ASSERT( p_adapter->ipoib_state == IPOIB_RUNNING );
+             }
+             KeReleaseInStackQueuedSpinLock( &hdl );
+
+             p_adapter->reset = FALSE;
+             NdisMResetComplete(
+                             p_adapter->h_adapter, Status, TRUE );
+}
+
 /* Implementation */
 ib_api_status_t
 ipoib_create_adapter(
@@ -800,9 +822,7 @@

                                if( p_adapter->reset && p_adapter->state != IB_PNP_PORT_INIT )
                                {
-                                              p_adapter->reset = FALSE;
-                                              NdisMResetComplete(
-                                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                                }
                                status = IB_SUCCESS;
                                break;
@@ -978,6 +998,7 @@
 {
                ib_api_status_t                                status;
                ib_pnp_handle_t                             h_pnp;
+             KLOCK_QUEUE_HANDLE              hdl;

                IPOIB_ENTER( IPOIB_DBG_INIT );

@@ -992,8 +1013,17 @@
                                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 );
+                             // Avoid shutting the shutter twice
+                             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+                             if ( p_adapter->ipoib_state == IPOIB_RUNNING ) {
+                                             shutter_shut( &p_adapter->recv_shutter );
+                                             // Notify that reset started and disable other shutter_shut
+                                             p_adapter->ipoib_state |= IPOIB_RESET_OR_DOWN;
+                             }
+                             KeReleaseInStackQueuedSpinLock( &hdl );
+
                                if( status == IB_SUCCESS )
                                                status = IB_NOT_DONE;
                }
@@ -1007,7 +1037,12 @@
                                p_adapter->reset = TRUE;
                }
                else {
-                              p_adapter->reset = FALSE;
+                             //do not call to  __ipoib_complete_reset, because we return completion status directly from here
+                             p_adapter->reset = FALSE;
+                             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+                             p_adapter->ipoib_state =  p_adapter->ipoib_state & ~IPOIB_RESET_OR_DOWN;
+                             shutter_alive( &p_adapter->recv_shutter );
+                             KeReleaseInStackQueuedSpinLock( &hdl );
                }
                IPOIB_EXIT( IPOIB_DBG_INIT );
                return status;
@@ -1074,19 +1109,15 @@
                                status = __ipoib_pnp_reg( p_adapter, IB_PNP_FLAG_REG_COMPLETE );
                                if( status != IB_SUCCESS )
                                {
-                                              p_adapter->reset = FALSE;
                                                IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
                                                                ("__ipoib_pnp_reg returned %s\n",
                                                                p_adapter->p_ifc->get_err_str( status )) );
-                                              NdisMResetComplete(
-                                                              p_adapter->h_adapter, NDIS_STATUS_HARD_ERRORS, TRUE );
+                                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_HARD_ERRORS );
                                }
                }
                else
                {
-                              p_adapter->reset = FALSE;
-                              NdisMResetComplete(
-                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                                status = IB_SUCCESS;
                }

@@ -1261,10 +1292,7 @@

                if( p_adapter->reset )
                {
-                              //ASSERT(FALSE);
-                              p_adapter->reset = FALSE;
-                              NdisMResetComplete(
-                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                }

                IPOIB_EXIT( IPOIB_DBG_INIT );
@@ -1323,9 +1351,7 @@

                if( p_adapter->reset )
                {
-                              p_adapter->reset = FALSE;
-                              NdisMResetComplete(
-                                              p_adapter->h_adapter, NDIS_STATUS_SUCCESS, TRUE );
+                             __ipoib_complete_reset( p_adapter, NDIS_STATUS_SUCCESS );
                }

                IPOIB_EXIT( IPOIB_DBG_INIT );
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h             (revision 2975)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h          (working copy)
@@ -70,14 +70,14 @@
                CSUM_BYPASS
 } csum_flag_t;

-typedef enum _ipoib_state
-{
-              IPOIB_INIT = -1,
-    IPOIB_PAUSED,
-    IPOIB_PAUSING,
-    IPOIB_RUNNING
-} ipoib_state_t;
+typedef             uint32_t ipoib_state_t;
+#define                             IPOIB_INIT                                          1
+#define             IPOIB_PAUSED                                 2
+#define                             IPOIB_PAUSING                               4
+#define                 IPOIB_RUNNING                         8
+#define                 IPOIB_RESET_OR_DOWN 16

+
 typedef struct _ipoib_offloads_cap_ {
                boolean_t           lso;
                boolean_t           send_chksum_offload;
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp             (revision 2975)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp          (working copy)
@@ -3155,8 +3155,8 @@
                p_adapter = (ipoib_adapter_t*)adapter_context;

                cl_obj_lock( &p_adapter->obj );
-              if( p_adapter->ipoib_state == IPOIB_PAUSING ||
-                              p_adapter->ipoib_state == IPOIB_PAUSED)
+             if( p_adapter->ipoib_state & IPOIB_PAUSING ||
+                             p_adapter->ipoib_state & IPOIB_PAUSED)
                {
                                status = NDIS_STATUS_PAUSED;
                                IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
@@ -3275,6 +3275,8 @@
                IN NDIS_HANDLE             adapter_context,
                IN NDIS_SHUTDOWN_ACTION  shutdown_action)
 {
+             KLOCK_QUEUE_HANDLE              hdl;
+
                IPOIB_ENTER( IPOIB_DBG_INIT ) ;
                UNUSED_PARAM( shutdown_action );

@@ -3287,7 +3289,16 @@
                                                ("Shutter shut, state = %d\n", p_adapter->ipoib_state));
                                IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
                                                ("Shutter shut, state = %d\n", p_adapter->ipoib_state));
-                              shutter_shut ( &p_adapter->recv_shutter );
+                             // Wait until NDIS will return all indicated NBLs that were received
+                             // Avoid shutting the shutter twice
+                             KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+                             ASSERT ( !(p_adapter->ipoib_state & IPOIB_RESET_OR_DOWN) );
+                             if ( p_adapter->ipoib_state == IPOIB_RUNNING ) { //ensure that there was no active reset
+                                             shutter_shut( &p_adapter->recv_shutter );
+                                             // Notify that shutter was already shut
+                                             p_adapter->ipoib_state |= IPOIB_RESET_OR_DOWN;
+                             }
+                             KeReleaseInStackQueuedSpinLock( &hdl );
                }

                IPOIB_EXIT( IPOIB_DBG_INIT );
@@ -3965,6 +3976,7 @@
                cl_free( context );
 }

+
 static NDIS_STATUS
 ipoib_pause(
                IN           NDIS_HANDLE                                                                                   adapter_context,
@@ -3972,28 +3984,37 @@
 {
                ipoib_adapter_t                               *p_adapter;
                KLOCK_QUEUE_HANDLE              hdl;
+             ipoib_state_t                     prev_state;

-              UNREFERENCED_PARAMETER(pause_parameters);
+             UNREFERENCED_PARAMETER( pause_parameters );
                IPOIB_ENTER( IPOIB_DBG_INIT );

-              CL_ASSERT(adapter_context);
+             CL_ASSERT( adapter_context );
                p_adapter = (ipoib_adapter_t*)adapter_context;
-              CL_ASSERT(p_adapter->ipoib_state == IPOIB_RUNNING);
+

                KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
+             prev_state = p_adapter->ipoib_state;
+             CL_ASSERT ( prev_state & IPOIB_RUNNING );
                p_adapter->ipoib_state = IPOIB_PAUSING;
                KeReleaseInStackQueuedSpinLock( &hdl );

-              if (p_adapter->p_port) {
+             if ( p_adapter->p_port ) {
                                cl_spinlock_acquire( &p_adapter->p_port->send_lock );
-                              ipoib_port_resume(p_adapter->p_port,FALSE);
+                             ipoib_port_resume( p_adapter->p_port,FALSE );
                                cl_spinlock_release( &p_adapter->p_port->send_lock );
                }
+             if ( prev_state == IPOIB_RUNNING ) { //i.e there was no active reset
+                             IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
+                                             ("Shutter shut, state = %d\n", p_adapter->ipoib_state) );
+                             shutter_shut ( &p_adapter->recv_shutter );
+             }
+             else {
+                             ASSERT ( prev_state & IPOIB_RESET_OR_DOWN);
+                             IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
+                             ("Got reset when IPoIB port wasn't active\n") );
+             }

-              IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
-                              ("Shutter shut, state = %d\n", p_adapter->ipoib_state));
-              shutter_shut ( &p_adapter->recv_shutter );
-
                KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
                p_adapter->ipoib_state = IPOIB_PAUSED;
                KeReleaseInStackQueuedSpinLock( &hdl );
@@ -4025,7 +4046,7 @@
                                // Check to see if we need to change any attributes
                }

-              if ( (p_adapter->ipoib_state == IPOIB_PAUSED) || (p_adapter->ipoib_state == IPOIB_INIT) ) {
+             if ( (p_adapter->ipoib_state & IPOIB_PAUSED) || (p_adapter->ipoib_state & IPOIB_INIT) ) {
                                IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
                                                ("Shutter Alive, ipoib_state = %d\n", p_adapter->ipoib_state));
                                shutter_alive( &p_adapter->recv_shutter );

Alexander (XaleX) Naslednikov
SW Networking Team
Mellanox Technologies

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20101101/9096bf64/attachment.html>


More information about the ofw mailing list