* [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
@ 2017-08-29 15:50 David Harton
2017-08-29 18:42 ` Stephen Hemminger
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: David Harton @ 2017-08-29 15:50 UTC (permalink / raw)
To: konstantin.ananyev; +Cc: dev, David Harton
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(-)
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
2017-08-29 15:50 [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets David Harton
@ 2017-08-29 18:42 ` Stephen Hemminger
2017-08-29 19:20 ` David Harton (dharton)
2017-08-29 18:44 ` Stephen Hemminger
2017-08-29 19:29 ` Ananyev, Konstantin
2 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2017-08-29 18:42 UTC (permalink / raw)
To: David Harton; +Cc: konstantin.ananyev, dev
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
2017-08-29 15:50 [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets David Harton
2017-08-29 18:42 ` Stephen Hemminger
@ 2017-08-29 18:44 ` Stephen Hemminger
2017-08-29 19:29 ` Ananyev, Konstantin
2 siblings, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2017-08-29 18:44 UTC (permalink / raw)
To: David Harton; +Cc: konstantin.ananyev, dev
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
2017-08-29 18:42 ` Stephen Hemminger
@ 2017-08-29 19:20 ` David Harton (dharton)
0 siblings, 0 replies; 8+ messages in thread
From: David Harton (dharton) @ 2017-08-29 19:20 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: konstantin.ananyev, dev
> -----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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
2017-08-29 15:50 [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets David Harton
2017-08-29 18:42 ` Stephen Hemminger
2017-08-29 18:44 ` Stephen Hemminger
@ 2017-08-29 19:29 ` Ananyev, Konstantin
2017-08-29 19:50 ` David Harton (dharton)
2 siblings, 1 reply; 8+ messages in thread
From: Ananyev, Konstantin @ 2017-08-29 19:29 UTC (permalink / raw)
To: David Harton; +Cc: dev
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
2017-08-29 19:29 ` Ananyev, Konstantin
@ 2017-08-29 19:50 ` David Harton (dharton)
2017-08-30 14:27 ` Ananyev, Konstantin
0 siblings, 1 reply; 8+ messages in thread
From: David Harton (dharton) @ 2017-08-29 19:50 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
2017-08-29 19:50 ` David Harton (dharton)
@ 2017-08-30 14:27 ` Ananyev, Konstantin
2017-08-30 15:32 ` David Harton (dharton)
0 siblings, 1 reply; 8+ messages in thread
From: Ananyev, Konstantin @ 2017-08-30 14:27 UTC (permalink / raw)
To: David Harton (dharton), Tahhan, Maryam; +Cc: dev
> >
> > > -----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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets
2017-08-30 14:27 ` Ananyev, Konstantin
@ 2017-08-30 15:32 ` David Harton (dharton)
0 siblings, 0 replies; 8+ messages in thread
From: David Harton (dharton) @ 2017-08-30 15:32 UTC (permalink / raw)
To: Ananyev, Konstantin, Tahhan, Maryam; +Cc: dev
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-08-30 15:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-29 15:50 [dpdk-dev] [PATCH] ixgbe: add counter to track sw tx packets David Harton
2017-08-29 18:42 ` Stephen Hemminger
2017-08-29 19:20 ` David Harton (dharton)
2017-08-29 18:44 ` Stephen Hemminger
2017-08-29 19:29 ` Ananyev, Konstantin
2017-08-29 19:50 ` David Harton (dharton)
2017-08-30 14:27 ` Ananyev, Konstantin
2017-08-30 15:32 ` David Harton (dharton)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).