Re: [PATCH net-next v8 09/10] bng_en: implement netdev_stat_ops

[Bhargava Chenna Marreddy]

>
> Can this race with bnge_close_core when reading bn->net_stats_prev?
> bnge_close_core writes to bn->net_stats_prev while this function reads
> it without any locking.
>
> Looking at bnge_get_stats64, it uses rcu_read_lock() before accessing
> bn->net_stats_prev. While this won't cause a crash like the other two
> issues, it could result in torn reads on 32-bit systems where reading
> u64 is not atomic, potentially returning inconsistent statistics to
> userspace.
>
> Should bnge_get_base_stats also use rcu_read_lock() when reading
> bn->net_stats_prev?

Thanks, Simon.

Regarding the RCU suggestions: bnge_get_queue_stats_rx,
bnge_get_queue_stats_tx, and bnge_get_base_stats should not
require RCU protection here. Unlike ndo_get_stats64, these ops
are synchronized by the netdev instance lock. Since the writer path
(bnge_close_core via ndo_stop) also holds the netdev instance lock,
they are already mutually exclusive.

Do you agree, or am I missing a case where these ops can be called
without that lock?

Thanks,
Bhargava Marreddy


>
> > +
> > +     tx->packets = bn->net_stats_prev.tx_packets;

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature