[ofw] RE: [IPoIB] Fix for improper handling of IPoIB params
Tzachi Dar
tzachid at mellanox.co.il
Tue Jul 8 07:34:37 PDT 2008
Applied at revision 1341.
Thanks
Tzachi
________________________________
From: Alex Naslednikov
Sent: Thursday, June 26, 2008 3:00 PM
To: ofw at lists.openfabrics.org; Tzachi Dar
Cc: Ishai Rabinovitz
Subject: [IPoIB] Fix for improper handling of IPoIB params
Reposting this message again (there was a porblem with my first
post)
________________________________
This patch fixes improper handling of IPoIB parameters received
from registers
Signed-off by: xalex (xalex at mellanox.co.il
<mailto:xalex at mellanox.co.il> )
Index: ulp/ipoib/kernel/SOURCES
===================================================================
--- ulp/ipoib/kernel/SOURCES (revision 2629)
+++ ulp/ipoib/kernel/SOURCES (revision 2630)
@@ -25,14 +25,16 @@
TARGETLIBS= \
$(TARGETPATH)\*\complib.lib \
- $(DDK_LIB_PATH)\ndis.lib
+ $(DDK_LIB_PATH)\ndis.lib \
+ $(DDK_LIB_PATH)\ntstrsafe.lib \
+ $(DDK_LIB_PATH)\strsafe.lib
!if !defined(DDK_TARGET_OS) || "$(DDK_TARGET_OS)"=="Win2K"
#
# The driver is built in the Win2K build environment
# - use the library version of safe strings
#
-TARGETLIBS= $(TARGETLIBS) $(DDK_LIB_PATH)\ntstrsafe.lib
+#TARGETLIBS= $(TARGETLIBS) $(DDK_LIB_PATH)\ntstrsafe.lib
!endif
!IFDEF ENABLE_EVENT_TRACING
Index: ulp/ipoib/kernel/ipoib_driver.c
===================================================================
--- ulp/ipoib/kernel/ipoib_driver.c (revision 2629)
+++ ulp/ipoib/kernel/ipoib_driver.c (revision 2630)
@@ -30,7 +30,7 @@
* $Id$
*/
-
+#include "limits.h"
#include "ipoib_driver.h"
#include "ipoib_debug.h"
@@ -47,8 +47,12 @@
#include <complib/cl_init.h>
#include <initguid.h>
#include <iba/ipoib_ifc.h>
+#include "ntstrsafe.h"
+#include "strsafe.h"
+
+
#if defined(NDIS50_MINIPORT)
#define MAJOR_NDIS_VERSION 5
#define MINOR_NDIS_VERSION 0
@@ -59,6 +63,8 @@
#error NDIS Version not defined, try defining NDIS50_MINIPORT
or NDIS51_MINIPORT
#endif
+PDRIVER_OBJECT g_p_drv_obj;
+
static const NDIS_OID SUPPORTED_OIDS[] =
{
OID_GEN_SUPPORTED_LIST,
@@ -125,7 +131,72 @@
uint32_t g_ipoib_dbg_flags = 0x00000fff;
ipoib_globals_t g_ipoib = {0};
+typedef struct _IPOIB_REG_ENTRY
+{
+ NDIS_STRING RegName; // variable name text
+ BOOLEAN bRequired; // 1 -> required, 0 ->
optional
+ UINT FieldOffset; // offset in parent struct
+ UINT FieldSize; // size (in bytes) of the
field
+ UINT Default; // default value to use
+ UINT Min; // minimum value allowed
+ UINT Max; // maximum value allowed
+} IPOIB_REG_ENTRY, *PIPOIB_REG_ENTRY;
+IPOIB_REG_ENTRY HCARegTable[] = {
+ // reg value name If Required Offset in parentr
struct Field size Default Min
Max
+ {NDIS_STRING_CONST("RqDepth"), 1,
IPOIB_OFFSET(rq_depth), IPOIB_SIZE(rq_depth),
512, 128, 1024},
+ {NDIS_STRING_CONST("RqLowWatermark"), 0,
IPOIB_OFFSET(rq_low_watermark), IPOIB_SIZE(rq_low_watermark), 4,
2, 8},
+ {NDIS_STRING_CONST("SqDepth"), 1,
IPOIB_OFFSET(sq_depth), IPOIB_SIZE(sq_depth),
512, 128, 1024},
+ {NDIS_STRING_CONST("SendChksum"), 1,
IPOIB_OFFSET(send_chksum_offload), IPOIB_SIZE(send_chksum_offload),0,
0, 1},
+ {NDIS_STRING_CONST("RecvChksum"), 1,
IPOIB_OFFSET(recv_chksum_offload), IPOIB_SIZE(recv_chksum_offload),0,
0, 1},
+ {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 , 60 , 2044}
+};
+
+#define IPOIB_NUM_REG_PARAMS (sizeof (HCARegTable) /
sizeof(IPOIB_REG_ENTRY))
+
+
+static void
+ipoib_create_log(
+ NDIS_HANDLE h_adapter,
+ UINT ind,
+ ULONG eventLogMsgId)
+
+{
+#define cMaxStrLen 40
+#define cArrLen 3
+
+ PWCHAR logMsgArray[cArrLen];
+ WCHAR strVal[cMaxStrLen];
+ NDIS_STRING AdapterInstanceName;
+
+ IPOIB_INIT_NDIS_STRING(&AdapterInstanceName);
+ if (NdisMQueryAdapterInstanceName(&AdapterInstanceName,
h_adapter)!= NDIS_STATUS_SUCCESS ){
+ ASSERT(FALSE);
+ IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR,IPOIB_DBG_ERROR,
("[IPoIB] Init:Failed to retreive adapter name.\n"));
+ return;
+ }
+ logMsgArray[0] = AdapterInstanceName.Buffer;
+
+ if (RtlStringCbPrintfW(strVal, sizeof(strVal), L"0x%x",
HCARegTable[ind].Default) != STATUS_SUCCESS) {
+ ASSERT(FALSE);
+ IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR,IPOIB_DBG_ERROR,
+ ("[IPoIB] Init: Problem copying string value: exiting\n"));
+ return;
+ }
+
+ logMsgArray[0] = AdapterInstanceName.Buffer;
+ logMsgArray[1] = HCARegTable[ind].RegName.Buffer;
+ logMsgArray[2] = strVal;
+
+ NdisWriteEventLogEntry(g_p_drv_obj, eventLogMsgId, 0, cArrLen,
&logMsgArray, 0, NULL);
+
+}
+
+
+
NTSTATUS
DriverEntry(
IN PDRIVER_OBJECT p_drv_obj,
@@ -248,6 +319,7 @@
NDIS_MINIPORT_CHARACTERISTICS characteristics;
IPOIB_ENTER( IPOIB_DBG_INIT );
+ g_p_drv_obj = p_drv_obj;
#ifdef _DEBUG_
PAGED_CODE();
@@ -397,6 +469,7 @@
}
+
NDIS_STATUS
ipoib_get_adapter_params(
IN NDIS_HANDLE* const wrapper_config_context,
@@ -405,9 +478,13 @@
NDIS_STATUS status;
NDIS_HANDLE h_config;
NDIS_CONFIGURATION_PARAMETER *p_param;
- NDIS_STRING keyword;
PUCHAR mac;
UINT len;
+ UINT value;
+ PIPOIB_REG_ENTRY pRegEntry;
+ UINT i;
+ PUCHAR structPointer;
+ int sq_depth_step = 128;
IPOIB_ENTER( IPOIB_DBG_INIT );
@@ -419,124 +496,96 @@
return status;
}
- /* Required: Receive queue depth. */
- RtlInitUnicodeString( &keyword, L"RqDepth" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
+ // read all the registry values
+ for (i = 0, pRegEntry = HCARegTable; i < IPOIB_NUM_REG_PARAMS;
++i)
{
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("Receive Queue Depth parameter missing.\n") );
- return status;
- }
- p_adapter->params.rq_depth =
p_param->ParameterData.IntegerData;
+ // initialize pointer to appropriate place inside 'params'
+ structPointer = (PUCHAR) &p_adapter->params +
pRegEntry[i].FieldOffset;
- /* Optional: Receive queue low watermark. */
- RtlInitUnicodeString( &keyword, L"RqLowWatermark" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS ||
!p_param->ParameterData.IntegerData )
- {
- p_adapter->params.rq_low_watermark =
p_adapter->params.rq_depth >> 2;
- }
- else
- {
- p_adapter->params.rq_low_watermark =
- p_adapter->params.rq_depth /
p_param->ParameterData.IntegerData;
- }
+ // Get the configuration value for a specific parameter.
Under NT the
+ // parameters are all read in as DWORDs.
+ NdisReadConfiguration(
+ &status,
+ &p_param,
+ h_config,
+ &pRegEntry[i].RegName,
+ NdisParameterInteger);
- /* Required: Send queue depth. */
- RtlInitUnicodeString( &keyword, L"SqDepth" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
- {
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("Send Queue Depth parameter missing.\n") );
- return status;
- }
- p_adapter->params.sq_depth =
p_param->ParameterData.IntegerData;
- /* Send queue depth needs to be a power of two. */
- if( p_adapter->params.sq_depth <= 128 )
- p_adapter->params.sq_depth = 128;
- else if( p_adapter->params.sq_depth <= 256 )
- p_adapter->params.sq_depth = 256;
- else if( p_adapter->params.sq_depth <= 512 )
- p_adapter->params.sq_depth = 512;
- else
- p_adapter->params.sq_depth = 1024;
+ // If the parameter was present, then check its value for
validity.
+ if (status == NDIS_STATUS_SUCCESS)
+ {
+ // Check that param value is not too small or too large
+ if (p_param->ParameterData.IntegerData < pRegEntry[i].Min ||
+ p_param->ParameterData.IntegerData > pRegEntry[i].Max)
+ {
+ value = pRegEntry[i].Default;
+ ipoib_create_log(p_adapter->h_adapter, i,
EVENT_IPOIB_WRONG_PARAMETER_WRN);
+ IPOIB_PRINT(TRACE_LEVEL_VERBOSE, IPOIB_DBG_INIT, ("Read
configuration.Registry %S value is out of range, setting default value=
0x%x\n", pRegEntry[i].RegName.Buffer, value));
- /* Required: Send Checksum Offload. */
- RtlInitUnicodeString( &keyword, L"SendChksum" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
- {
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("Send Checksum Offload parameter missing.\n") );
- return status;
- }
- p_adapter->params.send_chksum_offload =
(p_param->ParameterData.IntegerData != 0);
+ }
+ else
+ {
+ value = p_param->ParameterData.IntegerData;
+ IPOIB_PRINT(TRACE_LEVEL_VERBOSE, IPOIB_DBG_INIT, ("Read
configuration. Registry %S, Value= 0x%x\n", pRegEntry[i].RegName.Buffer,
value));
+ }
+ }
- /* Required: Send Checksum Offload. */
- RtlInitUnicodeString( &keyword, L"RecvChksum" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
- {
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("Recv Checksum Offload parameter missing.\n") );
- return status;
- }
- p_adapter->params.recv_chksum_offload =
(p_param->ParameterData.IntegerData != 0);
+ else
+ {
+ value = pRegEntry[i].Default;
+ status = NDIS_STATUS_SUCCESS;
+ if (pRegEntry[i].bRequired)
+ {
+ ipoib_create_log(p_adapter->h_adapter, i,
EVENT_IPOIB_WRONG_PARAMETER_ERR);
+ IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_INIT, ("Read
configuration.Registry %S value not found, setting default value=
0x%x\n", pRegEntry[i].RegName.Buffer, value));
+ }
+ else
+ {
+ ipoib_create_log(p_adapter->h_adapter, i,
EVENT_IPOIB_WRONG_PARAMETER_INFO);
+ IPOIB_PRINT(TRACE_LEVEL_VERBOSE, IPOIB_DBG_INIT, ("Read
configuration. Registry %S value not found, Value= 0x%x\n",
pRegEntry[i].RegName.Buffer, value));
+ }
- /* Required: SA query timeout, in milliseconds. */
- RtlInitUnicodeString( &keyword, L"SaTimeout" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
- {
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("SA query timeout parameter missing.\n") );
- return status;
- }
- p_adapter->params.sa_timeout =
p_param->ParameterData.IntegerData;
+ }
+ //
+ // Store the value in the adapter structure.
+ //
+ switch(pRegEntry[i].FieldSize)
+ {
+ case 1:
+ *((PUCHAR) structPointer) = (UCHAR) value;
+ break;
- /* Required: SA query retry count. */
- RtlInitUnicodeString( &keyword, L"SaRetries" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
- {
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("SA query retry count parameter missing.\n") );
- return status;
+ case 2:
+ *((PUSHORT) structPointer) = (USHORT) value;
+ break;
+
+ case 4:
+ *((PULONG) structPointer) = (ULONG) value;
+ break;
+
+ default:
+ IPOIB_PRINT(TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR, ("Bogus
field size %d\n", pRegEntry[i].FieldSize));
+ break;
+ }
}
- p_adapter->params.sa_retry_cnt =
p_param->ParameterData.IntegerData;
- /* Required: Receive pool to queue depth ratio. */
- RtlInitUnicodeString( &keyword, L"RecvRatio" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
- {
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("Receive pool to queue depth ratio parameter missing.\n")
);
- return status;
+ // Send queue depth needs to be a power of two
+ //static const INT sq_depth_step = 128;
+
+ if (p_adapter->params.sq_depth % sq_depth_step) {
+ static const c_sq_ind = 2;
+ p_adapter->params.sq_depth = sq_depth_step *(
+ p_adapter->params.sq_depth / sq_depth_step + !!(
(p_adapter->params.sq_depth % sq_depth_step) > (sq_depth_step/2) ));
+ ipoib_create_log(p_adapter->h_adapter, c_sq_ind,
EVENT_IPOIB_WRONG_PARAMETER_WRN);
+ IPOIB_PRINT(TRACE_LEVEL_VERBOSE, IPOIB_DBG_INIT, ("SQ DEPTH
value was rounded to the closest acceptable value of 0x%x\n",
p_adapter->params.sq_depth ));
+
}
- p_adapter->params.recv_pool_ratio =
p_param->ParameterData.IntegerData;
- /* required: MTU size. */
- RtlInitUnicodeString( &keyword, L"PayloadMtu" );
- NdisReadConfiguration(
- &status, &p_param, h_config, &keyword, NdisParameterInteger
);
- if( status != NDIS_STATUS_SUCCESS )
- {
- IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
- ("PayloadMtu parameter missing. Use the default.\n") );
- return status;
- }
- p_adapter->params.payload_mtu =
p_param->ParameterData.IntegerData;
+
+ // Adjusting the low watermark parameter
+ p_adapter->params.rq_low_watermark =
+ p_adapter->params.rq_depth /
p_adapter->params.rq_low_watermark;
+
p_adapter->params.xfer_block_size = (sizeof(eth_hdr_t) +
p_adapter->params.payload_mtu);
NdisReadNetworkAddress( &status, &mac, &len, h_config );
@@ -2344,8 +2393,14 @@
/* Must cast here because the service name is an array of
unsigned chars but
* strcpy want a pointer to a signed char */
- strcpy( (char *)ib_service.svc_rec.service_name, ATS_NAME );
-
+ if ( StringCchCopy( (char *)ib_service.svc_rec.service_name,
+ sizeof(ib_service.svc_rec.service_name) / sizeof(char),
ATS_NAME ) != S_OK) {
+ ASSERT(FALSE);
+ IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR,IPOIB_DBG_ERROR,
+ ("Problem copying ATS name: exiting\n"));
+ return;
+ }
+
/* IP Address in question will be put in below */
ib_service.port_guid = p_adapter->guids.port_guid;
ib_service.timeout_ms = p_adapter->params.sa_timeout;
Index: ulp/ipoib/kernel/ipoib_log.mc
===================================================================
--- ulp/ipoib/kernel/ipoib_log.mc (revision 2629)
+++ ulp/ipoib/kernel/ipoib_log.mc (revision 2630)
@@ -283,3 +283,27 @@
Language=English
%2: The local port rate is too slow for the existing broadcast
MC group.
.
+
+MessageId=0x0058
+Facility=IPoIB
+Severity=Error
+SymbolicName=EVENT_IPOIB_WRONG_PARAMETER_ERR
+Language=English
+%2: Incorrect value or non-existing registry for the required
IPoIB parameter %3, overriding it by default value: %4
+.
+
+MessageId=0x0059
+Facility=IPoIB
+Severity=Warning
+SymbolicName=EVENT_IPOIB_WRONG_PARAMETER_WRN
+Language=English
+%2: Incorrect value or non-existing registry entry for the
required IPoIB parameter %3, overriding it by default value: %4
+.
+
+MessageId=0x005A
+Facility=IPoIB
+Severity=Informational
+SymbolicName=EVENT_IPOIB_WRONG_PARAMETER_INFO
+Language=English
+%2: Incorrect value or non-existing registry for the optional
IPoIB parameter %3, overriding it by default value: %4
+.
Index: ulp/ipoib/kernel/ipoib_driver.h
===================================================================
--- ulp/ipoib/kernel/ipoib_driver.h (revision 2629)
+++ ulp/ipoib/kernel/ipoib_driver.h (revision 2630)
@@ -126,4 +126,13 @@
ipoib_resume_oids(
IN ipoib_adapter_t* const p_adapter );
+#define IPOIB_OFFSET(field)
((UINT)FIELD_OFFSET(ipoib_params_t,field))
+#define IPOIB_SIZE(field)
sizeof(((ipoib_params_t*)0)->field)
+#define IPOIB_INIT_NDIS_STRING(str) \
+ (str)->Length = 0; \
+ (str)->MaximumLength = 0; \
+ (str)->Buffer = NULL;
+
+
+
#endif /* _IPOIB_DRIVER_H_ */
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20080708/70dcdee5/attachment.html>
More information about the ofw
mailing list