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 <olivier.matz at 6wind.com>