[ofw] [Patch][IPoIB_NDIS6_CM] Datapath improvement
Smith, Stan
stan.smith at intel.com
Wed Dec 8 18:23:36 PST 2010
Hello,
What is the 'uint64_t phys_addr;' about? I find no references to phys_addr in the recv descriptor?
ipoib_prealloc_hdr_t.phys_addr is not 8-byte aligned, hence will cause IA64 alignment problems.
The ipoib_prealloc_hdr_t definition is why many people dislike C++ due to obscure data hiding......
typedef struct _ipoib_hdr
{
net16_t type;
net16_t resv;
} PACK_SUFFIX ipoib_hdr_t;
typedef struct _ipoib_prealloc_hdr:public ipoib_hdr_t
{
uint64_t phys_addr;
} PACK_SUFFIX ipoib_prealloc_hdr_t;
Why not define a C struct which contains the ipoib_hdr_t and the 64-bit phys_addr, which is infinitely more clear than C++ approach?
Not to mention correctly aligned for IA64.
BTW, ipoib_prealloc_hdr_t does not need to be 'packed'. Perhaps this would fix the C++ definition alignment problem?
typedef struct _ipoib_prealloc_hdr
{
uint64_t phys_addr;
ipoib_hdt_t h;
} ipoib_prealloc_hdr_t;
It would be good to include some performance information as to how effective this patch is?
I agree this patch is the correct performance enhancement, how much performance improvement did you find?
I found a 30 MB/sec decrease in bandwidth at 64KB MTU using ttcp -D, while at 2044 MTU the CM performance is equal to UD?
Adding cl_get_physaddr() back into the send data path, restores the lost 30MB/s bandwidth?
You are on the right track with removing cl_get_physaddr() from the send data path, although I need to understand what's up with the BW drop.
Under investigation.
stan.
________________________________
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Wednesday, December 01, 2010 2:28 AM
To: ofw at lists.openfabrics.org
Subject: [ofw] [Patch][IPoIB_NDIS6_CM] Datapath improvement
This patch removes calls to MmGetPhysAddr from datapath
Tested with SdpConnect.exe on Win2008x64 vs. R2, with and w\o LSO, with the following data sizes:
65536, 655360, 512000, 519317 (bytes)
In addition, it passed several regression runs
Signed-off by: Alexander Naslednikov (xalex at mellanox.co.il)
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp (revision 3007)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp (working copy)
@@ -111,7 +111,8 @@
******************************************************************************/
static void
__port_construct(
- IN ipoib_port_t* const p_port );
+ IN ipoib_port_t* const p_port,
+ IN int32_t alloc_size);
static ib_api_status_t
__port_init(
@@ -638,6 +639,7 @@
CL_ASSERT( !p_adapter->p_port );
+ // Allocate PORT object along with "send queue depth" - 1 IPoIB headers
p_port = (ipoib_port_t *) cl_zalloc( sizeof(ipoib_port_t) +
(sizeof(ipoib_prealloc_hdr_t) * (p_adapter->params.sq_depth - 1)) );
if( !p_port )
@@ -652,7 +654,7 @@
gp_ipoib_port = p_port;
#endif
- __port_construct( p_port );
+ __port_construct( p_port, p_adapter->params.sq_depth -1 );
status = __port_init( p_port, p_adapter, p_pnp_rec );
if( status != IB_SUCCESS )
@@ -689,7 +691,8 @@
static void
__port_construct(
- IN ipoib_port_t* const p_port )
+ IN ipoib_port_t* const p_port,
+ IN int32_t alloc_size)
{
IPOIB_ENTER( IPOIB_DBG_INIT );
@@ -710,6 +713,12 @@
KeInitializeEvent( &p_port->sa_event, NotificationEvent, TRUE );
KeInitializeEvent( &p_port->leave_mcast_event, NotificationEvent, TRUE );
+
+ for ( int i = 0; i < alloc_size; ++i )
+ {
+ p_port->hdr[i].phys_addr = cl_get_physaddr( &p_port->hdr[i] );
+ p_port->hdr[i].resv = 0;
+ }
IPOIB_EXIT( IPOIB_DBG_INIT );
}
@@ -5278,9 +5287,7 @@
ASSERT( hdr_idx < s_buf->p_port->p_adapter->params.sq_depth );
/* Set up IPoIB Header */
- s_buf->p_port->hdr[hdr_idx].type = p_eth_hdr->type;
- s_buf->p_port->hdr[hdr_idx].resv = 0;
-
+ s_buf->p_port->hdr[hdr_idx].type = p_eth_hdr->type;
//Init send buffer to 0
s_buf->p_send_buf = NULL;
@@ -5309,8 +5316,7 @@
cl_perf_start( SendMgrFilter );
/* Put first DS to be IPoIB Header */
- p_desc->send_wr[0].local_ds[0].vaddr =
- cl_get_physaddr( &s_buf->p_port->hdr[hdr_idx] );
+ p_desc->send_wr[0].local_ds[0].vaddr = s_buf->p_port->hdr[hdr_idx].phys_addr;
p_desc->send_wr[0].local_ds[0].length = sizeof(ipoib_hdr_t);
p_desc->send_wr[0].local_ds[0].lkey = s_buf->p_port->ib_mgr.lkey;
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_port.h
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_port.h (revision 2999)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_port.h (working copy)
@@ -168,6 +168,12 @@
* resv
* Reserved portion of IPoIB header.
*********/
+
+typedef struct _ipoib_prealloc_hdr:public ipoib_hdr_t
+{
+ uint64_t phys_addr;
+} PACK_SUFFIX ipoib_prealloc_hdr_t;
+
static const uint32_t EthHeaderOffset = sizeof(eth_hdr_t);
//We reuse eth header to put there IPoIB header for LSO Net Buffers.
@@ -369,6 +375,7 @@
NDIS_TCP_IP_CHECKSUM_PACKET_INFO ndis_csum;
#if IPOIB_INLINE_RECV
recv_buf_t buf;
+ uint64_t phys_addr;
#else
recv_buf_t *p_buf;
#endif
@@ -626,7 +633,7 @@
ib_net16_t base_lid;
LONG n_no_progress;
PIO_WORKITEM pPoWorkItem;
- ipoib_hdr_t hdr[1]; /* Must be last! */
+ ipoib_prealloc_hdr_t hdr[1]; /* Must be last! */
} ipoib_port_t;
#pragma warning(default:4324)
Alexander (XaleX) Naslednikov
SW Networking Team
Mellanox Technologies
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20101208/85f2696f/attachment.html>
More information about the ofw
mailing list