From e2bc472726c765a36dec0dae92980bc3860241b9 Mon Sep 17 00:00:00 2001 From: tzachid Date: Tue, 8 Jul 2008 14:34:09 +0000 Subject: [PATCH] [ipoib] Fix for improper handling of IPoIB params signed by:xalex git-svn-id: svn://openib.tc.cornell.edu/gen1@1341 ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86 --- trunk/ulp/ipoib/kernel/SOURCES | 6 +- trunk/ulp/ipoib/kernel/ipoib_driver.c | 277 +++++++++++++++----------- trunk/ulp/ipoib/kernel/ipoib_driver.h | 9 + trunk/ulp/ipoib/kernel/ipoib_log.mc | 24 +++ 4 files changed, 203 insertions(+), 113 deletions(-) diff --git a/trunk/ulp/ipoib/kernel/SOURCES b/trunk/ulp/ipoib/kernel/SOURCES index d98b10fd..b85a51ba 100644 --- a/trunk/ulp/ipoib/kernel/SOURCES +++ b/trunk/ulp/ipoib/kernel/SOURCES @@ -25,14 +25,16 @@ C_DEFINES=$(C_DEFINES) -DNDIS_MINIPORT_DRIVER -DNDIS_WDM=1 \ 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 diff --git a/trunk/ulp/ipoib/kernel/ipoib_driver.c b/trunk/ulp/ipoib/kernel/ipoib_driver.c index 48dacbe6..9d7f75f8 100644 --- a/trunk/ulp/ipoib/kernel/ipoib_driver.c +++ b/trunk/ulp/ipoib/kernel/ipoib_driver.c @@ -30,7 +30,7 @@ * $Id$ */ - +#include "limits.h" #include "ipoib_driver.h" #include "ipoib_debug.h" @@ -47,6 +47,10 @@ #include #include #include +#include "ntstrsafe.h" +#include "strsafe.h" + + #if defined(NDIS50_MINIPORT) @@ -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,6 +131,71 @@ uint32_t g_ipoib_dbg_level = TRACE_LEVEL_ERROR; 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( @@ -248,6 +319,7 @@ DriverEntry( NDIS_MINIPORT_CHARACTERISTICS characteristics; IPOIB_ENTER( IPOIB_DBG_INIT ); + g_p_drv_obj = p_drv_obj; #ifdef _DEBUG_ PAGED_CODE(); @@ -397,6 +469,7 @@ ipoib_unload( } + NDIS_STATUS ipoib_get_adapter_params( IN NDIS_HANDLE* const wrapper_config_context, @@ -405,9 +478,13 @@ ipoib_get_adapter_params( 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 @@ ipoib_get_adapter_params( 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; + + // 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); + + // 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)); - /* 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; - } + } + 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 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; + 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: 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); + } + // + // Store the value in the adapter structure. + // + switch(pRegEntry[i].FieldSize) + { + case 1: + *((PUCHAR) structPointer) = (UCHAR) value; + break; - /* 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); + case 2: + *((PUSHORT) structPointer) = (USHORT) value; + break; - /* 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; + case 4: + *((PULONG) structPointer) = (ULONG) 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; + 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; - } - p_adapter->params.recv_pool_ratio = p_param->ParameterData.IntegerData; + // 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 )); - /* 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 ); @@ -2350,8 +2399,14 @@ ipoib_reg_addrs( /* 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.guid; ib_service.timeout_ms = p_adapter->params.sa_timeout; diff --git a/trunk/ulp/ipoib/kernel/ipoib_driver.h b/trunk/ulp/ipoib/kernel/ipoib_driver.h index 01291c65..b80cb5ac 100644 --- a/trunk/ulp/ipoib/kernel/ipoib_driver.h +++ b/trunk/ulp/ipoib/kernel/ipoib_driver.h @@ -126,4 +126,13 @@ void 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_ */ diff --git a/trunk/ulp/ipoib/kernel/ipoib_log.mc b/trunk/ulp/ipoib/kernel/ipoib_log.mc index 9676ff29..646035e0 100644 --- a/trunk/ulp/ipoib/kernel/ipoib_log.mc +++ b/trunk/ulp/ipoib/kernel/ipoib_log.mc @@ -283,3 +283,27 @@ SymbolicName=EVENT_IPOIB_BCAST_RATE 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 +. -- 2.46.0