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