DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH] ixgbe: do not include CRC in Tx byte count
@ 2015-03-24 23:54 Ananyev, Konstantin
  2015-04-01  7:45 ` Thomas Monjalon
       [not found] ` <c8d4fd5c400c4beaa4577b8b1069fe04@BRMWP-EXMB12.corp.brocade.com>
  0 siblings, 2 replies; 14+ messages in thread
From: Ananyev, Konstantin @ 2015-03-24 23:54 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger


Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, March 24, 2015 3:55 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: Re: [dpdk-dev] [PATCH] ixgbe: do not include CRC in Tx byte count
> 
> On Mon, 23 Mar 2015 16:45:44 +0000
> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of stephen@networkplumber.org
> > > Sent: Friday, January 23, 2015 6:24 AM
> > > To: dev@dpdk.org
> > > Cc: Stephen Hemminger
> > > Subject: [dpdk-dev] [PATCH] ixgbe: do not include CRC in Tx byte count
> > >
> > > From: Stephen Hemminger <shemming@brocade.com>
> > >
> > > The ixgbe driver was including CRC in the transmit packet byte
> > > count, but not for packets received.
> > > This was notice when forwarding and
> > > the number of bytes received was greater than the number of bytes transmitted
> > > for the same number of packets. Make the driver behave like other
> > > virtual devices and not include CRC in byte count. Use the same queue
> > > counters already computed and used for Rx.
> >
> > About RX side stats - as I remember it depends to what value hw_stip_crc is set at configure().
> > If hw_stip_crc==1, then, yes CRC bytes are not included into  QBRC value.
> > I If hw_stip_crc==0, then CRC bytes are included into QBRC.
> 
> That is an additional bug!
>   * CRC should never be included in the byte count.
>     This is not how Linux or other drivers report byte count.

I don't have any strong opinion here...
For me any method (with or without CRC) of counting bytes are ok, as long as this method is identical across all PMDs we support.
Which makes we wonder, what approach other PMDs use? 

> 
>   * the byte count must be symmetrical Rx == Tx
> 
> The Brocade router always set strip_crc to 1. So did not see the additional bug
> of CRC being included in byte count.

Are you going to submit v2, to make Rx==Tx for both cases?
Konstantin

> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH] ixgbe: do not include CRC in Tx byte count
@ 2015-01-23  6:23 stephen
  2015-01-27 10:11 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: stephen @ 2015-01-23  6:23 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

The ixgbe driver was including CRC in the transmit packet byte
count, but not for packets received. This was notice when forwarding and
the number of bytes received was greater than the number of bytes transmitted
for the same number of packets. Make the driver behave like other
virtual devices and not include CRC in byte count. Use the same queue
counters already computed and used for Rx.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index b58ec45..27355eb 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -1724,12 +1724,15 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	struct ixgbe_hw_stats *hw_stats =
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
 	uint32_t bprc, lxon, lxoff, total;
-	uint64_t total_missed_rx, total_qbrc, total_qprc;
+	uint64_t total_missed_rx;
+	uint64_t total_qbrc, total_qprc, total_qbtc, total_qptc;
 	unsigned i;
 
 	total_missed_rx = 0;
 	total_qbrc = 0;
 	total_qprc = 0;
+	total_qbtc = 0;
+	total_qptc = 0;
 
 	hw_stats->crcerrs += IXGBE_READ_REG(hw, IXGBE_CRCERRS);
 	hw_stats->illerrc += IXGBE_READ_REG(hw, IXGBE_ILLERRC);
@@ -1770,6 +1773,8 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 		total_qprc += hw_stats->qprc[i];
 		total_qbrc += hw_stats->qbrc[i];
+		total_qptc += hw_stats->qptc[i];
+		total_qbtc += hw_stats->qbtc[i];
 	}
 	hw_stats->mlfc += IXGBE_READ_REG(hw, IXGBE_MLFC);
 	hw_stats->mrfc += IXGBE_READ_REG(hw, IXGBE_MRFC);
@@ -1860,8 +1865,8 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	/* Fill out the rte_eth_stats statistics structure */
 	stats->ipackets = total_qprc;
 	stats->ibytes = total_qbrc;
-	stats->opackets = hw_stats->gptc;
-	stats->obytes = hw_stats->gotc;
+	stats->opackets = total_qptc;
+	stats->obytes = total_qbtc;
 	stats->imcasts = hw_stats->mprc;
 
 	for (i = 0; i < IXGBE_QUEUE_STAT_COUNTERS; i++) {
-- 
2.1.4

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

end of thread, other threads:[~2015-07-09 23:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-24 23:54 [dpdk-dev] [PATCH] ixgbe: do not include CRC in Tx byte count Ananyev, Konstantin
2015-04-01  7:45 ` Thomas Monjalon
2015-07-09 23:03   ` Stephen Hemminger
     [not found] ` <c8d4fd5c400c4beaa4577b8b1069fe04@BRMWP-EXMB12.corp.brocade.com>
2015-04-01 18:36   ` Stephen Hemminger
2015-07-09 22:59     ` Thomas Monjalon
  -- strict thread matches above, loose matches on Subject: below --
2015-01-23  6:23 stephen
2015-01-27 10:11 ` Thomas Monjalon
2015-01-27 11:38   ` Stephen Hemminger
2015-03-04 20:55     ` Thomas Monjalon
     [not found]     ` <9265c0d3b03e4f33b7823139be7b3b91@BRMWP-EXMB11.corp.brocade.com>
2015-03-04 23:06       ` Stephen Hemminger
2015-03-23 14:08 ` Thomas Monjalon
2015-03-23 16:45 ` Ananyev, Konstantin
2015-03-24 15:55   ` Stephen Hemminger
2015-04-02 20:20     ` Butler, Siobhan A

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