net/ixgbe: include QPRDC in imissed stat
PATCH v3 1/1 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."
--- drivers/net/ixgbe/ixgbe_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ixgbe/ixgbe_ethdev.c +1 -1
@@ -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 +
Signature · 1 line
2.24.1.AMZN
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" <[email protected]> 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 <[email protected]>
---
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.AMZNHI，Cody Harris
Quoted 30 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Cody Harris
Sent: Tuesday, May 19, 2020 1:08 PM
To: [email protected]
Cc: Cody Harris <[email protected]>
Subject: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed statThe 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 <[email protected]>--- drivers/net/ixgbe/ixgbe_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/ixgbe/ixgbe_ethdev.c +0
-0
drivers/net/ixgbe/ixgbe_ethdev.c ixgbe_dev_stats_get(struct rte_eth_dev *dev, · line 3370
Quoted 6 lines
drivers/net/ixgbe/ixgbe_ethdev.c +1
-1@@ -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.
Quoted 5 lines
drivers/net/ixgbe/ixgbe_ethdev.c +0
-0stats->ierrors = hw_stats->crcerrs + hw_stats->mspdc +
hw_stats->rlec+
--
2.24.1.AMZN
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.
Quoted 60 lines
-----Original Message-----
From: Zhao1, Wei
Sent: Thursday, May 28, 2020 11:54 AM
To: Cody Harris <[email protected]>; [email protected]
Subject: RE: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed statHI，Cody Harris
Quoted 12 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Cody Harris
Sent: Tuesday, May 19, 2020 1:08 PM
To: [email protected]
Cc: Cody Harris <[email protected]>
Subject: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed statThe 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.
Quoted 7 lines
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."
Quoted 12 lines
Signed-off-by Cody Harris <[email protected]>--- drivers/net/ixgbe/ixgbe_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/ixgbe/ixgbe_ethdev.c +0
-0drivers/net/ixgbe/ixgbe_ethdev.c ixgbe_dev_stats_get(struct rte_eth_dev *dev, · line 3370
Quoted 6 lines
drivers/net/ixgbe/ixgbe_ethdev.c +1
-1@@ -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.Quoted 5 lines
drivers/net/ixgbe/ixgbe_ethdev.c +0
-0stats->ierrors = hw_stats->crcerrs + hw_stats->mspdc +
hw_stats->rlec+
--
2.24.1.AMZN
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" <[email protected]> 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 <[email protected]>; [email protected]
> Subject: RE: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed stat
>
> HI，Cody Harris
>
> > -----Original Message-----
> > From: dev <[email protected]> On Behalf Of Cody Harris
> > Sent: Tuesday, May 19, 2020 1:08 PM
> > To: [email protected]
> > Cc: Cody Harris <[email protected]>
> > 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 <[email protected]>
> > ---
> > 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.AMZNHi, Harris, Cody
I have reply in other mail for " why imissed doesn't include it ", thanks.
Quoted 105 lines
-----Original Message-----
From: Harris, Cody <[email protected]>
Sent: Thursday, May 28, 2020 4:17 PM
To: Zhao1, Wei <[email protected]>; [email protected]
Cc: Ye, Xiaolong <[email protected]>; Guo, Jia <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed statHi 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" <[email protected]> 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 <[email protected]>; [email protected]
> Subject: RE: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed
statQuoted 19 lines
HI，Cody Harris
Quoted 16 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Cody Harris
Sent: Tuesday, May 19, 2020 1:08 PM
To: [email protected]
Cc: Cody Harris <[email protected]>
Subject: [dpdk-dev] [PATCH v3] net/ixgbe: include QPRDC in imissed statThe 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
Quoted 1 line
Quoted 1 line
internal packet buffer. QPRDC counts packets that are dropped
because
Quoted 9 lines
Quoted 2 lines
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."
Quoted 6 lines
Signed-off-by Cody Harris <[email protected]>--- drivers/net/ixgbe/ixgbe_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 6 lines
drivers/net/ixgbe/ixgbe_ethdev.c ixgbe_dev_stats_get(struct rte_eth_dev
Quoted 6 lines
drivers/net/ixgbe/ixgbe_ethdev.c +0
-0@@ -3366,7 +3366,7 @@ ixgbe_dev_stats_get(struct rte_eth_dev*dev,
Quoted 8 lines
drivers/net/ixgbe/ixgbe_ethdev.c ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) · line 3369
Quoted 6 lines
drivers/net/ixgbe/ixgbe_ethdev.c +1
-1struct 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
Quoted 1 line
also dropped and count by this Register QPRDC, but the definition of
imissed is
Quoted 1 line
"Total of RX packets dropped by the HW because there are no available
buffer"
Quoted 10 lines
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.Quoted 5 lines
drivers/net/ixgbe/ixgbe_ethdev.c +0
-0stats->ierrors = hw_stats->crcerrs + hw_stats->mspdc +hw_stats->rlec +--
2.24.1.AMZN