> -----Original Message-----
> From: David Harton [mailto:dhar...@cisco.com]
> Sent: Tuesday, August 29, 2017 4:51 PM
> To: Ananyev, Konstantin <konstantin.anan...@intel.com>
> Cc: dev@dpdk.org; David Harton <dhar...@cisco.com>
> Subject: [PATCH] ixgbe: add counter to track sw tx packets
>
> Add counter to track packets transmitted at the software layer
> to help isolate output errors not reported otherwise.
So what is the reason why same stats couldn't be collected at the application
layer?
I.E:
nb_tx = rte_eth_tx_bulk(port, queue, ...);
sw_stats[...]->tx_pkts += nb_tx;
?
That's' generic way that would work for all PMDs (not ixgbe only)
and wouldn't affect actual PMD TX function in any way.
Konstantin
>
> Signed-off-by: David Harton <dhar...@cisco.com>
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 98
> ++++++++++++++++++++++++++++++++++++----
> drivers/net/ixgbe/ixgbe_ethdev.h | 9 ++++
> drivers/net/ixgbe/ixgbe_rxtx.c | 49 ++++++++++++++------
> 3 files changed, 132 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index ed21af5..a38a595 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -728,6 +728,14 @@ struct rte_ixgbe_xstats_name_off {
> #define IXGBE_NB_HW_STATS (sizeof(rte_ixgbe_stats_strings) / \
> sizeof(rte_ixgbe_stats_strings[0]))
>
> +/* SW statistics */
> +static const struct rte_ixgbe_xstats_name_off rte_ixgbe_sw_strings[] = {
> + {"tx_pkts", offsetof(struct ixgbe_sw_stats, tx_pkts)},
> +};
> +
> +#define IXGBE_NB_SW_STATS (sizeof(rte_ixgbe_sw_strings) / \
> + sizeof(rte_ixgbe_sw_strings[0]))
> +
> /* MACsec statistics */
> static const struct rte_ixgbe_xstats_name_off rte_ixgbe_macsec_strings[] = {
> {"out_pkts_untagged", offsetof(struct ixgbe_macsec_stats,
> @@ -3085,6 +3093,8 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device
> *pci_dev)
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> struct ixgbe_hw_stats *hw_stats =
> IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
> + struct ixgbe_sw_stats *sw_stats =
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(dev->data->dev_private);
> struct ixgbe_macsec_stats *macsec_stats =
> IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
> dev->data->dev_private);
> @@ -3130,7 +3140,10 @@ static int eth_ixgbevf_pci_remove(struct
> rte_pci_device *pci_dev)
> hw_stats->fclast;
>
> /* Tx Errors */
> - stats->oerrors = 0;
> + if (sw_stats->tx_pkts > stats->opackets)
> + stats->oerrors = sw_stats->tx_pkts - stats->opackets;
> + else
> + stats->oerrors = 0;
> }
>
> static void
> @@ -3138,18 +3151,21 @@ static int eth_ixgbevf_pci_remove(struct
> rte_pci_device *pci_dev)
> {
> struct ixgbe_hw_stats *stats =
> IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
> + struct ixgbe_sw_stats *sw_stats =
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(dev->data->dev_private);
>
> /* HW registers are cleared on read */
> ixgbe_dev_stats_get(dev, NULL);
>
> /* Reset software totals */
> memset(stats, 0, sizeof(*stats));
> + memset(sw_stats, 0, sizeof(*sw_stats));
> }
>
> /* This function calculates the number of xstats based on the current config
> */
> static unsigned
> ixgbe_xstats_calc_num(void) {
> - return IXGBE_NB_HW_STATS + IXGBE_NB_MACSEC_STATS +
> + return IXGBE_NB_HW_STATS + IXGBE_NB_MACSEC_STATS + IXGBE_NB_SW_STATS +
> (IXGBE_NB_RXQ_PRIO_STATS * IXGBE_NB_RXQ_PRIO_VALUES) +
> (IXGBE_NB_TXQ_PRIO_STATS * IXGBE_NB_TXQ_PRIO_VALUES);
> }
> @@ -3176,6 +3192,15 @@ static int ixgbe_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> count++;
> }
>
> + /* SW Stats */
> + for (i = 0; i < IXGBE_NB_SW_STATS; i++) {
> + snprintf(xstats_names[count].name,
> + sizeof(xstats_names[count].name),
> + "%s",
> + rte_ixgbe_sw_strings[i].name);
> + count++;
> + }
> +
> /* MACsec Stats */
> for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
> snprintf(xstats_names[count].name,
> @@ -3236,6 +3261,15 @@ static int ixgbe_dev_xstats_get_names_by_id(
> count++;
> }
>
> + /* SW Stats */
> + for (i = 0; i < IXGBE_NB_SW_STATS; i++) {
> + snprintf(xstats_names[count].name,
> + sizeof(xstats_names[count].name),
> + "%s",
> + rte_ixgbe_sw_strings[i].name);
> + count++;
> + }
> +
> /* MACsec Stats */
> for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
> snprintf(xstats_names[count].name,
> @@ -3291,17 +3325,23 @@ static int ixgbe_dev_xstats_get_names_by_id(
> static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> struct rte_eth_xstat_name *xstats_names, unsigned limit)
> {
> - unsigned i;
> + unsigned int i, j;
>
> - if (limit < IXGBEVF_NB_XSTATS && xstats_names != NULL)
> + if (limit < (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS) &&
> + xstats_names != NULL)
> return -ENOMEM;
>
> - if (xstats_names != NULL)
> + if (xstats_names != NULL) {
> for (i = 0; i < IXGBEVF_NB_XSTATS; i++)
> snprintf(xstats_names[i].name,
> sizeof(xstats_names[i].name),
> "%s", rte_ixgbevf_stats_strings[i].name);
> - return IXGBEVF_NB_XSTATS;
> + for (j = 0; j < IXGBE_NB_SW_STATS; j++, i++)
> + snprintf(xstats_names[i].name,
> + sizeof(xstats_names[i].name),
> + "%s", rte_ixgbe_sw_strings[j].name);
> + }
> + return (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS);
> }
>
> static int
> @@ -3312,6 +3352,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> struct ixgbe_hw_stats *hw_stats =
> IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
> + struct ixgbe_sw_stats *sw_stats =
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(dev->data->dev_private);
> struct ixgbe_macsec_stats *macsec_stats =
> IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
> dev->data->dev_private);
> @@ -3346,6 +3388,14 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> count++;
> }
>
> + /* SW Stats */
> + for (i = 0; i < IXGBE_NB_SW_STATS; i++) {
> + xstats[count].value = *(uint64_t *)(((char *)sw_stats) +
> + rte_ixgbe_sw_strings[i].offset);
> + xstats[count].id = count;
> + count++;
> + }
> +
> /* MACsec Stats */
> for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
> xstats[count].value = *(uint64_t *)(((char *)macsec_stats) +
> @@ -3388,6 +3438,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> struct ixgbe_hw_stats *hw_stats =
> IXGBE_DEV_PRIVATE_TO_STATS(
> dev->data->dev_private);
> + struct ixgbe_sw_stats *sw_stats =
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(
> + dev->data->dev_private);
> struct ixgbe_macsec_stats *macsec_stats =
> IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
> dev->data->dev_private);
> @@ -3422,6 +3475,13 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> count++;
> }
>
> + /* SW Stats */
> + for (i = 0; i < IXGBE_NB_SW_STATS; i++) {
> + values[count] = *(uint64_t *)(((char *)sw_stats) +
> + rte_ixgbe_sw_strings[i].offset);
> + count++;
> + }
> +
> /* MACsec Stats */
> for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
> values[count] = *(uint64_t *)(((char *)macsec_stats) +
> @@ -3522,10 +3582,12 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> {
> struct ixgbevf_hw_stats *hw_stats = (struct ixgbevf_hw_stats *)
> IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
> - unsigned i;
> + struct ixgbe_sw_stats *sw_stats = (struct ixgbe_sw_stats *)
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(dev->data->dev_private);
> + unsigned int i, j;
>
> - if (n < IXGBEVF_NB_XSTATS)
> - return IXGBEVF_NB_XSTATS;
> + if (n < (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS))
> + return (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS);
>
> ixgbevf_update_stats(dev);
>
> @@ -3538,8 +3600,13 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
> rte_ixgbevf_stats_strings[i].offset);
> }
> + for (j = 0; j < IXGBE_NB_SW_STATS; j++, i++) {
> + xstats[i].id = i;
> + xstats[i].value = *(uint64_t *)(((char *)sw_stats) +
> + rte_ixgbe_sw_strings[j].offset);
> + }
>
> - return IXGBEVF_NB_XSTATS;
> + return (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS);
> }
>
> static void
> @@ -3547,6 +3614,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> {
> struct ixgbevf_hw_stats *hw_stats = (struct ixgbevf_hw_stats *)
> IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
> + struct ixgbe_sw_stats *sw_stats = (struct ixgbe_sw_stats *)
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(dev->data->dev_private);
>
> ixgbevf_update_stats(dev);
>
> @@ -3557,6 +3626,11 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> stats->ibytes = hw_stats->vfgorc;
> stats->opackets = hw_stats->vfgptc;
> stats->obytes = hw_stats->vfgotc;
> +
> + if (sw_stats->tx_pkts > stats->opackets)
> + stats->oerrors = sw_stats->tx_pkts - stats->opackets;
> + else
> + stats->oerrors = 0;
> }
>
> static void
> @@ -3564,6 +3638,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> {
> struct ixgbevf_hw_stats *hw_stats = (struct ixgbevf_hw_stats *)
> IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
> + struct ixgbe_sw_stats *sw_stats = (struct ixgbe_sw_stats *)
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(dev->data->dev_private);
>
> /* Sync HW register to the last stats */
> ixgbevf_dev_stats_get(dev, NULL);
> @@ -3573,6 +3649,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused
> struct rte_eth_dev *dev,
> hw_stats->vfgorc = 0;
> hw_stats->vfgptc = 0;
> hw_stats->vfgotc = 0;
> +
> + memset(sw_stats, 0, sizeof(*sw_stats));
> }
>
> static int
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 3f1aeb5..ba9350c 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -434,6 +434,11 @@ struct ixgbe_macsec_stats {
> uint64_t in_pkts_notusingsa;
> };
>
> +/* Struct to track emulated stats */
> +struct ixgbe_sw_stats {
> + uint64_t tx_pkts;
> +};
> +
> /* The configuration of bandwidth */
> struct ixgbe_bw_conf {
> uint8_t tc_num; /* Number of TCs. */
> @@ -508,6 +513,7 @@ struct ixgbe_adapter {
> struct ixgbe_hw hw;
> struct ixgbe_hw_stats stats;
> struct ixgbe_macsec_stats macsec_stats;
> + struct ixgbe_sw_stats sw_stats;
> struct ixgbe_hw_fdir_info fdir;
> struct ixgbe_interrupt intr;
> struct ixgbe_stat_mapping_registers stat_mappings;
> @@ -541,6 +547,9 @@ struct ixgbe_adapter {
> #define IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(adapter) \
> (&((struct ixgbe_adapter *)adapter)->macsec_stats)
>
> +#define IXGBE_DEV_PRIVATE_TO_SW_STATS(adapter) \
> + (&((struct ixgbe_adapter *)adapter)->sw_stats)
> +
> #define IXGBE_DEV_PRIVATE_TO_INTR(adapter) \
> (&((struct ixgbe_adapter *)adapter)->intr)
>
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 64bff25..34e3968 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -347,24 +347,35 @@ uint16_t ixgbe_xmit_fixed_burst_vec(void *tx_queue,
> struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts)
> {
> uint16_t nb_tx;
> + struct ixgbe_tx_queue *txq = (struct ixgbe_tx_queue *)tx_queue;
> + struct ixgbe_sw_stats *sw_stats = (struct ixgbe_sw_stats *)
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(
> + rte_eth_devices[txq->port_id].data->dev_private);
>
> /* Try to transmit at least chunks of TX_MAX_BURST pkts */
> - if (likely(nb_pkts <= RTE_PMD_IXGBE_TX_MAX_BURST))
> - return tx_xmit_pkts(tx_queue, tx_pkts, nb_pkts);
> -
> - /* transmit more than the max burst, in chunks of TX_MAX_BURST */
> - nb_tx = 0;
> - while (nb_pkts) {
> - uint16_t ret, n;
> -
> - n = (uint16_t)RTE_MIN(nb_pkts, RTE_PMD_IXGBE_TX_MAX_BURST);
> - ret = tx_xmit_pkts(tx_queue, &(tx_pkts[nb_tx]), n);
> - nb_tx = (uint16_t)(nb_tx + ret);
> - nb_pkts = (uint16_t)(nb_pkts - ret);
> - if (ret < n)
> - break;
> + if (likely(nb_pkts <= RTE_PMD_IXGBE_TX_MAX_BURST)) {
> + nb_tx = tx_xmit_pkts(tx_queue, tx_pkts, nb_pkts);
> + } else {
> + /*
> + * transmit more than the max burst,
> + * in chunks of TX_MAX_BURST
> + */
> + nb_tx = 0;
> + while (nb_pkts) {
> + uint16_t ret, n;
> +
> + n = (uint16_t)RTE_MIN(
> + nb_pkts, RTE_PMD_IXGBE_TX_MAX_BURST);
> + ret = tx_xmit_pkts(tx_queue, &tx_pkts[nb_tx], n);
> + nb_tx = (uint16_t)(nb_tx + ret);
> + nb_pkts = (uint16_t)(nb_pkts - ret);
> + if (ret < n)
> + break;
> + }
> }
>
> + sw_stats->tx_pkts += nb_tx;
> +
> return nb_tx;
> }
>
> @@ -375,6 +386,9 @@ uint16_t ixgbe_xmit_fixed_burst_vec(void *tx_queue,
> struct rte_mbuf **tx_pkts,
> {
> uint16_t nb_tx = 0;
> struct ixgbe_tx_queue *txq = (struct ixgbe_tx_queue *)tx_queue;
> + struct ixgbe_sw_stats *sw_stats = (struct ixgbe_sw_stats *)
> + IXGBE_DEV_PRIVATE_TO_SW_STATS(
> + rte_eth_devices[txq->port_id].data->dev_private);
>
> while (nb_pkts) {
> uint16_t ret, num;
> @@ -388,6 +402,8 @@ uint16_t ixgbe_xmit_fixed_burst_vec(void *tx_queue,
> struct rte_mbuf **tx_pkts,
> break;
> }
>
> + sw_stats->tx_pkts += nb_tx;
> +
> return nb_tx;
> }
> #endif
> @@ -636,6 +652,7 @@ uint16_t ixgbe_xmit_fixed_burst_vec(void *tx_queue,
> struct rte_mbuf **tx_pkts,
> ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts)
> {
> + struct ixgbe_sw_stats *sw_stats;
> struct ixgbe_tx_queue *txq;
> struct ixgbe_tx_entry *sw_ring;
> struct ixgbe_tx_entry *txe, *txn;
> @@ -942,6 +959,10 @@ uint16_t ixgbe_xmit_fixed_burst_vec(void *tx_queue,
> struct rte_mbuf **tx_pkts,
> IXGBE_PCI_REG_WRITE_RELAXED(txq->tdt_reg_addr, tx_id);
> txq->tx_tail = tx_id;
>
> + sw_stats = (struct ixgbe_sw_stats *)IXGBE_DEV_PRIVATE_TO_SW_STATS(
> + rte_eth_devices[txq->port_id].data->dev_private);
> + sw_stats->tx_pkts += nb_tx;
> +
> return nb_tx;
> }
>
> --
> 1.8.3.1
