lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <ef9d4ca5-35e4-ff8c-c1aa-f77a4b04d0a2@intel.com> Date: Thu, 14 Sep 2023 17:22:00 +0200 From: Alexander Lobakin <aleksander.lobakin@...el.com> To: Yajun Deng <yajun.deng@...ux.dev> CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>, <horms@...nel.org>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH net-next v3] net/core: Export dev_core_stats_*_inc() From: Yajun Deng <yajun.deng@...ux.dev> Date: Thu, 14 Sep 2023 10:37:18 +0800 > Although there is a kfree_skb_reason() helper function that can be used to > find the reason why this skb is dropped, but most callers didn't increase > one of rx_dropped, tx_dropped, rx_nohandler and rx_otherhost_dropped. > > For the users, people are more concerned about why the dropped in ip > is increasing. So we can export dev_core_stats_rx_dropped_inc sets, > which users would trace them know why rx_dropped is increasing. > > Export dev_core_stats_{rx_dropped, tx_dropped, rx_nohandler, > rx_otherhost_dropped}_inc for trace. Also, move dev_core_stats() > and netdev_core_stats_alloc() to dev.c, as they are not called > externally. I'd like to hear some arguments against having them static inlines + one external that I proposed earlier. > > Signed-off-by: Yajun Deng <yajun.deng@...ux.dev> > --- > v3: __cold should be added to the netdev_core_stats_alloc(). > v2: use __cold instead of inline in dev_core_stats(). > v1: https://lore.kernel.org/netdev/20230911082016.3694700-1-yajun.deng@linux.dev/ ...as it's not at least mentioned here in the changelog. [...] > diff --git a/net/core/dev.c b/net/core/dev.c > index ccff2b6ef958..98592e4c1df0 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -10475,7 +10475,7 @@ void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64, > } > EXPORT_SYMBOL(netdev_stats_to_stats64); > > -struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev) > +static __cold struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev) This is way over 79/80 chars, break the line before "netdev_". > { > struct net_device_core_stats __percpu *p; > > @@ -10488,7 +10488,35 @@ struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device > /* This READ_ONCE() pairs with the cmpxchg() above */ > return READ_ONCE(dev->core_stats); > } > -EXPORT_SYMBOL(netdev_core_stats_alloc); > + > +static inline struct net_device_core_stats __percpu *dev_core_stats(struct net_device *dev) Same for the line length. Also notice that now some of the functions you touch have "dev_" prefix, others have "netdev_", I'd probably take a look into unifying this. (note for the maintainers that it would be probably better to leave explicit "inline" here, but no bloat-o-meter was provided by the author, so I can't say anything for sure) > +{ > + /* This READ_ONCE() pairs with the write in netdev_core_stats_alloc() */ > + struct net_device_core_stats __percpu *p = READ_ONCE(dev->core_stats); > + > + if (likely(p)) > + return p; > + > + return netdev_core_stats_alloc(dev); > +} [...] Thanks, Olek
Powered by blists - more mailing lists