rte_eth_dev_data reorganization needed

rte_eth_dev_data reorganization needed

[Stephen Hemminger]

While looking at ethdev (for pdump issues) noticed.
The rte_eth_dev_data structure layout is unorganized.
Lots of holes, fields not arranged in logical groups
and usage could be localized for better cache access.

Obviously, any reorg is ABI break. We should do this for 24.11

Re: rte_eth_dev_data reorganization needed

[Ferruh Yigit]

On 1/8/2024 5:14 PM, Stephen Hemminger wrote:
> While looking at ethdev (for pdump issues) noticed.
> The rte_eth_dev_data structure layout is unorganized.
> Lots of holes, fields not arranged in logical groups
> and usage could be localized for better cache access.
> 

For reference following is structure layout [1].

Agree that it can benefit from reorganization.

> Obviously, any reorg is ABI break. We should do this for 24.11
>

It may not be an ABI break, since structure moved to ethdev_driver.h
with recent changes, it is not exposed to user directly anymore.


eth_dev_data is device data and it is not directly involved in the
datapath, although I can see 'rx_mbuf_alloc_failed' stats is updated in
datapath, and some drivers access 'dev_private' in datapath, most of
other fields are configuration related.

So, although I expect changes in eth_dev_data doesn't impact the
performance it needs to be tested and verified.

As this release looks mostly quite, perhaps it can be good time to try
this kind of change, what do you think to start reorg in this release?





[1]
struct rte_eth_dev_data {
 char                       name[64];             /*     0    64 */
 /* --- cacheline 1 boundary (64 bytes) --- */
 void * *                   rx_queues;            /*    64     8 */
 void * *                   tx_queues;            /*    72     8 */
 uint16_t                   nb_rx_queues;         /*    80     2 */
 uint16_t                   nb_tx_queues;         /*    82     2 */
 struct rte_eth_dev_sriov   sriov;                /*    84     6 */

 /* XXX 6 bytes hole, try to pack */

 void *                     dev_private;          /*    96     8 */
 struct rte_eth_link        dev_link __attribute__((__aligned__(8))); /*
  104     8 */

 /* XXX last struct has 2 bytes of padding */

 struct rte_eth_conf        dev_conf;             /*   112  2280 */

 /* XXX last struct has 4 bytes of padding */

 /* --- cacheline 37 boundary (2368 bytes) was 24 bytes ago --- */
 uint16_t                   mtu;                  /*  2392     2 */

 /* XXX 2 bytes hole, try to pack */

 uint32_t                   min_rx_buf_size;      /*  2396     4 */
 uint64_t                   rx_mbuf_alloc_failed; /*  2400     8 */
 struct rte_ether_addr *    mac_addrs;            /*  2408     8 */
 uint64_t                   mac_pool_sel[128];    /*  2416  1024 */
 /* --- cacheline 53 boundary (3392 bytes) was 48 bytes ago --- */
 struct rte_ether_addr *    hash_mac_addrs;       /*  3440     8 */
 uint16_t                   port_id;              /*  3448     2 */
 uint8_t                    promiscuous:1;        /*  3450: 0  1 */
 uint8_t                    scattered_rx:1;       /*  3450: 1  1 */
 uint8_t                    all_multicast:1;      /*  3450: 2  1 */
 uint8_t                    dev_started:1;        /*  3450: 3  1 */
 uint8_t                    lro:1;                /*  3450: 4  1 */
 uint8_t                    dev_configured:1;     /*  3450: 5  1 */
 uint8_t                    flow_configured:1;    /*  3450: 6  1 */

 /* XXX 1 bit hole, try to pack */

 uint8_t                    rx_queue_state[1024]; /*  3451  1024 */
 /* --- cacheline 69 boundary (4416 bytes) was 59 bytes ago --- */
 uint8_t                    tx_queue_state[1024]; /*  4475  1024 */

 /* XXX 1 byte hole, try to pack */

 /* --- cacheline 85 boundary (5440 bytes) was 60 bytes ago --- */
 uint32_t                   dev_flags;            /*  5500     4 */
 /* --- cacheline 86 boundary (5504 bytes) --- */
 int                        numa_node;            /*  5504     4 */

 /* XXX 4 bytes hole, try to pack */

 struct rte_vlan_filter_conf vlan_filter_conf;    /*  5512   512 */
 /* --- cacheline 94 boundary (6016 bytes) was 8 bytes ago --- */
 struct rte_eth_dev_owner   owner;                /*  6024    72 */
 /* --- cacheline 95 boundary (6080 bytes) was 16 bytes ago --- */
 uint16_t                   representor_id;       /*  6096     2 */
 uint16_t                   backer_port_id;       /*  6098     2 */

 /* XXX 4 bytes hole, try to pack */

 pthread_mutex_t            flow_ops_mutex;       /*  6104    40 */

 /* size: 6144, cachelines: 96, members: 32 */
 /* sum members: 6126, holes: 5, sum holes: 17 */
 /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */
 /* paddings: 2, sum paddings: 6 */
 /* forced alignments: 1 */
} __attribute__((__aligned__(64)));