[dpdk-dev] [PATCH v2 6/6] mbuf: flatten struct vlan_macip into mbuf struct
Olivier MATZ
olivier.matz at 6wind.com
Mon Sep 8 11:07:11 CEST 2014
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>
More information about the dev
mailing list