[dpdk-dev] ixgbe: add counter to track sw tx packets

Message ID 20170829155056.12563-1-dharton@cisco.com (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

David Harton Aug. 29, 2017, 3:50 p.m. UTC
  Add counter to track packets transmitted at the software layer
to help isolate output errors not reported otherwise.

Signed-off-by: David Harton <dharton@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(-)
  

Comments

Stephen Hemminger Aug. 29, 2017, 6:42 p.m. UTC | #1
On Tue, 29 Aug 2017 11:50:56 -0400
David Harton <dharton@cisco.com> wrote:

> +	if (n < (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS))
> +		return (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS);

Please don't use BSD style.
Return does not need parenthesis.
  
Stephen Hemminger Aug. 29, 2017, 6:44 p.m. UTC | #2
On Tue, 29 Aug 2017 11:50:56 -0400
David Harton <dharton@cisco.com> wrote:

> Add counter to track packets transmitted at the software layer
> to help isolate output errors not reported otherwise.
> 
> Signed-off-by: David Harton <dharton@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(-)

Since the software stats are not per-queue they will cause significant
cache thrashing.
  
David Harton Aug. 29, 2017, 7:20 p.m. UTC | #3
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, August 29, 2017 2:43 PM
> To: David Harton (dharton) <dharton@cisco.com>
> Cc: konstantin.ananyev@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
> 
> On Tue, 29 Aug 2017 11:50:56 -0400
> David Harton <dharton@cisco.com> wrote:
> 
> > +	if (n < (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS))
> > +		return (IXGBEVF_NB_XSTATS + IXGBE_NB_SW_STATS);
> 
> Please don't use BSD style.
> Return does not need parenthesis.

Sure.
  
Ananyev, Konstantin Aug. 29, 2017, 7:29 p.m. UTC | #4
> -----Original Message-----
> From: David Harton [mailto:dharton@cisco.com]
> Sent: Tuesday, August 29, 2017 4:51 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; David Harton <dharton@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 <dharton@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
  
David Harton Aug. 29, 2017, 7:50 p.m. UTC | #5
> -----Original Message-----
> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> Sent: Tuesday, August 29, 2017 3:29 PM
> To: David Harton (dharton) <dharton@cisco.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] ixgbe: add counter to track sw tx packets
> 
> 
> 
> > -----Original Message-----
> > From: David Harton [mailto:dharton@cisco.com]
> > Sent: Tuesday, August 29, 2017 4:51 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org; David Harton <dharton@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

The motivation is to have rte_eth_stats_get() report the advertised output errors along with providing some insight to where the drops are happening with xstats (i.e. happening after passing the DPDK boundary).

Are you asking that DPDK track the tx stats in librte_ether so we can report oerrors?  The problem with that (similar to tracking at the application layer) is some drivers do report oerrors so how does that layer know when it should or shouldn't provide that stat?

If the concern is cache thrashing as Stephen implies then the stats could be made per Q.

Thanks,
Dave

> 
> >
> > Signed-off-by: David Harton <dharton@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
  
Ananyev, Konstantin Aug. 30, 2017, 2:27 p.m. UTC | #6
> >
> > > -----Original Message-----
> > > From: David Harton [mailto:dharton@cisco.com]
> > > Sent: Tuesday, August 29, 2017 4:51 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Cc: dev@dpdk.org; David Harton <dharton@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
> 
> The motivation is to have rte_eth_stats_get() report the advertised output errors along with providing some insight to where the drops are
> happening with xstats (i.e. happening after passing the DPDK boundary).
> 
> Are you asking that DPDK track the tx stats in librte_ether so we can report oerrors?

Yep, or even better - probably at the layer above rte_ethdev.

>  The problem with that (similar to tracking at the
> application layer) is some drivers do report oerrors so how does that layer know when it should or shouldn't provide that stat?

Something like that:
if (stats->oerrors == 0 && sw_stats->tx_pkts > stats->opackets)
		stats->oerrors = sw_stats->tx_pkts - stats->opackets;
?

> 
> If the concern is cache thrashing as Stephen implies then the stats could be made per Q.

Yes, one is cache sharing concern.
Though in more generic way - concern that fixing that small HW limitation might affect ixgbe TX performance.
BTW, as I remember it wasn't like that before.
It was done by the patch:
http://dpdk.org/ml/archives/dev/2015-July/022289.html
even though I acked it, I don't remember what was the issue :(
Maryam could you probably remind what was the exact problem with the old way
for calculating oerros (via TXDGPC)?
Thanks
Konstantin

> 
> Thanks,
> Dave
> 
> >
> > >
> > > Signed-off-by: David Harton <dharton@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
  
David Harton Aug. 30, 2017, 3:32 p.m. UTC | #7
> -----Original Message-----
> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> Sent: Wednesday, August 30, 2017 10:27 AM
> To: David Harton (dharton) <dharton@cisco.com>; Tahhan, Maryam
> <maryam.tahhan@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] ixgbe: add counter to track sw tx packets
> 
> 
> > >
> > > > -----Original Message-----
> > > > From: David Harton [mailto:dharton@cisco.com]
> > > > Sent: Tuesday, August 29, 2017 4:51 PM
> > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > > Cc: dev@dpdk.org; David Harton <dharton@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
> >
> > The motivation is to have rte_eth_stats_get() report the advertised
> > output errors along with providing some insight to where the drops are
> happening with xstats (i.e. happening after passing the DPDK boundary).
> >
> > Are you asking that DPDK track the tx stats in librte_ether so we can
> report oerrors?
> 
> Yep, or even better - probably at the layer above rte_ethdev.
> 
> >  The problem with that (similar to tracking at the application layer)
> > is some drivers do report oerrors so how does that layer know when it
> should or shouldn't provide that stat?
> 
> Something like that:
> if (stats->oerrors == 0 && sw_stats->tx_pkts > stats->opackets)
> 		stats->oerrors = sw_stats->tx_pkts - stats->opackets; ?
> 
> >
> > If the concern is cache thrashing as Stephen implies then the stats
> could be made per Q.
> 
> Yes, one is cache sharing concern.
> Though in more generic way - concern that fixing that small HW limitation
> might affect ixgbe TX performance.

Ok, so sounds like we aren't willing to tradeoff any performance to address the problem where it lies.

I don't really want to pollute the common code for a driver specific issue.  I'll just mark this patch as Rejected.

Thanks for the feedback,
Dave

> BTW, as I remember it wasn't like that before.
> It was done by the patch:
> http://dpdk.org/ml/archives/dev/2015-July/022289.html
> even though I acked it, I don't remember what was the issue :( Maryam
> could you probably remind what was the exact problem with the old way for
> calculating oerros (via TXDGPC)?
> Thanks
> Konstantin
> 
> >
> > Thanks,
> > Dave
> >
> > >
> > > >
> > > > Signed-off-by: David Harton <dharton@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
  

Patch

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;
 }