[Openib-windows] RE: A patch for listing the ip addresses and fetching remote gids .
Tzachi Dar
tzachid at mellanox.co.il
Sun Sep 18 12:54:55 PDT 2005
See bellow.
>-----Original Message-----
>From: Fab Tillier [mailto:ftillier at silverstorm.com]
>Sent: Friday, September 16, 2005 9:36 PM
>To: 'Tzachi Dar'; openib-windows at openib.org
>Subject: RE: A patch for listing the ip addresses and fetching remote gids.
>
>> From: Tzachi Dar [mailto:tzachid at mellanox.co.il]
>> Sent: Friday, September 16, 2005 1:09 AM
>>
>> Hi fab,
>>
>> Bellow is the patch that allows other components to communicate with
>ipoib,
>> and to get ip addresses as well as ports.
>
>This is great, thanks.
>
>A few nits about this patch:
>- Use tabs, not spaces.
>- The ip_addresses_shared.h file, by virtue of being in the ipoib directory
>and
>not the user or kernel subdirectory, is shared. No need to add the shared
>suffix to the filename.
>- Curly braces in the rest of the code base are on separate lines, and line
>up
>with one another vertically. While this is different than the Linux coding
>guidelines, we should stick to the existing model. It makes code much
>easier to
>read, especially for cases where you have multi-line conditional/loop
>statements
>by making it clear where the conditional/loop body begins and ends.
>- Lines should be <= 80 characters
>- For some reason, the PrintIp test case and its makefile and sources file
>were
>repeated in the patch you sent 3 times.
>- Try to avoid trailing white space.
>
>Also, I think the shared header should be named something like ib_at.h,
>since
>that's the functionality it provides
I agree on all the comment above.
>I will start merging some of these things into the code base, starting with
>simple functionality like:
>- Tracking adapters in a global list
>- Registering the unload handler
>
>> (the file is also attached).
>> Index: ulp/ipoib/ip_addresses_shared.h
>> ===================================================================
>> --- ulp/ipoib/ip_addresses_shared.h (revision 0)
>> +++ ulp/ipoib/ip_addresses_shared.h (revision 0)
>> @@ -0,0 +1,109 @@
>> +/*
>
><snip>
>
>> +// This file is shared between the user and kernel
>> +
>> +#ifndef _IP_ADDRESSES_SHARED_H_
>> +#define _IP_ADDRESSES_SHARED_H_
>> +
>> +#define IPOIB_IOCTL_VERSION 1
>> +
>> +#define HW_ADDR_LEN 6
>
>Only define HW_ADDR_LEN if it hasn't been defined yet. If you fear
>different
>sizes, then define it for use in the IOCTL definition only (change the name
>to
>something like AT_MAC_LEN).
>
Where is the HW_ADDR_LEN defined? If it is already defined to 6 we can keep
using it.
>> +
>> +
>> +// This IRP is used to return all available CAs ports number and port
>guid
>> +#define IOCTL_IPOIB_PORTS CTL_CODE(FILE_DEVICE_UNKNOWN, 0x801,
>> METHOD_BUFFERED ,FILE_ANY_ACCESS)
>> +
>> +struct IPOIB_AT_PORT_RECORD {
>> + ib_net64_t CaGuid;
>> + ib_net64_t PortGuid;
>> + ULONG PortNumber;
>> +};
>
>Should we just use native Windows types here (UINT64 rather than
>ib_net64_t)?
I believe that this is an InfiniBand field so we should probably stay with
that.
>
>> +
>> +struct IOCTL_IPOIB_PORTS_IN {
>> + ULONG Version;
>> + ULONG Size; //Total size,
>of
>> the output buffers
>> +};
>> +
>> +struct IOCTL_IPOIB_PORTS_OUT {
>> + ULONG Version;
>> + ULONG Size; //Total
>size,
>> of the output buffer needed if the
>> + // suplied buffer wasn't enough
>> + ULONG NumPorts;
>> + struct IPOIB_AT_PORT_RECORD Ports[1];
>> +};
>> +
>> +// This IRP is used to return all the ip addresses that are assigned to
>a
>> port
>> +#define IOCTL_IPOIB_IP_ADDRESSES CTL_CODE(FILE_DEVICE_UNKNOWN, 0x802,
>> METHOD_BUFFERED ,FILE_ANY_ACCESS)
>
>FILE_READ_ACCESS should be all that's needed, since the clients can only
>query
>information, and not set it. Or does that not work with IOCTLs?
That should probably work, I have copied this from some book, without giving
it too much attention.
>
>> +
>> +struct IOCTL_IPOIB_IP_ADDRESSES_IN {
>> + ULONG Version;
>> + ib_net64_t PortGuid; // The guid
>of
>> the port that we are queriyn for
>> +};
>> +
>> +struct IP_ADDRESS {
>> + char IpVersion; // might only be 4 or 6
>> + unsigned char Data[16]; // This allows to return ip v6
>as
>> well
>> +};
>> +
>> +struct IOCTL_IPOIB_IP_ADDRESSES_OUT {
>> + ULONG Version;
>> + ULONG Size; //Total
>size,
>> of the output buffer needed if the
>> + // suplied buffer wasn't enough
>> + ULONG NumIps;
>> + struct IP_ADDRESS Addreses[1];
>> +};
>> +
>> +
>> +// This IRP is used to convert a remote mac addresses to a remote GID
>> +#define IOCTL_IPOIB_MAC_2_GID CTL_CODE(FILE_DEVICE_UNKNOWN, 0x803,
>> METHOD_BUFFERED ,FILE_ANY_ACCESS)
>> +
>> +
>> +struct IOCTL_IPOIB_MAC_2_GID_IN {
>> + unsigned char DestMac[HW_ADDR_LEN];
>
>This should take as input the source port GUID. It is valid for different,
>disjoint subnets to have duplicate MAC addresses.
>
>This also optimizes the lookup by targeting a specific instance rather than
>searching all.
>
We can do it this way, still, for the efficiency, the caller will only have
the source IP, so he will have to look for that source IP in all adapters
any way.
>> +};
>> +
>> +struct IOCTL_IPOIB_MAC_2_GID_OUT {
>> + ib_gid_t DestGid;
>> +};
>> +
>> +// The following structure functions and structurs define the functions
>> that are needed in order to
>> +// hold an array of the ports that are currently available.
>> +
>> +#define Name L"IPOIB"
>> +
>> +#define IPOIB_DEV_NAME L"\\Device\\"Name
>> +#define IPOIB_DOS_DEV_NAME L"\\DosDevices\\Global\\"Name
>> +#define IPOIB_WIN32_NAME L"\\\\.\\"Name
>> +
>> +
>> +
>> +#endif /* _IP_ADDRESSES_SHARED_H_ */
>> Index: ulp/ipoib/ip_packet.h
>> ===================================================================
>> --- ulp/ipoib/ip_packet.h (revision 385)
>> +++ ulp/ipoib/ip_packet.h (working copy)
>> @@ -37,12 +37,6 @@
>> #include <complib/cl_types.h>
>> #include <complib/cl_byteswap.h>
>>
>> -
>> -#ifndef HW_ADDR_LEN
>> -#define HW_ADDR_LEN 6
>> -#endif /* HW_ADDR_LEN */
>> -
>> -
>
>This should be left here to allow other NDIS drivers to use this file
>without
>having to include the IPoIB address translation file - this is needed for
>IB to
>Ethernet gateways, for example. If the other definition is protected with
>a
>#ifndef, everything should be fine. Alternatively, the other definition
>should
>have its own name, so that it is consistent with the IOCTL buffer
>definition.
>
The IOCTL buffer definition can have it's own name.
>> #define ETH_PROT_TYPE_IP CL_HTON16(0x800)
>> #define ETH_PROT_TYPE_ARP CL_HTON16(0x806)
>>
>> Index: ulp/ipoib/kernel/ip_Addresses.c
>> ===================================================================
>> --- ulp/ipoib/kernel/ip_Addresses.c (revision 0)
>> +++ ulp/ipoib/kernel/ip_Addresses.c (revision 0)
>> @@ -0,0 +1,580 @@
>> +/*
>
><snip>
>
>> + */
>> +#include <iba/ib_types.h>
>> +#include "ip_addresses_shared.h"
>> +#include "ipoib_adapter.h"
>> +#include "ipoib_port.h"
>> +#include "ipoib_debug.h"
>> +#include "ip_addresses.h"
>> +#include "ipoib_endpoint.h"
>> +
>> +
>> +NTSTATUS
>> +__ipoib_dispatch(
>> + IN PDEVICE_OBJECT device_object,
>> + IN PIRP Irp
>> + );
>> +
>> +
>> +struct ipoib_addresses_config {
>> +
>> + PDEVICE_OBJECT device_object;
>> + NDIS_HANDLE ndis_device_handle;
>> + cl_list_t ports_list;
>
>Don't use cl_list - it performs allocations on the fly, which means you
>have to
>handle potential failures. Just use qlist here, and add a list item to the
>adapter object. If you want efficient lookups using the port GUID, use a
>quick
>map with the port GUID as key.
>
>The adapter object should remove itself from the list/map in its
>"destroying"
>handler or similar.
>
OK, I just wanted to have the change encapsulated in one place.
>> + cl_spinlock_t spin_lock; // To protect the list
>> +
>> +};
>> +
>> +struct ipoib_addresses_config g_ipoib_addresses_config;
>> +
>> +void __ipoib_lock_list()
>> +{
>> + cl_spinlock_acquire(&g_ipoib_addresses_config.spin_lock);
>> +}
>> +
>> +void __ipoib_unlock_list()
>> +{
>> + cl_spinlock_release(&g_ipoib_addresses_config.spin_lock);
>> +}
>
>Having wrapper functions to take and release a lock seems silly. Why not
>just
>make the call to the spinlock functions directly?
>
>Also, I would recommend using in stack queued spinlocks, as they supposedly
>perform better. For this, just call the native Windows calls.
>
As you can see, if one is using the wrapper functions he can replace the
lock in only one place... (I don't mind doing it your way).
>> +
>> +
>> +NDIS_STATUS __ipoib_ip_create_device(
>> + NDIS_HANDLE ndis_wrapper_handle
>> + );
>> +
>> +VOID __ipoib_dregister_device();
>> +
>> +NDIS_STATUS ipoib_get_ipoib_ports(
>> + struct IOCTL_IPOIB_PORTS_IN ports_in,
>> + struct IOCTL_IPOIB_PORTS_OUT *ports_out,
>> + IN ULONG OutputBufferLength,
>> + OUT ULONG *pOutputDataSize
>> + )
>> +{
>> + NDIS_STATUS status = NDIS_STATUS_SUCCESS;
>> + ULONG num_ports, i = 0;
>> + ULONG needed_size_bytes;
>> + cl_list_iterator_t it;
>> + ipoib_adapter_t *p_adapter;
>> + struct IPOIB_AT_PORT_RECORD *ports_records;
>> +
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> + ports_out->Version = IPOIB_IOCTL_VERSION;
>> + ports_out->Size = 0;
>> + if (ports_in.Version != IPOIB_IOCTL_VERSION) {
>> + IPOIB_TRACE( IPOIB_DBG_ERROR, ("unsupported version = 0x%x\n",
>> ports_in.Version ));
>> + return STATUS_INVALID_PARAMETER;
>> + }
>> + num_ports = cl_list_count(&g_ipoib_addresses_config.ports_list);
>> + if (num_ports == 0) {
>> + needed_size_bytes = sizeof (struct IOCTL_IPOIB_PORTS_OUT);
>> + } else {
>> + needed_size_bytes = sizeof (struct IOCTL_IPOIB_PORTS_OUT) +
>> + (num_ports - 1) * sizeof (struct IPOIB_AT_PORT_RECORD);
>> + }
>> + if (needed_size_bytes > OutputBufferLength) {
>> + // Not enough data for us to use, we should return only the
>> + // size
>
>Return the size, and as much data as will fit. This allows clients to
>query for
>the first address in the list without having to query for all.
At least for SDP I don't see a reason to handle only a part of the data. As
a result I didn't see any reason to write this code (and check it). If WSD
has a need for this information you can return this.
>
>> + ports_out->NumPorts = num_ports;
>> + ports_out->Size = needed_size_bytes;
>> + return NDIS_STATUS_SUCCESS;
>> + }
>> + *pOutputDataSize = needed_size_bytes;
>> + ports_out->NumPorts = num_ports;
>> + ports_records = ports_out->Ports;
>> +
>> + __ipoib_lock_list();
>
>Locking needs to either be moved up to protect the size checks so that we
>don't
>overflow here, or we need to perform boundary checks on the input buffer as
>we
>fill it here.
>
>I would prefer to see us perform the boundary checks here, and return a
>partial
>buffer if possible (along with the required size).
>
>> +
>> + for (it = cl_list_head(&g_ipoib_addresses_config.ports_list);
>> + it != cl_list_end(&g_ipoib_addresses_config.ports_list);
>> + it = cl_list_next(it), i++) {
>> + p_adapter = cl_list_obj(it);
>> +
>> + cl_obj_lock( &p_adapter->obj );
>> + ports_records[i].CaGuid = p_adapter->guids.ca_guid;
>> + ports_records[i].PortGuid = p_adapter->guids.port_guid;
>> + cl_obj_unlock( &p_adapter->obj );
>> +
>> + }
>> + __ipoib_unlock_list();
>> + CL_ASSERT(i == num_ports);
>
>This assert will be triggered if an adapter is enabled (or disabled) while
>a
>query is in progress, but before the lock was acquired. We don't want
>this.
>
We should probably take the lock first, and than the assert is fine.
Please note that calling num_ports = cl_list_count(..) without the lock is a
bug since the list might change, so the solution is probably to take the
lock earlier.
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return status;
>> +
>> +}
>> +
>> +
>> +NDIS_STATUS __ipoib_get_port_ips(
>> + struct IOCTL_IPOIB_IP_ADDRESSES_IN addresses_in,
>> + struct IOCTL_IPOIB_IP_ADDRESSES_OUT *addresses_out,
>> + IN ULONG OutputBufferLength,
>> + OUT ULONG *pOutputDataSize
>> + )
>> +{
>> + ULONG num_ips, i = 0;
>> + ULONG needed_size_bytes;
>> + cl_list_iterator_t it;
>> + ipoib_adapter_t *p_adapter;
>> + struct IP_ADDRESS *ip_records;
>> + net_address_item_t *p_addr_item;
>> +
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + addresses_out->Version = IPOIB_IOCTL_VERSION;
>> + addresses_out->Size = 0;
>> + if (addresses_in.Version != IPOIB_IOCTL_VERSION) {
>> + IPOIB_TRACE( IPOIB_DBG_ERROR, ("unsupported version = 0x%x\n",
>> addresses_in.Version ));
>> + return STATUS_INVALID_PARAMETER;
>> + }
>> + __ipoib_lock_list();
>> +
>> + for (it = cl_list_head(&g_ipoib_addresses_config.ports_list);
>> + it != cl_list_end(&g_ipoib_addresses_config.ports_list);
>> + it = cl_list_next(it)) {
>> + p_adapter = cl_list_obj(it);
>> +
>> + cl_obj_lock( &p_adapter->obj );
>> + if (p_adapter->guids.port_guid == addresses_in.PortGuid) {
>> + // We should return our answer based on our answer
>> + num_ips = cl_vector_get_size(&p_adapter->ip_vector);
>> + if (num_ips == 0) {
>> + needed_size_bytes = sizeof (struct
>> IOCTL_IPOIB_IP_ADDRESSES_OUT);
>> + } else {
>> + needed_size_bytes = sizeof (struct
>> IOCTL_IPOIB_IP_ADDRESSES_OUT) +
>> + (num_ips - 1) * sizeof (struct IP_ADDRESS);
>> + }
>> + if (needed_size_bytes > OutputBufferLength) {
>> + // Not enough data for us to use, we should return only
>the
>> + // size
>
>Return as much data as can fit (in integral IB_ADDRESS units, of course).
>
>> + addresses_out->NumIps = num_ips;
>> + addresses_out->Size = needed_size_bytes;
>> + cl_obj_unlock( &p_adapter->obj );
>> + __ipoib_unlock_list();
>> + return NDIS_STATUS_SUCCESS;
>
>Set status and break would be cleaner.
Agreed.
>
>> + }
>> + *pOutputDataSize = needed_size_bytes;
>> +
>> + addresses_out->NumIps = num_ips;
>> + ip_records = addresses_out->Addreses;
>> +
>> + // Copy all the IP's into the vector
>> + for (i = 0 ; i < num_ips; i++) {
>> +
>> + ip_records[i].IpVersion = 4;
>> + p_addr_item = (net_address_item_t*)
>> + cl_vector_get_ptr(
>> &p_adapter->ip_vector, i );
>> + ip_records[i].Data[12] = p_addr_item-
>>address.as_bytes[0];
>> + ip_records[i].Data[13] = p_addr_item-
>>address.as_bytes[1];
>> + ip_records[i].Data[14] = p_addr_item-
>>address.as_bytes[2];
>> + ip_records[i].Data[15] = p_addr_item-
>>address.as_bytes[3];
>
>The first 12 bytes should be cleared (matches how IPv4 addresses are
>specified
>in the SDP hello message, as well as ATS service registrations).
Agreed.
>
>> + }
>> +
>> + cl_obj_unlock( &p_adapter->obj );
>> +
>> + __ipoib_unlock_list();
>> + return NDIS_STATUS_SUCCESS;
>> +
>> + }
>> + cl_obj_unlock( &p_adapter->obj );
>
>It would be cleaner to release the lock, set the return value, and break
>out of
>the for loop.
>
Agreed
>> +
>> +
>> + }
>> + // If we have reached here, this means that the port was not found
>> + __ipoib_unlock_list();
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return STATUS_INVALID_PORT_HANDLE;
>> +
>> +}
>> +
>> +
>> +NDIS_STATUS __ipoib_get_gid(
>> + struct IOCTL_IPOIB_MAC_2_GID_IN mac_in,
>> + struct IOCTL_IPOIB_MAC_2_GID_OUT *gid_out,
>> + OUT ULONG *pOutputDataSize
>> + )
>> +{
>> + cl_list_iterator_t it;
>> + ipoib_adapter_t *p_adapter;
>> + NDIS_STATUS status;
>> + mac_addr_t mac;
>> +
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + *pOutputDataSize = sizeof (gid_out->DestGid);
>> +
>> + // Go over all addapters and try to find the correct endpoint
>> +
>> + memcpy(mac.addr, mac_in.DestMac, HW_ADDR_LEN);
>> +
>> + __ipoib_lock_list();
>> +
>> + for (it = cl_list_head(&g_ipoib_addresses_config.ports_list);
>> + it != cl_list_end(&g_ipoib_addresses_config.ports_list);
>> + it = cl_list_next(it)) {
>> + p_adapter = cl_list_obj(it);
>> +
>> + cl_obj_lock( &p_adapter->obj );
>> +
>> + status = gid_from_mac( p_adapter->p_port, mac, &gid_out->DestGid
>);
>> +
>> + if (status == NDIS_STATUS_SUCCESS) {
>> + cl_obj_unlock( &p_adapter->obj );
>> + __ipoib_unlock_list();
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return status;
>> + }
>> +
>> + cl_obj_unlock( &p_adapter->obj );
>> +
>> +
>> + }
>> + __ipoib_unlock_list();
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return NDIS_STATUS_NO_ROUTE_TO_DESTINATION;
>> +
>> +}
>> +
>> +#define VERIFY_BUFFERS(InputBufferLength, OutputBufferLength,
>InStructSize,
>> OutStructZize) \
>> +if ((InputBufferLength < InStructSize) ||
>> \
>> + (OutputBufferLength < OutStructZize)) {
>> \
>> + IPOIB_TRACE( IPOIB_DBG_ERROR,
>> \
>> + ("ipoib_dispatch_device_io_control bad buffer sizes\n" ));
>> \
>> + CL_ASSERT(FALSE);
>> \
>> + status = STATUS_ACCESS_VIOLATION;
>> \
>> + goto Cleanup;
>> \
>> +}
>
>Yuk. I hate macros, especially with an embedded goto. It completely
>obscures
>the code flow through the function, making the code harder to read. Make
>the
>checks explicit - it won't hurt readability.
>
Agreed.
>> +
>> +NDIS_STATUS __ipoib_dispatch_device_io_control(
>> + IN PVOID pInputBuffer,
>> + IN ULONG InputBufferLength ,
>> + IN PVOID pOutputBuffer,
>> + IN ULONG OutputBufferLength,
>> + IN ULONG IoControlCode,
>> + OUT ULONG *pOutputDataSize
>> + )
>> +{
>> + NDIS_STATUS status = NDIS_STATUS_SUCCESS;
>> + struct IOCTL_IPOIB_PORTS_IN ports_in;
>> + struct IOCTL_IPOIB_PORTS_OUT *ports_out;
>> + struct IOCTL_IPOIB_IP_ADDRESSES_IN addreses_in;
>> + struct IOCTL_IPOIB_IP_ADDRESSES_OUT *addreses_out;
>> + struct IOCTL_IPOIB_MAC_2_GID_IN ip_in;
>> + struct IOCTL_IPOIB_MAC_2_GID_OUT *ip_out;
>
>You should try to avoid making copies of the input parameters. I think
>simply
>by restructuring the handlers to not touch the output buffer first, you
>could
>eliminate all these. If you do want to keep them, you should declare all
>input
>parameters in a union, likewise for all output parameters.
>
I believe that the price of copying the input parameters is small enough
compared to the troubles that will happen if one touches the buffers after
starting to fill the output.
>> +
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + switch (IoControlCode) {
>> + case IOCTL_IPOIB_PORTS :
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL, ("IOCTL_IPOIB_PORTS
>recieved\n"
>> ));
>> + VERIFY_BUFFERS(InputBufferLength, OutputBufferLength, sizeof
>> (struct IOCTL_IPOIB_PORTS_IN), sizeof (struct IOCTL_IPOIB_PORTS_OUT));
>> +
>> +
>> + ports_in = *(struct IOCTL_IPOIB_PORTS_IN *) pInputBuffer;
>> + ports_out = (struct IOCTL_IPOIB_PORTS_OUT *) pOutputBuffer;
>> +
>> + status = ipoib_get_ipoib_ports(
>> + ports_in,
>> + ports_out,
>> + OutputBufferLength,
>> + pOutputDataSize
>> + );
>> + }
>> + break;
>> + case IOCTL_IPOIB_IP_ADDRESSES :
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL, ("IOCTL_IPOIB_IP_ADDRESSES
>> recieved\n" ));
>> + VERIFY_BUFFERS(InputBufferLength, OutputBufferLength, sizeof
>> (struct IOCTL_IPOIB_IP_ADDRESSES_IN), sizeof (struct
>> IOCTL_IPOIB_IP_ADDRESSES_OUT));
>> +
>> + addreses_in = *(struct IOCTL_IPOIB_IP_ADDRESSES_IN *)
>> pInputBuffer;
>> + addreses_out = (struct IOCTL_IPOIB_IP_ADDRESSES_OUT *)
>> pOutputBuffer;
>> +
>> + status = __ipoib_get_port_ips(
>> + addreses_in,
>> + addreses_out,
>> + OutputBufferLength,
>> + pOutputDataSize
>> + );
>> + }
>> + break;
>> +
>> + case IOCTL_IPOIB_MAC_2_GID :
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL, ("IOCTL_IPOIB_MAC_2_GID
>> recieved\n" ));
>> + VERIFY_BUFFERS(InputBufferLength, OutputBufferLength, sizeof
>> (struct IOCTL_IPOIB_MAC_2_GID_IN), sizeof (struct
>> IOCTL_IPOIB_MAC_2_GID_OUT));
>> +
>> + ip_in = *(struct IOCTL_IPOIB_MAC_2_GID_IN *) pInputBuffer;
>> + ip_out = (struct IOCTL_IPOIB_MAC_2_GID_OUT *) pOutputBuffer;
>> +
>> + status = __ipoib_get_gid(
>> + ip_in,
>> + ip_out,
>> + pOutputDataSize
>> + );
>> + }
>> + break;
>> +
>> +
>> +
>> +
>> + default:
>> + // This is an unrecgnized IOCTL
>> + CL_ASSERT(FALSE);
>> + IPOIB_TRACE( IPOIB_DBG_ERROR, ("unknow IOCTL code = 0x%x\n",
>> IoControlCode ));
>> + status = STATUS_INVALID_PARAMETER;
>> + }
>> +Cleanup:
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return status;
>> +
>> +}
>> +
>> +NDIS_STATUS ipoib_driver_up(NDIS_HANDLE ndis_wrapper_handle)
>> +{
>> + NDIS_STATUS status;
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> + status = cl_list_init(&g_ipoib_addresses_config.ports_list, 8);
>> + if( status != CL_SUCCESS )
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_ERROR,
>> + ("cl_list_init failed with status of
>> %d\n", status) );
>> + return status;
>> + }
>> +
>> + status = __ipoib_ip_create_device(ndis_wrapper_handle);
>> + if( status != NDIS_STATUS_SUCCESS )
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_ERROR,
>> + ("NdisMRegisterDevice failed with
>status
>> of %d\n", status) );
>> + cl_list_destroy(&g_ipoib_addresses_config.ports_list);
>> + return status;
>> + }
>> +
>> + status = cl_spinlock_init(&g_ipoib_addresses_config.spin_lock);
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return status;
>> +
>> +
>> +}
>> +
>> +void ipoib_driver_unload()
>> +{
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> + cl_list_destroy(&g_ipoib_addresses_config.ports_list);
>> + cl_spinlock_destroy(&g_ipoib_addresses_config.spin_lock);
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> +
>> +}
>
>I'll move this into the same file as DriverEntry - it makes more sense
>there.
>
OK.
>> +
>> +NDIS_STATUS __ipoib_ip_create_device(
>> + NDIS_HANDLE ndis_wrapper_handle
>> + )
>> +{
>> + NDIS_STATUS status;
>> + NDIS_STRING DeviceName;
>> + NDIS_STRING DeviceLinkUnicodeString;
>> + PDRIVER_DISPATCH DispatchTable[IRP_MJ_MAXIMUM_FUNCTION+1];
>> +
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + NdisInitUnicodeString(&DeviceName, IPOIB_DEV_NAME);
>> + NdisInitUnicodeString(&DeviceLinkUnicodeString, IPOIB_DOS_DEV_NAME);
>> +
>> + NdisZeroMemory(DispatchTable, (IRP_MJ_MAXIMUM_FUNCTION+1) *
>> sizeof(PDRIVER_DISPATCH));
>> +
>> + DispatchTable[IRP_MJ_CREATE] = __ipoib_dispatch;
>> + DispatchTable[IRP_MJ_CLEANUP] = __ipoib_dispatch;
>> + DispatchTable[IRP_MJ_CLOSE] = __ipoib_dispatch;
>> + DispatchTable[IRP_MJ_DEVICE_CONTROL] = __ipoib_dispatch;
>> + DispatchTable[IRP_MJ_INTERNAL_DEVICE_CONTROL] = __ipoib_dispatch;
>> +
>> + status = NdisMRegisterDevice(
>> + ndis_wrapper_handle,
>> + &DeviceName,
>> + &DeviceLinkUnicodeString,
>> + &DispatchTable[0],
>> + &g_ipoib_addresses_config.device_object,
>> + &g_ipoib_addresses_config.ndis_device_handle
>> + );
>> + if( status != NDIS_STATUS_SUCCESS )
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_ERROR,
>> + ("NdisMRegisterDevice failed with
>status
>> of %d\n", status) );
>> + } else {
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL,
>> + ("NdisMRegisterDevice succeeded\n") );
>> + }
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return status;
>> +
>> +
>> +}
>> +
>> +NDIS_STATUS ipoib_add_adapter(ipoib_adapter_t *p_adapter)
>> +{
>> + NDIS_STATUS status;
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + __ipoib_lock_list();
>> + status = cl_list_insert_head(&g_ipoib_addresses_config.ports_list,
>> p_adapter);
>> + __ipoib_unlock_list();
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> + return status;
>> +}
>> +
>> +
>> +VOID ipoib_remove_adapter(ipoib_adapter_t *p_adapter)
>> +{
>> + cl_status_t status;
>> + ULONG count;
>> +
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + __ipoib_lock_list();
>> + status = cl_list_remove_object(&g_ipoib_addresses_config.ports_list,
>> p_adapter);
>> + CL_ASSERT(status == CL_SUCCESS);
>> +
>> + count = cl_list_count(&g_ipoib_addresses_config.ports_list);
>> + __ipoib_unlock_list();
>> +
>> + if (count == 0 ) {
>> + __ipoib_dregister_device();
>> + }
>> +
>> +
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> +
>> +}
>> +
>> +VOID __ipoib_dregister_device()
>> +{
>> + NDIS_STATUS status;
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + if (g_ipoib_addresses_config.ndis_device_handle != NULL) {
>> + status =
>> NdisMDeregisterDevice(g_ipoib_addresses_config.ndis_device_handle);
>
>What happens if you disable the last IPoIB instance while an application
>has the
>associated file object open? The docs say the function will fail. If it
>does,
>the code should check the count when it receives a IRP_MJ_CLOSE, and try
>again
>if it's zero.
>
>> + if( status != NDIS_STATUS_SUCCESS )
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_ERROR,
>> + ("NdisMDeregisterDevice failed with
>> status of %d", status) );
>> + } else {
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL,
>> + ("NdisMDeregisterDevice succeeded\n")
>);
>> + }
>> +
>> + g_ipoib_addresses_config.ndis_device_handle = NULL;
>> + }
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> +}
>> +
>> +
>> +NTSTATUS
>> +__ipoib_dispatch(
>> + IN PDEVICE_OBJECT device_object,
>> + IN PIRP pIrp
>> + )
>> +/*++
>> +Routine Description:
>> +
>> + Process IRPs sent to this device.
>> +
>> +Arguments:
>> +
>> + device_object - pointer to a device object
>> + Irp - pointer to an I/O Request Packet
>> +
>> +Return Value:
>> +
>> + NTSTATUS - STATUS_SUCCESS always - change this when adding
>> + real code to handle ioctls.
>> +
>> +--*/
>
>Documenation before the function like the rest of the code base, please. I
>know
>the Windows headers do it this way, but it becomes unreadable if the
>documentation gets of any significant length (you can't see what the
>function
>prototype is without scrolling up.
>
>> +{
>> + PIO_STACK_LOCATION pIrpSp;
>> + NTSTATUS status = STATUS_SUCCESS;
>> + int Method;
>> + ULONG OutputDataSize = 0;
>> +
>> + UNREFERENCED_PARAMETER(device_object);
>> +
>> + IPOIB_ENTER(IPOIB_DBG_IOCTL);
>> +
>> + pIrpSp = IoGetCurrentIrpStackLocation(pIrp);
>> +
>> +
>> + switch (pIrpSp->MajorFunction)
>> + {
>> + case IRP_MJ_CREATE:
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL, ("IRP_MJ_CREATE recieved\n")
>);
>> + break;
>> +
>> + case IRP_MJ_CLEANUP:
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL, ("IRP_MJ_CLEANUP recieved\n")
>);
>>
>> + break;
>> +
>> + case IRP_MJ_CLOSE:
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL, ("IRP_MJ_CLOSE recieved\n") );
>>
>> + break;
>> +
>> + case IRP_MJ_DEVICE_CONTROL:
>> + IPOIB_TRACE( IPOIB_DBG_IOCTL, ("IRP_MJ_DEVICE_CONTROL
>> recieved\n") );
>> + Method = pIrpSp->Parameters.DeviceIoControl.IoControlCode &
>> 0x3;
>> + if (Method != METHOD_BUFFERED) {
>> + // We only support method buffered (very small requests)
>> + status = STATUS_INVALID_PARAMETER;
>> + IPOIB_TRACE( IPOIB_DBG_ERROR,
>> + ("Unsupporteed ioctl Method %d\n",
>> Method) );
>> + } else {
>> + status = __ipoib_dispatch_device_io_control(
>> + pIrp->AssociatedIrp.SystemBuffer,
>> + pIrpSp-
>>Parameters.DeviceIoControl.InputBufferLength,
>> + pIrp->AssociatedIrp.SystemBuffer,
>> + pIrpSp-
>>Parameters.DeviceIoControl.OutputBufferLength,
>> + pIrpSp->Parameters.DeviceIoControl.IoControlCode,
>> + &OutputDataSize
>> + );
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> + CL_ASSERT(status != STATUS_PENDING);
>> +
>> + pIrp->IoStatus.Status = status;
>> + pIrp->IoStatus.Information = OutputDataSize;
>> + IoCompleteRequest(pIrp, IO_NO_INCREMENT);
>> +
>> + IPOIB_EXIT(IPOIB_DBG_IOCTL);
>> +
>> + return status;
>> +
>> +}
>> +
>> Index: ulp/ipoib/kernel/ip_Addresses.h
>> ===================================================================
>> --- ulp/ipoib/kernel/ip_Addresses.h (revision 0)
>> +++ ulp/ipoib/kernel/ip_Addresses.h (revision 0)
>> @@ -0,0 +1,50 @@
>> +/*
>> + * Copyright (c) 2005 Mellanox Technologies. All rights reserved.
>> + *
>> + * This software is available to you under the OpenIB.org BSD license
>> + * below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + * - Redistributions of source code must retain the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer.
>> + *
>> + * - Redistributions in binary form must reproduce the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer in the documentation and/or other materials
>> + * provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + *
>> + * $Id: ip_packet.h 47 2005-05-30 18:00:40Z sleybo $
>> + */
>> +
>> +#ifndef _IP_ADDRESSES_H_
>> +#define _IP_ADDRESSES_H_
>> +
>> +#include <ip_addresses_shared.h>
>> +
>> +NDIS_STATUS ipoib_driver_up(
>> + NDIS_HANDLE ndis_wrapper_handle
>> + );
>> +
>> +void ipoib_driver_unload();
>> +
>> +//VOID dregister_device();
>> +
>> +NDIS_STATUS ipoib_add_adapter(ipoib_adapter_t *p_adapter);
>> +
>> +VOID ipoib_remove_adapter(ipoib_adapter_t *p_adapter);
>> +
>> +
>> +#endif /* _IP_ADDRESSES_H_ */
>> Index: ulp/ipoib/kernel/ipoib_adapter.c
>> ===================================================================
>> --- ulp/ipoib/kernel/ipoib_adapter.c (revision 385)
>> +++ ulp/ipoib/kernel/ipoib_adapter.c (working copy)
>> @@ -30,14 +30,15 @@
>> */
>>
>>
>> -
>> +#include <iba/ib_types.h>
>> +#include "ip_addresses_shared.h"
>> #include "ipoib_adapter.h"
>> #include "ipoib_port.h"
>> #include "ipoib_driver.h"
>> #include "ipoib_debug.h"
>> #include <complib/cl_init.h>
>> +#include <ip_addresses.h>
>>
>> -
>> #define ITEM_POOL_START 16
>> #define ITEM_POOL_GROW 16
>>
>> @@ -207,6 +208,16 @@
>> return status;
>> }
>>
>> + status = ipoib_add_adapter(p_adapter);
>> + if( status != IB_SUCCESS )
>> + {
>> + cl_obj_destroy( &p_adapter->obj ); // ??? is this
>the
>> correct way to stop
>
>Yes, that's the correct way to destroy the adapter. I don't think that's
>what
>you want to do, though - why disable IPoIB all together just because
>address
>translation won't work? Also, using quick list allows you to do this
>without
>having to check for success or failure.
I believe that if a driver is not supplying important services it shouldn't
go up. Currently (until we write to the event log) this is the only place
that we have to report errors.
>
>> + IPOIB_TRACE_EXIT( IPOIB_DBG_ERROR,
>> + ("adapter_init returned %s.\n",
>> + p_adapter->p_ifc->get_err_str( status
>))
>> );
>> + return status;
>> + }
>> +
>> *pp_adapter = p_adapter;
>>
>> IPOIB_EXIT( IPOIB_DBG_INIT );
>> @@ -235,6 +246,8 @@
>> {
>> IPOIB_ENTER( IPOIB_DBG_INIT );
>>
>> + ipoib_remove_adapter(p_adapter);
>> +
>> CL_ASSERT( p_adapter );
>>
>> /*
>> Index: ulp/ipoib/kernel/ipoib_debug.h
>> ===================================================================
>> --- ulp/ipoib/kernel/ipoib_debug.h (revision 385)
>> +++ ulp/ipoib/kernel/ipoib_debug.h (working copy)
>> @@ -59,7 +59,9 @@
>> #define IPOIB_DBG_MCAST (1 << 7)
>> #define IPOIB_DBG_ALLOC (1 << 8)
>> #define IPOIB_DBG_OID (1 << 9)
>> +#define IPOIB_DBG_IOCTL (1 << 10)
>>
>> +
>> #define IPOIB_DBG_FUNC (1 << 28) /* For function
>entry/exit
>> */
>> #define IPOIB_DBG_INFO (1 << 29) /* For verbose
>information
>> */
>> #define IPOIB_DBG_WARN (1 << 30) /* For warnings. */
>> Index: ulp/ipoib/kernel/ipoib_driver.c
>> ===================================================================
>> --- ulp/ipoib/kernel/ipoib_driver.c (revision 385)
>> +++ ulp/ipoib/kernel/ipoib_driver.c (working copy)
>> @@ -29,13 +29,15 @@
>> * $Id$
>> */
>>
>> -
>> +#include <iba/ib_types.h>
>> +#include "ip_addresses_shared.h"
>> #include "ipoib_driver.h"
>> #include "ipoib_debug.h"
>> #include "ipoib_port.h"
>> #include <complib/cl_bus_ifc.h>
>> #include <initguid.h>
>> #include <iba/ipoib_ifc.h>
>> +#include <ip_addresses.h>
>>
>>
>> /* Default parameters values for optional parameters */
>> @@ -119,7 +121,7 @@
>>
>>
>> /* Global driver debug level */
>> -uint32_t g_ipoib_dbg_lvl = IPOIB_DBG_ERROR;
>> +uint32_t g_ipoib_dbg_lvl = IPOIB_DBG_ERROR | IPOIB_DBG_IOCTL |
>> IPOIB_DBG_INIT |
>> IPOIB_DBG_FUNC;
>
>This should always be just IPOIB_DBG_ERROR for the code checked into SVN.
>
>> atomic32_t g_laa_idx = 0;
>>
>>
>> @@ -219,6 +221,15 @@
>> IN
>ipoib_adapter_t*
>> const p_adapter );
>>
>>
>> +
>> +VOID DriverUnload (
>> + IN PDRIVER_OBJECT pDriverObject
>> + )
>> +{
>> + IPOIB_ENTER( IPOIB_DBG_INIT );
>> + pDriverObject = NULL;
>> + ipoib_driver_unload();
>> +}
>> //! Standard Windows Device Driver Entry Point
>> /*! DriverEntry is the first routine called after a driver is loaded,
>and
>> is responsible for initializing the driver. On W2k this occurs when the
>> PnP
>> @@ -242,6 +253,8 @@
>>
>> IPOIB_ENTER( IPOIB_DBG_INIT );
>>
>> + p_drv_obj->DriverUnload = DriverUnload;
>> +
>> #ifdef _DEBUG_
>> PAGED_CODE();
>> #endif
>> @@ -250,7 +263,6 @@
>> ndis_handle = NULL;
>>
>> NdisMInitializeWrapper( &ndis_handle, p_drv_obj,
>> p_registry_path, NULL );
>> -
>> memset(&characteristics, 0, sizeof(characteristics));
>> characteristics.MajorNdisVersion =
>> MAJOR_NDIS_VERSION;
>> characteristics.MinorNdisVersion =
>> MINOR_NDIS_VERSION;
>> @@ -273,11 +285,26 @@
>> ndis_handle, &characteristics,
>> sizeof(characteristics) );
>> if( status != NDIS_STATUS_SUCCESS )
>> {
>> +
>> IPOIB_TRACE( IPOIB_DBG_ERROR,
>> - ("NdisMRegisterMiniport failed with
>> status of %s", status) );
>> + ("NdisMRegisterMiniport failed with
>> status of %d\n", status) );
>> NdisTerminateWrapper( ndis_handle, NULL );
>> + return status;
>> + }
>> +
>> +
>> + status = ipoib_driver_up(ndis_handle);
>> + if( status != NDIS_STATUS_SUCCESS )
>> + {
>> + IPOIB_TRACE( IPOIB_DBG_ERROR,
>> + ("ipoib_driver_up failed with status
>of
>> %d\n", status) );
>> + //??? should we "undo the" NdisMRegisterMiniport ?
>> + NdisTerminateWrapper( ndis_handle, NULL );
>> + return status;
>
>Don't fail the driver loading just because address translation won't work.
>We
>can still provide full connectivity via IPoIB even if SDP/WSD don't work.
>
>> }
>>
>> + NdisMRegisterUnloadHandler(ndis_handle, DriverUnload);
>> +
>> IPOIB_EXIT( IPOIB_DBG_INIT );
>> return status;
>> }
>> Index: ulp/ipoib/kernel/ipoib_endpoint.c
>> ===================================================================
>> --- ulp/ipoib/kernel/ipoib_endpoint.c (revision 385)
>> +++ ulp/ipoib/kernel/ipoib_endpoint.c (working copy)
>> @@ -30,7 +30,8 @@
>> */
>>
>>
>> -
>> +#include <iba/ib_types.h>
>> +#include "ip_addresses_shared.h"
>> #include "ipoib_endpoint.h"
>> #include "ipoib_port.h"
>> #include "ipoib_debug.h"
>
>Why was this change needed? Is it because of moving the HW_ADDR_LEN
>definition?
>
>> Index: ulp/ipoib/kernel/ipoib_port.c
>> ===================================================================
>> --- ulp/ipoib/kernel/ipoib_port.c (revision 385)
>> +++ ulp/ipoib/kernel/ipoib_port.c (working copy)
>> @@ -30,7 +30,8 @@
>> */
>>
>>
>> -
>> +#include <iba/ib_types.h>
>> +#include "ip_addresses_shared.h"
>> #include "ipoib_port.h"
>> #include "ipoib_adapter.h"
>> #include "ipoib_debug.h"
>> @@ -3856,8 +3857,39 @@
>> IPOIB_EXIT( IPOIB_DBG_ENDPT );
>> }
>>
>> +NDIS_STATUS
>> +gid_from_mac(
>> + IN ipoib_port_t*
>> const p_port,
>> + IN const mac_addr_t
>> mac,
>> + OUT ib_gid_t*
>> p_dest_gid )
>> +{
>> + ipoib_endpt_t* p_endpt;
>> + cl_map_item_t *p_item;
>> + uint64_t key = 0;
>> + cl_memcpy( &key, &mac, sizeof(mac_addr_t) );
>>
>> -static inline NDIS_STATUS
>> + IPOIB_ENTER( IPOIB_DBG_ENDPT );
>> +
>> + cl_obj_lock( &p_port->obj );
>> +
>> + p_item = cl_qmap_get( &p_port->endpt_mgr.mac_endpts, key );
>> + if( p_item == cl_qmap_end( &p_port->endpt_mgr.mac_endpts ) )
>> + {
>> + cl_obj_unlock( &p_port->obj );
>> + IPOIB_TRACE_EXIT( IPOIB_DBG_ENDPT, ("Failed
>endpoint
>> lookup.\n") );
>> + return NDIS_STATUS_NO_ROUTE_TO_DESTINATION;
>> + }
>> +
>> + p_endpt = PARENT_STRUCT( p_item, ipoib_endpt_t, mac_item );
>> + *p_dest_gid = p_endpt->dgid;
>> +
>> + cl_obj_unlock( &p_port->obj );
>> +
>> + IPOIB_EXIT( IPOIB_DBG_ENDPT );
>> + return NDIS_STATUS_SUCCESS;
>> +}
>> +
>> +NDIS_STATUS
>> __endpt_mgr_ref(
>> IN ipoib_port_t*
>> const p_port,
>> IN const mac_addr_t
>> mac,
>
>Why is this no longer static inline?
>
>> @@ -3872,6 +3904,7 @@
>> IPOIB_ENTER( IPOIB_DBG_ENDPT );
>>
>> key = 0;
>> + *pp_endpt = NULL;
>> cl_memcpy( &key, &mac, sizeof(mac_addr_t) );
>>
>> cl_obj_lock( &p_port->obj );
>
>This function should actually be changed to not touch pp_endpt in case of
>non-STATUS_SUCCESS.
I believe that it is always safer to start the function by zeroing the
output parameters.
>
>> Index: ulp/ipoib/kernel/ipoib_port.h
>> ===================================================================
>> --- ulp/ipoib/kernel/ipoib_port.h (revision 385)
>> +++ ulp/ipoib/kernel/ipoib_port.h (working copy)
>> @@ -584,5 +584,10 @@
>> ipoib_port_resume(
>> IN ipoib_port_t*
>> const p_port );
>>
>> +NDIS_STATUS
>> +gid_from_mac(
>> + IN ipoib_port_t*
>> const p_port,
>> + IN const mac_addr_t
>> mac,
>> + OUT ib_gid_t*
>> p_dest_gid );
>
>All non-static functions in the driver start with 'ipoib_'. This one
>should
>follow suit.
Agreed.
>
>>
>> #endif /* _IPOIB_PORT_H_ */
>> Index: ulp/ipoib/kernel/SOURCES
>> ===================================================================
>> --- ulp/ipoib/kernel/SOURCES (revision 385)
>> +++ ulp/ipoib/kernel/SOURCES (working copy)
>> @@ -6,7 +6,8 @@
>> ipoib_driver.c \
>> ipoib_adapter.c \
>> ipoib_endpoint.c \
>> - ipoib_port.c
>> + ipoib_port.c \
>> + ..\ip_Addresses.c
>
>The path for this file is wrong. It is a kernel only file and should be in
>the
>local directory. The patch file indicates that the file is indeed in the
>correct directory, so maybe it got picked up by the build.
>
You are of course right, I'm surprised that it was built any way.
>>
>> INCLUDES=..;..\..\..\inc;..\..\..\inc\kernel;
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20050918/77570494/attachment.html>
More information about the ofw
mailing list