> On Jan 9, 2019, at 1:52 AM, Olivier Matz <[email protected]> wrote: > > On Wed, Jan 09, 2019 at 10:38:07AM +0100, David Marchand wrote: >> On Wed, Jan 9, 2019 at 9:54 AM Yongseok Koh <[email protected]> wrote: >> >>> On replenishing Rx buffers for vectorized Rx, mbuf->buf_addr isn't needed >>> to be accessed as it is static and easily calculated from the mbuf address. >>> Accessing the mbuf content causes unnecessary load stall and it is worsened >>> on ARM. >>> >>> Fixes: 545b884b1da3 ("net/mlx5: fix buffer address posting in SSE Rx") >>> Cc: [email protected] >>> >>> Signed-off-by: Yongseok Koh <[email protected]> >>> --- >>> drivers/net/mlx5/mlx5_rxtx_vec.h | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/mlx5/mlx5_rxtx_vec.h >>> b/drivers/net/mlx5/mlx5_rxtx_vec.h >>> index fda7004e2d..ced5547307 100644 >>> --- a/drivers/net/mlx5/mlx5_rxtx_vec.h >>> +++ b/drivers/net/mlx5/mlx5_rxtx_vec.h >>> @@ -102,8 +102,12 @@ mlx5_rx_replenish_bulk_mbuf(struct mlx5_rxq_data >>> *rxq, uint16_t n) >>> return; >>> } >>> for (i = 0; i < n; ++i) { >>> - wq[i].addr = rte_cpu_to_be_64((uintptr_t)elts[i]->buf_addr >>> + >>> - RTE_PKTMBUF_HEADROOM); >>> + uintptr_t buf_addr = >>> + (uintptr_t)elts[i] + sizeof(struct rte_mbuf) + >>> + rte_pktmbuf_priv_size(rxq->mp) + >>> RTE_PKTMBUF_HEADROOM; >>> + >>> + assert(buf_addr == (uintptr_t)elts[i]->buf_addr); >>> + wq[i].addr = rte_cpu_to_be_64(buf_addr); >>> /* If there's only one MR, no need to replace LKey in WQE. >>> */ >>> if (unlikely(mlx5_mr_btree_len(&rxq->mr_ctrl.cache_bh) > >>> 1)) >>> wq[i].lkey = mlx5_rx_mb2mr(rxq, elts[i]); >>> -- >>> 2.11.0 >>> >>> >> How about having a macro / inline in the mbuf api to get this information >> in a consistent/unique way ? >> I can see we have this calculation at least in rte_pktmbuf_init() and >> rte_pktmbuf_detach(). > > Agree. Maybe rte_mbuf_default_buf_addr(m) ?
I'm also okay to add. Will come up with a new patch. > Side note, is the assert() correct in the patch? I'd say there's a > difference of RTE_PKTMBUF_HEADROOM between the 2 values. Oops, my fault. Thanks for the catch, you saved a crash. :-) Thanks, Yongseok

