DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2] include dropped packets from QPRDC in ixbe imissed
       [not found] <pw70410>
@ 2020-05-19  5:02 ` Cody Harris
  2020-05-19  5:08 ` [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat Cody Harris
  1 sibling, 0 replies; 7+ messages in thread
From: Cody Harris @ 2020-05-19  5:02 UTC (permalink / raw)
  To: dev; +Cc: Cody Harris

The ixgbe imissed statstic originally only contained dropped packets counted by
the RXMPC registers. This change includes additional types of packet drops
counted by the QPRDC registers.

Intel support confimed that the packet drops counted by the QPRDC registers do
not double-count drops counted by RXMPC registers:

"RXMPC counts packets that are dropped because there is no room in the internal
packet buffer.  QPRDC counts packets that are dropped because they can't be
transferred to system memory. These packets have been stored in the internal
packet buffer, so there should be no overlap with RXMPC."

Signed-off-by: Cody Harris <codh@amazon.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a4e5c539d..7284ca28e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	}
 
 	/* Rx Errors */
-	stats->imissed  = total_missed_rx;
+	stats->imissed  = total_missed_rx + total_qprdc;
 	stats->ierrors  = hw_stats->crcerrs +
 			  hw_stats->mspdc +
 			  hw_stats->rlec +
-- 
2.24.1.AMZN


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
       [not found] <pw70410>
  2020-05-19  5:02 ` [dpdk-dev] [PATCH v2] include dropped packets from QPRDC in ixbe imissed Cody Harris
@ 2020-05-19  5:08 ` Cody Harris
  2020-05-19  5:09   ` Harris, Cody
  2020-05-28  3:53   ` Zhao1, Wei
  1 sibling, 2 replies; 7+ messages in thread
From: Cody Harris @ 2020-05-19  5:08 UTC (permalink / raw)
  To: dev; +Cc: Cody Harris

The ixgbe imissed statstic originally only contained dropped packets
counted by the RXMPC registers. This change includes additional types of
packet drops counted by the QPRDC registers.

Intel support confimed that the packet drops counted by the QPRDC registers
do not double-count drops counted by RXMPC registers:

"RXMPC counts packets that are dropped because there is no room in the
internal packet buffer.  QPRDC counts packets that are dropped because they
can't be transferred to system memory. These packets have been stored in
the internal packet buffer, so there should be no overlap with RXMPC."

Signed-off-by: Cody Harris <codh@amazon.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a4e5c539d..7284ca28e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	}
 
 	/* Rx Errors */
-	stats->imissed  = total_missed_rx;
+	stats->imissed  = total_missed_rx + total_qprdc;
 	stats->ierrors  = hw_stats->crcerrs +
 			  hw_stats->mspdc +
 			  hw_stats->rlec +
-- 
2.24.1.AMZN


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
  2020-05-19  5:08 ` [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat Cody Harris
@ 2020-05-19  5:09   ` Harris, Cody
  2020-05-28  3:53   ` Zhao1, Wei
  1 sibling, 0 replies; 7+ messages in thread
From: Harris, Cody @ 2020-05-19  5:09 UTC (permalink / raw)
  To: dev

Sorry about all the revisions, I hit some style issues I didn't know about.

Be nice, it's my first time contributing anything :)

-Cody

On 5/18/20, 10:08 PM, "Cody Harris" <codh@amazon.com> wrote:

    The ixgbe imissed statstic originally only contained dropped packets
    counted by the RXMPC registers. This change includes additional types of
    packet drops counted by the QPRDC registers.
    
    Intel support confimed that the packet drops counted by the QPRDC registers
    do not double-count drops counted by RXMPC registers:
    
    "RXMPC counts packets that are dropped because there is no room in the
    internal packet buffer.  QPRDC counts packets that are dropped because they
    can't be transferred to system memory. These packets have been stored in
    the internal packet buffer, so there should be no overlap with RXMPC."
    
    Signed-off-by: Cody Harris <codh@amazon.com>
    ---
     drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
     1 file changed, 1 insertion(+), 1 deletion(-)
    
    diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
    index a4e5c539d..7284ca28e 100644
    --- a/drivers/net/ixgbe/ixgbe_ethdev.c
    +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
    @@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
     	}
     
     	/* Rx Errors */
    -	stats->imissed  = total_missed_rx;
    +	stats->imissed  = total_missed_rx + total_qprdc;
     	stats->ierrors  = hw_stats->crcerrs +
     			  hw_stats->mspdc +
     			  hw_stats->rlec +
    -- 
    2.24.1.AMZN
    
    


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
  2020-05-19  5:08 ` [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat Cody Harris
  2020-05-19  5:09   ` Harris, Cody
@ 2020-05-28  3:53   ` Zhao1, Wei
  2020-05-28  8:09     ` Zhao1, Wei
  1 sibling, 1 reply; 7+ messages in thread
From: Zhao1, Wei @ 2020-05-28  3:53 UTC (permalink / raw)
  To: Cody Harris, dev

HI,Cody Harris

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Cody Harris
> Sent: Tuesday, May 19, 2020 1:08 PM
> To: dev@dpdk.org
> Cc: Cody Harris <codh@amazon.com>
> Subject: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
> 
> The ixgbe imissed statstic originally only contained dropped packets counted by
> the RXMPC registers. This change includes additional types of packet drops
> counted by the QPRDC registers.
> 
> Intel support confimed that the packet drops counted by the QPRDC registers
> do not double-count drops counted by RXMPC registers:
> 
> "RXMPC counts packets that are dropped because there is no room in the
> internal packet buffer.  QPRDC counts packets that are dropped because they
> can't be transferred to system memory. These packets have been stored in the
> internal packet buffer, so there should be no overlap with RXMPC."
> 
> Signed-off-by: Cody Harris <codh@amazon.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index a4e5c539d..7284ca28e 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev,
> struct rte_eth_stats *stats)
>  	}
> 
>  	/* Rx Errors */
> -	stats->imissed  = total_missed_rx;
> +	stats->imissed  = total_missed_rx + total_qprdc;

If Rx queue is disabled in the RXDCTL register, packet direct to this queue is also dropped and count by this 
Register QPRDC, but the definition of imissed is "Total of RX packets dropped by the HW because there are no available buffer" in rte layer.
So, it maybe mislead users if we mix the 2 statistic, is that so?
Also, x550 has other definition for this register, we can not mix them.


>  	stats->ierrors  = hw_stats->crcerrs +
>  			  hw_stats->mspdc +
>  			  hw_stats->rlec +
> --
> 2.24.1.AMZN


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
  2020-05-28  3:53   ` Zhao1, Wei
@ 2020-05-28  8:09     ` Zhao1, Wei
  2020-05-28  8:17       ` Harris, Cody
  0 siblings, 1 reply; 7+ messages in thread
From: Zhao1, Wei @ 2020-05-28  8:09 UTC (permalink / raw)
  To: Cody Harris, dev; +Cc: Ye, Xiaolong, Guo, Jia

Hi, Cody Harris

  More info, you can get the number of dropped packet per queue by stats->q_errors, 
No need to add this into imissed.

> -----Original Message-----
> From: Zhao1, Wei
> Sent: Thursday, May 28, 2020 11:54 AM
> To: Cody Harris <codh@amazon.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
> 
> HI,Cody Harris
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Cody Harris
> > Sent: Tuesday, May 19, 2020 1:08 PM
> > To: dev@dpdk.org
> > Cc: Cody Harris <codh@amazon.com>
> > Subject: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed
> > stat
> >
> > The ixgbe imissed statstic originally only contained dropped packets
> > counted by the RXMPC registers. This change includes additional types
> > of packet drops counted by the QPRDC registers.
> >
> > Intel support confimed that the packet drops counted by the QPRDC
> > registers do not double-count drops counted by RXMPC registers:
> >
> > "RXMPC counts packets that are dropped because there is no room in the
> > internal packet buffer.  QPRDC counts packets that are dropped because
> > they can't be transferred to system memory. These packets have been
> > stored in the internal packet buffer, so there should be no overlap with
> RXMPC."
> >
> > Signed-off-by: Cody Harris <codh@amazon.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index a4e5c539d..7284ca28e 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev,
> > struct rte_eth_stats *stats)
> >  	}
> >
> >  	/* Rx Errors */
> > -	stats->imissed  = total_missed_rx;
> > +	stats->imissed  = total_missed_rx + total_qprdc;
> 
> If Rx queue is disabled in the RXDCTL register, packet direct to this queue is
> also dropped and count by this Register QPRDC, but the definition of imissed is
> "Total of RX packets dropped by the HW because there are no available buffer"
> in rte layer.
> So, it maybe mislead users if we mix the 2 statistic, is that so?
> Also, x550 has other definition for this register, we can not mix them.
> 
> 
> >  	stats->ierrors  = hw_stats->crcerrs +
> >  			  hw_stats->mspdc +
> >  			  hw_stats->rlec +
> > --
> > 2.24.1.AMZN


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
  2020-05-28  8:09     ` Zhao1, Wei
@ 2020-05-28  8:17       ` Harris, Cody
  2020-05-28  9:43         ` Zhao1, Wei
  0 siblings, 1 reply; 7+ messages in thread
From: Harris, Cody @ 2020-05-28  8:17 UTC (permalink / raw)
  To: Zhao1, Wei, dev; +Cc: Ye, Xiaolong, Guo, Jia

Hi Zhao Wei,

I understand that the QPRDC drop count is available by stats->q_errors. I'm interested to know why imissed doesn't include it. The name imissed makes it sound like the counter counts incoming missed (valid) packets regardless of where the drop occurs during reception.

I think the name imissed is misleading if it doesn't include all types of dropped valid packets, so I made this CR to see how receptive the DPDK maintainers would be to include all dropped packet counts.

Thanks for your review,

~Cody

On 5/28/20, 1:10 AM, "Zhao1, Wei" <wei.zhao1@intel.com> wrote:

    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
    
    
    
    Hi, Cody Harris
    
      More info, you can get the number of dropped packet per queue by stats->q_errors,
    No need to add this into imissed.
    
    > -----Original Message-----
    > From: Zhao1, Wei
    > Sent: Thursday, May 28, 2020 11:54 AM
    > To: Cody Harris <codh@amazon.com>; dev@dpdk.org
    > Subject: RE: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
    >
    > HI,Cody Harris
    >
    > > -----Original Message-----
    > > From: dev <dev-bounces@dpdk.org> On Behalf Of Cody Harris
    > > Sent: Tuesday, May 19, 2020 1:08 PM
    > > To: dev@dpdk.org
    > > Cc: Cody Harris <codh@amazon.com>
    > > Subject: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed
    > > stat
    > >
    > > The ixgbe imissed statstic originally only contained dropped packets
    > > counted by the RXMPC registers. This change includes additional types
    > > of packet drops counted by the QPRDC registers.
    > >
    > > Intel support confimed that the packet drops counted by the QPRDC
    > > registers do not double-count drops counted by RXMPC registers:
    > >
    > > "RXMPC counts packets that are dropped because there is no room in the
    > > internal packet buffer.  QPRDC counts packets that are dropped because
    > > they can't be transferred to system memory. These packets have been
    > > stored in the internal packet buffer, so there should be no overlap with
    > RXMPC."
    > >
    > > Signed-off-by: Cody Harris <codh@amazon.com>
    > > ---
    > >  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
    > >  1 file changed, 1 insertion(+), 1 deletion(-)
    > >
    > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
    > > b/drivers/net/ixgbe/ixgbe_ethdev.c
    > > index a4e5c539d..7284ca28e 100644
    > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
    > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
    > > @@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev,
    > > struct rte_eth_stats *stats)
    > >     }
    > >
    > >     /* Rx Errors */
    > > -   stats->imissed  = total_missed_rx;
    > > +   stats->imissed  = total_missed_rx + total_qprdc;
    >
    > If Rx queue is disabled in the RXDCTL register, packet direct to this queue is
    > also dropped and count by this Register QPRDC, but the definition of imissed is
    > "Total of RX packets dropped by the HW because there are no available buffer"
    > in rte layer.
    > So, it maybe mislead users if we mix the 2 statistic, is that so?
    > Also, x550 has other definition for this register, we can not mix them.
    >
    >
    > >     stats->ierrors  = hw_stats->crcerrs +
    > >                       hw_stats->mspdc +
    > >                       hw_stats->rlec +
    > > --
    > > 2.24.1.AMZN
    
    


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
  2020-05-28  8:17       ` Harris, Cody
@ 2020-05-28  9:43         ` Zhao1, Wei
  0 siblings, 0 replies; 7+ messages in thread
From: Zhao1, Wei @ 2020-05-28  9:43 UTC (permalink / raw)
  To: Harris, Cody, dev; +Cc: Ye, Xiaolong, Guo, Jia

Hi, Harris, Cody

  I have reply in other mail for " why imissed doesn't include it ", thanks.

> -----Original Message-----
> From: Harris, Cody <codh@amazon.com>
> Sent: Thursday, May 28, 2020 4:17 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: Ye, Xiaolong <xiaolong.ye@intel.com>; Guo, Jia <jia.guo@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
> 
> Hi Zhao Wei,
> 
> I understand that the QPRDC drop count is available by stats->q_errors. I'm
> interested to know why imissed doesn't include it. The name imissed makes it
> sound like the counter counts incoming missed (valid) packets regardless of
> where the drop occurs during reception.
> 
> I think the name imissed is misleading if it doesn't include all types of dropped
> valid packets, so I made this CR to see how receptive the DPDK maintainers
> would be to include all dropped packet counts.
> 
> Thanks for your review,
> 
> ~Cody
> 
> On 5/28/20, 1:10 AM, "Zhao1, Wei" <wei.zhao1@intel.com> wrote:
> 
>     CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you can confirm the sender and know the
> content is safe.
> 
> 
> 
>     Hi, Cody Harris
> 
>       More info, you can get the number of dropped packet per queue by
> stats->q_errors,
>     No need to add this into imissed.
> 
>     > -----Original Message-----
>     > From: Zhao1, Wei
>     > Sent: Thursday, May 28, 2020 11:54 AM
>     > To: Cody Harris <codh@amazon.com>; dev@dpdk.org
>     > Subject: RE: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed
> stat
>     >
>     > HI,Cody Harris
>     >
>     > > -----Original Message-----
>     > > From: dev <dev-bounces@dpdk.org> On Behalf Of Cody Harris
>     > > Sent: Tuesday, May 19, 2020 1:08 PM
>     > > To: dev@dpdk.org
>     > > Cc: Cody Harris <codh@amazon.com>
>     > > Subject: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed
>     > > stat
>     > >
>     > > The ixgbe imissed statstic originally only contained dropped packets
>     > > counted by the RXMPC registers. This change includes additional types
>     > > of packet drops counted by the QPRDC registers.
>     > >
>     > > Intel support confimed that the packet drops counted by the QPRDC
>     > > registers do not double-count drops counted by RXMPC registers:
>     > >
>     > > "RXMPC counts packets that are dropped because there is no room in
> the
>     > > internal packet buffer.  QPRDC counts packets that are dropped
> because
>     > > they can't be transferred to system memory. These packets have been
>     > > stored in the internal packet buffer, so there should be no overlap with
>     > RXMPC."
>     > >
>     > > Signed-off-by: Cody Harris <codh@amazon.com>
>     > > ---
>     > >  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
>     > >  1 file changed, 1 insertion(+), 1 deletion(-)
>     > >
>     > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>     > > b/drivers/net/ixgbe/ixgbe_ethdev.c
>     > > index a4e5c539d..7284ca28e 100644
>     > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>     > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>     > > @@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev
> *dev,
>     > > struct rte_eth_stats *stats)
>     > >     }
>     > >
>     > >     /* Rx Errors */
>     > > -   stats->imissed  = total_missed_rx;
>     > > +   stats->imissed  = total_missed_rx + total_qprdc;
>     >
>     > If Rx queue is disabled in the RXDCTL register, packet direct to this
> queue is
>     > also dropped and count by this Register QPRDC, but the definition of
> imissed is
>     > "Total of RX packets dropped by the HW because there are no available
> buffer"
>     > in rte layer.
>     > So, it maybe mislead users if we mix the 2 statistic, is that so?
>     > Also, x550 has other definition for this register, we can not mix them.
>     >
>     >
>     > >     stats->ierrors  = hw_stats->crcerrs +
>     > >                       hw_stats->mspdc +
>     > >                       hw_stats->rlec +
>     > > --
>     > > 2.24.1.AMZN
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-28  9:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <pw70410>
2020-05-19  5:02 ` [dpdk-dev] [PATCH v2] include dropped packets from QPRDC in ixbe imissed Cody Harris
2020-05-19  5:08 ` [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat Cody Harris
2020-05-19  5:09   ` Harris, Cody
2020-05-28  3:53   ` Zhao1, Wei
2020-05-28  8:09     ` Zhao1, Wei
2020-05-28  8:17       ` Harris, Cody
2020-05-28  9:43         ` Zhao1, Wei

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).