> -----Original Message----- > From: Jack Min <jack...@nvidia.com> > Sent: Thursday, August 17, 2023 9:32 PM > To: Stephen Hemminger <step...@networkplumber.org>; Honnappa > Nagarahalli <honnappa.nagaraha...@arm.com> > Cc: dev@dpdk.org; Matan Azrad <ma...@nvidia.com>; > viachesl...@nvidia.com; Tyler Retzlaff <roret...@linux.microsoft.com>; > Wathsala Wathawana Vithanage <wathsala.vithan...@arm.com>; nd > <n...@arm.com> > Subject: Re: MLX5 PMD access ring library private data > > On 2023/8/17 22:06, Stephen Hemminger wrote: > > On Thu, 17 Aug 2023 05:06:20 +0000 > > Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> wrote: > > > >> Hi Matan, Viacheslav, > >> Tyler pointed out that the function > __mlx5_hws_cnt_pool_enqueue_revert is accessing the ring private structure > members (prod.head and prod.tail) directly. Even though ' struct rte_ring' is > a > public structure (mainly because the library provides inline functions), the > structure members are considered private to the ring library. So, this needs > to > be corrected. > >> > >> It looks like the function __mlx5_hws_cnt_pool_enqueue_revert is trying > to revert things that were enqueued. It is not clear to me why this > functionality is required. Can you provide the use case for this? We can > discuss possible solutions. > > How can reverting be thread safe? Consumer could have already looked at > them? > > Hey, > > In our case, this ring is SC/SP, only accessed by one thread > (enqueue/dequeue/revert). You could implement a more simpler and more efficient (For ex: such an implementation would not need any atomic operations, would require less number of cache lines) ring for this. Is this function being used in the dataplane?
> > The scenario we have "revert" is: > > We use ring to manager our HW objects (counter in this case) and for each > core (thread) has "cache" (a SC/SP ring) for sake of performance. > > 1. Get objects from "cache" firstly, if cache is empty, we fetch a bulk of > free > objects from global ring into cache. > > 2. Put (free) objects also into "cache" firstly, if cache is full, we flush a > bulk of > objects into global ring in order to make some rooms in cache. > > However, this HW object cannot be immediately reused after free. It needs > time to be reset and then can be used again. > > So when we flush cache, we want to keep the first enqueued objects still stay > there because they have more chance already be reset than the latest > enqueued objects. > > Only flush recently enqueued objects back into global ring, act as "LIFO" > behavior. > > This is why we require "revert" enqueued objects. You could use 'rte_ring_free_count' API before you enqueue to check for available space. > > -Jack >
