dummy_can: fix packet statistics
PATCH 1/1 dummy_can: fix packet statistics
The former implementation was only counting the tx_packets value. Adopt the skb handling from vcan.c to correctly count the statistics and fix the packet flow for looped packets.
The CAN echo support (IFF_ECHO) is enabled in dummy_can_init() to provide a more realistic behaviour of real CAN hardware interfaces.
--- drivers/net/can/dummy_can.c | 50 ++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-)
drivers/net/can/dummy_can.c +47 -3
@@ -207,17 +207,60 @@ static int dummy_can_netdev_close(struct net_device *dev) }static netdev_tx_t dummy_can_start_xmit (struct sk_buff *skb,struct net_device *dev) { +struct net_device_stats *stats = &dev->stats; +unsigned int len; +bool loop; +if (can_dev_dropped_skb (dev, skb))return NETDEV_TX_OK ; -can_put_echo_skb (skb, dev,0 ,0 ); - dev->stats.tx_packets++; - dev->stats.tx_bytes +=can_get_echo_skb (dev,0 ,NULL ); + len =can_skb_get_data_len (skb); + stats->tx_packets++; + stats->tx_bytes += len; + +/* set flag whether this packet has to be looped back */ + loop = skb->pkt_type ==PACKET_LOOPBACK ; + +skb_tx_timestamp (skb); + +/* driver supports echo handling - see dummy_can_init() */ +if (!(dev->flags &IFF_ECHO )) { +/* no echo handling available inside this driver */ +if (loop) { +/* only count the packets here, because the +* CAN core already did the echo for us +*/ + stats->rx_packets++; + stats->rx_bytes += len; + } +consume_skb (skb); +return NETDEV_TX_OK ; + } + +/* perform standard echo handling for CAN network interfaces */ + +if (loop) { + skb =can_create_echo_skb (skb); +if (!skb) +return NETDEV_TX_OK ; +/* receive with packet counting */ + stats->rx_packets++; + stats->rx_bytes += len; + + skb->pkt_type =PACKET_BROADCAST ; + skb->dev = dev; + skb->ip_summed =CHECKSUM_UNNECESSARY ; + +netif_rx (skb); + }else { +/* no looped packets => no counting */ +consume_skb (skb); + }return NETDEV_TX_OK ; }static const struct net_device_ops dummy_can_netdev_ops = { .ndo_open = dummy_can_netdev_open,
@@ -239,10 +282,11 @@ static int __init dummy_can_init(void)if (!dev)return -ENOMEM ; dev->netdev_ops = &dummy_can_netdev_ops; dev->ethtool_ops = &dummy_can_ethtool_ops; + dev->flags |=IFF_ECHO ;/* enable echo handling */ priv =netdev_priv (dev); priv->can.bittiming_const = &dummy_can_bittiming_const; priv->can.bitrate_max =20 *MEGA /* BPS */ ; priv->can.clock.freq =160 *MEGA /* Hz */ ; priv->can.fd.data_bittiming_const = &dummy_can_fd_databittiming_const;
Signature · 1 line
2.47.3
Vincent Mailhol
maintainerHi Oliver,
Quoted 3 lines · Oliver Hartkopp
The former implementation was only counting the
tx_packetsvalue. Adopt the skb handling fromvcan.cto correctly count the statistics and fix the packet flow for looped packets.
My intent was to have the dummy_can's xmit() behave like the other hardware interface: just have the echo skb. I am not sure why a real vcan-style loop back would be needed here.
That also means that dummy_can would never be able to receive a packet, but I do not see the issue with that.
Quoted 2 lines
The CAN echo support (
IFF_ECHO) is enabled indummy_can_init()to provide a more realistic behaviour of real CAN hardware interfaces.
Ack. The IFF_ECHO is needed, otherwise the echo skb is discarded right away and the tx_bytes are never increased. Actually, this is not the only driver which forget to set up its IFF_ECHO flag.
So, for me, the fix would be to just add the IFF_ECHO. After doing just this locally, the statistics behave the same as with my real hardware
interfaces: when I send a packet only the TX stats are increased and the RX stats remains unchanged.
Yours sincerely,
Vincent Mailhol
Quoted 17 lines · Vincent Mailhol
Hi Oliver,
Quoted 3 lines · Oliver Hartkopp
The former implementation was only counting the
tx_packetsvalue. Adopt the skb handling fromvcan.cto correctly count the statistics and fix the packet flow for looped packets.My intent was to have the dummy_can's
xmit()behave like the other hardware interface: just have the echo skb. I am not sure why a real vcan-style loop back would be needed here.That also means that
dummy_canwould never be able to receive a packet, but I do not see the issue with that.Quoted 2 lines
The CAN echo support (
IFF_ECHO) is enabled indummy_can_init()to provide a more realistic behaviour of real CAN hardware interfaces.
Quoted 8 lines
Ack. The
IFF_ECHOis needed, otherwise the echo skb is discarded right away and thetx_bytesare never increased. Actually, this is not the only driver which forget to set up itsIFF_ECHOflag.So, for me, the fix would be to just add the
IFF_ECHO. After doing just this locally, the statistics behave the same as with my real hardware
interfaces: when I send a packet only the TX stats are increased and the RX stats remains unchanged.
Correct!
I've sent a v2 patch.
Thanks for the review!
Best regards,
Oliver