Skip to contentSkip to conversation

rte_eth_dev_data reorganization needed

DISCUSSION Stephen Hemminger 2 participants 2 messages Jan 8, 2024 – Jan 9, 2024

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 5 lines · 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.

For reference following is structure layout [1].

Agree that it can benefit from reorganization.

Quoted 2 lines

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)));