[PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero

[PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero

[Narendra_K]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

Hello,

[Apologies if you are receiving this message twice. I am resending the message, as I got message delivery failure note].

While exploring SR-IOV on Intel 82599EB 10-Gigabit SFP+ adapter, I had the following observation.  I enabled two VFs by passing 'max_vfs=2' to ixgbe driver. One of the VFs was assigned to a guest.
In the guest, the ifconfig and ip tools reported 'RX packets' and 'TX packets' as zero, after pinging to a remote host. Looking into it further, the commit 4197aa7bb81877ebb06e4f2cc1b5fea2da23a7bd implements 64 bit per ring statistics. It seemed like the 'total_bytes' and 'total_packets' of RX and TX ring were being reset to zero by the RX and TX interrupt handlers, resulting in the user space tools reporting zero RX and TX bytes. 

The attached patch addresses the issue by preventing the resetting of RX and TX ring statistics to zero. The patch was taken against latest mainline 3.5-rc6 kernel.

I tested the patch by pinging  from the guest OS to a remote host.

ping -f <remote host> -c 10000

The ip and ifcofig showed the statistics increased by 10000 packets.  

# lspci | grep 82599
04:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFP+ Network Connection (rev 01)
04:00.1 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFP+ Network Connection (rev 01)
04:10.0 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)
04:10.1 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)
04:10.2 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)
04:10.3 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)

# lspci -s 04:00.0 -n
04:00.0 0200: 8086:154d (rev 01)
# lspci -s 04:10.0 -n
04:10.0 0200: 8086:10ed (rev 01) 

Please let me know if additional details and logs are required. 

With regards,
Narendra K




[-- Attachment #2: 0001-ixgbevf-Prevent-RX-TX-statistics-getting-reset-to-ze.patch --]
[-- Type: application/octet-stream, Size: 2033 bytes --]

From 37b3c2d1b7a1f5d45566b22fbaa038b3e5ed3218 Mon Sep 17 00:00:00 2001
From: Narendra K <narendra_k@dell.com>
Date: Thu, 12 Jul 2012 18:20:55 +0530
Subject: [PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero

The commit 4197aa7bb81877ebb06e4f2cc1b5fea2da23a7bd implements 64 bit
per ring statistics. But the driver resets the 'total_bytes' and
'total_packets' from RX and TX rings in the RX and TX interrupt
handlers to zero. This results in statistics being lost and user space
reporting RX and TX statistics as zero. This patch addresses the
issue by preventing the resetting of RX and TX ring statistics to
zero.

Signed-off-by: Narendra K <narendra_k@dell.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index f69ec42..8b304a4 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -969,8 +969,6 @@ static irqreturn_t ixgbevf_msix_clean_tx(int irq, void *data)
 	r_idx = find_first_bit(q_vector->txr_idx, adapter->num_tx_queues);
 	for (i = 0; i < q_vector->txr_count; i++) {
 		tx_ring = &(adapter->tx_ring[r_idx]);
-		tx_ring->total_bytes = 0;
-		tx_ring->total_packets = 0;
 		ixgbevf_clean_tx_irq(adapter, tx_ring);
 		r_idx = find_next_bit(q_vector->txr_idx, adapter->num_tx_queues,
 				      r_idx + 1);
@@ -994,16 +992,6 @@ static irqreturn_t ixgbevf_msix_clean_rx(int irq, void *data)
 	struct ixgbe_hw *hw = &adapter->hw;
 	struct ixgbevf_ring  *rx_ring;
 	int r_idx;
-	int i;
-
-	r_idx = find_first_bit(q_vector->rxr_idx, adapter->num_rx_queues);
-	for (i = 0; i < q_vector->rxr_count; i++) {
-		rx_ring = &(adapter->rx_ring[r_idx]);
-		rx_ring->total_bytes = 0;
-		rx_ring->total_packets = 0;
-		r_idx = find_next_bit(q_vector->rxr_idx, adapter->num_rx_queues,
-				      r_idx + 1);
-	}
 
 	if (!q_vector->rxr_count)
 		return IRQ_HANDLED;
-- 
1.7.10.1

Re: [PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]

On 07/12/2012 06:55 AM, Narendra_K@Dell.com wrote:
> Hello,
>
> [Apologies if you are receiving this message twice. I am resending the message, as I got message delivery failure note].
>
> While exploring SR-IOV on Intel 82599EB 10-Gigabit SFP+ adapter, I had the following observation.  I enabled two VFs by passing 'max_vfs=2' to ixgbe driver. One of the VFs was assigned to a guest.
> In the guest, the ifconfig and ip tools reported 'RX packets' and 'TX packets' as zero, after pinging to a remote host. Looking into it further, the commit 4197aa7bb81877ebb06e4f2cc1b5fea2da23a7bd implements 64 bit per ring statistics. It seemed like the 'total_bytes' and 'total_packets' of RX and TX ring were being reset to zero by the RX and TX interrupt handlers, resulting in the user space tools reporting zero RX and TX bytes. 
>
> The attached patch addresses the issue by preventing the resetting of RX and TX ring statistics to zero. The patch was taken against latest mainline 3.5-rc6 kernel.
>
> I tested the patch by pinging  from the guest OS to a remote host.
>
> ping -f <remote host> -c 10000
>
> The ip and ifcofig showed the statistics increased by 10000 packets.  
>
> # lspci | grep 82599
> 04:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFP+ Network Connection (rev 01)
> 04:00.1 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFP+ Network Connection (rev 01)
> 04:10.0 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)
> 04:10.1 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)
> 04:10.2 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)
> 04:10.3 Ethernet controller: Intel Corporation 82599 Ethernet Controller Virtual Function (rev 01)
>
> # lspci -s 04:00.0 -n
> 04:00.0 0200: 8086:154d (rev 01)
> # lspci -s 04:10.0 -n
> 04:10.0 0200: 8086:10ed (rev 01) 
>
> Please let me know if additional details and logs are required. 
>
> With regards,
> Narendra K
>
>
>

Thanks, I will add the patch to my queue



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]

RE: [PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero

[Narendra_K]

> -----Original Message-----
> From: Jeff Kirsher [mailto:tarbal@gmail.com]
> Sent: Thursday, July 12, 2012 11:56 PM
> To: K, Narendra; gregory.v.rose@intel.com
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero
> 
> On 07/12/2012 06:55 AM, Narendra_K@Dell.com wrote:
> > Hello,
> >
> > [Apologies if you are receiving this message twice. I am resending the
> message, as I got message delivery failure note].
> >
> > While exploring SR-IOV on Intel 82599EB 10-Gigabit SFP+ adapter, I had the
> following observation.  I enabled two VFs by passing 'max_vfs=2' to ixgbe
> driver. One of the VFs was assigned to a guest.
> > In the guest, the ifconfig and ip tools reported 'RX packets' and 'TX packets'
> as zero, after pinging to a remote host. Looking into it further, the commit
> 4197aa7bb81877ebb06e4f2cc1b5fea2da23a7bd implements 64 bit per ring
> statistics. It seemed like the 'total_bytes' and 'total_packets' of RX and TX
> ring were being reset to zero by the RX and TX interrupt handlers, resulting in
> the user space tools reporting zero RX and TX bytes.
> >
> > The attached patch addresses the issue by preventing the resetting of RX
> and TX ring statistics to zero. The patch was taken against latest mainline 3.5-
> rc6 kernel.
> >
> > I tested the patch by pinging  from the guest OS to a remote host.
> >
> > ping -f <remote host> -c 10000
> >
> > The ip and ifcofig showed the statistics increased by 10000 packets.
> >
> > # lspci | grep 82599
> > 04:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFP+
> Network Connection (rev 01)
> > 04:00.1 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFP+
> Network Connection (rev 01)
> > 04:10.0 Ethernet controller: Intel Corporation 82599 Ethernet Controller
> Virtual Function (rev 01)
> > 04:10.1 Ethernet controller: Intel Corporation 82599 Ethernet Controller
> Virtual Function (rev 01)
> > 04:10.2 Ethernet controller: Intel Corporation 82599 Ethernet Controller
> Virtual Function (rev 01)
> > 04:10.3 Ethernet controller: Intel Corporation 82599 Ethernet Controller
> Virtual Function (rev 01)
> >
> > # lspci -s 04:00.0 -n
> > 04:00.0 0200: 8086:154d (rev 01)
> > # lspci -s 04:10.0 -n
> > 04:10.0 0200: 8086:10ed (rev 01)
> >
> > Please let me know if additional details and logs are required.[>]  
> >
> > With regards,
> > Narendra K
> >
> >
> >
> 
> Thanks, I will add the patch to my queue
> 
[>] 
 
Hi Greg,

I was re-looking at why ' rx_ring->total_packets' and ' rx_ring->total_bytes' were being set to zero in ' ixgbevf_msix_clean_rx'.  It looks like ' rx_ring->total_packets' and ' rx_ring->total_packets'  are computed per one run of 'ixgbevf_clean_rx_irq' .  Then in 'ixgbevf_clean_rxonly' if 'adapter->itr_setting & 1' is true, the count is  used in  'ixgbevf_set_itr_msix'. When the interrupts are enabled, the 
' rx_ring->total_packets'  and ' rx_ring->total_bytes' are set to zero so that they can be re-computed in the poll function and  fed to the 'ixgbevf_set_itr_msix'.

This results in statistics reported by 'ip' and 'ifconfig' as zero. The patch addresses the scenario.  But it seems it would change the intended behavior in the scenario  when 'adapter->itr_setting & 1' is true.  It could be addressed by storing the 'total_rx_packets' and 'total_rx_bytes'  computed every time in the poll function in 'struct ixgbevf_adapter' . Then the interrupt handler could reset them to zero instead of resetting  ' rx_ring->total_packets' and  ' rx_ring->total_bytes'.

Also, I observed that  'adapter->itr_setting & 1'  was not true by default.  I tried setting it by 'ethtool  -C eth0 adaptive-rx on', and it returned 'operation not supported'. 

I could be missing something here, please let me know.

With regards,
Narendra K

Re: [PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero

[Greg Rose]

On Fri, 13 Jul 2012 05:36:37 -0700
<Narendra_K@Dell.com> wrote:

> > -----Original Message-----
> > From: Jeff Kirsher [mailto:tarbal@gmail.com]
> > Sent: Thursday, July 12, 2012 11:56 PM
> > To: K, Narendra; gregory.v.rose@intel.com
> > Cc: netdev@vger.kernel.org
> > Subject: Re: [PATCH] ixgbevf - Prevent RX/TX statistics getting
> > reset to zero
> > 
> > On 07/12/2012 06:55 AM, Narendra_K@Dell.com wrote:
> > > Hello,
> > >
> > > [Apologies if you are receiving this message twice. I am
> > > resending the
> > message, as I got message delivery failure note].
> > >
> > > While exploring SR-IOV on Intel 82599EB 10-Gigabit SFP+ adapter,
> > > I had the
> > following observation.  I enabled two VFs by passing 'max_vfs=2' to
> > ixgbe driver. One of the VFs was assigned to a guest.
> > > In the guest, the ifconfig and ip tools reported 'RX packets' and
> > > 'TX packets'
> > as zero, after pinging to a remote host. Looking into it further,
> > the commit 4197aa7bb81877ebb06e4f2cc1b5fea2da23a7bd implements 64
> > bit per ring statistics. It seemed like the 'total_bytes' and
> > 'total_packets' of RX and TX ring were being reset to zero by the
> > RX and TX interrupt handlers, resulting in the user space tools
> > reporting zero RX and TX bytes.
> > >
> > > The attached patch addresses the issue by preventing the
> > > resetting of RX
> > and TX ring statistics to zero. The patch was taken against latest
> > mainline 3.5- rc6 kernel.
> > >
> > > I tested the patch by pinging  from the guest OS to a remote host.
> > >
> > > ping -f <remote host> -c 10000
> > >
> > > The ip and ifcofig showed the statistics increased by 10000
> > > packets.
> > >
> > > # lspci | grep 82599
> > > 04:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit
> > > SFP+
> > Network Connection (rev 01)
> > > 04:00.1 Ethernet controller: Intel Corporation 82599EB 10-Gigabit
> > > SFP+
> > Network Connection (rev 01)
> > > 04:10.0 Ethernet controller: Intel Corporation 82599 Ethernet
> > > Controller
> > Virtual Function (rev 01)
> > > 04:10.1 Ethernet controller: Intel Corporation 82599 Ethernet
> > > Controller
> > Virtual Function (rev 01)
> > > 04:10.2 Ethernet controller: Intel Corporation 82599 Ethernet
> > > Controller
> > Virtual Function (rev 01)
> > > 04:10.3 Ethernet controller: Intel Corporation 82599 Ethernet
> > > Controller
> > Virtual Function (rev 01)
> > >
> > > # lspci -s 04:00.0 -n
> > > 04:00.0 0200: 8086:154d (rev 01)
> > > # lspci -s 04:10.0 -n
> > > 04:10.0 0200: 8086:10ed (rev 01)
> > >
> > > Please let me know if additional details and logs are
> > > required.[>]  
> > >
> > > With regards,
> > > Narendra K
> > >
> > >
> > >
> > 
> > Thanks, I will add the patch to my queue
> > 
> [>] 
>  
> Hi Greg,
> 
> I was re-looking at why ' rx_ring->total_packets' and '
> rx_ring->total_bytes' were being set to zero in '
> ixgbevf_msix_clean_rx'.  It looks like ' rx_ring->total_packets' and
> ' rx_ring->total_packets'  are computed per one run of
> 'ixgbevf_clean_rx_irq' .  Then in 'ixgbevf_clean_rxonly' if
> 'adapter->itr_setting & 1' is true, the count is  used in
> 'ixgbevf_set_itr_msix'. When the interrupts are enabled, the '
> rx_ring->total_packets'  and ' rx_ring->total_bytes' are set to zero
> so that they can be re-computed in the poll function and  fed to the
> 'ixgbevf_set_itr_msix'.
> 
> This results in statistics reported by 'ip' and 'ifconfig' as zero.
> The patch addresses the scenario.  But it seems it would change the
> intended behavior in the scenario  when 'adapter->itr_setting & 1' is
> true.  It could be addressed by storing the 'total_rx_packets' and
> 'total_rx_bytes'  computed every time in the poll function in 'struct
> ixgbevf_adapter' . Then the interrupt handler could reset them to
> zero instead of resetting  ' rx_ring->total_packets' and  '
> rx_ring->total_bytes'.
> 
> Also, I observed that  'adapter->itr_setting & 1'  was not true by
> default.  I tried setting it by 'ethtool  -C eth0 adaptive-rx on',
> and it returned 'operation not supported'. 
> 
> I could be missing something here, please let me know.

Nope, you're correct in your analysis.  The ixgbevf driver hasn't
supported adaptive interrupt moderation in the past.  However, a set of
patches we have in the pipeline will turn it on by default.  Also,
as a result of those patches the bug you've reported will be fixed.
We'll go ahead and accept your patch for the net tree and then fix up
any conflicts between that and our new set of patches when they get
pushed to net-next.

Thanks for your work on this.

- Greg

RE: [PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero

[Narendra_K]

> -----Original Message-----
> From: Greg Rose [mailto:gregory.v.rose@intel.com]
> Sent: Friday, July 13, 2012 10:44 PM
> To: K, Narendra
> Cc: jeffrey.t.kirsher@intel.com; netdev@vger.kernel.org
> Subject: Re: [PATCH] ixgbevf - Prevent RX/TX statistics getting reset to zero
> 
> On Fri, 13 Jul 2012 05:36:37 -0700
> <Narendra_K@Dell.com> wrote:
> 
> > > -----Original Message-----
> > > From: Jeff Kirsher [mailto:tarbal@gmail.com]
> > > Sent: Thursday, July 12, 2012 11:56 PM
> > > To: K, Narendra; gregory.v.rose@intel.com
> > > Cc: netdev@vger.kernel.org
> > > Subject: Re: [PATCH] ixgbevf - Prevent RX/TX statistics getting
> > > reset to zero
> > >
> > > On 07/12/2012 06:55 AM, Narendra_K@Dell.com wrote:
> > > > Hello,
> > > >
> > > > [Apologies if you are receiving this message twice. I am resending
> > > > the
> > > message, as I got message delivery failure note].
> > > >
> > > > While exploring SR-IOV on Intel 82599EB 10-Gigabit SFP+ adapter, I
> > > > had the
> > > following observation.  I enabled two VFs by passing 'max_vfs=2' to
> > > ixgbe driver. One of the VFs was assigned to a guest.
> > > > In the guest, the ifconfig and ip tools reported 'RX packets' and
> > > > 'TX packets'
> > > as zero, after pinging to a remote host. Looking into it further,
> > > the commit 4197aa7bb81877ebb06e4f2cc1b5fea2da23a7bd implements
> 64
> > > bit per ring statistics. It seemed like the 'total_bytes' and
> > > 'total_packets' of RX and TX ring were being reset to zero by the RX
> > > and TX interrupt handlers, resulting in the user space tools
> > > reporting zero RX and TX bytes.
> > > >
> > > > The attached patch addresses the issue by preventing the resetting
> > > > of RX
> > > and TX ring statistics to zero. The patch was taken against latest
> > > mainline 3.5- rc6 kernel.
> > > >
> > > > I tested the patch by pinging  from the guest OS to a remote host.
> > > >
> > > > ping -f <remote host> -c 10000
> > > >
> > > > The ip and ifcofig showed the statistics increased by 10000
> > > > packets.
> > > >
> > > > # lspci | grep 82599
> > > > 04:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit
> > > > SFP+
> > > Network Connection (rev 01)
> > > > 04:00.1 Ethernet controller: Intel Corporation 82599EB 10-Gigabit
> > > > SFP+
> > > Network Connection (rev 01)
> > > > 04:10.0 Ethernet controller: Intel Corporation 82599 Ethernet
> > > > Controller
> > > Virtual Function (rev 01)
> > > > 04:10.1 Ethernet controller: Intel Corporation 82599 Ethernet
> > > > Controller
> > > Virtual Function (rev 01)
> > > > 04:10.2 Ethernet controller: Intel Corporation 82599 Ethernet
> > > > Controller
> > > Virtual Function (rev 01)
> > > > 04:10.3 Ethernet controller: Intel Corporation 82599 Ethernet
> > > > Controller
> > > Virtual Function (rev 01)
> > > >
> > > > # lspci -s 04:00.0 -n
> > > > 04:00.0 0200: 8086:154d (rev 01)
> > > > # lspci -s 04:10.0 -n
> > > > 04:10.0 0200: 8086:10ed (rev 01)
> > > >
> > > > Please let me know if additional details and logs are required.[>]
> > > >
> > > > With regards,
> > > > Narendra K
> > > >
> > > >
> > > >
> > >
> > > Thanks, I will add the patch to my queue
> > >
> > [>]
> >
> > Hi Greg,
> >
> > I was re-looking at why ' rx_ring->total_packets' and '
> > rx_ring->total_bytes' were being set to zero in '
> > ixgbevf_msix_clean_rx'.  It looks like ' rx_ring->total_packets' and '
> > rx_ring->total_packets'  are computed per one run of
> > 'ixgbevf_clean_rx_irq' .  Then in 'ixgbevf_clean_rxonly' if
> > 'adapter->itr_setting & 1' is true, the count is  used in
> > 'ixgbevf_set_itr_msix'. When the interrupts are enabled, the '
> > rx_ring->total_packets'  and ' rx_ring->total_bytes' are set to zero
> > so that they can be re-computed in the poll function and  fed to the
> > 'ixgbevf_set_itr_msix'.
> >
> > This results in statistics reported by 'ip' and 'ifconfig' as zero.
> > The patch addresses the scenario.  But it seems it would change the
> > intended behavior in the scenario  when 'adapter->itr_setting & 1' is
> > true.  It could be addressed by storing the 'total_rx_packets' and
> > 'total_rx_bytes'  computed every time in the poll function in 'struct
> > ixgbevf_adapter' . Then the interrupt handler could reset them to zero
> > instead of resetting  ' rx_ring->total_packets' and  '
> > rx_ring->total_bytes'.
> >
> > Also, I observed that  'adapter->itr_setting & 1'  was not true by
> > default.  I tried setting it by 'ethtool  -C eth0 adaptive-rx on', and
> > it returned 'operation not supported'.
> >
> > I could be missing something here, please let me know.
> 
> Nope, you're correct in your analysis.  The ixgbevf driver hasn't supported
> adaptive interrupt moderation in the past.  However, a set of patches we
> have in the pipeline will turn it on by default.  Also, as a result of those
> patches the bug you've reported will be fixed.
> We'll go ahead and accept your patch for the net tree and then fix up any
> conflicts between that and our new set of patches when they get pushed to
> net-next.
> 
> Thanks for your work on this.
> 
> - Greg

Thank you Greg.

With regards,
Narendra K