Hi Maryam,
On 06/26/2015 02:59 PM, Maryam Tahhan wrote:
> Extend rte_eth_xstats_get to retrieve additional stats from the device
> driver as well the ethdev generic stats.
>
> Signed-off-by: Maryam Tahhan <maryam.tahhan at intel.com>
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
> lib/librte_ether/rte_ethdev.c | 20 ++++++++++++++------
> 2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0f62bcb..099e464 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2035,7 +2035,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct
> rte_eth_xstats *xstats,
> total_qprdc = 0;
> rxnfgpc = 0;
> txdgpc = 0;
> - count = 0;
> + count = n;
>
> ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc,
> &total_qprc,
> &rxnfgpc, &txdgpc, &total_qprdc);
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 02cd07f..6451621 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1741,7 +1741,7 @@ rte_eth_xstats_get(uint8_t port_id, struct
> rte_eth_xstats *xstats,
> {
> struct rte_eth_stats eth_stats;
> struct rte_eth_dev *dev;
> - unsigned count, i, q;
> + unsigned count = 0, xcount = 0, i, q;
> uint64_t val, *stats_ptr;
>
> if (!rte_eth_dev_is_valid_port(port_id)) {
> @@ -1751,14 +1751,18 @@ rte_eth_xstats_get(uint8_t port_id, struct
> rte_eth_xstats *xstats,
>
> dev = &rte_eth_devices[port_id];
>
> - /* implemented by the driver */
> - if (dev->dev_ops->xstats_get != NULL)
> - return (*dev->dev_ops->xstats_get)(dev, xstats, n);
> -
> - /* else, return generic statistics */
> + /* Return generic statistics */
> count = RTE_NB_STATS;
> count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
> count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
> +
> + /* implemented by the driver */
> + if (dev->dev_ops->xstats_get != NULL) {
> + /* Retrieve the additional count size */
> + xcount = (*dev->dev_ops->xstats_get)(dev, xstats, 0);
> + count += xcount;
> + }
> +
> if (n < count)
> return count;
>
> @@ -1805,6 +1809,10 @@ rte_eth_xstats_get(uint8_t port_id, struct
> rte_eth_xstats *xstats,
> }
> }
>
> + /* Display stats after the generic stats*/
> + if (dev->dev_ops->xstats_get != NULL)
> + (*dev->dev_ops->xstats_get)(dev, xstats, count);
> +
> return count;
> }
>
I think we can avoid to have 2 calls to dev->dev_ops->xstats_get.
The first call can be removed.
The second call could be as below:
if (dev->dev_ops->xstats_get != NULL) {
ret = (*dev->dev_ops->xstats_get)(dev, &xstats[count],
n - count);
if (ret < 0)
return ret;
return ret + count;
}
- if the driver returns -1, the error code is propagated
- if the driver returns a positive value, it is added to "count",
which is the number of generic stats. It can be higher than "n"
if the xstats table is too small
Regards,
Olivier
