Hi, Morten > -----邮件原件----- > 发件人: Morten Brørup <m...@smartsharesystems.com> > 发送时间: Tuesday, February 28, 2023 4:09 PM > 收件人: Konstantin Ananyev <konstantin.anan...@huawei.com>; Feifei > Wang <feifei.wa...@arm.com>; Konstantin Ananyev > <konstantin.v.anan...@yandex.ru>; tho...@monjalon.net; Ferruh Yigit > <ferruh.yi...@amd.com>; Andrew Rybchenko > <andrew.rybche...@oktetlabs.ru> > 抄送: dev@dpdk.org; nd <n...@arm.com>; Honnappa Nagarahalli > <honnappa.nagaraha...@arm.com>; Ruifeng Wang > <ruifeng.w...@arm.com>; nd <n...@arm.com> > 主题: RE: [PATCH v3 1/3] ethdev: enable direct rearm with separate API > > Hi Feifei, > > > From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com] > > Sent: Monday, 27 February 2023 20.32 > > > > Hi Feifei , > > > > > > > > > + uint16_t *rearm_start; > > > > > + uint16_t *rearm_nb; > > > > > > > > I know that for Intel NICs uint16_t is sufficient, wonder would it > > > > always > > be > > > > for other vendors? > > > > Another thing to consider the case when ring position wrapping? > > > > Again I know that it is not required for Intel NICs, but would it > > > > be > > sufficient > > > > for API that supposed to be general? > > > > > > > For this, we re-define this structure: > > > rte_eth_rxq_rearm_data { > > > void *rx_sw_ring; > > > uint16_t *rearm_start; > > > uint16_t *rearm_nb; > > > } > > > -> > > > struct *rxq_recycle_info { > > > rte_mbuf **buf_ring; > > > uint16_t *offset = (uint16 *)(&rq->ci); > > > uint16_t *end; > > > uint16_t ring_size; > > > > > > } > > > For the new structure, *offset is a pointer for rearm-start index of > > > Rx buffer ring (consumer index). *end is a pointer for rearm-end > > > index Of Rx buffer ring (producer index). > > > > > > 1. we look up different pmds, some pmds using 'uint_16t' as index > > > size like > > intel PMD, > > > some pmds using 'uint32_t' as index size like MLX5 or thunderx PMD. > > > For pmd using 'uint32_t', rearm starts at 'buf_ring[offset & > > > (ring_size - > > 1)]', and 'uint16_t' > > > is enough for ring size. > > > > Sounds like a smart idea to me. > > When configuring an Ethernet device queue, the nb_rx/tx_desc parameter > to rte_eth_rx/tx_queue_setup() is uint16_t, so I agree that uint16_t should > suffice here too. > > I had the following thought, but am not sure. So please take this comment > for consideration only: > > I think the "& (ring_size -1)" is superfluous, unless a PMD allows its index > pointer to exceed the ring size, and performs the same "& (ring_size -1)" > when using the index pointer to access its ring. > > And if a PMD uses the index pointer like that (i.e. exceeding the ring size), > you would need the same wrap protection for a 16 bit index pointer. > > > > > > > > > > > 2. Good question. In general path, there is a constraint that > > > 'nb_rearm < > > ring_size - rq->ci', > > > This can ensure no ring wrapping in rearm. Thus in direct-rearm, we > > > will > > refer to this to > > > solve ring wrapping. > > > > Should work, I think... > > Just need not to forget to document it :) > > It is this constraint (the guarantee that there is no ring wrapping in a rearm > burst) that makes me think that the "& (ring_size -1)" is superfluous.
Actually there is some misleading here, I can explain about this. (ring_size -1 ) is for some pmds whose index can exceed the ring_size, such as MLX5: uint16_t *offset = &mlx5_rxq->rq_ci, rq_ci is an index which can exceed the ring_size.
