| Message ID | a46f539e-a54d-7e92-0372-cd96bb280729@gmail.com |
|---|---|
| Headers | show |
| Series | net: add and use function dev_fetch_sw_netstats for fetching pcpu_sw_netstats | expand |
On Sun, 11 Oct 2020 21:36:43 +0200 Heiner Kallweit <hkallweit1@gmail.com> wrote: > +void dev_fetch_sw_netstats(struct rtnl_link_stats64 *s, > + struct pcpu_sw_netstats __percpu *netstats) netstats is unmodified, should it be const? > +{ > + int cpu; > + > + if (IS_ERR_OR_NULL(netstats)) > + return; Any code calling this with a null pointer is broken/buggy, please don't ignore that.
On 11.10.2020 21:54, Stephen Hemminger wrote: > On Sun, 11 Oct 2020 21:36:43 +0200 > Heiner Kallweit <hkallweit1@gmail.com> wrote: > >> +void dev_fetch_sw_netstats(struct rtnl_link_stats64 *s, >> + struct pcpu_sw_netstats __percpu *netstats) > > netstats is unmodified, should it be const? > >> +{ >> + int cpu; >> + >> + if (IS_ERR_OR_NULL(netstats)) >> + return; > > Any code calling this with a null pointer is broken/buggy, please don't > ignore that. > Thanks, I'll consider both points in a v2.
On Sun, 11 Oct 2020 21:36:43 +0200 Heiner Kallweit wrote: > In several places the same code is used to populate rtnl_link_stats64 > fields with data from pcpu_sw_netstats. Therefore factor out this code > to a new function dev_fetch_sw_netstats(). > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > +/** > + * dev_fetch_sw_netstats - get per-cpu network device statistics > + * @s: place to store stats > + * @netstats: per-cpu network stats to read from > + * > + * Read per-cpu network statistics and populate the related fields in s. in @s? > + */ > +void dev_fetch_sw_netstats(struct rtnl_link_stats64 *s, > + struct pcpu_sw_netstats __percpu *netstats) > +} > +EXPORT_SYMBOL(dev_fetch_sw_netstats); Your pick, but _GPL would be fine too even if most exports here are non-GPL-exclusive.
On Sun, 11 Oct 2020 21:34:58 +0200 Heiner Kallweit wrote: > In several places the same code is used to populate rtnl_link_stats64 > fields with data from pcpu_sw_netstats. Therefore factor out this code > to a new function dev_fetch_sw_netstats(). FWIW probably fine to convert nfp_repr_get_host_stats64() as well, just take out the drop counter and make it a separate atomic. If you're up for that.
On 12.10.2020 00:10, Jakub Kicinski wrote: > On Sun, 11 Oct 2020 21:34:58 +0200 Heiner Kallweit wrote: >> In several places the same code is used to populate rtnl_link_stats64 >> fields with data from pcpu_sw_netstats. Therefore factor out this code >> to a new function dev_fetch_sw_netstats(). > > FWIW probably fine to convert nfp_repr_get_host_stats64() as well, just > take out the drop counter and make it a separate atomic. If you're up > for that. > Looking at nfp_repr_get_host_stats64() I'm not sure why the authors decided to add a 64bit tx drop counter, struct net_device_stats has an unsigned long tx_dropped counter already. And that the number of dropped tx packets exceeds 32bit (on 32bit systems) seems not very likely.
On Mon, 12 Oct 2020 00:29:52 +0200 Heiner Kallweit wrote: > On 12.10.2020 00:10, Jakub Kicinski wrote: > > On Sun, 11 Oct 2020 21:34:58 +0200 Heiner Kallweit wrote: > >> In several places the same code is used to populate rtnl_link_stats64 > >> fields with data from pcpu_sw_netstats. Therefore factor out this code > >> to a new function dev_fetch_sw_netstats(). > > > > FWIW probably fine to convert nfp_repr_get_host_stats64() as well, just > > take out the drop counter and make it a separate atomic. If you're up > > for that. > > > Looking at nfp_repr_get_host_stats64() I'm not sure why the authors > decided to add a 64bit tx drop counter, struct net_device_stats has > an unsigned long tx_dropped counter already. And that the number of > dropped tx packets exceeds 32bit (on 32bit systems) seems not very > likely. struct net_device::stats? That's not per-cpu. Or do you mean struct net_device::tx_dropped? That'd work nicely.
In several places the same code is used to populate rtnl_link_stats64 fields with data from pcpu_sw_netstats. Therefore factor out this code to a new function dev_fetch_sw_netstats(). Heiner Kallweit (12): net: core: add function dev_fetch_sw_netstats for fetching pcpu_sw_netstats IB/hfi1: use new function dev_fetch_sw_netstats net: macsec: use new function dev_fetch_sw_netstats net: usb: qmi_wwan: use new function dev_fetch_sw_netstats net: usbnet: use new function dev_fetch_sw_netstats qtnfmac: use new function dev_fetch_sw_netstats net: bridge: use new function dev_fetch_sw_netstats net: dsa: use new function dev_fetch_sw_netstats iptunnel: use new function dev_fetch_sw_netstats mac80211: use new function dev_fetch_sw_netstats net: openvswitch: use new function dev_fetch_sw_netstats xfrm: use new function dev_fetch_sw_netstats drivers/infiniband/hw/hfi1/ipoib_main.c | 34 +----------------- drivers/net/macsec.c | 25 +------------ drivers/net/usb/qmi_wwan.c | 24 +------------ drivers/net/usb/usbnet.c | 24 +------------ drivers/net/wireless/quantenna/qtnfmac/core.c | 27 +------------- include/linux/netdevice.h | 2 ++ net/bridge/br_device.c | 21 +---------- net/core/dev.c | 36 +++++++++++++++++++ net/dsa/slave.c | 21 +---------- net/ipv4/ip_tunnel_core.c | 23 +----------- net/mac80211/iface.c | 23 +----------- net/openvswitch/vport-internal_dev.c | 20 +---------- net/xfrm/xfrm_interface.c | 22 +----------- 13 files changed, 49 insertions(+), 253 deletions(-)