[ofw] [Patch] Adding instrumental code for reset flow

[Alex Naslednikov]

Correction: applied at 3006 and 3007

From: Alex Naslednikov
Sent: Monday, November 29, 2010 4:37 PM
To: 'Smith, Stan'; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch] Adding instrumental code for reset flow

Applied at 3006 (together with missing file)

From: Smith, Stan [mailto:stan.smith at intel.com]
Sent: Friday, November 26, 2010 8:51 PM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch] Adding instrumental code for reset flow

OK, I understand you logic.
ACK on the missing line.
Patch needs to include actual declarations for 'ULONG g_reset & g_reset_complete'.

stan.

________________________________
From: Alex Naslednikov [mailto:xalex at mellanox.co.il]
Sent: Thursday, November 25, 2010 1:47 AM
To: Smith, Stan; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch] Adding instrumental code for reset flow
Stan,
Thank you for the comments.
We did it this way for the all IPoIB counters to enable debug at 'fre' version. See, for example, definitions of
ULONG                                 g_ipoib_send    = 0;
ULONG                                 g_ipoib_send_ack           = 0;
ULONG                                 g_ipoib_send_SW           = 0;
ULONG                                 g_ipoib_send_SG             = 0;
ULONG                                 g_ipoib_send_SW_in_loop = 0;
ULONG                                 g_ipoib_send_SG_pending = 0;
ULONG                                 g_ipoib_send_SG_real = 0;
ULONG                                 g_ipoib_send_SG_failed = 0;
ULONG                                 g_ipoib_send_reset = 0;

The ASSERT is for the checked version only. We don't want to stop here when running fre version of the driver.
But I would like to check this counters if I got some other errors during free run.

But on the other hand, we decided to unite all these counters into the struct on PORT object, to make this mechanism more robust and correct

And more important, I forgot to include to the patch the following lines. Without it, the ASSERT will be always true, I apologize for it.

Index: B:/users/xalex/MLNX_VPI_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
===================================================================
--- B:/users/xalex/MLNX_VPI_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp       (revision 6779)
+++ B:/users/xalex/MLNX_VPI_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp     (revision 6780)
@@ -3013,7 +3013,8 @@
               CL_ASSERT( p_addr_reset );
               CL_ASSERT( adapter_context );
               p_adapter = (ipoib_adapter_t*)adapter_context;
-
+              ++g_reset;
+
               switch( ipoib_reset_adapter( p_adapter ) )
               {
               case IB_NOT_DONE:

From: Smith, Stan [mailto:stan.smith at intel.com]
Sent: Thursday, November 25, 2010 6:04 AM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Subject: RE: [ofw] [Patch] Adding instrumental code for reset flow

Hello,
  Since this patch is basically long term debug support, why not put all of it under '#if DBG' ?
Incrementing g_reset, for the free build the ASSERT() is gone hence no check, makes questionable sense?
Full on or full off.
Good catch on the typo.

stan.

________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Tuesday, November 23, 2010 12:16 AM
To: ofw at lists.openfabrics.org
Subject: [ofw] [Patch] Adding instrumental code for reset flow
New counters added; no functionality changes
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 3000)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp      (working copy)
@@ -51,7 +51,10 @@
 #define FOUR_X_IN_100BPS    100000000
 #define TWELVE_X_IN_100BPS                300000000

+extern ULONG g_reset;
+extern ULONG g_reset_complete;

+
 /* Declarations */
 static void
 adapter_construct(
@@ -139,6 +142,9 @@
                                ("ipoib_state remained IPOIB_PAUSED and will be changed at ipoib_restart()\n") );
                }
                KeReleaseInStackQueuedSpinLock( &hdl );
+
+             ++g_reset_complete;
+             ASSERT( g_reset == g_reset_complete );

                p_adapter->reset = FALSE;
                NdisMResetComplete(
@@ -1067,7 +1073,7 @@
                                }
                                else
                                {
-                                              // This sittuation happened in real life:
+                                             // This situation 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 );
@@ -1075,6 +1081,8 @@
                                                ("ipoib_state remained IPOIB_PAUSED and will be changed at ipoib_restart()\n") );
                                }
                                KeReleaseInStackQueuedSpinLock( &hdl );
+                             ++g_reset_complete;
+                             ASSERT( g_reset == g_reset_complete );

                }
                IPOIB_EXIT( IPOIB_DBG_INIT );
@@ -1762,4 +1770,4 @@
                cl_spinlock_release( &p_adapter->send_stat_lock );

                IPOIB_EXIT( IPOIB_DBG_STAT );
-}
+}
\ No newline at end of file

Alexander (XaleX) Naslednikov
SW Networking Team
Mellanox Technologies

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20101129/1df44dff/attachment.html>