[net,v1,1/1] net: fec: Fix Transmitted bytes counter

The tx_bytes field was not being updated so the
network card statistics showed 0.0B transmitted.

Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
---
 drivers/net/ethernet/freescale/fec_main.c |    1 +
 1 file changed, 1 insertion(+)

On 06/28/13 02:25, Jim Baxter wrote:

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Jim Baxter
> Sent: Friday, June 28, 2013 2:25 AM
> To: David S. Miller
> Cc: Estevam Fabio-R49496; Li Frank-B20596; Shawn Guo; netdev@vger.kernel.org
> Subject: [PATCH net v1 1/1] net: fec: Fix Transmitted bytes counter
>
> The tx_bytes field was not being updated so the network card statistics showed 0.0B transmitted.
>
> Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index ed6180e..05a5f76 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -738,6 +738,7 @@ fec_enet_tx(struct net_device *ndev)
>  				ndev->stats.tx_carrier_errors++;
>  		} else {
>  			ndev->stats.tx_packets++;
> +			ndev->stats.tx_bytes += bdp->cbd_datlen;
>  		}
>  
>  		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) &&
> --
You cannot stat. tx_bytes in here, since stat.tx_bytes indicate that all bytes sent by MAC regardless whether there have error packets or not.
You must add the stat. at xmit function as below:
...
fep->tx_skbuff[index] = skb;
+ ndev->stats.tx_bytes += skb->len;
...

Thanks,
Andy

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Duan Fugang-B38611 <B38611@freescale.com>
Date: Fri, 28 Jun 2013 02:11:30 +0000

> On 06/28/13 02:25, Jim Baxter wrote:
> 
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
>> index ed6180e..05a5f76 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -738,6 +738,7 @@ fec_enet_tx(struct net_device *ndev)
>>  				ndev->stats.tx_carrier_errors++;
>>  		} else {
>>  			ndev->stats.tx_packets++;
>> +			ndev->stats.tx_bytes += bdp->cbd_datlen;
>>  		}
>>  
>>  		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) &&
>> --
> You cannot stat. tx_bytes in here, since stat.tx_bytes indicate that all bytes sent by MAC regardless whether there have error packets or not.
> You must add the stat. at xmit function as below:

I completely disagree.

tx_bytes indicates what actually made it to the wires, so Jim's original
patch is the correct one.

Every single driver I have ever written, always increments tx_bytes in
the transmit completion handler, not when the device layer gives the
packet to the driver.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 01/07/13 21:40, David Miller wrote:
> From: Duan Fugang-B38611 <B38611@freescale.com>
> Date: Fri, 28 Jun 2013 02:11:30 +0000
> 
>> On 06/28/13 02:25, Jim Baxter wrote:
>>
>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
>>> index ed6180e..05a5f76 100644
>>> --- a/drivers/net/ethernet/freescale/fec_main.c
>>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>>> @@ -738,6 +738,7 @@ fec_enet_tx(struct net_device *ndev)
>>>  				ndev->stats.tx_carrier_errors++;
>>>  		} else {
>>>  			ndev->stats.tx_packets++;
>>> +			ndev->stats.tx_bytes += bdp->cbd_datlen;
>>>  		}
>>>  
>>>  		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) &&
>>> --
>> You cannot stat. tx_bytes in here, since stat.tx_bytes indicate that all bytes sent by MAC regardless whether there have error packets or not.
>> You must add the stat. at xmit function as below:
> 
> I completely disagree.
> 
> tx_bytes indicates what actually made it to the wires, so Jim's original
> patch is the correct one.
> 
> Every single driver I have ever written, always increments tx_bytes in
> the transmit completion handler, not when the device layer gives the
> packet to the driver.
> 

If this should be the first version of the patch, can I cancel the
second version and set the first version to new in patchwork, or is it
best to submit a third version?

Thank you,
Jim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Jim Baxter <jim_baxter@mentor.com>
Date: Tue, 2 Jul 2013 09:32:31 +0100

> If this should be the first version of the patch, can I cancel the
> second version and set the first version to new in patchwork, or is it
> best to submit a third version?

I applied your v1 patch, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html