[ofw] [IPoIB_CM] patch for two MTU parameters. Was [IPoIB_CM] Problems with large pings

Alex Naslednikov xalex at mellanox.co.il
Thu Jan 22 00:20:51 PST 2009


Hello, Alex,
Thank you for the patch.
I reviewed the patch and it looks good. Have you performed also basic
testing of UD mode in addition to CM mode?
IMHO, both solutions are ok, but if you suppose that the 2-parameter
solutions will be more intuitive for customer, let's go for it.

-----Original Message-----
From: Alex Estrin [mailto:alex.estrin at qlogic.com] 
Sent: Tuesday, January 20, 2009 9:09 PM
To: Alex Naslednikov
Cc: ofw at lists.openfabrics.org
Subject: RE: [ofw] [IPoIB_CM] patch for two MTU parameters. Was
[IPoIB_CM] Problems with large pings

Hello,

> -----Original Message-----
> From: Alex Naslednikov [mailto:xalex at mellanox.co.il]
> Sent: Monday, January 12, 2009 2:09 PM
> To: Alex Estrin
> Cc: ofw at lists.openfabrics.org
> Subject: RE: [ofw] [IPoIB_CM] Problems with large pings
> 
> Hello, Alex,
> Thank you for the patch, it looks good.
> I still have one remark regards (ud) payload :
>    If one uses IPoIB in UD mode, he will not have the possibility now 
> to configure MTU to the value lower than DEFAULT_MTU.
> 
> I would like to propose 2 solutions:
> 1.  using 2 different parameters for UD and CM, or 2. If CM is 
> disabled, ud payload will take the value from the registry; otherwise,

> cm payload will take the value from the registry and UD will take the 
> maximum possible value
> 
> XaleX
> 

I thought more about your proposal of using 2 MTU parameters instead of
one.
I think it makes sense as it will be less confusing for a customer, also
will simplify a bit initialization code.
Proposed patch will create and use two MTU parameters.

Please review.


Index: kernel/ipoib_driver.c
===================================================================
--- kernel/ipoib_driver.c	(revision 1856)
+++ kernel/ipoib_driver.c	(working copy)
@@ -160,11 +160,12 @@
 	{NDIS_STRING_CONST("SaTimeout"),        1,
IPOIB_OFFSET(sa_timeout),            IPOIB_SIZE(sa_timeout),
1000,       250,    UINT_MAX},
 	{NDIS_STRING_CONST("SaRetries"),        1,
IPOIB_OFFSET(sa_retry_cnt),          IPOIB_SIZE(sa_retry_cnt),       10,
1,      UINT_MAX},
 	{NDIS_STRING_CONST("RecvRatio"),        1,
IPOIB_OFFSET(recv_pool_ratio),       IPOIB_SIZE(recv_pool_ratio),    1,
1,      10},
-	{NDIS_STRING_CONST("PayloadMtu"),       1,
IPOIB_OFFSET(payload_mtu),           IPOIB_SIZE(payload_mtu),
2044,       512,   MAX_CM_PAYLOAD_MTU},
+	{NDIS_STRING_CONST("PayloadMtu"),       1,
IPOIB_OFFSET(payload_mtu),           IPOIB_SIZE(payload_mtu),
2044,       512,   MAX_UD_PAYLOAD_MTU},
 	{NDIS_STRING_CONST("lso"),              0, IPOIB_OFFSET(lso),
IPOIB_SIZE(lso),                0,          0,      1},
 	{NDIS_STRING_CONST("MCLeaveRescan"),    1,
IPOIB_OFFSET(mc_leave_rescan),       IPOIB_SIZE(mc_leave_rescan),
260,        1,    3600},
 	{NDIS_STRING_CONST("BCJoinRetry"),	    1,
IPOIB_OFFSET(bc_join_retry),		    IPOIB_SIZE(bc_join_retry),
50,         0,    1000},
-	{NDIS_STRING_CONST("CmEnabled"),        0,
IPOIB_OFFSET(cm_enabled),            IPOIB_SIZE(cm_enabled),
FALSE,     FALSE, TRUE}
+	{NDIS_STRING_CONST("CmEnabled"),        0,
IPOIB_OFFSET(cm_enabled),            IPOIB_SIZE(cm_enabled),
FALSE,     FALSE, TRUE},
+	{NDIS_STRING_CONST("CmPayloadMtu"),     1,
IPOIB_OFFSET(cm_payload_mtu),        IPOIB_SIZE(cm_payload_mtu),
MAX_CM_PAYLOAD_MTU, 512, MAX_CM_PAYLOAD_MTU}
 
 };  
 
@@ -614,18 +615,18 @@
 				EVENT_IPOIB_CONNECTED_MODE_ERR, 1,
0xbadc0de0 );
 		}
 	}
-	/* adjust ud mtu upper limit if cm is disabled */
-	if( !p_adapter->params.cm_enabled )
+
+	if( p_adapter->params.cm_enabled )
 	{
-		p_adapter->params.payload_mtu = 
-			min( MAX_UD_PAYLOAD_MTU,
p_adapter->params.payload_mtu );
-		p_adapter->params.xfer_block_size = 
-			(sizeof(eth_hdr_t) +
p_adapter->params.payload_mtu);
+		p_adapter->params.cm_xfer_block_size = 
+			(sizeof(eth_hdr_t) +
p_adapter->params.cm_payload_mtu);
 	}
 
+	p_adapter->params.xfer_block_size = 
+		(sizeof(eth_hdr_t) + p_adapter->params.payload_mtu);
+
 	NdisReadNetworkAddress( &status, p_mac, p_len, h_config );
 
-
 	NdisCloseConfiguration( h_config );
 
 	IPOIB_EXIT( IPOIB_DBG_INIT );
Index: kernel/ipoib_port.c
===================================================================
--- kernel/ipoib_port.c	(revision 1856)
+++ kernel/ipoib_port.c	(working copy)
@@ -899,18 +899,12 @@
 
 	if( p_port->p_adapter->params.cm_enabled )
 	{
-		uint32_t	ud_payload_mtu =
__port_attr_to_mtu_size( 
-			p_port->p_ca_attrs->p_port_attr[p_port->port_num
- 1].mtu );
-		/* setup CM payload mtu */
-		p_port->p_adapter->params.cm_payload_mtu =
-			min( MAX_CM_PAYLOAD_MTU,
p_port->p_adapter->params.payload_mtu );
-		p_port->p_adapter->params.cm_xfer_block_size = 
-			p_port->p_adapter->params.cm_payload_mtu +
sizeof(eth_hdr_t);
-		
+		uint32_t payload_mtu = __port_attr_to_mtu_size( 
+			p_port->p_ca_attrs->p_port_attr[p_port->port_num
- 1].mtu )
+			- sizeof(ipoib_hdr_t);
 		/* adjust ipoib UD payload MTU to actual port MTU size.
*/
-		/* TODO: should initialization fail if port mtu < 2048 ?
*/
 		p_port->p_adapter->params.payload_mtu = 
-			( max( DEFAULT_MTU, ud_payload_mtu ) -
sizeof(ipoib_hdr_t) );
+				max( DEFAULT_PAYLOAD_MTU, payload_mtu );
 		p_port->p_adapter->params.xfer_block_size = 
 			(sizeof(eth_hdr_t) +
p_port->p_adapter->params.payload_mtu);
 	}
@@ -919,14 +913,19 @@
 	if ( !p_port->p_adapter->reset )
 	{
 		ULONG max_phys_mapping;
-
-		max_phys_mapping = p_port->p_adapter->params.cm_enabled
?
-			p_port->p_adapter->params.cm_xfer_block_size :
-			p_port->p_adapter->params.xfer_block_size;
-		max_phys_mapping = p_port->p_adapter->params.lso ?
-			max(LARGE_SEND_OFFLOAD_SIZE, max_phys_mapping):
max_phys_mapping;
-		status =
-			NdisMInitializeScatterGatherDma( 
+		if( p_port->p_adapter->params.cm_enabled )
+		{
+			max_phys_mapping =
p_port->p_adapter->params.cm_xfer_block_size;
+		}
+		else if( p_port->p_adapter->params.lso )
+		{
+			max_phys_mapping = LARGE_SEND_OFFLOAD_SIZE;
+		}
+		else
+		{
+			max_phys_mapping =
p_port->p_adapter->params.xfer_block_size;
+		}
+		status = NdisMInitializeScatterGatherDma(
 			p_port->p_adapter->h_adapter, TRUE,
max_phys_mapping );
 		if( status != NDIS_STATUS_SUCCESS )
 		{
Index: kernel/netipoib-xp32.inf
===================================================================
--- kernel/netipoib-xp32.inf	(revision 1856)
+++ kernel/netipoib-xp32.inf	(working copy)
@@ -138,7 +138,7 @@
 HKR, Ndi\Params\PayloadMtu,		Type,		0, "dword"
 HKR, Ndi\Params\PayloadMtu,		Default,	0, "2044"
 HKR, Ndi\Params\PayloadMtu,		Min,		0, "512"
-HKR, Ndi\Params\PayloadMtu,		Max,		0, "65520"
+HKR, Ndi\Params\PayloadMtu,		Max,		0, "4092"
 
 HKR, Ndi\Params\MCLeaveRescan,		ParamDesc,	0,
%MC_RESCAN_STR%
 HKR, Ndi\Params\MCLeaveRescan,		Type,		0, "dword"
@@ -168,6 +168,12 @@
 HKR, Ndi\Params\CmEnabled\enum,	"0",	0,	%DISABLED_STR%
 HKR, Ndi\Params\CmEnabled\enum,	"1",	0,	%ENABLED_STR%
 
+HKR, Ndi\Params\CmPayloadMtu,		ParamDesc,	0,
%CONNECTED_MODE_MTU_STR%
+HKR, Ndi\Params\CmPayloadMtu,		Type,		0, "dword"
+HKR, Ndi\Params\CmPayloadMtu,		Default,	0, "65520"
+HKR, Ndi\Params\CmPayloadMtu,		Min,		0, "512"
+HKR, Ndi\Params\CmPayloadMtu,		Max,		0, "65520"
+
 [IpoibService]
 DisplayName     = %IpoibServiceDispName%
 ServiceType     = 1 ;%SERVICE_KERNEL_DRIVER%
@@ -269,3 +275,5 @@
 DISABLED_STR		= "Disabled"
 BYPASS_STR		= "Bypass"
 CONNECTED_MODE_STR	= "Connected mode"
+CONNECTED_MODE_MTU_STR	= "Connected Mode Payload Mtu size"
+
Index: kernel/netipoib.inx
===================================================================
--- kernel/netipoib.inx	(revision 1856)
+++ kernel/netipoib.inx	(working copy)
@@ -141,7 +141,7 @@
 HKR, Ndi\Params\PayloadMtu,		Type,		0, "dword"
 HKR, Ndi\Params\PayloadMtu,		Default,	0, "2044"
 HKR, Ndi\Params\PayloadMtu,		Min,		0, "512"
-HKR, Ndi\Params\PayloadMtu,		Max,		0, "65520"
+HKR, Ndi\Params\PayloadMtu,		Max,		0, "4092"
 
 HKR, Ndi\Params\MCLeaveRescan,		ParamDesc,	0,
%MC_RESCAN_STR%
 HKR, Ndi\Params\MCLeaveRescan,		Type,		0, "dword"
@@ -171,6 +171,12 @@
 HKR, Ndi\Params\CmEnabled\enum,	"0",	0,	%DISABLED_STR%
 HKR, Ndi\Params\CmEnabled\enum,	"1",	0,	%ENABLED_STR%
 
+HKR, Ndi\Params\CmPayloadMtu,		ParamDesc,	0,
%CONNECTED_MODE_MTU_STR%
+HKR, Ndi\Params\CmPayloadMtu,		Type,		0, "dword"
+HKR, Ndi\Params\CmPayloadMtu,		Default,	0, "65520"
+HKR, Ndi\Params\CmPayloadMtu,		Min,		0, "512"
+HKR, Ndi\Params\CmPayloadMtu,		Max,		0, "65520"
+
 [IpoibService]
 DisplayName     = %IpoibServiceDispName%
 ServiceType     = 1 ;%SERVICE_KERNEL_DRIVER%
@@ -275,4 +281,5 @@
 ENABLED_STR		= "Enabled"
 DISABLED_STR		= "Disabled"
 BYPASS_STR		= "Bypass"
-CONNECTED_MODE_STR	= "Connected mode"
\ No newline at end of file
+CONNECTED_MODE_STR	= "Connected mode"
+CONNECTED_MODE_MTU_STR	= "Connected Mode Payload Mtu size"





More information about the ofw mailing list