diff mbox series

[net-next,3/3] net: hsr: add tx stats for master interface

Message ID 20190412201325.29001-4-m-karicheri2@ti.com
State New
Headers show
Series [net-next,1/3] net: hsr: fix naming of file and functions | expand

Commit Message

Murali Karicheri April 12, 2019, 8:13 p.m. UTC
Add tx stats to hsr interface. Without this
ifconfig for hsr interface doesn't show tx packet stats.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>

---
 net/hsr/hsr_device.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.17.0

Comments

David Miller April 13, 2019, 12:14 a.m. UTC | #1
From: Murali Karicheri <m-karicheri2@ti.com>

Date: Fri, 12 Apr 2019 16:13:25 -0400

> Add tx stats to hsr interface. Without this

> ifconfig for hsr interface doesn't show tx packet stats.

> 

> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>

> ---

>  net/hsr/hsr_device.c | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c

> index 15c72065df79..f22902eac083 100644

> --- a/net/hsr/hsr_device.c

> +++ b/net/hsr/hsr_device.c

> @@ -229,6 +229,8 @@ static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)

>  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);

>  	skb->dev = master->dev;

>  	hsr_forward_skb(skb, master);

> +	master->dev->stats.tx_packets++;

> +	master->dev->stats.tx_bytes += skb->len;

>  


This introduced a use after free of 'skb' as hsr_forward_skb() can free it.

Also, why are there so many changes in your "internal" tree and why are
they not all upstream already?
Murali Karicheri April 15, 2019, 12:53 p.m. UTC | #2
Hi David,

On 04/12/2019 08:14 PM, David Miller wrote:
> From: Murali Karicheri <m-karicheri2@ti.com>

> Date: Fri, 12 Apr 2019 16:13:25 -0400

> 

>> Add tx stats to hsr interface. Without this

>> ifconfig for hsr interface doesn't show tx packet stats.

>>

>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>

>> ---

>>   net/hsr/hsr_device.c | 2 ++

>>   1 file changed, 2 insertions(+)

>>

>> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c

>> index 15c72065df79..f22902eac083 100644

>> --- a/net/hsr/hsr_device.c

>> +++ b/net/hsr/hsr_device.c

>> @@ -229,6 +229,8 @@ static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)

>>   	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);

>>   	skb->dev = master->dev;

>>   	hsr_forward_skb(skb, master);

>> +	master->dev->stats.tx_packets++;

>> +	master->dev->stats.tx_bytes += skb->len;

>>   

> 

> This introduced a use after free of 'skb' as hsr_forward_skb() can free it.

You are right. I think I should move this to inside hsr_forward_skb().
> 

> Also, why are there so many changes in your "internal" tree and why are

> they not all upstream already?

> That is a good question. That was a management decision. Priority was 

prototyping of hsr and prp solution internally first using TI' ICSS PRU 
Ethernet for industrial applications and then once we have it working, 
push the changes upstream. We are starting this process already 
beginning with upstream of ICSS PRU driver. I have plan to send an RFC 
patch for PRP support soon.

Murali
diff mbox series

Patch

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 15c72065df79..f22902eac083 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -229,6 +229,8 @@  static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
 	skb->dev = master->dev;
 	hsr_forward_skb(skb, master);
+	master->dev->stats.tx_packets++;
+	master->dev->stats.tx_bytes += skb->len;
 
 	return NETDEV_TX_OK;
 }