Re: [PATCH v5 2/5] ethdev: add enum type and relevant structures for hash filter control

[Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>]

2014-10-21 11:14, Helin Zhang:
> +enum rte_eth_hash_filter_info_type {
> +	RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0,
> +	RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE,

PCTYPE is an unknown word in the API layer.
Could you replace it by something more generic?

> +	RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT,
> +	RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP,
> +	RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION,
> +	RTE_ETH_HASH_FILTER_INFO_TYPE_MAX,
> +};

You should comment each constant.

> +struct rte_eth_sym_hash_ena_info {
> +	/**< packet classification type, defined in rte_ethdev.h */
> +	uint8_t pctype;

No, PCTYPE is not anymore defined in ethdev.

> +/**
> + * A structure used to set or get filter swap information, to support
> + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET',
> + * with information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'.
> + */
> +struct rte_eth_filter_swap_info {
> +	/**< Packet classification type, defined in rte_ethdev.h */
> +	uint8_t pctype;
> +	/**< Offset of the 1st field of the 1st couple to be swapped. */
> +	uint8_t off0_src0;
> +	/**< Offset of the 2nd field of the 1st couple to be swapped. */
> +	uint8_t off0_src1;
> +	/**< Field length of the first couple. */
> +	uint8_t len0;
> +	/**< Offset of the 1st field of the 2nd couple to be swapped. */
> +	uint8_t off1_src0;
> +	/**< Offset of the 2nd field of the 2nd couple to be swapped. */
> +	uint8_t off1_src1;
> +	/**< Field length of the second couple. */
> +	uint8_t len1;
> +};

I guess it would be easier to understand if
RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP was defined previously.

-- 
Thomas