| Message ID | 92805c7dbdec5be2572798177196467fef7c108f.1524671282.git.echaudro@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | [ovs-dev,1/1] netdev-dpdk: Fixed netdev_dpdk structure alignment | expand |
Thanks for this, Eelco. (Sorry for the double email, missed the ML on the first one.) On 25/04/2018 16:48, Eelco Chaudron wrote: > Currently, the code tells us we have 4 pad bytes left in cacheline0 > while actually we are 8 bytes short: > This was caused by commit 5e925cc ("netdev-dpdk: DPDK v17.11 upgrade"), where the dpdk_port_t typedef was changed from uint8_t to uint16_t, messing up the alignment. > struct netdev_dpdk { > union { > OVS_CACHE_LINE_MARKER cacheline0; /* 1 */ > struct { > dpdk_port_t port_id; /* 0 2 */ > _Bool attached; /* 2 1 */ > struct eth_addr hwaddr; /* 4 6 */ > int mtu; /* 12 4 */ > int socket_id; /* 16 4 */ > int buf_size; /* 20 4 */ > int max_packet_len; /* 24 4 */ > enum dpdk_dev_type type; /* 28 4 */ > enum netdev_flags flags; /* 32 4 */ > char * devargs; /* 40 8 */ > struct dpdk_tx_queue * tx_q; /* 48 8 */ > struct rte_eth_link link; /* 56 8 */ > int link_reset_cnt; /* 64 4 */ > }; /* 72 */ > uint8_t pad9[128]; /* 128 */ > }; /* 0 128 */ > /* --- cacheline 2 boundary (128 bytes) --- */ > > Re-located one member, link_reset_cnt, and now it's one cache line: > > struct netdev_dpdk { > union { > OVS_CACHE_LINE_MARKER cacheline0; /* 1 */ > struct { > dpdk_port_t port_id; /* 0 2 */ > _Bool attached; /* 2 1 */ > struct eth_addr hwaddr; /* 4 6 */ > int mtu; /* 12 4 */ > int socket_id; /* 16 4 */ > int buf_size; /* 20 4 */ > int max_packet_len; /* 24 4 */ > enum dpdk_dev_type type; /* 28 4 */ > enum netdev_flags flags; /* 32 4 */ > int link_reset_cnt; /* 36 4 */ > char * devargs; /* 40 8 */ > struct dpdk_tx_queue * tx_q; /* 48 8 */ > struct rte_eth_link link; /* 56 8 */ > }; /* 64 */ > uint8_t pad9[64]; /* 64 */ > }; /* 0 64 */ > /* --- cacheline 1 boundary (64 bytes) --- */ > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Acked-by: Tiago Lam <tiago.lam@intel.com> Tiago
> Thanks for this, Eelco. > > (Sorry for the double email, missed the ML on the first one.) > > On 25/04/2018 16:48, Eelco Chaudron wrote: > > Currently, the code tells us we have 4 pad bytes left in cacheline0 > > while actually we are 8 bytes short: > > > > This was caused by commit 5e925cc ("netdev-dpdk: DPDK v17.11 upgrade"), > where the dpdk_port_t typedef was changed from uint8_t to uint16_t, > messing up the alignment. > Thanks all, I merged this to DPDK_MERGE and backported for OVS 2.9. Thanks Ian > > struct netdev_dpdk { > > union { > > OVS_CACHE_LINE_MARKER cacheline0; /* 1 */ > > struct { > > dpdk_port_t port_id; /* 0 2 */ > > _Bool attached; /* 2 1 */ > > struct eth_addr hwaddr; /* 4 6 */ > > int mtu; /* 12 4 */ > > int socket_id; /* 16 4 */ > > int buf_size; /* 20 4 */ > > int max_packet_len; /* 24 4 */ > > enum dpdk_dev_type type; /* 28 4 */ > > enum netdev_flags flags; /* 32 4 */ > > char * devargs; /* 40 8 */ > > struct dpdk_tx_queue * tx_q; /* 48 8 */ > > struct rte_eth_link link; /* 56 8 */ > > int link_reset_cnt; /* 64 4 */ > > }; /* 72 */ > > uint8_t pad9[128]; /* 128 */ > > }; /* 0 128 */ > > /* --- cacheline 2 boundary (128 bytes) --- */ > > > > Re-located one member, link_reset_cnt, and now it's one cache line: > > > > struct netdev_dpdk { > > union { > > OVS_CACHE_LINE_MARKER cacheline0; /* 1 */ > > struct { > > dpdk_port_t port_id; /* 0 2 */ > > _Bool attached; /* 2 1 */ > > struct eth_addr hwaddr; /* 4 6 */ > > int mtu; /* 12 4 */ > > int socket_id; /* 16 4 */ > > int buf_size; /* 20 4 */ > > int max_packet_len; /* 24 4 */ > > enum dpdk_dev_type type; /* 28 4 */ > > enum netdev_flags flags; /* 32 4 */ > > int link_reset_cnt; /* 36 4 */ > > char * devargs; /* 40 8 */ > > struct dpdk_tx_queue * tx_q; /* 48 8 */ > > struct rte_eth_link link; /* 56 8 */ > > }; /* 64 */ > > uint8_t pad9[64]; /* 64 */ > > }; /* 0 64 */ > > /* --- cacheline 1 boundary (64 bytes) --- */ > > > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > > Acked-by: Tiago Lam <tiago.lam@intel.com> > > Tiago > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ee39cbe4a..b14140571 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -363,11 +363,10 @@ struct netdev_dpdk { int max_packet_len; enum dpdk_dev_type type; enum netdev_flags flags; + int link_reset_cnt; char *devargs; /* Device arguments for dpdk ports */ struct dpdk_tx_queue *tx_q; struct rte_eth_link link; - int link_reset_cnt; - /* 4 pad bytes here. */ ); PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline1,
Currently, the code tells us we have 4 pad bytes left in cacheline0 while actually we are 8 bytes short: struct netdev_dpdk { union { OVS_CACHE_LINE_MARKER cacheline0; /* 1 */ struct { dpdk_port_t port_id; /* 0 2 */ _Bool attached; /* 2 1 */ struct eth_addr hwaddr; /* 4 6 */ int mtu; /* 12 4 */ int socket_id; /* 16 4 */ int buf_size; /* 20 4 */ int max_packet_len; /* 24 4 */ enum dpdk_dev_type type; /* 28 4 */ enum netdev_flags flags; /* 32 4 */ char * devargs; /* 40 8 */ struct dpdk_tx_queue * tx_q; /* 48 8 */ struct rte_eth_link link; /* 56 8 */ int link_reset_cnt; /* 64 4 */ }; /* 72 */ uint8_t pad9[128]; /* 128 */ }; /* 0 128 */ /* --- cacheline 2 boundary (128 bytes) --- */ Re-located one member, link_reset_cnt, and now it's one cache line: struct netdev_dpdk { union { OVS_CACHE_LINE_MARKER cacheline0; /* 1 */ struct { dpdk_port_t port_id; /* 0 2 */ _Bool attached; /* 2 1 */ struct eth_addr hwaddr; /* 4 6 */ int mtu; /* 12 4 */ int socket_id; /* 16 4 */ int buf_size; /* 20 4 */ int max_packet_len; /* 24 4 */ enum dpdk_dev_type type; /* 28 4 */ enum netdev_flags flags; /* 32 4 */ int link_reset_cnt; /* 36 4 */ char * devargs; /* 40 8 */ struct dpdk_tx_queue * tx_q; /* 48 8 */ struct rte_eth_link link; /* 56 8 */ }; /* 64 */ uint8_t pad9[64]; /* 64 */ }; /* 0 64 */ /* --- cacheline 1 boundary (64 bytes) --- */ Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- lib/netdev-dpdk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)