* Re: rte_eth_dev_data reorganization needed
2024-01-08 17:14 rte_eth_dev_data reorganization needed Stephen Hemminger
@ 2024-01-09 16:58 ` Ferruh Yigit
0 siblings, 0 replies; 2+ messages in thread
From: Ferruh Yigit @ 2024-01-09 16:58 UTC (permalink / raw)
To: Stephen Hemminger, dev
Cc: Thomas Monjalon, Jerin Jacob Kollanukkaran, Qi Z Zhang
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)));
^ permalink raw reply [flat|nested] 2+ messages in thread