Alex,
I like your patch, in that a single call to NDIS is made. I do not see the
path to keeping g_NBL correct without a for loop?
Below is a slight modification in that 'Completing NBL....' message is not
output two times and the NDIS error code is output in hex to make it easier to
find in ndis.h
#if DBG
if (NET_BUFFER_LIST_STATUS(NetBufferLists) != NDIS_STATUS_SUCCESS) {
IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_ALL,
("NBL completed with error %#x to NDIS\n",
NET_BUFFER_LIST_STATUS(NetBufferLists)));
} else {
ASSERT(NET_BUFFER_LIST_NEXT_NBL(NetBufferLists) == NULL);
IPOIB_PRINT( TRACE_LEVEL_VERBOSE, IPOIB_DBG_SEND,
("Completing NBL=%x, g_NBL=%d, g_NBL_completed=%d \n",
NetBufferLists, g_NBL, g_NBL_complete) );
}
#endif
From: Alex Naslednikov [mailto:[email protected]]
Sent: Tuesday, March 01, 2011 12:42 AM
To: Smith, Stan; Fab Tillier; [email protected]
Subject: RE: [IPoIB] NdisMSendNetBufferListCompleteX assert
Hello Stan,
You are right - we have the following code for the regular flow (prior calling
to ipoib_port_send()):
// Important issue, break the connection between the different
nbls
NET_BUFFER_LIST_NEXT_NBL(curr_net_buffer_list) = NULL;
Of course, when we get into bad flow, the assertion mentioned by Fab is no
longer valid.
Thus, to avoid big changes, we can simply modify the code of
NdisMSendNetBufferListsCompleteX in a following manner:
#if DBG
if (NET_BUFFER_LIST_STATUS(NetBufferLists) != NDIS_STATUS_SUCCESS) {
IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_ALL,
("NBL completed with error %d to NDIS\n",
NET_BUFFER_LIST_STATUS(NetBufferLists)));
} else {
ASSERT(NET_BUFFER_LIST_NEXT_NBL(NetBufferLists) == NULL);
}
IPOIB_PRINT( TRACE_LEVEL_VERBOSE, IPOIB_DBG_SEND,
("Completing NBL=%x, g_NBL=%d, g_NBL_completed=%d \n",
NetBufferLists, g_NBL, g_NBL_complete) );
#endif
In addition, we should take into account that g_NBL was increased by an actual
numbers of NBLs inside the list. Thus, g_NBL_completed should be increased
accordingly (and not only once)
The other possibility was proposed by Stan. In this case the change will be
even smaller, but we will call to NDIS several times instead of one call only.
I am ok with both of them
From: [email protected]
[mailto:[email protected]] On Behalf Of Smith, Stan
Sent: Tuesday, March 01, 2011 3:42 AM
To: Fab Tillier; [email protected]
Subject: Re: [ofw] [IPoIB] NdisMSendNetBufferListCompleteX assert
>-----Original Message-----
>From: [email protected]
>[mailto:[email protected]] On Behalf Of Fab
>Tillier
>Sent: Monday, February 28, 2011 4:46 PM
>To: [email protected]
>Subject: [ofw] [IPoIB] NdisMSendNetBufferListCompleteX assert
>
>The assertion at line 1122 in ipoib_port.h seems wrong since the following
>code dereferences the NBL.
>Is it wrong, or there to cause an assert because of a send error?
>
>If the latter, a comment would help make the intent clear, as the code looks
>wrong as is.
>
>-Fab
NdisMSendNetBufferListsCompleteX() is completing a 'single' NBL
(NetworkBufferList); not a list of NBLs.
The code is correctly used in ipoib_port.cpp @ ipoib_port_send().
Although it's usage in ipoib_driver.cpp @ ipoib_send_net_buffer_list() after
label 'compl_status:' is incorrect.
Appears the comment is wrong w.r.t. NdisMSendNetBufferListsCompleteX() and the
for() loop should be extended down to encompass the
NdisMSendNetBufferListsCompleteX() call with a single NBL.
The dispatch check should be done once prior to entering the for()loop.
Tzachi, Alex - do you agree?
Stan.
>_______________________________________________
>ofw mailing list
>[email protected]
>http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
