From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v4 11/21] ethdev: define structures for getting flow director information Date: Tue, 28 Oct 2014 14:44:34 +0100 Message-ID: <2263852.MKHN7tgybq@xps13> References: <1411711418-12881-1-git-send-email-jingjing.wu@intel.com> <1413939687-11177-1-git-send-email-jingjing.wu@intel.com> <1413939687-11177-12-git-send-email-jingjing.wu@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev-VfR2kkLFssw@public.gmane.org To: Jingjing Wu Return-path: In-Reply-To: <1413939687-11177-12-git-send-email-jingjing.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" 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