2014-10-22 09:01, Jingjing Wu:
> +/**
> + * A structure used to report the status of the flow director filters in use.
> + */
> +struct rte_eth_fdir {
> + /** Number of filters with collision indication. */
> + uint16_t collision;
> + /** Number of free (non programmed) filters. */
> + uint16_t free;
> + /** The Lookup hash value of the added filter that updated the value
> + of the MAXLEN field */
> + uint16_t maxhash;
> + /** Longest linked list of filters in the table. */
> + uint8_t maxlen;
> + /** Number of added filters. */
> + uint64_t add;
> + /** Number of removed filters. */
> + uint64_t remove;
> + /** Number of failed added filters (no more space in device). */
> + uint64_t f_add;
> + /** Number of failed removed filters. */
> + uint64_t f_remove;
> +};
rte_eth_fdir is a name which doesn't say what it really is.
This structure looks like a collection of statistics.
Why not rte_eth_fdir_stats?
> +struct rte_eth_fdir_ext {
> + uint16_t guarant_spc; /**< guaranteed spaces.*/
> + uint16_t guarant_cnt; /**< Number of filters in guaranteed spaces. */
> + uint16_t best_spc; /**< best effort spaces.*/
> + uint16_t best_cnt; /**< Number of filters in best effort spaces. */
> +};
I don't understand why this "extended" structure is not merged in the first one.
Adding new fields don't break old API.
> +/**
> + * A structure used to get the status information of flow director filter.
> + * to support RTE_ETH_FILTER_FDIR with RTE_ETH_FILTER_INFO operation.
> + */
OK content of this comment is good.
But the second sentence has no start.
Please try to have an uppercase letter at the beginning of your sentences,
and a subject followed by a verb.
(side note: this is also true for commit logs)
> +struct rte_eth_fdir_info {
> + int mode; /**< if 0 disbale, if 1 enable*/
Typo: disbale
--
Thomas
