[dpdk-dev,v3] net/i40evf: fix reporting of imissed packets

Message ID 1481055381-14243-1-git-send-email-tcrugnale@sandvine.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
checkpatch/checkpatch success coding style OK

Commit Message

Tom Crugnale Dec. 6, 2016, 8:16 p.m. UTC
  Missed packets on RX were erroneously being assigned to the ierrors
struct member. Change it to be assigned to imissed.

Fixes: 4861cde4 ("i40e: new poll mode driver")

Signed-off-by: Tom Crugnale <tcrugnale@sandvine.com>
---
v3:
* Fixed coding style issues
v2:
* Fixed line breaks in original patch submission
---

 drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

Comments

Ferruh Yigit Dec. 7, 2016, 6:29 p.m. UTC | #1
On 12/6/2016 8:16 PM, Tom Crugnale wrote:
> Missed packets on RX were erroneously being assigned to the ierrors
> struct member. Change it to be assigned to imissed.
> 
> Fixes: 4861cde4 ("i40e: new poll mode driver")
> 
> Signed-off-by: Tom Crugnale <tcrugnale@sandvine.com>
> ---
> v3:
> * Fixed coding style issues
> v2:
> * Fixed line breaks in original patch submission
> ---
> 
>  drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index aa306d6..90876c8 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -952,7 +952,7 @@ struct rte_i40evf_xstats_name_off {
>  }
>  
>  static int
> -i40evf_get_statics(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
> +i40evf_get_stats(struct rte_eth_dev *dev, struct rte_eth_stats *stats)

I aware this is simple modification, but if you can, can you please make
this separate patch?
Stable trees may not interested in internal function name changes but
may want to get stats counter fix.

>  {
>  	int ret;
>  	struct i40e_eth_stats *pstats = NULL;
> @@ -965,7 +965,7 @@ struct rte_i40evf_xstats_name_off {
>  						pstats->rx_broadcast;
>  	stats->opackets = pstats->tx_broadcast + pstats->tx_multicast +
>  						pstats->tx_unicast;
> -	stats->ierrors = pstats->rx_discards;
> +	stats->imissed = pstats->rx_discards;

so won't VF driver report any rx_error?

>  	stats->oerrors = pstats->tx_errors + pstats->tx_discards;

This line is not part of the patch, but what do you think about this
one, should oerrors contain tx_discards values?

>  	stats->ibytes = pstats->rx_bytes;
>  	stats->obytes = pstats->tx_bytes;
> @@ -2277,8 +2277,8 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev,
>  static void
>  i40evf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  {
> -	if (i40evf_get_statics(dev, stats))
> -		PMD_DRV_LOG(ERR, "Get statics failed");
> +	if (i40evf_get_stats(dev, stats))
> +		PMD_DRV_LOG(ERR, "Get stats failed");
>  }
>  
>  static void
>
  
Jingjing Wu Feb. 7, 2017, 3:35 p.m. UTC | #2
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, December 8, 2016 2:30 AM
> To: Tom Crugnale <tcrugnale@sandvine.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] net/i40evf: fix reporting of imissed packets
> 
> On 12/6/2016 8:16 PM, Tom Crugnale wrote:
> > Missed packets on RX were erroneously being assigned to the ierrors
> > struct member. Change it to be assigned to imissed.
> >
> > Fixes: 4861cde4 ("i40e: new poll mode driver")
> >
> > Signed-off-by: Tom Crugnale <tcrugnale@sandvine.com>
> > ---
> > v3:
> > * Fixed coding style issues
> > v2:
> > * Fixed line breaks in original patch submission
> > ---
> >
> >  drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index aa306d6..90876c8 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -952,7 +952,7 @@ struct rte_i40evf_xstats_name_off {  }
> >
> >  static int
> > -i40evf_get_statics(struct rte_eth_dev *dev, struct rte_eth_stats
> > *stats)
> > +i40evf_get_stats(struct rte_eth_dev *dev, struct rte_eth_stats
> > +*stats)
> 
> I aware this is simple modification, but if you can, can you please make this
> separate patch?
> Stable trees may not interested in internal function name changes but may want
> to get stats counter fix.
> 
> >  {
> >  	int ret;
> >  	struct i40e_eth_stats *pstats = NULL; @@ -965,7 +965,7 @@ struct
> > rte_i40evf_xstats_name_off {
> >  						pstats->rx_broadcast;
> >  	stats->opackets = pstats->tx_broadcast + pstats->tx_multicast +
> >  						pstats->tx_unicast;
> > -	stats->ierrors = pstats->rx_discards;
> > +	stats->imissed = pstats->rx_discards;
> 
> so won't VF driver report any rx_error?
Yes, i40evf can not get the error stats.

I agree with this change.
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
  
Ferruh Yigit Feb. 8, 2017, 2:12 p.m. UTC | #3
On 2/7/2017 3:35 PM, Wu, Jingjing wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Thursday, December 8, 2016 2:30 AM
>> To: Tom Crugnale <tcrugnale@sandvine.com>; Wu, Jingjing
>> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v3] net/i40evf: fix reporting of imissed packets
>>
>> On 12/6/2016 8:16 PM, Tom Crugnale wrote:
>>> Missed packets on RX were erroneously being assigned to the ierrors
>>> struct member. Change it to be assigned to imissed.
>>>
>>> Fixes: 4861cde4 ("i40e: new poll mode driver")
>>>
>>> Signed-off-by: Tom Crugnale <tcrugnale@sandvine.com>

> I agree with this change.
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>

    Fixes: 4861cde46116 ("i40e: new poll mode driver")
    Cc: stable@dpdk.org

Applied to dpdk-next-net/master, thanks.

(function name change already done by another patch)
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index aa306d6..90876c8 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -952,7 +952,7 @@  struct rte_i40evf_xstats_name_off {
 }
 
 static int
-i40evf_get_statics(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
+i40evf_get_stats(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	int ret;
 	struct i40e_eth_stats *pstats = NULL;
@@ -965,7 +965,7 @@  struct rte_i40evf_xstats_name_off {
 						pstats->rx_broadcast;
 	stats->opackets = pstats->tx_broadcast + pstats->tx_multicast +
 						pstats->tx_unicast;
-	stats->ierrors = pstats->rx_discards;
+	stats->imissed = pstats->rx_discards;
 	stats->oerrors = pstats->tx_errors + pstats->tx_discards;
 	stats->ibytes = pstats->rx_bytes;
 	stats->obytes = pstats->tx_bytes;
@@ -2277,8 +2277,8 @@  static int i40evf_dev_xstats_get(struct rte_eth_dev *dev,
 static void
 i40evf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
-	if (i40evf_get_statics(dev, stats))
-		PMD_DRV_LOG(ERR, "Get statics failed");
+	if (i40evf_get_stats(dev, stats))
+		PMD_DRV_LOG(ERR, "Get stats failed");
 }
 
 static void