| 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
| ||
|
Date: Fri, 27 Oct 2023 18:33:14 -0700
From: Coco Li <lixiaoyan@...gle.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Jakub Kicinski <kuba@...nel.org>, Neal Cardwell <ncardwell@...gle.com>,
Mubashir Adnan Qureshi <mubashirq@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew@...n.ch>,
Jonathan Corbet <corbet@....net>, David Ahern <dsahern@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org, Chao Wu <wwchao@...gle.com>,
Wei Wang <weiwan@...gle.com>, Pradeep Nemavat <pnemavat@...gle.com>
Subject: Re: [PATCH v4 net-next 5/6] net-device: reorganize net_device fast
path variables
On Thu, Oct 26, 2023 at 2:42 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Thu, Oct 26, 2023 at 10:20 AM Coco Li <lixiaoyan@...gle.com> wrote:
> >
> > Reorganize fast path variables on tx-txrx-rx order
> > Fastpath variables end after npinfo.
> >
> > Below data generated with pahole on x86 architecture.
> >
> > Fast path variables span cache lines before change: 12
> > Fast path variables span cache lines after change: 4
> >
> > Signed-off-by: Coco Li <lixiaoyan@...gle.com>
> > Suggested-by: Eric Dumazet <edumazet@...gle.com>
> > Reviewed-by: David Ahern <dsahern@...nel.org>
> > ---
> > include/linux/netdevice.h | 113 ++++++++++++++++++++------------------
> > net/core/dev.c | 51 +++++++++++++++++
> > 2 files changed, 111 insertions(+), 53 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index b8bf669212cce..26c4d57451bf0 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -2076,6 +2076,66 @@ enum netdev_ml_priv_type {
> > */
> >
> > struct net_device {
> > + /* Cacheline organization can be found documented in
> > + * Documentation/networking/net_cachelines/net_device.rst.
> > + * Please update the document when adding new fields.
> > + */
> > +
> > + /* TX read-mostly hotpath */
> > + __cacheline_group_begin(net_device_read);
>
> This should be net_device_write ? Or perhaps simply tx ?
>
>
> > + unsigned long long priv_flags;
> > + const struct net_device_ops *netdev_ops;
> > + const struct header_ops *header_ops;
> > + struct netdev_queue *_tx;
> > + unsigned int real_num_tx_queues;
> > + unsigned int gso_max_size;
> > + unsigned int gso_ipv4_max_size;
> > + u16 gso_max_segs;
> > + s16 num_tc;
> > + /* Note : dev->mtu is often read without holding a lock.
> > + * Writers usually hold RTNL.
> > + * It is recommended to use READ_ONCE() to annotate the reads,
> > + * and to use WRITE_ONCE() to annotate the writes.
> > + */
> > + unsigned int mtu;
> > + unsigned short needed_headroom;
> > + struct netdev_tc_txq tc_to_txq[TC_MAX_QUEUE];
> > +#ifdef CONFIG_XPS
> > + struct xps_dev_maps __rcu *xps_maps[XPS_MAPS_MAX];
> > +#endif
> > +#ifdef CONFIG_NETFILTER_EGRESS
> > + struct nf_hook_entries __rcu *nf_hooks_egress;
> > +#endif
> > +#ifdef CONFIG_NET_XGRESS
> > + struct bpf_mprog_entry __rcu *tcx_egress;
> > +#endif
> > +
> __cacheline_group_end(tx);
>
> __cacheline_group_begin(txrx);
>
>
> > + /* TXRX read-mostly hotpath */
> > + unsigned int flags;
> > + unsigned short hard_header_len;
> > + netdev_features_t features;
> > + struct inet6_dev __rcu *ip6_ptr;
> > +
>
> __cacheline_group_end(txrx);
>
> __cacheline_group_begin(rx);
>
> > + /* RX read-mostly hotpath */
> > + struct list_head ptype_specific;
> > + int ifindex;
> > + unsigned int real_num_rx_queues;
> > + struct netdev_rx_queue *_rx;
> > + unsigned long gro_flush_timeout;
> > + int napi_defer_hard_irqs;
> > + unsigned int gro_max_size;
> > + unsigned int gro_ipv4_max_size;
> > + rx_handler_func_t __rcu *rx_handler;
> > + void __rcu *rx_handler_data;
> > + possible_net_t nd_net;
> > +#ifdef CONFIG_NETPOLL
> > + struct netpoll_info __rcu *npinfo;
> > +#endif
> > +#ifdef CONFIG_NET_XGRESS
> > + struct bpf_mprog_entry __rcu *tcx_ingress;
> > +#endif
> > + __cacheline_group_end(net_device_read);
> > +
> > char name[IFNAMSIZ];
> > struct netdev_name_node *name_node;
> > struct dev_ifalias __rcu *ifalias;
> > @@ -2100,7 +2160,6 @@ struct net_device {
> > struct list_head unreg_list;
> > struct list_head close_list;
> > struct list_head ptype_all;
> > - struct list_head ptype_specific;
> >
> > struct {
> > struct list_head upper;
> > @@ -2108,25 +2167,12 @@ struct net_device {
> > } adj_list;
> >
> > /* Read-mostly cache-line for fast-path access */
> > - unsigned int flags;
> > xdp_features_t xdp_features;
> > - unsigned long long priv_flags;
> > - const struct net_device_ops *netdev_ops;
> > const struct xdp_metadata_ops *xdp_metadata_ops;
> > - int ifindex;
> > unsigned short gflags;
> > - unsigned short hard_header_len;
> >
> > - /* Note : dev->mtu is often read without holding a lock.
> > - * Writers usually hold RTNL.
> > - * It is recommended to use READ_ONCE() to annotate the reads,
> > - * and to use WRITE_ONCE() to annotate the writes.
> > - */
> > - unsigned int mtu;
> > - unsigned short needed_headroom;
> > unsigned short needed_tailroom;
> >
> > - netdev_features_t features;
> > netdev_features_t hw_features;
> > netdev_features_t wanted_features;
> > netdev_features_t vlan_features;
> > @@ -2170,8 +2216,6 @@ struct net_device {
> > const struct tlsdev_ops *tlsdev_ops;
> > #endif
> >
> > - const struct header_ops *header_ops;
> > -
> > unsigned char operstate;
> > unsigned char link_mode;
> >
> > @@ -2212,9 +2256,7 @@ struct net_device {
> >
> >
> > /* Protocol-specific pointers */
> > -
> > struct in_device __rcu *ip_ptr;
> > - struct inet6_dev __rcu *ip6_ptr;
> > #if IS_ENABLED(CONFIG_VLAN_8021Q)
> > struct vlan_info __rcu *vlan_info;
> > #endif
> > @@ -2249,26 +2291,14 @@ struct net_device {
> > /* Interface address info used in eth_type_trans() */
> > const unsigned char *dev_addr;
> >
> > - struct netdev_rx_queue *_rx;
> > unsigned int num_rx_queues;
> > - unsigned int real_num_rx_queues;
> > -
> > struct bpf_prog __rcu *xdp_prog;
> > - unsigned long gro_flush_timeout;
> > - int napi_defer_hard_irqs;
> > #define GRO_LEGACY_MAX_SIZE 65536u
> > /* TCP minimal MSS is 8 (TCP_MIN_GSO_SIZE),
> > * and shinfo->gso_segs is a 16bit field.
> > */
> > #define GRO_MAX_SIZE (8 * 65535u)
> > - unsigned int gro_max_size;
> > - unsigned int gro_ipv4_max_size;
> > unsigned int xdp_zc_max_segs;
> > - rx_handler_func_t __rcu *rx_handler;
> > - void __rcu *rx_handler_data;
> > -#ifdef CONFIG_NET_XGRESS
> > - struct bpf_mprog_entry __rcu *tcx_ingress;
> > -#endif
> > struct netdev_queue __rcu *ingress_queue;
> > #ifdef CONFIG_NETFILTER_INGRESS
> > struct nf_hook_entries __rcu *nf_hooks_ingress;
> > @@ -2283,25 +2313,13 @@ struct net_device {
> > /*
> > * Cache lines mostly used on transmit path
> > */
> > - struct netdev_queue *_tx ____cacheline_aligned_in_smp;
> > unsigned int num_tx_queues;
> > - unsigned int real_num_tx_queues;
> > struct Qdisc __rcu *qdisc;
> > unsigned int tx_queue_len;
> > spinlock_t tx_global_lock;
> >
> > struct xdp_dev_bulk_queue __percpu *xdp_bulkq;
> >
> > -#ifdef CONFIG_XPS
> > - struct xps_dev_maps __rcu *xps_maps[XPS_MAPS_MAX];
> > -#endif
> > -#ifdef CONFIG_NET_XGRESS
> > - struct bpf_mprog_entry __rcu *tcx_egress;
> > -#endif
> > -#ifdef CONFIG_NETFILTER_EGRESS
> > - struct nf_hook_entries __rcu *nf_hooks_egress;
> > -#endif
> > -
> > #ifdef CONFIG_NET_SCHED
> > DECLARE_HASHTABLE (qdisc_hash, 4);
> > #endif
> > @@ -2340,12 +2358,6 @@ struct net_device {
> > bool needs_free_netdev;
> > void (*priv_destructor)(struct net_device *dev);
> >
> > -#ifdef CONFIG_NETPOLL
> > - struct netpoll_info __rcu *npinfo;
> > -#endif
> > -
> > - possible_net_t nd_net;
> > -
> > /* mid-layer private */
> > void *ml_priv;
> > enum netdev_ml_priv_type ml_priv_type;
> > @@ -2379,20 +2391,15 @@ struct net_device {
> > */
> > #define GSO_MAX_SIZE (8 * GSO_MAX_SEGS)
> >
> > - unsigned int gso_max_size;
> > #define TSO_LEGACY_MAX_SIZE 65536
> > #define TSO_MAX_SIZE UINT_MAX
> > unsigned int tso_max_size;
> > - u16 gso_max_segs;
> > #define TSO_MAX_SEGS U16_MAX
> > u16 tso_max_segs;
> > - unsigned int gso_ipv4_max_size;
> >
> > #ifdef CONFIG_DCB
> > const struct dcbnl_rtnl_ops *dcbnl_ops;
> > #endif
> > - s16 num_tc;
> > - struct netdev_tc_txq tc_to_txq[TC_MAX_QUEUE];
> > u8 prio_tc_map[TC_BITMASK + 1];
> >
> > #if IS_ENABLED(CONFIG_FCOE)
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index a37a932a3e145..ca7e653e6c348 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -11511,6 +11511,55 @@ static struct pernet_operations __net_initdata default_device_ops = {
> > .exit_batch = default_device_exit_batch,
> > };
> >
> > +static void __init net_dev_struct_check(void)
> > +{
> > + /* TX read-mostly hotpath */
>
> Of course, change net_device_read to either rx, txrx, or tx, depending
> of each field purpose/location.
The group names need to be unique, hence the verbosity. I will update
the patch series with more detailed cache line group separations.
Thank you!
>
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, priv_flags);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, netdev_ops);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, header_ops);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, _tx);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, real_num_tx_queues);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, gso_max_size);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, gso_ipv4_max_size);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, gso_max_segs);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, num_tc);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, mtu);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, needed_headroom);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, tc_to_txq);
> > +#ifdef CONFIG_XPS
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, xps_maps);
> > +#endif
> > +#ifdef CONFIG_NETFILTER_EGRESS
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, nf_hooks_egress);
> > +#endif
> > +#ifdef CONFIG_NET_XGRESS
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, tcx_egress);
> > +#endif
> > + /* TXRX read-mostly hotpath */
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, flags);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, hard_header_len);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, features);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, ip6_ptr);
> > + /* RX read-mostly hotpath */
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, ptype_specific);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, ifindex);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, real_num_rx_queues);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, _rx);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, gro_flush_timeout);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, napi_defer_hard_irqs);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, gro_max_size);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, gro_ipv4_max_size);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, rx_handler);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, rx_handler_data);
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, nd_net);
> > +#ifdef CONFIG_NETPOLL
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, npinfo);
> > +#endif
> > +#ifdef CONFIG_NET_XGRESS
> > + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read, tcx_ingress);
> > +#endif
> > +}
> > +
> > /*
> > * Initialize the DEV module. At boot time this walks the device list and
> > * unhooks any devices that fail to initialise (normally hardware not
> > @@ -11528,6 +11577,8 @@ static int __init net_dev_init(void)
> >
> > BUG_ON(!dev_boot_phase);
> >
> > + net_dev_struct_check();
> > +
> > if (dev_proc_init())
> > goto out;
> >
> > --
> > 2.42.0.758.gaed0368e0e-goog
> >
Powered by blists - more mailing lists