RE: MLX5 PMD access ring library private data

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

> -----Original Message-----
> From: Jack Min <jackmin@nvidia.com>
> Sent: Thursday, August 17, 2023 9:32 PM
> To: Stephen Hemminger <stephen@networkplumber.org>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@nvidia.com>;
> viacheslavo@nvidia.com; Tyler Retzlaff <roretzla@linux.microsoft.com>;
> Wathsala Wathawana Vithanage <wathsala.vithanage@arm.com>; nd
> <nd@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.Nagarahalli@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
>