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.
