On Sun, Aug 20, 2023 at 11:07:33AM +0200, Morten Brørup wrote: > Bruce, Honnappa, Konstantin, > > Back in 2017, Bruce added support for non-power-of-2 rings with this patch > [1]. > > [1]: > https://git.dpdk.org/dpdk/commit/lib/librte_ring/rte_ring.h?id=b74461155543430f5253e96ad6d413ebcad36693 > > I think that the calculation of "entries" in __rte_ring_move_cons_head() > [2][3] is incorrect when the ring capacity is not power-of-2, because it is > missing the capacity comparison you added to rte_ring_count() [4]. Please > review if I'm mistaken. > > [2]: > https://elixir.bootlin.com/dpdk/v23.07/source/lib/ring/rte_ring_c11_pvt.h#L159 > [3]: > https://elixir.bootlin.com/dpdk/v23.07/source/lib/ring/rte_ring_generic_pvt.h#L150 > [4]: https://elixir.bootlin.com/dpdk/v23.07/source/lib/ring/rte_ring.h#L502 > thanks for flagging this inconsistency, but I think we are ok.
For consumer, I think this is correct, because we are only ever reducing the number of entries in the ring, and the calculation of the number of entries is made in the usual way using modulo arithmetic. We should never have more than capacity entries in the ring so the check in ring count I believe is superflous. [The exception would be if someone bypassed the inline functions and accessed the ring directly themselves - at which point "all bets are off", to use the English phrase] The producer code (__rte_ring_move_prod_head) does do a capacity check, which is where one is required to ensure we never exceed capacity. /Bruce
