[dpdk-dev] [PATCH 0/8] oerrors stats fixes for virtual pmds

[dpdk-dev] [PATCH 0/8] oerrors stats fixes for virtual pmds

[David Marchand]

I did a pass on the pmds about oerrors some time ago, but still did not
find the motivation to finish yet.
For now, here is a series of cleanups on the virtual pmds.

The former patches are just about removing counters that adds nothing to
the stats.

The latter patches make sure that unsent packets (because of a temporary
situation) are not added to oerrors since an application can decide to
send them again later.

Wrt oerrors, the pcap pmd should be fixed with the series [1].
But the tap pmd still needs to be inspected.

1: https://patchwork.dpdk.org/project/dpdk/list/?series=5701

-- 
David Marchand

David Marchand (8):
  net/af_packet: remove unused Rx counter
  net/af_xdp: remove unused Tx counter
  net/null: remove unused Tx error counter
  net/virtio: remove unused Tx error counter
  net/kni: do not count unsent packets as errors
  net/memif: do not count unsent packets as errors
  net/ring: do not count unsent packets as errors
  net/vhost: do not count unsent packets as errors

 drivers/net/af_packet/rte_eth_af_packet.c | 1 -
 drivers/net/af_xdp/rte_eth_af_xdp.c       | 2 --
 drivers/net/kni/rte_eth_kni.c             | 3 ---
 drivers/net/memif/rte_eth_memif.c         | 2 --
 drivers/net/null/rte_eth_null.c           | 9 ++-------
 drivers/net/ring/rte_eth_ring.c           | 4 +---
 drivers/net/vhost/rte_eth_vhost.c         | 4 +---
 drivers/net/virtio/virtio_ethdev.c        | 3 ---
 8 files changed, 4 insertions(+), 24 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/8] net/af_packet: remove unused Rx counter

[David Marchand]

This Rx counter has never been used.

Fixes: 364e08f2bbc0 ("af_packet: add PMD for AF_PACKET-based virtual devices")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/af_packet/rte_eth_af_packet.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 2f6508f..82bf2cd 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -51,7 +51,6 @@ struct pkt_rx_queue {
 	uint16_t in_port;
 
 	volatile unsigned long rx_pkts;
-	volatile unsigned long err_pkts;
 	volatile unsigned long rx_bytes;
 };
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 1/8] net/af_packet: remove unused Rx counter

[Ferruh Yigit]

On 7/25/2019 10:14 AM, David Marchand wrote:
> This Rx counter has never been used.
> 
> Fixes: 364e08f2bbc0 ("af_packet: add PMD for AF_PACKET-based virtual devices")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH 2/8] net/af_xdp: remove unused Tx counter

[David Marchand]

This Tx counter is now unused.

Fixes: 10edf857fde4 ("net/af_xdp: make reserve/submit peek/release consistent")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 33352e1..41ed5b2 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -97,7 +97,6 @@ struct pkt_rx_queue {
 
 struct tx_stats {
 	uint64_t tx_pkts;
-	uint64_t err_pkts;
 	uint64_t tx_bytes;
 };
 
@@ -456,7 +455,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		stats->imissed += xdp_stats.rx_dropped;
 
 		stats->opackets += stats->q_opackets[i];
-		stats->oerrors += txq->stats.err_pkts;
 		stats->obytes += stats->q_obytes[i];
 	}
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 2/8] net/af_xdp: remove unused Tx counter

[Ye Xiaolong]

On 07/25, David Marchand wrote:
>This Tx counter is now unused.
>
>Fixes: 10edf857fde4 ("net/af_xdp: make reserve/submit peek/release consistent")

err_pkts seems not introduced by this commit?

>
>Signed-off-by: David Marchand <david.marchand@redhat.com>
>---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
>index 33352e1..41ed5b2 100644
>--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
>+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
>@@ -97,7 +97,6 @@ struct pkt_rx_queue {
> 
> struct tx_stats {
> 	uint64_t tx_pkts;
>-	uint64_t err_pkts;
> 	uint64_t tx_bytes;
> };
> 
>@@ -456,7 +455,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
> 		stats->imissed += xdp_stats.rx_dropped;
> 
> 		stats->opackets += stats->q_opackets[i];
>-		stats->oerrors += txq->stats.err_pkts;
> 		stats->obytes += stats->q_obytes[i];
> 	}
> 
>-- 
>1.8.3.1
>

Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>

Re: [dpdk-dev] [PATCH 2/8] net/af_xdp: remove unused Tx counter

[David Marchand]

On Thu, Jul 25, 2019 at 5:49 PM Ye Xiaolong <xiaolong.ye@intel.com> wrote:
>
> On 07/25, David Marchand wrote:
> >This Tx counter is now unused.
> >
> >Fixes: 10edf857fde4 ("net/af_xdp: make reserve/submit peek/release consistent")
>
> err_pkts seems not introduced by this commit?

This commit removed the only place where it was used.

>
> >
> >Signed-off-by: David Marchand <david.marchand@redhat.com>
> >---
> > drivers/net/af_xdp/rte_eth_af_xdp.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> >diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
> >index 33352e1..41ed5b2 100644
> >--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> >+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> >@@ -97,7 +97,6 @@ struct pkt_rx_queue {
> >
> > struct tx_stats {
> >       uint64_t tx_pkts;
> >-      uint64_t err_pkts;
> >       uint64_t tx_bytes;
> > };
> >
> >@@ -456,7 +455,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
> >               stats->imissed += xdp_stats.rx_dropped;
> >
> >               stats->opackets += stats->q_opackets[i];
> >-              stats->oerrors += txq->stats.err_pkts;
> >               stats->obytes += stats->q_obytes[i];
> >       }
> >
> >--
> >1.8.3.1
> >
>
> Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>

Thanks.

-- 
David Marchand

Re: [dpdk-dev] [PATCH 2/8] net/af_xdp: remove unused Tx counter

[Ye Xiaolong]

On 07/25, David Marchand wrote:
>On Thu, Jul 25, 2019 at 5:49 PM Ye Xiaolong <xiaolong.ye@intel.com> wrote:
>>
>> On 07/25, David Marchand wrote:
>> >This Tx counter is now unused.
>> >
>> >Fixes: 10edf857fde4 ("net/af_xdp: make reserve/submit peek/release consistent")
>>
>> err_pkts seems not introduced by this commit?
>
>This commit removed the only place where it was used.
>

Got it, make sense.

Thanks,
Xiaolong
>>
>> >
>> >Signed-off-by: David Marchand <david.marchand@redhat.com>
>> >---
>> > drivers/net/af_xdp/rte_eth_af_xdp.c | 2 --
>> > 1 file changed, 2 deletions(-)
>> >
>> >diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> >index 33352e1..41ed5b2 100644
>> >--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> >+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> >@@ -97,7 +97,6 @@ struct pkt_rx_queue {
>> >
>> > struct tx_stats {
>> >       uint64_t tx_pkts;
>> >-      uint64_t err_pkts;
>> >       uint64_t tx_bytes;
>> > };
>> >
>> >@@ -456,7 +455,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>> >               stats->imissed += xdp_stats.rx_dropped;
>> >
>> >               stats->opackets += stats->q_opackets[i];
>> >-              stats->oerrors += txq->stats.err_pkts;
>> >               stats->obytes += stats->q_obytes[i];
>> >       }
>> >
>> >--
>> >1.8.3.1
>> >
>>
>> Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>
>
>Thanks.
>
>-- 
>David Marchand

[dpdk-dev] [PATCH 3/8] net/null: remove unused Tx error counter

[David Marchand]

This Tx counter has never been used.

Fixes: c743e50c475f ("null: new poll mode driver")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/null/rte_eth_null.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 31cbb84..b2c92ab 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -62,7 +62,6 @@ struct null_queue {
 
 	rte_atomic64_t rx_pkts;
 	rte_atomic64_t tx_pkts;
-	rte_atomic64_t err_pkts;
 };
 
 struct pmd_internals {
@@ -311,7 +310,7 @@ static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 {
 	unsigned i, num_stats;
-	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
+	unsigned long rx_total = 0, tx_total = 0;
 	const struct pmd_internals *internal;
 
 	if ((dev == NULL) || (igb_stats == NULL))
@@ -334,12 +333,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 		igb_stats->q_opackets[i] =
 			internal->tx_null_queues[i].tx_pkts.cnt;
 		tx_total += igb_stats->q_opackets[i];
-		tx_err_total += internal->tx_null_queues[i].err_pkts.cnt;
 	}
 
 	igb_stats->ipackets = rx_total;
 	igb_stats->opackets = tx_total;
-	igb_stats->oerrors = tx_err_total;
 
 	return 0;
 }
@@ -356,10 +353,8 @@ eth_stats_reset(struct rte_eth_dev *dev)
 	internal = dev->data->dev_private;
 	for (i = 0; i < RTE_DIM(internal->rx_null_queues); i++)
 		internal->rx_null_queues[i].rx_pkts.cnt = 0;
-	for (i = 0; i < RTE_DIM(internal->tx_null_queues); i++) {
+	for (i = 0; i < RTE_DIM(internal->tx_null_queues); i++)
 		internal->tx_null_queues[i].tx_pkts.cnt = 0;
-		internal->tx_null_queues[i].err_pkts.cnt = 0;
-	}
 }
 
 static void
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 3/8] net/null: remove unused Tx error counter

[Ferruh Yigit]

On 7/25/2019 10:14 AM, David Marchand wrote:
> This Tx counter has never been used.
> 
> Fixes: c743e50c475f ("null: new poll mode driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH 4/8] net/virtio: remove unused Tx error counter

[David Marchand]

This Tx counter has never been used.

Fixes: 9658d17da27b ("virtio: maintain stats per queue")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 62c8274..20840bf 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -121,7 +121,6 @@ static const struct rte_virtio_xstats_name_off rte_virtio_rxq_stat_strings[] = {
 static const struct rte_virtio_xstats_name_off rte_virtio_txq_stat_strings[] = {
 	{"good_packets",           offsetof(struct virtnet_tx, stats.packets)},
 	{"good_bytes",             offsetof(struct virtnet_tx, stats.bytes)},
-	{"errors",                 offsetof(struct virtnet_tx, stats.errors)},
 	{"multicast_packets",      offsetof(struct virtnet_tx, stats.multicast)},
 	{"broadcast_packets",      offsetof(struct virtnet_tx, stats.broadcast)},
 	{"undersize_packets",      offsetof(struct virtnet_tx, stats.size_bins[0])},
@@ -944,7 +943,6 @@ virtio_update_stats(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 		stats->opackets += txvq->stats.packets;
 		stats->obytes += txvq->stats.bytes;
-		stats->oerrors += txvq->stats.errors;
 
 		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
 			stats->q_opackets[i] = txvq->stats.packets;
@@ -1082,7 +1080,6 @@ virtio_dev_stats_reset(struct rte_eth_dev *dev)
 
 		txvq->stats.packets = 0;
 		txvq->stats.bytes = 0;
-		txvq->stats.errors = 0;
 		txvq->stats.multicast = 0;
 		txvq->stats.broadcast = 0;
 		memset(txvq->stats.size_bins, 0,
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 4/8] net/virtio: remove unused Tx error counter

[Tiwei Bie]

On Thu, Jul 25, 2019 at 11:14:24AM +0200, David Marchand wrote:
> This Tx counter has never been used.
> 
> Fixes: 9658d17da27b ("virtio: maintain stats per queue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

[dpdk-dev] [PATCH 5/8] net/kni: do not count unsent packets as errors

[David Marchand]

err_pkts reflects the number of packets that the driver did not manage to
send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: 75e2bc54c018 ("net/kni: add KNI PMD")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/kni/rte_eth_kni.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 9e0c6bd..8026566 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -270,7 +270,6 @@ eth_kni_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	unsigned long rx_packets_total = 0, rx_bytes_total = 0;
 	unsigned long tx_packets_total = 0, tx_bytes_total = 0;
 	struct rte_eth_dev_data *data = dev->data;
-	unsigned long tx_packets_err_total = 0;
 	unsigned int i, num_stats;
 	struct pmd_queue *q;
 
@@ -292,14 +291,12 @@ eth_kni_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		stats->q_obytes[i] = q->tx.bytes;
 		tx_packets_total += stats->q_opackets[i];
 		tx_bytes_total += stats->q_obytes[i];
-		tx_packets_err_total += q->tx.err_pkts;
 	}
 
 	stats->ipackets = rx_packets_total;
 	stats->ibytes = rx_bytes_total;
 	stats->opackets = tx_packets_total;
 	stats->obytes = tx_bytes_total;
-	stats->oerrors = tx_packets_err_total;
 
 	return 0;
 }
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 5/8] net/kni: do not count unsent packets as errors

[Ferruh Yigit]

On 7/25/2019 10:14 AM, David Marchand wrote:
> err_pkts reflects the number of packets that the driver did not manage to
> send.
> This is a temporary situation, those packets are not freed and the
> application can still retry to send them later.
> Hence, we can't count them as transmit failed.

'err_pkts' seems calculated wrong both in Rx & Tx and KNI PMD, looks like we can
remove it completely, what do you think?

> 
> Fixes: 75e2bc54c018 ("net/kni: add KNI PMD")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/kni/rte_eth_kni.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
> index 9e0c6bd..8026566 100644
> --- a/drivers/net/kni/rte_eth_kni.c
> +++ b/drivers/net/kni/rte_eth_kni.c
> @@ -270,7 +270,6 @@ eth_kni_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  	unsigned long rx_packets_total = 0, rx_bytes_total = 0;
>  	unsigned long tx_packets_total = 0, tx_bytes_total = 0;
>  	struct rte_eth_dev_data *data = dev->data;
> -	unsigned long tx_packets_err_total = 0;
>  	unsigned int i, num_stats;
>  	struct pmd_queue *q;
>  
> @@ -292,14 +291,12 @@ eth_kni_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  		stats->q_obytes[i] = q->tx.bytes;
>  		tx_packets_total += stats->q_opackets[i];
>  		tx_bytes_total += stats->q_obytes[i];
> -		tx_packets_err_total += q->tx.err_pkts;
>  	}
>  
>  	stats->ipackets = rx_packets_total;
>  	stats->ibytes = rx_bytes_total;
>  	stats->opackets = tx_packets_total;
>  	stats->obytes = tx_bytes_total;
> -	stats->oerrors = tx_packets_err_total;
>  
>  	return 0;
>  }
> 

Re: [dpdk-dev] [PATCH 5/8] net/kni: do not count unsent packets as errors

[David Marchand]

On Thu, Jul 25, 2019 at 6:12 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 7/25/2019 10:14 AM, David Marchand wrote:
> > err_pkts reflects the number of packets that the driver did not manage to
> > send.
> > This is a temporary situation, those packets are not freed and the
> > application can still retry to send them later.
> > Hence, we can't count them as transmit failed.
>
> 'err_pkts' seems calculated wrong both in Rx & Tx and KNI PMD, looks like we can
> remove it completely, what do you think?

On the rx side, I agree that err_pkts makes little sense.
On the tx side, it can be seen as a "tx full" counter.

So not sure about the tx side, but I sure can remove the rx part (in a
separate patch?).


-- 
David Marchand

Re: [dpdk-dev] [PATCH 5/8] net/kni: do not count unsent packets as errors

[David Marchand]

On Thu, Jul 25, 2019 at 9:07 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Thu, Jul 25, 2019 at 6:12 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >
> > On 7/25/2019 10:14 AM, David Marchand wrote:
> > > err_pkts reflects the number of packets that the driver did not manage to
> > > send.
> > > This is a temporary situation, those packets are not freed and the
> > > application can still retry to send them later.
> > > Hence, we can't count them as transmit failed.
> >
> > 'err_pkts' seems calculated wrong both in Rx & Tx and KNI PMD, looks like we can
> > remove it completely, what do you think?
>
> On the rx side, I agree that err_pkts makes little sense.
> On the tx side, it can be seen as a "tx full" counter.
>
> So not sure about the tx side, but I sure can remove the rx part (in a
> separate patch?).

On second thought, we have nothing to report "tx full" for now (unless
adding xstats support but it is too late for 19.08).
I will go with dropping those bits if you are still okay with this
(and idem with the others drivers for which you commented in the same
way).


-- 
David Marchand

Re: [dpdk-dev] [PATCH 5/8] net/kni: do not count unsent packets as errors

[Ferruh Yigit]

On 7/26/2019 8:24 AM, David Marchand wrote:
> On Thu, Jul 25, 2019 at 9:07 PM David Marchand
> <david.marchand@redhat.com> wrote:
>>
>> On Thu, Jul 25, 2019 at 6:12 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>
>>> On 7/25/2019 10:14 AM, David Marchand wrote:
>>>> err_pkts reflects the number of packets that the driver did not manage to
>>>> send.
>>>> This is a temporary situation, those packets are not freed and the
>>>> application can still retry to send them later.
>>>> Hence, we can't count them as transmit failed.
>>>
>>> 'err_pkts' seems calculated wrong both in Rx & Tx and KNI PMD, looks like we can
>>> remove it completely, what do you think?
>>
>> On the rx side, I agree that err_pkts makes little sense.
>> On the tx side, it can be seen as a "tx full" counter.
>>
>> So not sure about the tx side, but I sure can remove the rx part (in a
>> separate patch?).
> 
> On second thought, we have nothing to report "tx full" for now (unless
> adding xstats support but it is too late for 19.08).
> I will go with dropping those bits if you are still okay with this

+1 to drop, thanks.

> (and idem with the others drivers for which you commented in the same
> way).
> 
> 

[dpdk-dev] [PATCH 6/8] net/memif: do not count unsent packets as errors

[David Marchand]

n_err reflects the number of packets that the driver did not manage to
send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/memif/rte_eth_memif.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 00c9b39..080729a 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -938,7 +938,6 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->ibytes = 0;
 	stats->opackets = 0;
 	stats->obytes = 0;
-	stats->oerrors = 0;
 
 	tmp = (pmd->role == MEMIF_ROLE_SLAVE) ? pmd->run.num_s2m_rings :
 	    pmd->run.num_m2s_rings;
@@ -966,7 +965,6 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		stats->q_obytes[i] = mq->n_bytes;
 		stats->opackets += mq->n_pkts;
 		stats->obytes += mq->n_bytes;
-		stats->oerrors += mq->n_err;
 	}
 	return 0;
 }
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 6/8] net/memif: do not count unsent packets as errors

[Ferruh Yigit]

On 7/25/2019 10:14 AM, David Marchand wrote:
> n_err reflects the number of packets that the driver did not manage to
> send.
> This is a temporary situation, those packets are not freed and the
> application can still retry to send them later.
> Hence, we can't count them as transmit failed.
> 
> Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/memif/rte_eth_memif.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
> index 00c9b39..080729a 100644
> --- a/drivers/net/memif/rte_eth_memif.c
> +++ b/drivers/net/memif/rte_eth_memif.c
> @@ -938,7 +938,6 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  	stats->ibytes = 0;
>  	stats->opackets = 0;
>  	stats->obytes = 0;
> -	stats->oerrors = 0;
>  
>  	tmp = (pmd->role == MEMIF_ROLE_SLAVE) ? pmd->run.num_s2m_rings :
>  	    pmd->run.num_m2s_rings;
> @@ -966,7 +965,6 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  		stats->q_obytes[i] = mq->n_bytes;
>  		stats->opackets += mq->n_pkts;
>  		stats->obytes += mq->n_bytes;
> -		stats->oerrors += mq->n_err;

Can we also delete 'n_err', it seems calculated wrong already?

>  	}
>  	return 0;
>  }
> 

[dpdk-dev] [PATCH 7/8] net/ring: do not count unsent packets as errors

[David Marchand]

err_pkts reflects the number of packets that the driver did not manage
to send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: e1e4017751f1 ("ring: add new driver")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/ring/rte_eth_ring.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index eb347bc..eef7606 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -172,7 +172,7 @@ static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned int i;
-	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
+	unsigned long rx_total = 0, tx_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
@@ -185,12 +185,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 			i < dev->data->nb_tx_queues; i++) {
 		stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
 		tx_total += stats->q_opackets[i];
-		tx_err_total += internal->tx_ring_queues[i].err_pkts.cnt;
 	}
 
 	stats->ipackets = rx_total;
 	stats->opackets = tx_total;
-	stats->oerrors = tx_err_total;
 
 	return 0;
 }
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 7/8] net/ring: do not count unsent packets as errors

[Ferruh Yigit]

On 7/25/2019 10:14 AM, David Marchand wrote:
> err_pkts reflects the number of packets that the driver did not manage
> to send.
> This is a temporary situation, those packets are not freed and the
> application can still retry to send them later.
> Hence, we can't count them as transmit failed.
> 
> Fixes: e1e4017751f1 ("ring: add new driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/ring/rte_eth_ring.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
> index eb347bc..eef7606 100644
> --- a/drivers/net/ring/rte_eth_ring.c
> +++ b/drivers/net/ring/rte_eth_ring.c
> @@ -172,7 +172,7 @@ static int
>  eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  {
>  	unsigned int i;
> -	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
> +	unsigned long rx_total = 0, tx_total = 0;
>  	const struct pmd_internals *internal = dev->data->dev_private;
>  
>  	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
> @@ -185,12 +185,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  			i < dev->data->nb_tx_queues; i++) {
>  		stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
>  		tx_total += stats->q_opackets[i];
> -		tx_err_total += internal->tx_ring_queues[i].err_pkts.cnt;

Can we remove 'err_pkts' completely, it seems already calculated wrong?

>  	}
>  
>  	stats->ipackets = rx_total;
>  	stats->opackets = tx_total;
> -	stats->oerrors = tx_err_total;
>  
>  	return 0;
>  }
> 

[dpdk-dev] [PATCH 8/8] net/vhost: do not count unsent packets as errors

[David Marchand]

missed_pkts reflects the number of packets that the driver did not manage
to send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: 5f05e95cd5d9 ("net/vhost: fix Tx error counting")
Fixes: ee584e9710b9 ("vhost: add driver on top of the library")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 57f382c..a4892d7 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1081,7 +1081,7 @@ static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned i;
-	unsigned long rx_total = 0, tx_total = 0, tx_missed_total = 0;
+	unsigned long rx_total = 0, tx_total = 0;
 	unsigned long rx_total_bytes = 0, tx_total_bytes = 0;
 	struct vhost_queue *vq;
 
@@ -1103,7 +1103,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 			continue;
 		vq = dev->data->tx_queues[i];
 		stats->q_opackets[i] = vq->stats.pkts;
-		tx_missed_total += vq->stats.missed_pkts;
 		tx_total += stats->q_opackets[i];
 
 		stats->q_obytes[i] = vq->stats.bytes;
@@ -1112,7 +1111,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	stats->ipackets = rx_total;
 	stats->opackets = tx_total;
-	stats->oerrors = tx_missed_total;
 	stats->ibytes = rx_total_bytes;
 	stats->obytes = tx_total_bytes;
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 8/8] net/vhost: do not count unsent packets as errors

[Tiwei Bie]

On Thu, Jul 25, 2019 at 11:14:28AM +0200, David Marchand wrote:
> missed_pkts reflects the number of packets that the driver did not manage
> to send.
> This is a temporary situation, those packets are not freed and the
> application can still retry to send them later.
> Hence, we can't count them as transmit failed.
> 
> Fixes: 5f05e95cd5d9 ("net/vhost: fix Tx error counting")
> Fixes: ee584e9710b9 ("vhost: add driver on top of the library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

[dpdk-dev] [PATCH v2 0/9] oerrors stats fixes for virtual pmds

[David Marchand]

I did a pass on the pmds about oerrors some time ago, but still did not
find the motivation to finish yet.
For now, here is a series of cleanups on the virtual pmds.

The former patches are just about removing counters that adds nothing to
the stats.

The latter patches make sure that unsent packets (because of a temporary
situation) are not added to oerrors since an application can decide to
send them again later.

Wrt oerrors, the pcap pmd should be fixed with the series [1].
But the tap pmd still needs to be inspected.

1: https://patchwork.dpdk.org/project/dpdk/list/?series=5701

Changelog since v1:
- added a patch on kni that drops a Rx counter
- dropped the Tx counter for kni, memif and ring

-- 
David Marchand

David Marchand (9):
  net/af_packet: remove unused Rx counter
  net/af_xdp: remove unused Tx counter
  net/null: remove unused Tx error counter
  net/virtio: remove unused Tx error counter
  net/kni: remove unused Rx "error" counter
  net/kni: do not count unsent packets as errors
  net/memif: do not count unsent packets as errors
  net/ring: do not count unsent packets as errors
  net/vhost: do not count unsent packets as errors

 drivers/net/af_packet/rte_eth_af_packet.c |  1 -
 drivers/net/af_xdp/rte_eth_af_xdp.c       |  2 --
 drivers/net/kni/rte_eth_kni.c             |  7 -------
 drivers/net/memif/rte_eth_memif.c         |  7 -------
 drivers/net/memif/rte_eth_memif.h         |  1 -
 drivers/net/null/rte_eth_null.c           |  9 ++-------
 drivers/net/ring/rte_eth_ring.c           | 16 ++++------------
 drivers/net/vhost/rte_eth_vhost.c         |  4 +---
 drivers/net/virtio/virtio_ethdev.c        |  3 ---
 9 files changed, 7 insertions(+), 43 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 1/9] net/af_packet: remove unused Rx counter

[David Marchand]

This Rx counter has never been used.

Fixes: 364e08f2bbc0 ("af_packet: add PMD for AF_PACKET-based virtual devices")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/af_packet/rte_eth_af_packet.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 2f6508f..82bf2cd 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -51,7 +51,6 @@ struct pkt_rx_queue {
 	uint16_t in_port;
 
 	volatile unsigned long rx_pkts;
-	volatile unsigned long err_pkts;
 	volatile unsigned long rx_bytes;
 };
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/9] net/af_xdp: remove unused Tx counter

[David Marchand]

This Tx counter is now unused.

Fixes: 10edf857fde4 ("net/af_xdp: make reserve/submit peek/release consistent")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 33352e1..41ed5b2 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -97,7 +97,6 @@ struct pkt_rx_queue {
 
 struct tx_stats {
 	uint64_t tx_pkts;
-	uint64_t err_pkts;
 	uint64_t tx_bytes;
 };
 
@@ -456,7 +455,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		stats->imissed += xdp_stats.rx_dropped;
 
 		stats->opackets += stats->q_opackets[i];
-		stats->oerrors += txq->stats.err_pkts;
 		stats->obytes += stats->q_obytes[i];
 	}
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 3/9] net/null: remove unused Tx error counter

[David Marchand]

This Tx counter has never been used.

Fixes: c743e50c475f ("null: new poll mode driver")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/null/rte_eth_null.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 31cbb84..b2c92ab 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -62,7 +62,6 @@ struct null_queue {
 
 	rte_atomic64_t rx_pkts;
 	rte_atomic64_t tx_pkts;
-	rte_atomic64_t err_pkts;
 };
 
 struct pmd_internals {
@@ -311,7 +310,7 @@ static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 {
 	unsigned i, num_stats;
-	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
+	unsigned long rx_total = 0, tx_total = 0;
 	const struct pmd_internals *internal;
 
 	if ((dev == NULL) || (igb_stats == NULL))
@@ -334,12 +333,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 		igb_stats->q_opackets[i] =
 			internal->tx_null_queues[i].tx_pkts.cnt;
 		tx_total += igb_stats->q_opackets[i];
-		tx_err_total += internal->tx_null_queues[i].err_pkts.cnt;
 	}
 
 	igb_stats->ipackets = rx_total;
 	igb_stats->opackets = tx_total;
-	igb_stats->oerrors = tx_err_total;
 
 	return 0;
 }
@@ -356,10 +353,8 @@ eth_stats_reset(struct rte_eth_dev *dev)
 	internal = dev->data->dev_private;
 	for (i = 0; i < RTE_DIM(internal->rx_null_queues); i++)
 		internal->rx_null_queues[i].rx_pkts.cnt = 0;
-	for (i = 0; i < RTE_DIM(internal->tx_null_queues); i++) {
+	for (i = 0; i < RTE_DIM(internal->tx_null_queues); i++)
 		internal->tx_null_queues[i].tx_pkts.cnt = 0;
-		internal->tx_null_queues[i].err_pkts.cnt = 0;
-	}
 }
 
 static void
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 4/9] net/virtio: remove unused Tx error counter

[David Marchand]

This Tx counter has never been used.

Fixes: 9658d17da27b ("virtio: maintain stats per queue")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 62c8274..20840bf 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -121,7 +121,6 @@ static const struct rte_virtio_xstats_name_off rte_virtio_rxq_stat_strings[] = {
 static const struct rte_virtio_xstats_name_off rte_virtio_txq_stat_strings[] = {
 	{"good_packets",           offsetof(struct virtnet_tx, stats.packets)},
 	{"good_bytes",             offsetof(struct virtnet_tx, stats.bytes)},
-	{"errors",                 offsetof(struct virtnet_tx, stats.errors)},
 	{"multicast_packets",      offsetof(struct virtnet_tx, stats.multicast)},
 	{"broadcast_packets",      offsetof(struct virtnet_tx, stats.broadcast)},
 	{"undersize_packets",      offsetof(struct virtnet_tx, stats.size_bins[0])},
@@ -944,7 +943,6 @@ virtio_update_stats(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 		stats->opackets += txvq->stats.packets;
 		stats->obytes += txvq->stats.bytes;
-		stats->oerrors += txvq->stats.errors;
 
 		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
 			stats->q_opackets[i] = txvq->stats.packets;
@@ -1082,7 +1080,6 @@ virtio_dev_stats_reset(struct rte_eth_dev *dev)
 
 		txvq->stats.packets = 0;
 		txvq->stats.bytes = 0;
-		txvq->stats.errors = 0;
 		txvq->stats.multicast = 0;
 		txvq->stats.broadcast = 0;
 		memset(txvq->stats.size_bins, 0,
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 5/9] net/kni: remove unused Rx "error" counter

[David Marchand]

The delta between what the application asked to receive and what was
indeed received, can not be called an error counter.
This counter is not reported anywhere, remove it.

Fixes: 75e2bc54c018 ("net/kni: add KNI PMD")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/kni/rte_eth_kni.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 9e0c6bd..884280c 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -83,7 +83,6 @@ eth_kni_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	nb_pkts = rte_kni_rx_burst(kni, bufs, nb_bufs);
 
 	kni_q->rx.pkts += nb_pkts;
-	kni_q->rx.err_pkts += nb_bufs - nb_pkts;
 
 	return nb_pkts;
 }
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 5/9] net/kni: remove unused Rx "error" counter

[Ferruh Yigit]

On 7/26/2019 11:21 AM, David Marchand wrote:
> The delta between what the application asked to receive and what was
> indeed received, can not be called an error counter.
> This counter is not reported anywhere, remove it.
> 
> Fixes: 75e2bc54c018 ("net/kni: add KNI PMD")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v2 6/9] net/kni: do not count unsent packets as errors

[David Marchand]

err_pkts reflects the number of packets that the driver did not manage to
send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: 75e2bc54c018 ("net/kni: add KNI PMD")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- dropped the err_pkts counter entirely as nothing reports it

---
 drivers/net/kni/rte_eth_kni.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 884280c..515c0aa 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -35,7 +35,6 @@ struct eth_kni_args {
 struct pmd_queue_stats {
 	uint64_t pkts;
 	uint64_t bytes;
-	uint64_t err_pkts;
 };
 
 struct pmd_queue {
@@ -97,7 +96,6 @@ eth_kni_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	nb_pkts =  rte_kni_tx_burst(kni, bufs, nb_bufs);
 
 	kni_q->tx.pkts += nb_pkts;
-	kni_q->tx.err_pkts += nb_bufs - nb_pkts;
 
 	return nb_pkts;
 }
@@ -269,7 +267,6 @@ eth_kni_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	unsigned long rx_packets_total = 0, rx_bytes_total = 0;
 	unsigned long tx_packets_total = 0, tx_bytes_total = 0;
 	struct rte_eth_dev_data *data = dev->data;
-	unsigned long tx_packets_err_total = 0;
 	unsigned int i, num_stats;
 	struct pmd_queue *q;
 
@@ -291,14 +288,12 @@ eth_kni_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		stats->q_obytes[i] = q->tx.bytes;
 		tx_packets_total += stats->q_opackets[i];
 		tx_bytes_total += stats->q_obytes[i];
-		tx_packets_err_total += q->tx.err_pkts;
 	}
 
 	stats->ipackets = rx_packets_total;
 	stats->ibytes = rx_bytes_total;
 	stats->opackets = tx_packets_total;
 	stats->obytes = tx_bytes_total;
-	stats->oerrors = tx_packets_err_total;
 
 	return 0;
 }
@@ -319,7 +314,6 @@ eth_kni_stats_reset(struct rte_eth_dev *dev)
 		q = data->tx_queues[i];
 		q->tx.pkts = 0;
 		q->tx.bytes = 0;
-		q->tx.err_pkts = 0;
 	}
 }
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 6/9] net/kni: do not count unsent packets as errors

[Ferruh Yigit]

On 7/26/2019 11:21 AM, David Marchand wrote:
> err_pkts reflects the number of packets that the driver did not manage to
> send.
> This is a temporary situation, those packets are not freed and the
> application can still retry to send them later.
> Hence, we can't count them as transmit failed.
> 
> Fixes: 75e2bc54c018 ("net/kni: add KNI PMD")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v2 7/9] net/memif: do not count unsent packets as errors

[David Marchand]

n_err reflects the number of packets that the driver did not manage to
send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- dropped the n_err counter entirely as nothing reports it

---
 drivers/net/memif/rte_eth_memif.c | 7 -------
 drivers/net/memif/rte_eth_memif.h | 1 -
 2 files changed, 8 deletions(-)

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 00c9b39..bcda426 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -479,7 +479,6 @@ no_free_slots:
 		}
 	}
 
-	mq->n_err += nb_pkts - n_tx_pkts;
 	mq->n_pkts += n_tx_pkts;
 	return n_tx_pkts;
 }
@@ -857,7 +856,6 @@ memif_tx_queue_setup(struct rte_eth_dev *dev,
 	    (pmd->role == MEMIF_ROLE_SLAVE) ? MEMIF_RING_S2M : MEMIF_RING_M2S;
 	mq->n_pkts = 0;
 	mq->n_bytes = 0;
-	mq->n_err = 0;
 	mq->intr_handle.fd = -1;
 	mq->intr_handle.type = RTE_INTR_HANDLE_EXT;
 	dev->data->tx_queues[qid] = mq;
@@ -886,7 +884,6 @@ memif_rx_queue_setup(struct rte_eth_dev *dev,
 	mq->type = (pmd->role == MEMIF_ROLE_SLAVE) ? MEMIF_RING_M2S : MEMIF_RING_S2M;
 	mq->n_pkts = 0;
 	mq->n_bytes = 0;
-	mq->n_err = 0;
 	mq->intr_handle.fd = -1;
 	mq->intr_handle.type = RTE_INTR_HANDLE_EXT;
 	mq->mempool = mb_pool;
@@ -938,7 +935,6 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->ibytes = 0;
 	stats->opackets = 0;
 	stats->obytes = 0;
-	stats->oerrors = 0;
 
 	tmp = (pmd->role == MEMIF_ROLE_SLAVE) ? pmd->run.num_s2m_rings :
 	    pmd->run.num_m2s_rings;
@@ -966,7 +962,6 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		stats->q_obytes[i] = mq->n_bytes;
 		stats->opackets += mq->n_pkts;
 		stats->obytes += mq->n_bytes;
-		stats->oerrors += mq->n_err;
 	}
 	return 0;
 }
@@ -983,14 +978,12 @@ memif_stats_reset(struct rte_eth_dev *dev)
 		    dev->data->rx_queues[i];
 		mq->n_pkts = 0;
 		mq->n_bytes = 0;
-		mq->n_err = 0;
 	}
 	for (i = 0; i < pmd->run.num_m2s_rings; i++) {
 		mq = (pmd->role == MEMIF_ROLE_SLAVE) ? dev->data->rx_queues[i] :
 		    dev->data->tx_queues[i];
 		mq->n_pkts = 0;
 		mq->n_bytes = 0;
-		mq->n_err = 0;
 	}
 }
 
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 24e8a09..8269212 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -66,7 +66,6 @@ struct memif_queue {
 	/* rx/tx info */
 	uint64_t n_pkts;			/**< number of rx/tx packets */
 	uint64_t n_bytes;			/**< number of rx/tx bytes */
-	uint64_t n_err;				/**< number of tx errors */
 
 	memif_ring_t *ring;			/**< pointer to ring */
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 7/9] net/memif: do not count unsent packets as errors

[Ferruh Yigit]

On 7/26/2019 11:21 AM, David Marchand wrote:
> n_err reflects the number of packets that the driver did not manage to
> send.
> This is a temporary situation, those packets are not freed and the
> application can still retry to send them later.
> Hence, we can't count them as transmit failed.
> 
> Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v2 8/9] net/ring: do not count unsent packets as errors

[David Marchand]

err_pkts reflects the number of packets that the driver did not manage
to send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: e1e4017751f1 ("ring: add new driver")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- dropped the err_pkts counter entirely as nothing reports it

---
 drivers/net/ring/rte_eth_ring.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index eb347bc..634da63 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -41,7 +41,6 @@ struct ring_queue {
 	struct rte_ring *rng;
 	rte_atomic64_t rx_pkts;
 	rte_atomic64_t tx_pkts;
-	rte_atomic64_t err_pkts;
 };
 
 struct pmd_internals {
@@ -89,13 +88,10 @@ eth_ring_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	struct ring_queue *r = q;
 	const uint16_t nb_tx = (uint16_t)rte_ring_enqueue_burst(r->rng,
 			ptrs, nb_bufs, NULL);
-	if (r->rng->flags & RING_F_SP_ENQ) {
+	if (r->rng->flags & RING_F_SP_ENQ)
 		r->tx_pkts.cnt += nb_tx;
-		r->err_pkts.cnt += nb_bufs - nb_tx;
-	} else {
+	else
 		rte_atomic64_add(&(r->tx_pkts), nb_tx);
-		rte_atomic64_add(&(r->err_pkts), nb_bufs - nb_tx);
-	}
 	return nb_tx;
 }
 
@@ -172,7 +168,7 @@ static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned int i;
-	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
+	unsigned long rx_total = 0, tx_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
@@ -185,12 +181,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 			i < dev->data->nb_tx_queues; i++) {
 		stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
 		tx_total += stats->q_opackets[i];
-		tx_err_total += internal->tx_ring_queues[i].err_pkts.cnt;
 	}
 
 	stats->ipackets = rx_total;
 	stats->opackets = tx_total;
-	stats->oerrors = tx_err_total;
 
 	return 0;
 }
@@ -203,10 +197,8 @@ eth_stats_reset(struct rte_eth_dev *dev)
 
 	for (i = 0; i < dev->data->nb_rx_queues; i++)
 		internal->rx_ring_queues[i].rx_pkts.cnt = 0;
-	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++)
 		internal->tx_ring_queues[i].tx_pkts.cnt = 0;
-		internal->tx_ring_queues[i].err_pkts.cnt = 0;
-	}
 }
 
 static void
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 8/9] net/ring: do not count unsent packets as errors

[Ferruh Yigit]

On 7/26/2019 11:21 AM, David Marchand wrote:
> err_pkts reflects the number of packets that the driver did not manage
> to send.
> This is a temporary situation, those packets are not freed and the
> application can still retry to send them later.
> Hence, we can't count them as transmit failed.
> 
> Fixes: e1e4017751f1 ("ring: add new driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v2 9/9] net/vhost: do not count unsent packets as errors

[David Marchand]

missed_pkts reflects the number of packets that the driver did not manage
to send.
This is a temporary situation, those packets are not freed and the
application can still retry to send them later.
Hence, we can't count them as transmit failed.

Fixes: 5f05e95cd5d9 ("net/vhost: fix Tx error counting")
Fixes: ee584e9710b9 ("vhost: add driver on top of the library")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 57f382c..a4892d7 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1081,7 +1081,7 @@ static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned i;
-	unsigned long rx_total = 0, tx_total = 0, tx_missed_total = 0;
+	unsigned long rx_total = 0, tx_total = 0;
 	unsigned long rx_total_bytes = 0, tx_total_bytes = 0;
 	struct vhost_queue *vq;
 
@@ -1103,7 +1103,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 			continue;
 		vq = dev->data->tx_queues[i];
 		stats->q_opackets[i] = vq->stats.pkts;
-		tx_missed_total += vq->stats.missed_pkts;
 		tx_total += stats->q_opackets[i];
 
 		stats->q_obytes[i] = vq->stats.bytes;
@@ -1112,7 +1111,6 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	stats->ipackets = rx_total;
 	stats->opackets = tx_total;
-	stats->oerrors = tx_missed_total;
 	stats->ibytes = rx_total_bytes;
 	stats->obytes = tx_total_bytes;
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 0/9] oerrors stats fixes for virtual pmds

[Ferruh Yigit]

On 7/26/2019 11:21 AM, David Marchand wrote:
> I did a pass on the pmds about oerrors some time ago, but still did not
> find the motivation to finish yet.
> For now, here is a series of cleanups on the virtual pmds.
> 
> The former patches are just about removing counters that adds nothing to
> the stats.
> 
> The latter patches make sure that unsent packets (because of a temporary
> situation) are not added to oerrors since an application can decide to
> send them again later.
> 
> Wrt oerrors, the pcap pmd should be fixed with the series [1].
> But the tap pmd still needs to be inspected.
> 
> 1: https://patchwork.dpdk.org/project/dpdk/list/?series=5701
> 
> Changelog since v1:
> - added a patch on kni that drops a Rx counter
> - dropped the Tx counter for kni, memif and ring
> 

Series applied to dpdk-next-net/master, thanks.