[ofw] patch: [ipoib] when checking if a packet is a dhcp packet, make sure to check the offset and not the flags

Smith, Stan stan.smith at intel.com
Tue Oct 4 11:06:48 PDT 2011


Done in Revision: 3286
Author: stan.smith at intel.com
Date: 11:05:54 AM, Tuesday, October 04, 2011
Message:
[IPOIB] IP header 'offset' member renamed to 'offset_flags' to more accurately identify field contents.
Use IP_FRAGMENT_OFFSET() macro to correctly isolate offset from flags.
Add () around macro argument definition to ensure correct usage of argument.

Signed-off-by: stan smith (stan.smith at intel.com<mailto:stan.smith at intel.com>) Tzachi Dar (tzachid at mellanox.co.il)
----
Modified : /gen1/trunk/inc/kernel/ip_packet.h
Modified : /gen1/trunk/ulp/ipoib/kernel/ipoib_port.cpp


--- C:/Users/scsmith/AppData/Local/Temp/ip_packet.-revBASE.svn001.tmp.h  Tue Oct 04 10:52:52 2011

+++ C:/Users/scsmith/Documents/openIB-windows/OFA_SVN/ofw/gen1/trunk/inc/kernel/ip_packet.h  Tue Oct 04 10:44:22 2011

@@ -221,7 +221,7 @@

    uint8_t         svc_type;

    net16_t         length;

    net16_t         id;

-    net16_t         offset;

+    net16_t         offset_flags;

    uint8_t         ttl;

    uint8_t         prot;

    net16_t         chksum;

@@ -243,8 +243,8 @@

*   id

*         Packet identification.

*

-*   offset

-*         Fragment offset.

+*   offset_flags

+*         Fragment offset(13 bits) and flags(3 bits).

*

*   ttl

*         Time to live.

@@ -435,19 +435,19 @@

          ((pTcpHdr->offset & 0xF0) >> 2)

 #define IP_FRAGMENT_OFFSET(p_ip_hdr) \

-          ( cl_ntoh16( p_ip_hdr->offset & CL_HTON16(0x1fff) ) )

+          ( cl_ntoh16( (p_ip_hdr)->offset_flags & CL_HTON16(0x1fff) ) )

 #define IP_DONT_FRAGMENT(p_ip_hdr) \

-          ( (BOOLEAN)( p_ip_hdr->offset & CL_HTON16(0x4000 ) ) )

+          ( (BOOLEAN)( (p_ip_hdr)->offset_flags & CL_HTON16(0x4000 ) ) )



 #define IP_MORE_FRAGMENTS( p_ip_hdr ) \

-          ( (BOOLEAN)( p_ip_hdr->offset & CL_HTON16(0x2000) ) )

+          ( (BOOLEAN)( (p_ip_hdr)->offset_flags & CL_HTON16(0x2000) ) )

 #define IP_SET_MORE_FRAGMENTS( p_ip_hdr ) \

-          ( p_ip_hdr->offset |= CL_HTON16(0x2000) )

+          ( (p_ip_hdr)->offset_flags |= CL_HTON16(0x2000) )

 #define IP_SET_LAST_FRAGMENT( p_ip_hdr ) \

-          ( p_ip_hdr->offset &= (CL_HTON16(~0x2000) ) )

+          ( (p_ip_hdr)->offset_flags &= (CL_HTON16(~0x2000) ) )

 #define PROTOCOL_TCP         IP_PROT_TCP


--- C:/Users/scsmith/AppData/Local/Temp/ipoib_port.cpp-revBASE.svn000.tmp.cpp     Tue Oct 04 10:53:38 2011

+++ C:/Users/scsmith/Documents/openIB-windows/OFA_SVN/ofw/gen1/trunk/ulp/ipoib/kernel/ipoib_port.cpp     Tue Oct 04 10:42:32 2011

@@ -451,7 +451,7 @@

                    p_ip_hdr->svc_type,

                     cl_ntoh16( p_ip_hdr->length ),

                     cl_ntoh16( p_ip_hdr->id ),

-                     cl_ntoh16( p_ip_hdr->offset ),

+                     cl_ntoh16( p_ip_hdr->offset_flags ),

                     p_ip_hdr->ttl,

                     p_ip_hdr->prot,

                     cl_ntoh16( p_ip_hdr->chksum ),

@@ -2987,7 +2987,7 @@

                     break;

               }

-               if( p_ipoib->type.ip.hdr.offset ||

+               if( IP_FRAGMENT_OFFSET(&p_ipoib->type.ip.hdr) ||

                     p_ipoib->type.ip.hdr.prot != IP_PROT_UDP )

               {

                     /* Unfiltered.  Setup the ethernet header and report. */

@@ -5415,7 +5415,8 @@

    PERF_DECLARE( FilterDhcp );

     XIPOIB_ENTER( IPOIB_DBG_SEND );

-    if ( (ethertype == ETH_PROT_TYPE_IP) && ((ip_hdr_t*)p_ip_hdr)->offset > 0 )

+    if ( (ethertype == ETH_PROT_TYPE_IP) &&

+          (IP_FRAGMENT_OFFSET((ip_hdr_t*)p_ip_hdr) > 0) )

     {

          /* This is a fragmented part of UDP packet

           * Only first packet will contain UDP header in such case

@@ -9713,7 +9714,7 @@

    uint16_t*  p_hdr = (uint16_t*)p_ip_hdr;

     p_ip_hdr->length = cl_hton16( fragment_size ); // bytes

-    p_ip_hdr->offset = cl_hton16( fragment_offset ); // 8-byte units

+    p_ip_hdr->offset_flags = cl_hton16( fragment_offset ); // 8-byte units

     if( more_fragments )

    {



From: Tzachi Dar [mailto:tzachid at mellanox.co.il]
Sent: Tuesday, October 04, 2011 12:05 AM
To: Smith, Stan; ofw at lists.openfabrics.org
Subject: RE: [ofw] patch: [ipoib] when checking if a packet is a dhcp packet, make sure to check the offset and not the flags

You are very right.

Please commit.

Thanks
Tzachi

From: Smith, Stan [mailto:stan.smith at intel.com]
Sent: Tuesday, October 04, 2011 12:11 AM
To: Tzachi Dar; ofw at lists.openfabrics.org
Subject: RE: [ofw] patch: [ipoib] when checking if a packet is a dhcp packet, make sure to check the offset and not the flags

Hello,
  Oops, there are cases where the offset+flags word is not handled correctly.
  While applying this patch, I realized we already have a macro defined in ip_packet.h which embodies your patch: IP_FRAGMENT_OFFSET().
By using IP_FRAGMENT() instead of 'cl_ntoh16(((ip_hdr_t*)p_ip_hdr)->offset_flags) & OFFSET_MASK' it cleans the code up and we do not need the #define OFFSET_MASK.

The patch becomes

Index: inc/kernel/ip_packet.h
===================================================================
--- inc/kernel/ip_packet.h           (revision 7290)
+++ inc/kernel/ip_packet.h        (working copy)
@@ -221,7 +221,7 @@
               uint8_t                 svc_type;
               net16_t                                length;
               net16_t                                id;
-              net16_t                                offset;
+             net16_t                                offset_flags;
               uint8_t                 ttl;
               uint8_t                 prot;
               net16_t                                chksum;


--- ulp/ipoib/kernel/ipoib_port.cpp   Fri Sep 23 11:57:50 2011

+++ ulp/ipoib/kernel/ipoib_port.cpp   (working copy) Mon Oct 03 14:51:35 2011

@@ -451,7 +451,7 @@

                     p_ip_hdr->svc_type,

                     cl_ntoh16( p_ip_hdr->length ),

                     cl_ntoh16( p_ip_hdr->id ),

-                     cl_ntoh16( p_ip_hdr->offset ),

+                     cl_ntoh16( p_ip_hdr->offset_flags ),

                     p_ip_hdr->ttl,

                     p_ip_hdr->prot,

                     cl_ntoh16( p_ip_hdr->chksum ),

@@ -2987,7 +2987,7 @@

                     break;

               }



-               if( p_ipoib->type.ip.hdr.offset ||

+               if( IP_FRAGMENT_OFFSET(&p_ipoib->type.ip.hdr) ||

                     p_ipoib->type.ip.hdr.prot != IP_PROT_UDP )

               {

                     /* Unfiltered.  Setup the ethernet header and report. */

@@ -5415,7 +5415,7 @@

    PERF_DECLARE( FilterDhcp );



     XIPOIB_ENTER( IPOIB_DBG_SEND );

-    if ( (ethertype == ETH_PROT_TYPE_IP) && ((ip_hdr_t*)p_ip_hdr)->offset > 0 )

+    if ( (ethertype == ETH_PROT_TYPE_IP) && IP_FRAGMENT_OFFSET((ip_hdr_t*)p_ip_hdr) > 0 )

     {

          /* This is a fragmented part of UDP packet

           * Only first packet will contain UDP header in such case

@@ -9713,7 +9721,7 @@

    uint16_t*  p_hdr = (uint16_t*)p_ip_hdr;



     p_ip_hdr->length = cl_hton16( fragment_size ); // bytes

-    p_ip_hdr->offset = cl_hton16( fragment_offset ); // 8-byte units

+    p_ip_hdr->offset_flags = cl_hton16( fragment_offset ); // 8-byte units



     if( more_fragments )

    {

Thoughts?

Stan.


From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
Sent: Sunday, October 02, 2011 2:44 AM
To: ofw at lists.openfabrics.org
Subject: [ofw] patch: [ipoib] when checking if a packet is a dhcp packet, make sure to check the offset and not the flags

On an ip header there are 3 bits flags and 13 bits of offset.
We used to look at them as one word, which cause us not to use flags well.

The following code fixes this.

Thanks
Tzachi


Index: inc/kernel/ip_packet.h
===================================================================
--- inc/kernel/ip_packet.h           (revision 7290)
+++ inc/kernel/ip_packet.h        (working copy)
@@ -221,7 +221,7 @@
               uint8_t                 svc_type;
               net16_t                                length;
               net16_t                                id;
-              net16_t                                offset;
+             net16_t                                offset_flags;
               uint8_t                 ttl;
               uint8_t                 prot;
               net16_t                                chksum;
@@ -264,6 +264,10 @@
* SEE ALSO
*            IB Network Drivers, eth_hdr_t, arp_pkt_t, tcp_hdr_t, udp_hdr_t
*********/
+
+#define OFFSET_MASK  0x1fff
+
+
#include <complib/cl_packoff.h>

 #include <complib/cl_packon.h>
Index: ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
===================================================================
--- ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp                (revision 7290)
+++ ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp             (working copy)
@@ -441,7 +441,7 @@
                                                               p_ip_hdr->svc_type,
                                                               cl_ntoh16( p_ip_hdr->length ),
                                                               cl_ntoh16( p_ip_hdr->id ),
-                                                              cl_ntoh16( p_ip_hdr->offset ),
+                                                             cl_ntoh16( p_ip_hdr->offset_flags ),
                                                               p_ip_hdr->ttl,
                                                               p_ip_hdr->prot,
                                                               cl_ntoh16( p_ip_hdr->chksum ),
@@ -2821,7 +2821,7 @@
                                                               break;
                                               }

-                                              if( p_ipoib->type.ip.hdr.offset ||
+                                             if( (cl_ntoh16(p_ipoib->type.ip.hdr.offset_flags) & OFFSET_MASK) ||
                                                                p_ipoib->type.ip.hdr.prot != IP_PROT_UDP )
                                               {
                                                               /* Unfiltered.    Setup the ethernet header and report. */
@@ -5102,7 +5102,7 @@
               PERF_DECLARE( FilterDhcp );
               IPOIB_ENTER( IPOIB_DBG_SEND );

-              if ( (ethertype == ETH_PROT_TYPE_IP) && ((ip_hdr_t*)p_ip_hdr)->offset > 0 )
+             if ( (ethertype == ETH_PROT_TYPE_IP) && (cl_ntoh16(((ip_hdr_t*)p_ip_hdr)->offset_flags) & OFFSET_MASK) > 0 )
               {
                               /* This is a fragmented part of UDP packet
                                * Only first packet will contain UDP header in such case
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20111004/f8d6287f/attachment.html>


More information about the ofw mailing list