Fix a race in the SDP code (buffers were sent before RTU was received) (Integrate...

git-svn-id: svn://openib.tc.cornell.edu/gen1@180 ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86

diff --git a/trunk/ulp/sdp/kernel/SdpBufferPool.cpp b/trunk/ulp/sdp/kernel/SdpBufferPool.cpp
index 750edbd3416fefd02668b57426d1390721d0e5a2..bceb4ac6563d9d802c01d27f6023ef2581ac6c10 100644 (file)
--- a/trunk/ulp/sdp/kernel/SdpBufferPool.cpp
+++ b/trunk/ulp/sdp/kernel/SdpBufferPool.cpp
@@ -293,6 +293,18 @@ BufferPool::SendBuffersIfCan()
     AssertLocked();\r
     NTSTATUS rc = STATUS_SUCCESS;\r
 \r
+// Check if such code should be here ???? what if we have packets ????\r
+    if (m_PostCreditsWhenCan == true) {\r
+        m_PostCreditsWhenCan = false;\r
+        rc = PostCredits();\r
+        if (!NT_SUCCESS(rc)) {\r
+            SDP_PRINT(SDP_ERR, SDP_BUFFER_POOL, ("PostCredits failed rc = 0x%x\n", rc ));\r
+            goto Cleanup;\r
+        }        \r
+    }\r
+\r
+\r
+\r
     while ((m_QueuedPackets.Size() > 0) && \r
            (m_CurrentlySentBuffers < m_MaxConcurrentSends) &&\r
            (m_rRecvBuf > 2)) {\r
@@ -391,7 +403,7 @@ BufferPool::SendBuffer(BufferDescriptor *pBufferDescriptor)
     ib_send_wr_t    send_wr;\r
 \r
     send_wr.p_next = NULL;\r
-    send_wr.wr_id = (uintn_t)pBufferDescriptor;//?????(uint64_t) (uintptr_t) wr;\r
+    send_wr.wr_id = (uintn_t)pBufferDescriptor;\r
     send_wr.wr_type = WR_SEND;\r
     send_wr.send_opt = IB_SEND_OPT_SIGNALED;//socket_info->send_opt;\r
 \r
@@ -415,6 +427,7 @@ BufferPool::SendBuffer(BufferDescriptor *pBufferDescriptor)
         rc = IB2Status(ib_status);\r
         goto Cleanup;\r
     }\r
+    //????? Should we clear the post credits here ????????\r
     m_CurrentlySentBuffers ++;\r
     m_rRecvBuf--;\r
     m_pSdpSocket->m_RecvBufferPool.UpdateLocaleAdvertisedBuffers();\r
@@ -439,6 +452,14 @@ BufferPool::PostCredits()
         m_PostCreditsWhenCan = true;\r
         goto Cleanup;\r
     }\r
+\r
+    if (m_pSdpSocket->GetState() != SS_CONNECTED) {\r
+        SDP_PRINT(SDP_TRACE, SDP_BUFFER_POOL, ("this = 0x%p - Not sending credits,"\r
+            " because state = %s \n",this, SdpSocket::SS2String(m_pSdpSocket->GetState() )));        \r
+        // We will have to send them once we can\r
+        m_PostCreditsWhenCan = true;\r
+        goto Cleanup;\r
+    }\r
     \r
     // Post the credit\r
     if (m_CreditdBufferDescriptor == NULL) {\r
@@ -499,7 +520,7 @@ NTSTATUS BufferPool::PostDisConn()
     pBufferDescriptor->SetMid(SDP_MID_DISCONNECT);\r
     rc = AddBufferToQueuedList(pBufferDescriptor);\r
     if (!NT_SUCCESS(rc)) {\r
-        SDP_PRINT(SDP_ERR, SDP_BUFFER_POOL, ("SendBuffer failed rc = 0x%x\n", rc ));\r
+        SDP_PRINT(SDP_ERR, SDP_BUFFER_POOL, ("AddBufferToQueuedList failed rc = 0x%x\n", rc ));\r
         goto Cleanup;\r
     }\r
        \r

diff --git a/trunk/ulp/sdp/kernel/SdpRecvPool.cpp b/trunk/ulp/sdp/kernel/SdpRecvPool.cpp
index a49e75299480163e8563477d59b743c9ba9b8aaa..eafa0a3d12d5edd6dba42139b3131177e9ac9bbe 100644 (file)
--- a/trunk/ulp/sdp/kernel/SdpRecvPool.cpp
+++ b/trunk/ulp/sdp/kernel/SdpRecvPool.cpp
@@ -51,8 +51,6 @@ RecvPool::Init(
         pBufferDescriptor->Reset();\r
         m_FreePackets.InsertTailList(&pBufferDescriptor->BuffersList);\r
     }\r
-    m_FirstPostingOfBuffers = 0;\r
-\r
         \r
     return STATUS_SUCCESS;\r
 }\r
@@ -298,13 +296,6 @@ RecvPool::ReceiveIfCan()
     while (m_CurrentlyPostedRecievedBuffers < m_MaxConcurrentRecieves) {\r
         // do we have a free packet ?\r
         if (m_FreePackets.Size() > 0) {\r
-            //??????????? lET'S Sleep here for some time (FIND THE RACE) ??????????\r
-            {\r
-                if (m_FirstPostingOfBuffers ==0 ) {\r
-                    m_FirstPostingOfBuffers++;\r
-                    Sleep(0xfffff);\r
-                }\r
-            }\r
             // we can take a packet from the list\r
             LIST_ENTRY *item = m_FreePackets.RemoveHeadList();\r
             pBufferDescriptor = CONTAINING_RECORD(item, BufferDescriptor , BuffersList);            \r

diff --git a/trunk/ulp/sdp/kernel/SdpRecvPool.h b/trunk/ulp/sdp/kernel/SdpRecvPool.h
index 625b94c64933f5ffc0317adfe6d865257b4de94d..cfc26740104ed0f94eb07d7a1f16ab542aea2b3e 100644 (file)
--- a/trunk/ulp/sdp/kernel/SdpRecvPool.h
+++ b/trunk/ulp/sdp/kernel/SdpRecvPool.h
@@ -90,8 +90,6 @@ private:
     // This signals that the remote side will not be sending any data any more\r
     bool       m_DisConnRecieved; \r
 \r
-    int m_FirstPostingOfBuffers;//????????????????? USED TO HANDLE SOME RACE\r
-\r
 VOID AssertLocked();\r
 \r
 };\r

diff --git a/trunk/ulp/sdp/kernel/SdpSocket.cpp b/trunk/ulp/sdp/kernel/SdpSocket.cpp
index b68379782fdf805647279bf196662d69d519f3b7..de3717bbb74181c9f5debbd78d7babc8b1ab7bd4 100644 (file)
--- a/trunk/ulp/sdp/kernel/SdpSocket.cpp
+++ b/trunk/ulp/sdp/kernel/SdpSocket.cpp
@@ -390,7 +390,7 @@ SdpSocket::WSPRecv(
                 UserMode,\r
                 FALSE,\r
                 NULL\r
-                );    \r
+                );\r
             pBuffersEvent = NULL;\r
             if (( rc == STATUS_ALERTED ) ||( rc == STATUS_USER_APC )) {\r
                 // BUGBUG: Think what to do here, we should be able to stop the\r
@@ -1225,7 +1225,7 @@ NTSTATUS SdpSocket::CmSendRTU()
 \r
     cm_rtu.pfn_cm_apr_cb = cm_apr_callback;\r
     cm_rtu.pfn_cm_dreq_cb = cm_dreq_callback;\r
-    \r
+\r
     ib_status = ib_cm_rtu( m_cm_handle_t, &cm_rtu );\r
     if( ib_status != IB_SUCCESS ) {\r
         SDP_PRINT(SDP_ERR, SDP_SOCKET, ("ib_cm_rtu failed ib_status = 0x%d\n", ib_status ));\r
@@ -1253,7 +1253,7 @@ NTSTATUS SdpSocket::CmSendRTU()
 \r
     // We now start the recieve processing\r
 \r
-    rc = m_Lock.Lock();\r
+    rc = m_Lock.Lock(); //??????????\r
     if (!NT_SUCCESS(rc)) {\r
         SDP_PRINT(SDP_ERR, SDP_SOCKET, ("m_Lock.Lock failed rc = 0x%x\n", rc ));\r
         goto Cleanup;\r
@@ -1312,7 +1312,7 @@ SdpSocket::CmReqCallback(IN   ib_cm_req_rec_t *p_cm_req_rec)
     }\r
 \r
     // Take the lock and verify the state\r
-    rc = m_Lock.Lock();\r
+    rc = m_Lock.Lock(); //???????\r
     if (!NT_SUCCESS(rc)) {\r
         SDP_PRINT(SDP_ERR, SDP_SOCKET, ("m_Lock.Lock failed rc = 0x%x\n", rc ));\r
         goto Cleanup;\r
@@ -1468,6 +1468,8 @@ SdpSocket::CmReqCallback(IN   ib_cm_req_rec_t *p_cm_req_rec)
     sdp_msg_hello_ack hello_ack_msg;\r
     CreateHelloAckHeader(&hello_ack_msg);\r
 \r
+    pNewSocket->m_RecvBufferPool.SetLocaleAdvertisedBuffers(CL_NTOH16(hello_ack_msg.bsdh.recv_bufs));\r
+\r
     cm_rep.p_rep_pdata = (uint8_t *) &hello_ack_msg;\r
     cm_rep.rep_length = sizeof(hello_ack_msg);\r
 \r
@@ -1497,7 +1499,7 @@ SdpSocket::CmReqCallback(IN   ib_cm_req_rec_t *p_cm_req_rec)
         goto ErrorLocked;\r
     }\r
 \r
-    rc = pNewSocket->m_Lock.Lock();\r
+    rc = pNewSocket->m_Lock.Lock(); //????????\r
     if (!NT_SUCCESS(rc)) {\r
         SDP_PRINT(SDP_ERR, SDP_SOCKET, ("pNewSocket.Init() failed rc = 0x%x\n", rc ));\r
         goto ErrorLocked;\r
@@ -1529,7 +1531,7 @@ SdpSocket::CmReqCallback(IN   ib_cm_req_rec_t *p_cm_req_rec)
     // Sucess - we can now release the lock and update our state\r
     pNewSocket->m_state = SS_REP_SENT;\r
 \r
-    rc = pNewSocket->m_Lock.Unlock(); // Error is ignored, since this is already an error path\r
+    rc = pNewSocket->m_Lock.Unlock(); // ???????? Error is ignored, since this is already an error path\r
     if (!NT_SUCCESS(rc)) {\r
         SDP_PRINT(SDP_ERR, SDP_SOCKET, ("pNewSocket->m_Lock.Unlock() failed rc = 0x%x\n", rc ));\r
         // BUGBUG: who is responsibale for the cleanup ???????\r
@@ -1575,7 +1577,7 @@ SdpSocket::CmRtuCallback(IN   ib_cm_rtu_rec_t *p_cm_rtu_rec)
 \r
 \r
     // Take the lock and verify the state\r
-    rc = m_Lock.Lock();\r
+    rc = m_Lock.Lock(); //?????\r
     if (!NT_SUCCESS(rc)) {\r
         SDP_PRINT(SDP_ERR, SDP_SOCKET, ("m_Lock.Lock failed rc = 0x%x\n", rc ));\r
         goto Cleanup;\r
@@ -1597,7 +1599,7 @@ SdpSocket::CmRtuCallback(IN   ib_cm_rtu_rec_t *p_cm_rtu_rec)
     }\r
 \r
     // Next step is to move the new socket to the SS_CONNECTED state\r
-    rc = pSocket->m_Lock.Lock();\r
+    rc = pSocket->m_Lock.Lock(); //???????\r
     if (!NT_SUCCESS(rc)) {\r
         SDP_PRINT(SDP_ERR, SDP_SOCKET, ("m_Lock.Lock failed rc = 0x%x\n", rc ));\r
         goto ErrorLocked;\r
@@ -1839,6 +1841,7 @@ SdpSocket::__send_cb1(
     IN  void    *cq_context )\r
 {\r
     SdpSocket *pSocket = (SdpSocket *) cq_context;\r
+    SDP_PRINT(SDP_DEBUG, SDP_SOCKET, ("called this = 0x%x\n", pSocket ));    \r
     pSocket->m_Lock.SignalCB(SEND_CB_CALLED);\r
 }\r
 \r
@@ -2067,19 +2070,19 @@ NTSTATUS SdpSocket::CreateQp()
         rc = IB2Status(ib_status);\r
         goto Cleanup;\r
     }\r
-#if 0    \r
+#if DBG\r
     /* Query the QP so we can get our QPN. */\r
-    status = p_port->p_adapter->p_ifc->query_qp(\r
-       p_port->ib_mgr.h_qp, &qp_attr );\r
-    if( status != IB_SUCCESS )\r
+    ib_qp_attr_t qp_attr;\r
+    ib_status = ib_query_qp(\r
+        m_qp, &qp_attr );\r
+    if( ib_status != IB_SUCCESS )\r
     {\r
-       IPOIB_TRACE_EXIT( IPOIB_DBG_ERROR,\r
-               ("ib_query_qp returned %s\n", \r
-               p_port->p_adapter->p_ifc->get_err_str( status )) );\r
-       return status;\r
+        SDP_PRINT(SDP_ERR, SDP_SOCKET, ("ib_query_qp failed ib_status = 0x%d\n", ib_status ));\r
+        // ignore the error, this is only needed for debug\r
     }\r
-    p_port->ib_mgr.qpn = qp_attr.num;\r
-#endif\r
+    SDP_PRINT(SDP_TRACE, SDP_SOCKET,("QP number is %x\n", CL_NTOH32(qp_attr.num)));\r
+#endif    \r
+\r
     const net64_t MEM_REG_SIZE = 0xFFFFFFFFFFFFFFFF;\r
     /* Register all of physical memory */\r
     phys_create.length = MEM_REG_SIZE;\r

diff --git a/trunk/ulp/sdp/kernel/SdpSocket.h b/trunk/ulp/sdp/kernel/SdpSocket.h
index 28136ddfefb018c4b2e4fd97996bd21ce6dcec6e..15c5c9cec394700e4a7ffc5f3940cf16581218f0 100644 (file)
--- a/trunk/ulp/sdp/kernel/SdpSocket.h
+++ b/trunk/ulp/sdp/kernel/SdpSocket.h
@@ -63,6 +63,7 @@ class SdpSocket : public RefCountImpl {
 \r
     friend void cm_rtu_callback(IN ib_cm_rtu_rec_t *p_cm_rtu_rec );\r
     friend class ConnectionList;\r
+\r
 private: \r
    \r
     SocketStates m_state;\r
@@ -212,7 +213,8 @@ public:
     VOID CloseSocketThread();\r
 \r
     VOID DisconectSentEvent();\r
-    \r
+\r
+    SocketStates GetState() {return m_state;};\r
 \r
     // Two varibales that are needed for passing REP data\r
     struct sdp_msg_hello_ack m_hello_ack;\r
@@ -225,7 +227,8 @@ public:
     LIST_ENTRY m_UserFileList;\r
 \r
 #if DBG\r
-    char * SS2String(SocketStates state) {\r
+\r
+    static char * SS2String(SocketStates state) {\r
         switch (state) {\r
             case SS_IDLE                    : return "SS_IDLE";\r
             case SS_CONNECTING_QPR_SENT     : return "SS_CONNECTING_QPR_SENT";            \r

diff --git a/trunk/ulp/sdp/kernel/SdpTrace.cpp b/trunk/ulp/sdp/kernel/SdpTrace.cpp
index 6a7724d406bb223aab70f4126bd1e3d67104cbc3..bd7da2abc0c146b4234a8624d5473312eb95ad1d 100644 (file)
--- a/trunk/ulp/sdp/kernel/SdpTrace.cpp
+++ b/trunk/ulp/sdp/kernel/SdpTrace.cpp
@@ -3,10 +3,13 @@
 \r
 #include "Precompile.h"\r
 \r
+//int g_SdpDbgLevel = SDP_WARN;\r
+//int g_SdpDbgLevel = SDP_TRACE;\r
+int g_SdpDbgLevel = SDP_DEBUG;\r
+\r
 BOOLEAN CheckCondition(int sev, int top, char *file, int line, char * func)\r
 {\r
-    if (sev < SDP_TRACE) return FALSE;\r
-//  if (sev < SDP_WARN) return FALSE;\r
+    if (sev < g_SdpDbgLevel) return FALSE;\r
     if (top == SDP_PERFORMANCE) return FALSE;\r
     \r
     DbgPrint ("%s: ", func);\r

diff --git a/trunk/ulp/sdp/todo b/trunk/ulp/sdp/todo
index b5bb951a65c71f9eca61eb811a00252c32a77a53..2d455f72dd2c7cd0663dad4bf3d1165f83c08c62 100644 (file)
--- a/trunk/ulp/sdp/todo
+++ b/trunk/ulp/sdp/todo
@@ -28,6 +28,8 @@ general:
        \r
        Find a better solution for threading and remove the current solution.\r
        Fix the race of sdp user file\r
+\r
+       Fix the lock implmentation to have also a void implmentation as well as an RC implmentation\r
        \r
 \r
 USER MODE:\r