From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 5536EB39C for ; Mon, 8 Sep 2014 11:02:25 +0200 (CEST) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1XQuxN-0007fR-FT; Mon, 08 Sep 2014 11:09:59 +0200 Message-ID: <540D71BF.8050006@6wind.com> Date: Mon, 08 Sep 2014 11:07:11 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: Bruce Richardson , dev@dpdk.org References: <1409154628-30825-1-git-send-email-bruce.richardson@intel.com> <1409240559-14447-7-git-send-email-bruce.richardson@intel.com> In-Reply-To: <1409240559-14447-7-git-send-email-bruce.richardson@intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 6/6] mbuf: flatten struct vlan_macip into mbuf struct X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Sep 2014 09:02:26 -0000 Hi Bruce, Just small typos below. On 08/28/2014 05:42 PM, Bruce Richardson wrote: > The vlan_macip structure combined a vlan tag id with l2 and l3 headers > lengths for tracking offloads. However, this structure was only used as > a unit by the e1000 and ixgbe drivers, not generally. > > This patch removes the structure from the mbuf header and places the > fields into the mbuf structure directly at the required point, without > any net effect on the structure layout. This allows us to treat the vlan > tags and header length fields as separate for future mbuf changes. The > drivers which were written to use the combined structure still do so, > using a driver-local definition of it. > > Changes in V2: > * None > > [...] > > diff --git a/lib/librte_pmd_e1000/em_rxtx.c b/lib/librte_pmd_e1000/em_rxtx.c > index 074c9b3..4f46bdf 100644 > --- a/lib/librte_pmd_e1000/em_rxtx.c > +++ b/lib/librte_pmd_e1000/em_rxtx.c > @@ -144,13 +144,34 @@ enum { > EM_CTX_NUM = 1, /**< CTX NUM */ > }; > > +/** Offload features */ > +union em_vlan_macip { > + uint32_t data; > + struct { > + uint16_t l3_len:9; /**< L3 (IP) Header Length. */ > + uint16_t l2_len:7; /**< L2 (MAC) Header Length. */ > + uint16_t vlan_tci; > + /**< VLAN Tag Control Identifier (CPU order). */ > + } f; > +}; > + > +/* > + * Compare mask for vlan_macip_len.data, > + * should be in sync with em_vlan_macip.f layout. > + * */ > +#define TX_VLAN_CMP_MASK 0xFFFF0000 /**< VLAN length - 16-bits. */ > +#define TX_MAC_LEN_CMP_MASK 0x0000FE00 /**< MAC length - 7-bits. */ > +#define TX_IP_LEN_CMP_MASK 0x000001FF /**< IP length - 9-bits. */ > +/**< MAC+IP length. */ > +#define TX_MACIP_LEN_CMP_MASK (TX_MAC_LEN_CMP_MASK | TX_IP_LEN_CMP_MASK) > + I think the doxygen syntax "/**<" is only used for comments that are after the name. It was already like this before your patch (in rte_mbuf.h), but maybe it's a good occasion to fix this typo. There is the same in igb and ixgbe. > /** > * Structure to check if new context need be built > */ > struct em_ctx_info { > uint16_t flags; /**< ol_flags related to context build. */ > uint32_t cmp_mask; /**< compare mask */ > - union rte_vlan_macip hdrlen; /**< L2 and L3 header lenghts */ > + union em_vlan_macip hdrlen; /**< L2 and L3 header lenghts */ > }; The comment is not aligned with the others. Acked-by: Olivier Matz