Re: [PATCH v10 3/3] net: ax88796c: ASIX AX88796C SPI Ethernet Adapter Driver

On Wed, 20 Jan 2021 20:30:14 +0100 Lukasz Stelmach wrote:

quoted

You need to use 64 bit stats, like struct rtnl_link_stats64.
On a 32bit system at 100Mbps ulong can wrap in minutes.
 

Let me see. At first glance

git grep -l ndo_get_stats\\\> drivers/net/ethernet/  | xargs grep -li SPEED_100\\\>

quite a number of Fast Ethernet drivers use net_device_stats. Let me
calculate.

- bytes
  100Mbps is ~10MiB/s
  sending 4GiB at 10MiB/s takes 27 minutes

- packets
  minimum frame size is 84 bytes (840 bits on the wire) on 100Mbps means
  119048 pps at this speed it takse 10 hours to transmit 2^32 packets

Anyway, I switched to rtnl_link_stats64. Tell me, is it OK to just
memcpy() in .ndo_get_stats64?

Yup, you can just memcpy() your local copy over the one you get as an
argument of ndo_get_stats64

quoted

quoted

+	struct work_struct	ax_work;  

I don't see you ever canceling / flushing this work.
You should do that at least on driver remove if not close.  

Done.

Does it mean most drivers do it wrong?

    git grep INIT_WORK drivers/net/ethernet/ | \
    sed -e 's/\(^[^:]*\):[^>]*->\([^,]*\),.*/\1        \2/' | \
    while read file var; do \
        grep -H $var $file;
    done | grep INIT_WORK\\\|cancel_work

Some may use flush, but I wouldn't be surprised if there were bugs like
this out there.