[ofw] [IPoIB, IPoIB_CM] Bugfix for UDP packets

[Alex Naslednikov]

Yes, I wrote about it in the bug description.
Sorry if I wasn't clear.

The bug was exist always, for all kind of large UDP packets.
But it was shown only for the packets that were fragmented in such a way
that the last fragmented packets was less than 8 bytes of data.
The reason is that for the small packets (less than 8 bytes of data) the
following check failed:
if( buf_len < sizeof(udp_hdr_t) )
	{
		IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
			("Buffer not large enough for UDP packet.\n") );
		return NDIS_STATUS_BUFFER_TOO_SHORT;
	}

Size of udo_hdr_t is 8, so all other packets passed this checked and
also returnrd NDIS_STATUS_PENDING on the next check:
f( (p_udp_hdr->src_port != DHCP_PORT_CLIENT ||
   p_udp_hdr->dst_port != DHCP_PORT_SERVER) &&
   (p_udp_hdr->src_port != DHCP_PORT_SERVER ||
   p_udp_hdr->dst_port != DHCP_PORT_CLIENT) )  {
   /* Not a DHCP packet. */
   return NDIS_STATUS_PENDING;
  }

Of course, it was by mistake, because udp header doesn't exist for
fragmented packets (except the first one)

XaleX

-----Original Message-----
From: Alex Estrin [mailto:alex.estrin at qlogic.com] 
Sent: Monday, January 19, 2009 5:06 PM
To: Alex Naslednikov; ofw at lists.openfabrics.org
Cc: Tzachi Dar
Subject: RE: [ofw] [IPoIB, IPoIB_CM] Bugfix for UDP packets

Hello,

Would you please clarify if this is the case when OS fragments large
send packet to interface-MTU-size chunks?

Thanks,
Alex.


> -----Original Message-----
> From: Alex Naslednikov [mailto:xalex at mellanox.co.il]
> Sent: Sunday, January 18, 2009 9:37 AM
> To: ofw at lists.openfabrics.org
> Cc: Tzachi Dar; Alex Estrin
> Subject: [ofw] [IPoIB, IPoIB_CM] Bugfix for UDP packets
> 
> Several test were failed on sending UDP packets with MTU-like sizes.
> Fragmented UDP packets do not contain UDP header, except the first 
> one.
> So, appropriate check was inserted.
>  
> This bug was shown rarely (only when the last fragment was less than 8

> bytes), because the following check always worked (by mistake):
>  
> if( (p_udp_hdr->src_port != DHCP_PORT_CLIENT ||
>   p_udp_hdr->dst_port != DHCP_PORT_SERVER) &&
>   (p_udp_hdr->src_port != DHCP_PORT_SERVER ||
>   p_udp_hdr->dst_port != DHCP_PORT_CLIENT) )  {
>   /* Not a DHCP packet. */
>   return NDIS_STATUS_PENDING;
>  }
>  
> Index: ipoib_port.c
> ===================================================================
> --- ipoib_port.c (revision 3752)
> +++ ipoib_port.c (working copy)
> @@ -3763,6 +3763,15 @@
>    p_udp_hdr = (udp_hdr_t*)GetIpPayloadPtr(p_ip_hdr);
>   }
>   /* Get the UDP header and check the destination port numbers. */
> + 
> + if (p_ip_hdr->offset > 0) {
> +  /* This is a fragmented part of UDP packet
> +   * Only first packet will contain UDP header in such case
> +   * So, return if offset > 0
> +   */
> +   return NDIS_STATUS_PENDING;
> + }
> +   
>   if( buf_len < sizeof(udp_hdr_t) )
>   {
>    IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR
>