<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD>
<META http-equiv=Content-Type content="text/html; charset=us-ascii">
<META content="MSHTML 6.00.2900.2802" name=GENERATOR></HEAD>
<BODY>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>Hi
Fab,</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>As part of the
virtualization support, there is also a need to support DHCP for the guest
OSes.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>During this day I
have learned the DHCP code that exists in IPOIB and I have made some changes to
it, so it now works.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>In general the old
code was based on replacing the client identifier with a string that contained
the GID + QP number. This combination promises uniqueness, as long as there was
one IP per QP. (this is also the process that is described in <A
href="http://www.ietf.org/internet-drafts/draft-ietf-ipoib-dhcp-over-infiniband-10.txt">http://www.ietf.org/internet-drafts/draft-ietf-ipoib-dhcp-over-infiniband-10.txt</A>)</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>With the new virtual
server, there are now more than one IP addresses per QP, so I had to do a
change: I have added the client identifier to be GID + QP + MAC addresses.
Assuming that the MAC addresses are unique at least in one physical machine,
this should work fine.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>The next point is
about what the receive side sees. In the current implementation, the sender
(actually windows) puts it's MAC (6 bytes) as it's identifier. Later IPOIB
changes that to GID+QP, and the receiver changes that to MAC again (* see
bellow). As there are no limitations to what the DHCP server can
receive, I believe that it is more correct to pass the entire string as is. (22
bytes goes to the server).</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006></SPAN></FONT><FONT
face=Arial size=2><SPAN class=470491416-08032006></SPAN></FONT><FONT face=Arial
size=2><SPAN class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>* Please note that
this implementation was wrong if there wasn't a match between the GUID and
the MAC as a new MAC was generated for each packet.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>One more issue in
the same area, is what happens if a packet comes and it's format is not
recognized by you. For example wrong magic number. Currently the code drops it,
and I believe that it would be better to pass the packet to windows, and hope
for the good (this will allow other clients to work with
us).</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>Please note that the
old code and the new one don't handle the case that the client identifier is not
what we expected. (for example some configuration in the
registry).</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>Last thing to note
is that in the current code, an IP packet is passed to us by windows, we
enlarge the client identifier but don't change the IP header size and UDP header
size, and later we decrease the packet, and not changing anything, so we are
fine. However I believe that we should fix the headers each
time.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>Attached is the
partial patch so you can look at it. Based on your comments I'll create the
complete patch so that you can submit it.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>I'll be also happy
to get comments to the last patch.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006>Thanks</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006>Tzachi</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN
class=470491416-08032006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=470491416-08032006>Index:
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_debug.h<BR>===================================================================<BR>---
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_debug.h (revision 226)<BR>+++
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_debug.h (working copy)<BR>@@
-60,7 +60,9 @@<BR> #define IPOIB_DBG_ALLOC (1 <<
8)<BR> #define IPOIB_DBG_OID (1 << 9)<BR> #define
IPOIB_DBG_IOCTL (1 << 10)<BR>+#define IPOIB_DBG_VM (1 <<
11)<BR> <BR>+<BR> #define IPOIB_DBG_FUNC (1 << 28) /*
For function entry/exit */<BR> #define IPOIB_DBG_INFO (1 <<
29) /* For verbose information */<BR> #define IPOIB_DBG_WARN (1
<< 30) /* For warnings. */<BR>Index:
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_port.c<BR>===================================================================<BR>---
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_port.c (revision 226)<BR>+++
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_port.c (working copy)<BR>@@
-237,6 +237,7 @@<BR> <BR> static
ib_api_status_t<BR> __recv_gen(<BR>+ IN const
ipoib_port_t*
const p_port,<BR> IN const ipoib_pkt_t*
const p_ipoib,<BR> OUT eth_pkt_t*
const p_eth,<BR> IN ipoib_endpt_t*
const p_src,<BR>@@ -461,7 +462,34 @@<BR> return cl_memcmp(
p_key1, p_key2, sizeof(ib_gid_t)
);<BR> }<BR> <BR>+/******************************************************************************<BR>+*<BR>+*
Virtual server ip to mac
translations<BR>+*<BR>+******************************************************************************/<BR>+static
void<BR>+__init_vs_ip_to_mac_translation(<BR>+ IN VS_ip_mac_manager
* p_manager);<BR> <BR>+static
void<BR>+__shutdown_vs_ip_to_mac_translation(<BR>+ IN VS_ip_mac_manager
* p_manager);<BR>+<BR>+static
ib_api_status_t<BR>+__get_mac_from_ip(<BR>+ IN const VS_ip_mac_manager
* const p_manager,<BR>+ IN net32_t dst_ip,<BR>+ OUT mac_addr_t
* p_dst_mac);<BR>+<BR>+static
ib_api_status_t<BR>+__put_mac_ip_pair(<BR>+ IN VS_ip_mac_manager
* p_manager,<BR>+ IN net32_t dst_ip,<BR>+ IN mac_addr_t dst_mac);<BR>+<BR>+<BR>+<BR>+<BR> /******************************************************************************<BR> *<BR> *
Implementation<BR>@@ -548,6 +576,8
@@<BR> <BR> __endpt_mgr_construct( p_port
);<BR> <BR>+ __init_vs_ip_to_mac_translation(
&p_port->vs_manager );<BR>+<BR> IPOIB_EXIT( IPOIB_DBG_INIT
);<BR> }<BR> <BR>@@ -702,6 +732,8
@@<BR> <BR> cl_obj_deinit( p_obj
);<BR> <BR>+ __shutdown_vs_ip_to_mac_translation(
&p_port->vs_manager );<BR>+<BR> cl_free( p_port
);<BR> <BR> IPOIB_EXIT( IPOIB_DBG_INIT );<BR>@@ -1780,7 +1812,7
@@<BR> continue;<BR> }<BR> <BR>- len
= p_wc->length - sizeof(ib_grh_t);<BR>+ len = p_wc->length -
sizeof(ib_grh_t); //????? Can there be a buffer overrun here
????<BR> <BR> if( len < sizeof(ipoib_hdr_t)
)<BR> {<BR>@@ -1838,7 +1870,7
@@<BR> {<BR> /*
Unfiltered. Setup the ethernet header and report.
*/<BR> cl_perf_start( RecvTcp
);<BR>- status = __recv_gen( p_ipoib, p_eth, p_src, p_dst
);<BR>+ status = __recv_gen( p_port, p_ipoib, p_eth,
p_src, p_dst );<BR> cl_perf_stop(
&p_port->p_adapter->perf, RecvTcp
);<BR> break;<BR> }<BR>@@
-1876,7 +1908,7
@@<BR> {<BR> /*
Unfiltered. Setup the ethernet header and report.
*/<BR> cl_perf_start( RecvUdp
);<BR>- status = __recv_gen( p_ipoib, p_eth, p_src, p_dst
);<BR>+ status = __recv_gen( p_port, p_ipoib, p_eth,
p_src, p_dst );<BR> cl_perf_stop(
&p_port->p_adapter->perf, RecvUdp
);<BR> }<BR> break;<BR>@@ -1898,7
+1930,7 @@<BR> default:<BR> /*
Unfiltered. Setup the ethernet header and report.
*/<BR> cl_perf_start( RecvGen
);<BR>- status = __recv_gen( p_ipoib, p_eth, p_src, p_dst
);<BR>+ status = __recv_gen( p_port, p_ipoib, p_eth, p_src,
p_dst );<BR> cl_perf_stop(
&p_port->p_adapter->perf, RecvGen
);<BR> }<BR> <BR>@@ -1943,11 +1975,15
@@<BR> <BR> static
ib_api_status_t<BR> __recv_gen(<BR>+ IN const
ipoib_port_t*
const p_port,<BR> IN const ipoib_pkt_t*
const p_ipoib,<BR> OUT eth_pkt_t*
const p_eth,<BR> IN ipoib_endpt_t*
const p_src,<BR> IN ipoib_endpt_t*
const p_dst
)<BR> {<BR>+ net16_t OriginalType;<BR>+ ib_api_status_t status;<BR>+ mac_addr_t
dst_mac;<BR> IPOIB_ENTER( IPOIB_DBG_RECV
);<BR> <BR> if( !p_src || !p_dst )<BR>@@ -1957,6 +1993,8
@@<BR> return
IB_NOT_DONE;<BR> }<BR> <BR>+ OriginalType =
p_ipoib->hdr.type;<BR>+<BR> /*<BR> * Fill in the
ethernet header. Note that doing so will overwrite<BR> * the
IPoIB header, so start by moving the information from the IPoIB<BR>@@ -1966,6
+2004,14 @@<BR> p_eth->hdr.src =
p_src->mac;<BR> p_eth->hdr.dst =
p_dst->mac;<BR> <BR>+ if (OriginalType == ETH_PROT_TYPE_IP)
<BR>+ {<BR>+ status =
__get_mac_from_ip(&p_port->vs_manager, p_ipoib->type.ip.hdr.dst_ip,
&dst_mac);<BR>+ if ( status == IB_SUCCESS)
<BR>+ {<BR>+ p_eth->hdr.dst =
dst_mac;<BR>+ }<BR>+ }<BR> IPOIB_EXIT( IPOIB_DBG_RECV
);<BR> return IB_SUCCESS;<BR> }<BR>@@ -1991,7 +2037,7
@@<BR> UNUSED_PARAM( p_port );<BR> <BR> /* Create the
ethernet header. */<BR>- status = __recv_gen( p_ipoib, p_eth, p_src, p_dst
);<BR>+ status = __recv_gen( p_port, p_ipoib, p_eth, p_src, p_dst
);<BR> if( status != IB_SUCCESS
)<BR> {<BR> IPOIB_TRACE_EXIT( IPOIB_DBG_ERROR,<BR>@@
-2128,6 +2174,7 @@<BR> const
ipoib_arp_pkt_t *p_ib_arp;<BR> ib_gid_t gid;<BR> mac_addr_t mac;<BR>+ mac_addr_t
dst_mac;<BR> ipoib_hw_addr_t null_hw =
{0};<BR> <BR> IPOIB_ENTER( IPOIB_DBG_RECV );<BR>@@ -2279,7
+2326,7 @@<BR> * Create the ethernet header. Note that this is
done last so that<BR> * we have a chance to create a new
endpoint.<BR> */<BR>- status = __recv_gen( p_ipoib, p_eth,
*pp_src, p_dst );<BR>+ status = __recv_gen( p_port, p_ipoib, p_eth,
*pp_src, p_dst );<BR> if( status != IB_SUCCESS
)<BR> {<BR> IPOIB_TRACE_EXIT( IPOIB_DBG_ERROR,<BR>@@
-2288,6 +2335,20 @@<BR> return
status;<BR> }<BR> <BR>+ if (p_eth->hdr.type
== ETH_PROT_TYPE_ARP) {<BR>+ if ((p_eth->type.arp.op ==
ARP_OP_REP)) <BR>+ {<BR>+ status =
__get_mac_from_ip(&p_port->vs_manager, p_eth->type.arp.dst_ip,
&dst_mac);<BR>+ if ( status == IB_SUCCESS)
<BR>+ {<BR>+ p_eth->hdr.dst =
dst_mac;<BR>+ p_eth->type.arp.dst_hw =
dst_mac;<BR>+ }<BR>+ }
<BR>+<BR>+<BR>+ }<BR>+<BR> IPOIB_EXIT(
IPOIB_DBG_RECV );<BR> return IB_SUCCESS;<BR> }<BR>@@ -3133,7
+3194,7 @@<BR> p_cid[1] =
21;<BR> }<BR> <BR>- CL_ASSERT( p_cid[1] == 21
);<BR>+//?????? CL_ASSERT( p_cid[1] == 21 ); // This asserts seems to
bounce, nothing happens if ignored ???<BR> p_cid[23]=
DHCP_OPT_END;<BR> ib_gid_set_default( &gid,
p_port->p_adapter->guids.port_guid );<BR> cl_memcpy(
&p_cid[7], &gid, sizeof(ib_gid_t) );<BR>@@ -3219,6 +3280,11
@@<BR> return
NDIS_STATUS_INVALID_DATA;<BR> }<BR> <BR>+ if ((p_arp->op
== ARP_OP_REQ))
<BR>+ {<BR>+ __put_mac_ip_pair(&p_port->vs_manager,p_arp->src_ip,
p_arp->src_hw );<BR>+ }<BR>+<BR> /* Allocate our scratch
buffer. */<BR> p_desc->p_buf =
(send_buf_t*)<BR> ExAllocateFromNPagedLookasideList(
&p_port->buf_mgr.send_buf_list );<BR>@@ -5211,3 +5277,124
@@<BR> <BR> IPOIB_EXIT( IPOIB_DBG_MCAST
);<BR> }<BR>+<BR>+static
void<BR>+__init_vs_ip_to_mac_translation(<BR>+ IN VS_ip_mac_manager
* p_manager)<BR>+{<BR>+ p_manager->p_pairs =
NULL;<BR>+ p_manager->data_size = 0;<BR>+ p_manager->array_size
= 0;<BR>+}<BR>+<BR>+static
void<BR>+__shutdown_vs_ip_to_mac_translation(<BR>+ IN VS_ip_mac_manager
* p_manager)<BR>+{<BR>+ if ( p_manager->p_pairs != NULL
) <BR>+ {<BR>+ cl_free( p_manager->p_pairs
);<BR>+ }<BR>+}<BR>+<BR>+static
ib_api_status_t<BR>+__get_mac_from_ip(<BR>+ IN const VS_ip_mac_manager
* const p_manager,<BR>+ IN net32_t dst_ip,<BR>+ OUT mac_addr_t
* p_dst_mac)<BR>+{<BR>+ uint32_t i;<BR>+ for (i
= 0 ; i < p_manager->data_size; i++ ) <BR>+ {<BR>+ if
(p_manager->p_pairs[i].dst_ip == dst_ip)
<BR>+ {<BR>+ // We have found the IP that we are
looking for<BR>+ *p_dst_mac =
p_manager->p_pairs[i].mac;<BR>+ IPOIB_TRACE(
IPOIB_DBG_VM,("__get_mac_from_ip dst_ip = %d.%d.%d.%d found in table\n",
<BR>+ ((dst_ip & 0xff
) ),<BR>+ ((dst_ip
& 0xff00 ) >> 8
),<BR>+ ((dst_ip & 0xff0000 ) >> 16
),<BR>+ ((dst_ip & 0xff000000) >> 24
)));<BR>+ <BR>+ return
IB_SUCCESS;<BR>+ }<BR>+ <BR>+ }<BR>+ //
Not found<BR>+ IPOIB_TRACE( IPOIB_DBG_VM,("__get_mac_from_ip dst_ip =
%d.%d.%d.%d not found \n", <BR>+ ((dst_ip &
0xff )
),<BR>+ ((dst_ip & 0xff00 ) >>
8 ),<BR>+ ((dst_ip & 0xff0000 ) >> 16
),<BR>+ ((dst_ip & 0xff000000) >> 24
)));<BR>+<BR>+ <BR>+ return IB_NOT_FOUND;<BR>+}<BR>+<BR>+static
ib_api_status_t<BR>+__put_mac_ip_pair(<BR>+ IN VS_ip_mac_manager
* p_manager,<BR>+ IN net32_t dst_ip,<BR>+ IN mac_addr_t dst_mac)<BR>+{<BR>+ uint32_t
i;<BR>+ uint32_t new_size = 0;<BR>+ VS_ip_mac_pair
*new_array;<BR>+ IPOIB_ENTER( IPOIB_DBG_VM );<BR>+<BR>+ IPOIB_TRACE(
IPOIB_DBG_VM,("__put_mac_ip_pair dst_ip = %d.%d.%d.%d \n",
<BR>+ ((dst_ip & 0xff
) ),<BR>+ ((dst_ip
& 0xff00 ) >> 8
),<BR>+ ((dst_ip & 0xff0000 ) >> 16
),<BR>+ ((dst_ip & 0xff000000) >> 24
)));<BR>+<BR>+ // First step is to look if this is actually an update and
not adding<BR>+ for (i = 0 ; i < p_manager->data_size; i++ )
<BR>+ {<BR>+ if (p_manager->p_pairs[i].dst_ip == dst_ip)
<BR>+ {<BR>+ // We have found the IP that we are
looking for, update it<BR>+ p_manager->p_pairs[i].mac =
dst_mac;<BR>+ return
IB_SUCCESS;<BR>+ }<BR>+ <BR>+ }<BR>+ //
Not found, let see if we need to increase the table<BR>+ if (
p_manager->array_size <= p_manager->data_size
)<BR>+ {<BR>+ // Need to increase the array<BR>+ if
(p_manager->array_size < 4)
<BR>+ {<BR>+ new_size = 4;<BR>+ } else
{<BR>+ new_size = p_manager->array_size *
2;<BR>+ }<BR>+ new_array = cl_zalloc(new_size * sizeof
(VS_ip_mac_pair));<BR>+ if ( new_array == NULL
)<BR>+ {<BR>+ IPOIB_TRACE_EXIT(
IPOIB_DBG_ERROR,<BR>+ ("Failed to allocate new_array.\n")
);<BR>+ return
CL_INSUFFICIENT_MEMORY;<BR>+ }<BR>+ // copy the data to
the new array<BR>+ if ( p_manager->array_size > 0 )
<BR>+ {<BR>+ cl_memcpy ( new_array,
p_manager->p_pairs, p_manager->data_size * sizeof
(VS_ip_mac_pair));<BR>+ }<BR>+ if ( p_manager->p_pairs
!= NULL ) <BR>+ {<BR>+ cl_free(
p_manager->p_pairs );<BR>+ }<BR>+ p_manager->p_pairs
= new_array;<BR>+ new_array =
NULL;<BR>+ p_manager->array_size =
new_size; <BR>+ }<BR>+<BR>+ p_manager->p_pairs[p_manager->data_size].dst_ip
= dst_ip;<BR>+ p_manager->p_pairs[p_manager->data_size].mac =
dst_mac;<BR>+ p_manager->data_size++;<BR>+<BR>+ return
IB_SUCCESS;<BR>+<BR>+}<BR>+<BR>Index:
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_port.h<BR>===================================================================<BR>---
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_port.h (revision 226)<BR>+++
Q:/OpenIb/gen1/trunk/ulp/ipoib/kernel/ipoib_port.h (working copy)<BR>@@
-468,7 +468,20 @@<BR> * are inserted in the LID
map.<BR> *********/<BR> <BR>+typedef struct
_VS_ip_mac_pair<BR>+{<BR>+ mac_addr_t mac;<BR>+ net32_t dst_ip;<BR>+}
VS_ip_mac_pair;<BR> <BR>+typedef struct
_VS_ip_mac_manager<BR>+{<BR>+ VS_ip_mac_pair *p_pairs;<BR>+ uint32_t array_size;<BR>+ uint32_t data_size;<BR>+}
VS_ip_mac_manager;<BR>+<BR>+<BR> typedef struct
_ipoib_port<BR> {<BR> cl_obj_t obj;<BR>@@
-496,8 +509,12
@@<BR> atomic32_t endpt_rdr;<BR> <BR> atomic32_t hdr_idx;<BR>- ipoib_hdr_t hdr[1];<BR> <BR>+ VS_ip_mac_manager vs_manager;<BR>+<BR>+ //
Must be last<BR>+ ipoib_hdr_t hdr[1];
<BR>+<BR> } ipoib_port_t;<BR> /*<BR> * FIELDS<BR>@@ -536,7
+553,6 @@<BR> * Endpoint
manager.<BR> *********/<BR> <BR>-<BR> ib_api_status_t<BR> ipoib_create_port(<BR> IN struct
_ipoib_adapter* const p_adapter,<BR></SPAN></FONT></DIV></BODY></HTML>