[ofw] WWG 1-26-10 minutes - WinOF 2.2 release status [PATCH] [IPoIB_NDIS6_CM]

Smith, Stan stan.smith at intel.com
Wed Feb 3 09:20:47 PST 2010


Patches do not correctly apply against trunk\ulp\ipoib_ndis6_cm or branches\WOF2-2\ulp\ipoib_ndis6_cm.
I tried applying patches by hand and things got UGLY very quickly.

Please supply patches generated against trunk\ulp\ipoib_ndis6_cm\ ASAP as today is my last day at work until 2/15.

thank you,

Stan.

Alex Naslednikov wrote:
> Stan,
> The patch was revisited.
> For your covnenience, I am resending it again with the new changes.
> In spite of the fact that the patch was tested with WHQL NDIS 6.5 MPE
> test, I highly recommend to run independent test cycle.
>
> Index:
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp
> ===================================================================
> ---
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp
>                         (revision 5504) +++
>
>
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.cpp
> (revision 5529) @@ -392,6 +392,13 @@ p_adapter->p_ifc->get_err_str(
> status )) ); return status; } + + +       IPOIB_PRINT(
> TRACE_LEVEL_INFORMATION, IPOIB_DBG_INIT, +               ("Shutter
> Init, state = %d\n", p_adapter->ipoib_state) ); +       IPOIB_PRINT(
> TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV, +               ("Shutter
> Init, state = %d\n", p_adapter->ipoib_state) ); +       shutter_init(
> &p_adapter->recv_shutter );
>
>         IPOIB_EXIT( IPOIB_DBG_INIT );
>         return status;
> Index:
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
> ===================================================================
> ---
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
>         (revision 5504) +++
>
>  D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
> (revision 5529) @@ -824,7 +824,21 @@ 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. +    // +
> +       if ( p_adapter->ipoib_state == IPOIB_INIT) {
> +               IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_RECV,
> +               ("Shutter shut, state = %d\n",
> p_adapter->ipoib_state)); +               shutter_shut (
> &p_adapter->recv_shutter ); +       }
> +       else {
> +               IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_RECV,
> +               ("*****Shutter wasn't shut, state = %d*****\n",
> p_adapter->ipoib_state)); +       }
>         IPOIB_EXIT( IPOIB_DBG_INIT );
>         return IB_SUCCESS;
>  }
> @@ -1843,45 +1857,28 @@
>         return p_port->p_adapter->params.rq_low_watermark -
>  p_port->recv_mgr.depth; }
>
> -void
> -ipoib_return_net_buffer_list(
> -       IN                              NDIS_HANDLE
> adapter_context,
> -       IN                              NET_BUFFER_LIST
> *p_net_buffer_lists,
> -       IN                              ULONG
> return_flags) +inline ULONG __free_received_NBL (
> +       IN ipoib_port_t         *p_port,
> +       IN NET_BUFFER_LIST      *p_net_buffer_lists
> +       )
>  {
> -       ipoib_port_t            *p_port;
> +
>         ipoib_recv_desc_t       *p_desc;
> -       NET_BUFFER_LIST
> *cur_net_buffer_list,*next_net_buffer_list;
> -       int32_t                         shortage;
> +       NET_BUFFER_LIST         *cur_net_buffer_list,
> *next_net_buffer_list; +       LONG
> NBL_cnt = 0; +
>
> -       PERF_DECLARE( ReturnPacket );
> -       PERF_DECLARE( ReturnPutRecv );
> -       PERF_DECLARE( ReturnRepostRecv );
> -       PERF_DECLARE( ReturnPreparePkt );
> -       PERF_DECLARE( ReturnNdisIndicate );
> -
> -       IPOIB_ENTER( IPOIB_DBG_RECV );
> -
> -       UNUSED_PARAM( return_flags );
> -
> -       p_port = ((ipoib_adapter_t*)adapter_context)->p_port;
> -       CL_ASSERT( p_net_buffer_lists );
> -
> -       cl_perf_start( ReturnPacket );
> -       cl_spinlock_acquire( &p_port->recv_lock );
>         for (cur_net_buffer_list = p_net_buffer_lists;
>                  cur_net_buffer_list != NULL;
>                  cur_net_buffer_list = next_net_buffer_list)
>         {
> +               ++NBL_cnt;
>                 next_net_buffer_list =
> NET_BUFFER_LIST_NEXT_NBL(cur_net_buffer_list);
>
>                 /* Get the port and descriptor from the
>                 NET_BUFFER_LIST. */ CL_ASSERT(p_port ==
>                 IPOIB_PORT_FROM_NBL( cur_net_buffer_list )); p_desc =
> IPOIB_RECV_FROM_NBL( cur_net_buffer_list ); -
>
> -               //TODO: NDIS60, rewrite this block
> -
>  #if 0 //TODO CM flow
>                 if( p_desc->type == PKT_TYPE_CM_UCAST )
>                 {
> @@ -1916,9 +1913,44 @@
>                 __buf_mgr_put_recv( p_port, p_desc,
>                 cur_net_buffer_list ); cl_perf_stop(
>         &p_port->p_adapter->perf, ReturnPutRecv ); }
> +       return NBL_cnt;
> +}
>
> +void
> +ipoib_return_net_buffer_list(
> +       IN                              NDIS_HANDLE
> adapter_context, +       IN
> NET_BUFFER_LIST                         *p_net_buffer_lists, +
> IN                              ULONG
> return_flags) +{ +       ipoib_port_t            *p_port;
> +       int32_t                         shortage;
> +       LONG                            NBL_cnt = 0;
> +
> +       PERF_DECLARE( ReturnPacket );
> +       PERF_DECLARE( ReturnPutRecv );
> +       PERF_DECLARE( ReturnRepostRecv );
> +       PERF_DECLARE( ReturnPreparePkt );
> +       PERF_DECLARE( ReturnNdisIndicate );
>
> +       IPOIB_ENTER( IPOIB_DBG_RECV );
>
> +       UNUSED_PARAM( return_flags );
> +
> +       p_port = ((ipoib_adapter_t*)adapter_context)->p_port;
> +       CL_ASSERT( p_net_buffer_lists );
> +       if ( !p_port ) {
> +               ASSERT(p_port);
> +               IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
> +                       ("return_NBL callback called when port
> pointer was already cleared\n") ); +               return;
> +       }
> +
> +       cl_perf_start( ReturnPacket );
> +       cl_spinlock_acquire( &p_port->recv_lock );
> +       NBL_cnt = __free_received_NBL( p_port, p_net_buffer_lists );
> +
> +       shutter_sub( &p_port->p_adapter->recv_shutter, -NBL_cnt );
> +
>         /* Repost buffers to HW */
>         cl_perf_start( ReturnRepostRecv );
>         shortage = __recv_mgr_repost( p_port );
> @@ -1958,6 +1990,7 @@
>         cl_qlist_t                      done_list, bad_list;
>         size_t                          i;
>         ULONG                           recv_complete_flags = 0;
> +       BOOLEAN                         res;
>
>         PERF_DECLARE( RecvCompBundle );
>         PERF_DECLARE( RecvCb );
> @@ -2069,13 +2102,40 @@
>
>                 cl_perf_start( RecvNdisIndicate );
>
> -               NdisMIndicateReceiveNetBufferLists(
> -                       p_port->p_adapter->h_adapter,
> -                       p_port->recv_mgr.recv_NBL_array[0],
> -                       NDIS_DEFAULT_PORT_NUMBER,
> -                       NBL_cnt,
> -                       recv_complete_flags);
> +               if (shortage <= 0) {
>
> +                       res = shutter_add(
> &p_port->p_adapter->recv_shutter, NBL_cnt ); +
> if (res) { +
> NdisMIndicateReceiveNetBufferLists( +
> p_port->p_adapter->h_adapter, +
> p_port->recv_mgr.recv_NBL_array[0], +
> NDIS_DEFAULT_PORT_NUMBER, +
> NBL_cnt, +                                       recv_complete_flags);
> +                       }
> +                       else {
> +                               __free_received_NBL (p_port,
> p_port->recv_mgr.recv_NBL_array[0]); +                       }
> +
> +               } else {
> +
> +                       // If shortage >0, we already set the status
> to NDIS_RECEIVE_FLAGS_RESOURCES +                       // That is,
> IPoIB driver regain ownership of the NET_BUFFER_LIST structures
> immediately +                       res = shutter_add(
> &p_port->p_adapter->recv_shutter, 1 ); +                       if
> (res) { +
> NdisMIndicateReceiveNetBufferLists( +
> p_port->p_adapter->h_adapter, +
> p_port->recv_mgr.recv_NBL_array[0], +
> NDIS_DEFAULT_PORT_NUMBER, +
> NBL_cnt, +
> recv_complete_flags); +                               shutter_sub(
> &p_port->p_adapter->recv_shutter, -1 ); +                       } +
> +               }
> +
> +
> +
>                 cl_perf_stop( &p_port->p_adapter->perf,
> RecvNdisIndicate );
>
>                 /*
> @@ -3243,14 +3303,8 @@
>  {
>         cl_list_item_t          *p_item;
>         ipoib_send_NB_SG        *s_buf;
> -       ULONG                           send_complete_flags = 0;
> +       ULONG                           send_complete_flags =
> NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL;
>
> -       if (KeGetCurrentIrql() == DISPATCH_LEVEL)
> -       {
> -               NDIS_SET_SEND_COMPLETE_FLAG(send_complete_flags,
> NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL);
> -       }
> -
> -       cl_spinlock_acquire( &p_port->send_lock );
>         /* Complete any pending packets. */
>         for( p_item = cl_qlist_remove_head(
>                 &p_port->send_mgr.pending_list ); p_item !=
> cl_qlist_end( &p_port->send_mgr.pending_list ); @@ -3260,13 +3314,10
>                 @@ ASSERT(s_buf->p_port == p_port);
>                 ASSERT(s_buf->p_nbl);
>
> -
> +               //TODO
>                 //__send_complete_net_buffer(s_buf,
>
>         NDIS_STATUS_RESET_IN_PROGRESS,send_complete_flags,TRUE);
> __send_complete_net_buffer(s_buf,
> NDIS_STATUS_FAILURE,send_complete_flags,TRUE); } - -
> -       cl_spinlock_release( &p_port->send_lock );
>
>  }
>
> @@ -3278,7 +3329,9 @@
>         //Destroy pending list and put all the send buffers back to
>         pool //The list should be already destroyed at this point
>         ASSERT(p_port->send_mgr.pending_list.count == 0);
> +       cl_spinlock_acquire( &p_port->send_lock );
>         __pending_list_destroy(p_port);
> +       cl_spinlock_release( &p_port->send_lock );
>
>         // Now, destroy the send pool
>         cl_qpool_destroy(&p_port->send_mgr.send_pool);
> @@ -5299,7 +5352,6 @@
>                 //TODO Tzachid: make an assert here to validate your
>                 IRQL ASSERT (KeGetCurrentIrql() == DISPATCH_LEVEL);
>                 old_irql = DISPATCH_LEVEL;
> -               NDIS_SET_SEND_COMPLETE_FLAG(send_complete_flags,
>         NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL); } else {
>                 NDIS_RAISE_IRQL_TO_DISPATCH(&old_irql);
>                 //ASSERT (KeGetCurrentIrql() == PASSIVE_LEVEL); //
> Happens @@ -7158,6 +7210,7 @@
>          * object lock since that is the order taken when reposting.
>          */
>         cl_spinlock_acquire( &p_port->recv_lock );
> +       cl_spinlock_acquire( &p_port->send_lock );
>         cl_obj_lock( &p_port->obj );
>         p_port->state = IB_QPS_ERROR;
>
> @@ -7172,6 +7225,7 @@
>                 p_port->ib_mgr.h_query = NULL;
>         }
>         cl_obj_unlock( &p_port->obj );
> +       cl_spinlock_release( &p_port->send_lock );
>         cl_spinlock_release( &p_port->recv_lock );
>
>         KeWaitForSingleObject(
> @@ -7670,10 +7724,10 @@
>                 IPOIB_PRINT_EXIT( TRACE_LEVEL_INFORMATION,
>                 IPOIB_DBG_MCAST ,("Invalid state - Aborting.\n") );
> IPOIB_PRINT_EXIT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_INIT ,("Invalid
> state - Aborting.\n") );
>
> -               //cl_spinlock_acquire(&p_port->send_lock);
> +               cl_spinlock_acquire(&p_port->send_lock);
>                 //ipoib_port_resume(p_port , FALSE);
>                 __pending_list_destroy( p_port );
> -               //cl_spinlock_release(&p_port->send_lock);
> +               cl_spinlock_release(&p_port->send_lock);
>                 return;
>         }
>
> Index:
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h
> ===================================================================
> ---
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h
>  (revision 5504) +++
>  D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_adapter.h
>  (revision 5529) @@ -47,8 +47,10 @@ #include <ip_packet.h> #include
> "ip_stats.h" #include "ipoib_stat.h" +#include "shutter.h"
>
>
> +
>  /*
>   * Definitions
>   */
> @@ -70,6 +72,7 @@
>
>  typedef enum _ipoib_state
>  {
> +       IPOIB_INIT = -1,
>      IPOIB_PAUSED,
>      IPOIB_PAUSING,
>      IPOIB_RUNNING
> @@ -238,6 +241,9 @@
>         ULONG                                   n_send_NBL;
>         // number of send NBLs, gotten from NDIS ULONG
> n_send_NBL_done;        // number of send NBLs, completed
>
> +       //
> +       shutter_t               recv_shutter;
> +
>  }      ipoib_adapter_t;
>  /*
>  * FIELDS
> Index:
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
> ===================================================================
> ---
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
>                                 (revision 5504) +++
>
>
> D:/windows/MLNX_WinOF_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_driver.cpp
> (revision 5529) @@ -1865,7 +1865,7 @@ ("ipoib_create_adapter returned
> status %d.\n", ib_status ) ); return NDIS_STATUS_FAILURE; } -
> p_adapter->ipoib_state = IPOIB_PAUSED; +
> p_adapter->ipoib_state = IPOIB_INIT;
>
>                 status  = SetAttributes(p_adapter, h_adapter);
>                 if (status != NDIS_STATUS_SUCCESS) {
> @@ -3111,13 +3111,12 @@
>
>         CL_ASSERT( adapter_context );
>         p_adapter = (ipoib_adapter_t*)adapter_context;
> -       p_port = p_adapter->p_port;
>
>         cl_obj_lock( &p_adapter->obj );
>         if( p_adapter->ipoib_state == IPOIB_PAUSING ||
>                 p_adapter->ipoib_state == IPOIB_PAUSED)
>         {
> -               status = NDIS_STATUS_PAUSED; //NDIS_STATUS_PAUSED;
> +               status = NDIS_STATUS_PAUSED;
>                 IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
>                                 ("Got send during PAUSE, complete
>                 with error \n") ); cl_obj_unlock( &p_adapter->obj );
> @@ -3239,9 +3238,21 @@
>         IN NDIS_HANDLE  adapter_context,
>         IN NDIS_SHUTDOWN_ACTION  shutdown_action)
>  {
> -       IPOIB_ENTER( IPOIB_DBG_INIT );
> -       UNUSED_PARAM( adapter_context );
> +       IPOIB_ENTER( IPOIB_DBG_INIT ) ;
>         UNUSED_PARAM( shutdown_action );
> +
> +       ipoib_adapter_t *p_adapter = (ipoib_adapter_t *)
> adapter_context; +
> +       if (shutdown_action == NdisShutdownPowerOff ) {
> +               ASSERT(KeGetCurrentIrql() == PASSIVE_LEVEL);
> +               // We need to wait only if this is not a blue screen
> any way +               IPOIB_PRINT( TRACE_LEVEL_INFORMATION,
> IPOIB_DBG_INIT, +                       ("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 ); +       }
> +
>         IPOIB_EXIT( IPOIB_DBG_INIT );
>  }
>
> @@ -3941,13 +3952,15 @@
>         KeReleaseInStackQueuedSpinLock( &hdl );
>
>         if (p_adapter->p_port) {
> -               //TODO improve this flow !
> -               //TODO Be sure we stopped all sends and receives
>                 cl_spinlock_acquire( &p_adapter->p_port->send_lock );
>                 ipoib_port_resume(p_adapter->p_port,FALSE);
>                 cl_spinlock_release( &p_adapter->p_port->send_lock );
>         }
> -
> +
> +       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 );
> @@ -3978,6 +3991,16 @@
>          //
>          // Check to see if we need to change any attributes
>      }
> +
> +       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 ); +       }
> +       else {
> +               IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_RECV,
> +               ("*****Shutter Was not \"Alived\", state =
> %d*****\n", p_adapter->ipoib_state)); +       }
>
>         KeAcquireInStackQueuedSpinLock( &g_ipoib.lock, &hdl );
>         p_adapter->ipoib_state = IPOIB_RUNNING;
>
> -----Original Message-----
> From: Alex Naslednikov
> Sent: Wednesday, January 27, 2010 6:52 PM
> To: 'Smith, Stan'; ofw at lists.openfabrics.org
> Subject: RE: [ofw] WWG 1-26-10 minutes - WinOF 2.2 release status
>
> Hi Stan,
> I started to test my patch with WHQL tests.
> It seems that the original problem was resolved, but I have several
> improvements I hope to complete it by tomorrow (28/12/10)
>
> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org
> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Smith, Stan
> Sent: Tuesday, January 26, 2010 8:06 PM
> To: ofw at lists.openfabrics.org
> Subject: [ofw] WWG 1-26-10 minutes - WinOF 2.2 release status
>
>
> Windows Working Group - Tuesday 1/26 9:00AM PST
>
> Attending:
>
> Alex Naslednikov - Mellanox
> John Russo - Qlogic
> Rupert Dance - Lamprey Networks
> Stan Smith - Intel
>
> ===============================================
>
> WinOF 2.2 Status
> ----------------
>
> Patch 'Release HCA interface when loading of ibal fails' is intended
> for trunk only as it's a rare occurrence.
>
> The IPoIB shutter code addressing IPoIB receiving packets during
> shutdown will be included in WinOF 2.2 RC3 as soon as the code is
> pushed into the trunk.
> Alex will make this a priority as the release of RC3 is waiting on
> this patch.
>
> Sean submitted 2.2 patches for the DAPL provider socket-CM & rdma-CM.
>
> Stan's testing of RC2 shows no signs of regression.
>
> Mellanox will engage in more QA testing once RC3 is released.
>
> IPoIB is tested on Svr 2008 R2 as IPoIB WHQL testing is specifically
> targeted for Svr 2008 R2 only.
>
> ===============================================
>
> Delineation of OFED version numbers
> -----------------------------------
>
> The WWG (Windows Working Group) has voted to change the OFA Windows
> SW release nomenclature from WinOF to 'OpenFabrics Enterprise
> Distribution' in order to leverage and strengthen OFA name branding.
>
> In those situations where the operating environment context is
> ambiguous, the Windows version of OFED will be referred to as 'OFED
> for Windows'.
>
> In aligning with the OFED name the question surfaced about how to
> delineate OFED release versions and 'if' there should be some
> consolidation of OFED SW release cycles between the Linux and Windows
> sides of the OFA house.
>
> The WWG resolved, with no reserve, that the release cycles and
> versioning of OFED for Windows remain distinctly separate from the
> OFED for Linux release cycles and versioning.
>
> OpenFabrics Enterprise Distribution for Windows will be formally
> announced at the Sonoma'10 conference.
>
> Changes in the OFED for Windows installer will consist of changing
> names WinOF --> OFED.
> The only functional change will be the installation path:
> %ProgramFiles%\WinOF --> %ProgramFiles%\OFED
>
> At the OFA Sonoma workshop the WWG (and anyone else) will meet
> face-2-face to discuss futures.
>
>
>
>
>
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw




More information about the ofw mailing list