[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-22 Thread Tahhan, Maryam 
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, March 17, 2016 4:41 PM
> To: Igor Ryzhov 
> Cc: dev at dpdk.org; Tahhan, Maryam ;
> olivier.matz at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH] ethdev: don't count missed packets in
> erroneous packets counter
> 
> CC Maryam and Olivier who had discussions about imissed and other
> stats:
>   http://dpdk.org/ml/archives/dev/2015-August/022905.html
>   http://dpdk.org/ml/archives/dev/2015-September/023351.html
>   http://dpdk.org/ml/archives/dev/2015-September/023612.html
> 
> 2016-03-10 16:03, Igor Ryzhov:
> > Comment for "ierrors" counter says that it counts erroneous received
> packets. But for some reason "imissed" counter is added to "ierrors"
> counter in most drivers. It is a mistake, because missed packets are
> obviously not received. This patch fixes it.
> 
> According to this patch
>   http://dpdk.org/browse/dpdk/commit/?id=70bdb186
> imissed was kept in ierrors because of backward compatibility.
> I'm OK to remove imissed from ierrors.
> 
> Fixes: 70bdb18657da ("ethdev: add Rx error counters for missed, badcrc
> and badlen packets")
> Fixes: 6bfe648406b5 ("i40e: add Rx error statistics")
> Fixes: 856505d303f4 ("cxgbe: add port statistics")
> 
> Acked-by: Thomas Monjalon 

Looks fine, but make sure to add an explicit comment in release notes somewhere 
to flag the change. In case any apps were accounting for imissed as part of 
ierrors like testpmd was: 

-   if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
{
+   if ((stats->ierrors + stats->rx_nombuf) > 0) {
printf("  RX-error:%"PRIu64"\n", stats->ierrors);
printf("  RX-nombufs: %14"PRIu64"\n",
   stats->rx_nombuf);

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-22 Thread Stephen Hemminger 
On Tue, 22 Mar 2016 15:23:22 +
"Tahhan, Maryam"  wrote:

> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > Sent: Thursday, March 17, 2016 4:41 PM
> > To: Igor Ryzhov 
> > Cc: dev at dpdk.org; Tahhan, Maryam ;
> > olivier.matz at 6wind.com
> > Subject: Re: [dpdk-dev] [PATCH] ethdev: don't count missed packets in
> > erroneous packets counter
> > 
> > CC Maryam and Olivier who had discussions about imissed and other
> > stats:
> > http://dpdk.org/ml/archives/dev/2015-August/022905.html
> > http://dpdk.org/ml/archives/dev/2015-September/023351.html
> > http://dpdk.org/ml/archives/dev/2015-September/023612.html
> > 
> > 2016-03-10 16:03, Igor Ryzhov:  
> > > Comment for "ierrors" counter says that it counts erroneous received  
> > packets. But for some reason "imissed" counter is added to "ierrors"
> > counter in most drivers. It is a mistake, because missed packets are
> > obviously not received. This patch fixes it.
> > 
> > According to this patch
> > http://dpdk.org/browse/dpdk/commit/?id=70bdb186
> > imissed was kept in ierrors because of backward compatibility.
> > I'm OK to remove imissed from ierrors.
> > 
> > Fixes: 70bdb18657da ("ethdev: add Rx error counters for missed, badcrc
> > and badlen packets")
> > Fixes: 6bfe648406b5 ("i40e: add Rx error statistics")
> > Fixes: 856505d303f4 ("cxgbe: add port statistics")
> > 
> > Acked-by: Thomas Monjalon   
> 
> Looks fine, but make sure to add an explicit comment in release notes 
> somewhere to flag the change. In case any apps were accounting for imissed as 
> part of ierrors like testpmd was: 
> 
> - if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
> {
> + if ((stats->ierrors + stats->rx_nombuf) > 0) {

Extra () in that expression.

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-22 Thread Thomas Monjalon 
2016-03-17 17:40, Thomas Monjalon:
> CC Maryam and Olivier who had discussions about imissed and other stats:
>   http://dpdk.org/ml/archives/dev/2015-August/022905.html
>   http://dpdk.org/ml/archives/dev/2015-September/023351.html
>   http://dpdk.org/ml/archives/dev/2015-September/023612.html
> 
> 2016-03-10 16:03, Igor Ryzhov:
> > Comment for "ierrors" counter says that it counts erroneous received 
> > packets. But for some reason "imissed" counter is added to "ierrors" 
> > counter in most drivers. It is a mistake, because missed packets are 
> > obviously not received. This patch fixes it.
> 
> According to this patch
>   http://dpdk.org/browse/dpdk/commit/?id=70bdb186
> imissed was kept in ierrors because of backward compatibility.
> I'm OK to remove imissed from ierrors.
> 
> Fixes: 70bdb18657da ("ethdev: add Rx error counters for missed, badcrc and 
> badlen packets")
> Fixes: 6bfe648406b5 ("i40e: add Rx error statistics")
> Fixes: 856505d303f4 ("cxgbe: add port statistics")
> 
> Acked-by: Thomas Monjalon 

Applied, thanks

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-17 Thread Thomas Monjalon 
CC Maryam and Olivier who had discussions about imissed and other stats:
http://dpdk.org/ml/archives/dev/2015-August/022905.html
http://dpdk.org/ml/archives/dev/2015-September/023351.html
http://dpdk.org/ml/archives/dev/2015-September/023612.html

2016-03-10 16:03, Igor Ryzhov:
> Comment for "ierrors" counter says that it counts erroneous received packets. 
> But for some reason "imissed" counter is added to "ierrors" counter in most 
> drivers. It is a mistake, because missed packets are obviously not received. 
> This patch fixes it.

According to this patch
http://dpdk.org/browse/dpdk/commit/?id=70bdb186
imissed was kept in ierrors because of backward compatibility.
I'm OK to remove imissed from ierrors.

Fixes: 70bdb18657da ("ethdev: add Rx error counters for missed, badcrc and 
badlen packets")
Fixes: 6bfe648406b5 ("i40e: add Rx error statistics")
Fixes: 856505d303f4 ("cxgbe: add port statistics")

Acked-by: Thomas Monjalon

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-17 Thread Rahul Lakkireddy 
On Thursday, March 03/10/16, 2016 at 16:03:30 +0300, Igor Ryzhov wrote:
> Comment for "ierrors" counter says that it counts erroneous received packets. 
> But for some reason "imissed" counter is added to "ierrors" counter in most 
> drivers. It is a mistake, because missed packets are obviously not received. 
> This patch fixes it.
> 
> Signed-off-by: Igor Ryzhov 
> ---

For the cxgbe part,
Acked-by: Rahul Lakkireddy

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-14 Thread Igor Ryzhov 
Ping.

CCing to maintainers of affected drivers.

> 10 ? 2016 ?., ? 16:03, Igor Ryzhov  ???(?):
> 
> Comment for "ierrors" counter says that it counts erroneous received packets. 
> But for some reason "imissed" counter is added to "ierrors" counter in most 
> drivers. It is a mistake, because missed packets are obviously not received. 
> This patch fixes it.
> 
> Signed-off-by: Igor Ryzhov 
> ---
> app/test-pmd/testpmd.c   | 4 ++--
> drivers/net/cxgbe/cxgbe_ethdev.c | 2 +-
> drivers/net/e1000/em_ethdev.c| 1 -
> drivers/net/e1000/igb_ethdev.c   | 1 -
> drivers/net/i40e/i40e_ethdev.c   | 3 +--
> drivers/net/ixgbe/ixgbe_ethdev.c | 1 -
> 6 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 269ef81..d3d733b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -753,7 +753,7 @@ fwd_port_stats_display(portid_t port_id, struct 
> rte_eth_stats *stats)
>   if (cur_fwd_eng == &csum_fwd_engine)
>   printf("  Bad-ipcsum: %-14"PRIu64" Bad-l4csum: 
> %-14"PRIu64" \n",
>  port->rx_bad_ip_csum, port->rx_bad_l4_csum);
> - if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
> {
> + if ((stats->ierrors + stats->rx_nombuf) > 0) {
>   printf("  RX-error: %-"PRIu64"\n",  stats->ierrors);
>   printf("  RX-nombufs: %-14"PRIu64"\n", 
> stats->rx_nombuf);
>   }
> @@ -772,7 +772,7 @@ fwd_port_stats_display(portid_t port_id, struct 
> rte_eth_stats *stats)
>   if (cur_fwd_eng == &csum_fwd_engine)
>   printf("  Bad-ipcsum:%14"PRIu64"
> Bad-l4csum:%14"PRIu64"\n",
>  port->rx_bad_ip_csum, port->rx_bad_l4_csum);
> - if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
> {
> + if ((stats->ierrors + stats->rx_nombuf) > 0) {
>   printf("  RX-error:%"PRIu64"\n", stats->ierrors);
>   printf("  RX-nombufs: %14"PRIu64"\n",
>  stats->rx_nombuf);
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c 
> b/drivers/net/cxgbe/cxgbe_ethdev.c
> index 97ef152..0070e2a 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -662,7 +662,7 @@ static void cxgbe_dev_stats_get(struct rte_eth_dev 
> *eth_dev,
> ps.rx_trunc2 + ps.rx_trunc3;
>   eth_stats->ierrors  = ps.rx_symbol_err + ps.rx_fcs_err +
> ps.rx_jabber + ps.rx_too_long + ps.rx_runt +
> -   ps.rx_len_err + eth_stats->imissed;
> +   ps.rx_len_err;
> 
>   /* TX Stats */
>   eth_stats->opackets = ps.tx_frames;
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 4a843fe..27ace6d 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -914,7 +914,6 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
> rte_eth_stats *rte_stats)
>   rte_stats->imissed = stats->mpc;
>   rte_stats->ierrors = stats->crcerrs +
>stats->rlec + stats->ruc + stats->roc +
> -  rte_stats->imissed +
>stats->rxerrc + stats->algnerrc + stats->cexterr;
> 
>   /* Tx Errors */
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 4ed5e95..6e93214 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -1640,7 +1640,6 @@ eth_igb_stats_get(struct rte_eth_dev *dev, struct 
> rte_eth_stats *rte_stats)
>   rte_stats->imissed = stats->mpc;
>   rte_stats->ierrors = stats->crcerrs +
>stats->rlec + stats->ruc + stats->roc +
> -  rte_stats->imissed +
>stats->rxerrc + stats->algnerrc + stats->cexterr;
> 
>   /* Tx Errors */
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7e68c61..7d68d4d 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2062,8 +2062,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct 
> rte_eth_stats *stats)
>   pf->main_vsi->eth_stats.rx_discards;
>   stats->ierrors  = ns->crc_errors +
>   ns->rx_length_errors + ns->rx_undersize +
> - ns->rx_oversize + ns->rx_fragments + ns->rx_jabber +
> - stats->imissed;
> + ns->rx_oversize + ns->rx_fragments + ns->rx_jabber;
> 
>   PMD_DRV_LOG(DEBUG, "* PF stats start 
> ***");
>   PMD_DRV_LOG(DEBUG, "rx_bytes:%"PRIu64"", ns->eth.rx_bytes);
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c 
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 3e6fe86..ba84544 100644
> --- a/drivers/

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-14 Thread Lu, Wenzhuo 
Hi,


> -Original Message-
> From: Igor Ryzhov [mailto:iryzhov at nfware.com]
> Sent: Monday, March 14, 2016 3:31 PM
> To: ? ?
> Cc: dev at dpdk.org; Rahul Lakkireddy; Lu, Wenzhuo; Zhang, Helin; Ananyev,
> Konstantin
> Subject: Re: [dpdk-dev] [PATCH] ethdev: don't count missed packets in
> erroneous packets counter
> 
> Ping.
> 
> CCing to maintainers of affected drivers.
> 
> > 10 ? 2016 ?., ? 16:03, Igor Ryzhov  ???(?):
> >
> > Comment for "ierrors" counter says that it counts erroneous received 
> > packets.
> But for some reason "imissed" counter is added to "ierrors" counter in most
> drivers. It is a mistake, because missed packets are obviously not received. 
> This
> patch fixes it.
> >
> > Signed-off-by: Igor Ryzhov 
Acked-by: Wenzhuo Lu 
Agree with Igor. As we have a counter for imissed, we need not to mix it with 
ierror.

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

2016-03-10 Thread Igor Ryzhov 
Comment for "ierrors" counter says that it counts erroneous received packets. 
But for some reason "imissed" counter is added to "ierrors" counter in most 
drivers. It is a mistake, because missed packets are obviously not received. 
This patch fixes it.

Signed-off-by: Igor Ryzhov 
---
 app/test-pmd/testpmd.c   | 4 ++--
 drivers/net/cxgbe/cxgbe_ethdev.c | 2 +-
 drivers/net/e1000/em_ethdev.c| 1 -
 drivers/net/e1000/igb_ethdev.c   | 1 -
 drivers/net/i40e/i40e_ethdev.c   | 3 +--
 drivers/net/ixgbe/ixgbe_ethdev.c | 1 -
 6 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 269ef81..d3d733b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -753,7 +753,7 @@ fwd_port_stats_display(portid_t port_id, struct 
rte_eth_stats *stats)
if (cur_fwd_eng == &csum_fwd_engine)
printf("  Bad-ipcsum: %-14"PRIu64" Bad-l4csum: 
%-14"PRIu64" \n",
   port->rx_bad_ip_csum, port->rx_bad_l4_csum);
-   if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
{
+   if ((stats->ierrors + stats->rx_nombuf) > 0) {
printf("  RX-error: %-"PRIu64"\n",  stats->ierrors);
printf("  RX-nombufs: %-14"PRIu64"\n", 
stats->rx_nombuf);
}
@@ -772,7 +772,7 @@ fwd_port_stats_display(portid_t port_id, struct 
rte_eth_stats *stats)
if (cur_fwd_eng == &csum_fwd_engine)
printf("  Bad-ipcsum:%14"PRIu64"
Bad-l4csum:%14"PRIu64"\n",
   port->rx_bad_ip_csum, port->rx_bad_l4_csum);
-   if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
{
+   if ((stats->ierrors + stats->rx_nombuf) > 0) {
printf("  RX-error:%"PRIu64"\n", stats->ierrors);
printf("  RX-nombufs: %14"PRIu64"\n",
   stats->rx_nombuf);
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 97ef152..0070e2a 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -662,7 +662,7 @@ static void cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
  ps.rx_trunc2 + ps.rx_trunc3;
eth_stats->ierrors  = ps.rx_symbol_err + ps.rx_fcs_err +
  ps.rx_jabber + ps.rx_too_long + ps.rx_runt +
- ps.rx_len_err + eth_stats->imissed;
+ ps.rx_len_err;

/* TX Stats */
eth_stats->opackets = ps.tx_frames;
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 4a843fe..27ace6d 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -914,7 +914,6 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
rte_stats->imissed = stats->mpc;
rte_stats->ierrors = stats->crcerrs +
 stats->rlec + stats->ruc + stats->roc +
-rte_stats->imissed +
 stats->rxerrc + stats->algnerrc + stats->cexterr;

/* Tx Errors */
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 4ed5e95..6e93214 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1640,7 +1640,6 @@ eth_igb_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
rte_stats->imissed = stats->mpc;
rte_stats->ierrors = stats->crcerrs +
 stats->rlec + stats->ruc + stats->roc +
-rte_stats->imissed +
 stats->rxerrc + stats->algnerrc + stats->cexterr;

/* Tx Errors */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7e68c61..7d68d4d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2062,8 +2062,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
pf->main_vsi->eth_stats.rx_discards;
stats->ierrors  = ns->crc_errors +
ns->rx_length_errors + ns->rx_undersize +
-   ns->rx_oversize + ns->rx_fragments + ns->rx_jabber +
-   stats->imissed;
+   ns->rx_oversize + ns->rx_fragments + ns->rx_jabber;

PMD_DRV_LOG(DEBUG, "* PF stats start 
***");
PMD_DRV_LOG(DEBUG, "rx_bytes:%"PRIu64"", ns->eth.rx_bytes);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 3e6fe86..ba84544 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2552,7 +2552,6 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
  hw_stats->rlec +