[ofw][patch] fixes client id insertion

[Tzachi Dar]

Hi Anatoly,

I have looked at this patch, and there are some issues that I don't
quite understand:

* what about UDP checksum? It doesn't seem as you are calculating it.

* In this patch you are doing an assumption that the packet size will
always be the size of ip + the size of UDP + the size of dhcp 
"p_desc->p_buf->ip.hdr.length = cl_ntoh16( sizeof(ip_hdr_t) +
sizeof(udp_hdr_t) + sizeof(dhcp_pkt_t) );"

This assumes that the size is always constant. This brings two questions
as far as I understand:
1) If the size is always constant, why do we have to touch this in any
case?
2) at least the size of the ip header is not constant. So you should
probably replace sizeof(ip_hdr_t) with IP_HEADER_LENGTH(IpHdr);

Where #define IP_HEADER_LENGTH(pIpHdr)   \
        ( (ULONG)((pIpHdr->iph_verlen & 0x0F) << 2) )


Thanks
Tzachi

> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org 
> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of 
> Anatoly Greenblatt
> Sent: Wednesday, June 25, 2008 5:13 PM
> To: ofw at lists.openfabrics.org
> Subject: [ofw][patch] fixes client id insertion
> 
>  <<fix_client_id_insertion.patch>> Hi,
> 
> The client id inserted in dhcp packets w/o updating lengths 
> in udp and ip headers. As a result in some cases the dhcp 
> packet was not accepted by dhcp server ("parameter is larger 
> than buffer" error).
> 
> The fix updates length in udp and ip header and calculate ip 
> header checksum if needed. Also fixes incorrect return code.
> 
> Best regards,
> Anatoly
> 
> Index: ipoib_port.c
> ===================================================================
> --- ipoib_port.c	(revision 1295)
> +++ ipoib_port.c	(working copy)
> @@ -3216,6 +3216,19 @@
>  }
>  
>  
> +unsigned short ipchksum(unsigned short *ip, int len) {
> +    unsigned long sum = 0;
> +
> +    len >>= 1;
> +    while (len--) {
> +        sum += *(ip++);
> +        if (sum > 0xFFFF)
> +            sum -= 0xFFFF;
> +    }
> +    return (unsigned short)((~sum) & 0x0000FFFF); }
> +
>  static NDIS_STATUS
>  __send_mgr_filter_dhcp(
>  	IN				ipoib_port_t* const
> p_port,
> @@ -3317,7 +3330,7 @@
>  				{
>  					IPOIB_PRINT_EXIT(
> TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
>  						("Can't convert CID to
> IPoIB format.\n") );
> -					return IB_INSUFFICIENT_MEMORY;
> +					return NDIS_STATUS_RESOURCES;
>  				}
>  				/* Move the existing options 
> down, and add a new CID option */
>  				len = p_option - ( p_cid + 
> p_cid[1] + 2 ); @@ -3357,10 +3370,21 @@
>  
>  		CL_ASSERT( p_cid[1] == 21 );
>  		p_cid[23]= DHCP_OPT_END;
> -		ib_gid_set_default( &gid,
> p_port->p_adapter->guids.port_guid.guid );
> +		ib_gid_set_default( &gid,
> p_port->p_adapter->guids.port_guid );
>  		cl_memcpy( &p_cid[7], &gid, sizeof(ib_gid_t) );
>  		cl_memcpy( &p_cid[3], &p_port->ib_mgr.qpn,
> sizeof(p_port->ib_mgr.qpn) );		
>  		p_ib_dhcp->htype = DHCP_HW_TYPE_IB;
> +
> +		/* update lengths to include any change we made */
> +		p_desc->p_buf->ip.hdr.length = cl_ntoh16(
> sizeof(ip_hdr_t) + sizeof(udp_hdr_t) + sizeof(dhcp_pkt_t) );
> +		p_desc->p_buf->ip.prot.udp.hdr.length = cl_ntoh16(
> sizeof(udp_hdr_t) + sizeof(dhcp_pkt_t) );
> +
> +		/* update crc in ip header */
> +		if( !p_port->p_adapter->params.send_chksum_offload )
> +		{
> +			p_desc->p_buf->ip.hdr.chksum = 0;
> +			p_desc->p_buf->ip.hdr.chksum =
> ipchksum((unsigned short*) &p_desc->p_buf->ip.hdr, sizeof(ip_hdr_t));
> +		}
>  		break;
>  
>  	/* Server messages. */
>