[ofw] [IPoIB_CM] patch for two MTU parameters. Was [IPoIB_CM] Problems with large pings
Alex Estrin
alex.estrin at qlogic.com
Thu Jan 29 12:23:00 PST 2009
Applied in rev. 1919
Thanks,
Alex.
> -----Original Message-----
> From: Alex Naslednikov [mailto:xalex at mellanox.co.il]
> Sent: Thursday, January 22, 2009 3:21 AM
> To: Alex Estrin
> Cc: ofw at lists.openfabrics.org
> Subject: RE: [ofw] [IPoIB_CM] patch for two MTU parameters.
> Was [IPoIB_CM] Problems with large pings
>
> 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