MLX5 PMD access ring library private data

[Jack Min]

On 2023/8/19 19:57, Konstantin Ananyev wrote:
> 18/08/2023 10:38, Jack Min пишет:
>> On 2023/8/18 17:05, Konstantin Ananyev wrote:
>>>> On 2023/8/17 22:06, Stephen Hemminger wrote:
>>>>> On Thu, 17 Aug 2023 05:06:20 +0000
>>>>> Honnappa Nagarahalli<Honnappa.Nagarahalli at 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).
>>>>
>>>> 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.
>>>>
>>> Wouldn't then simple stack fit you better?
>>> Something like lib/stack/rte_stack_std.h, but even without spinlock 
>>> around?
>>
>> No, stack is always a "LIFO" struct, right?
>
> Yep.
>
>>
>> Here first we need this cache works as "FIFO" in most cases (get/put) 
>> because the first enqueued objects have more chance that are already 
>> reset so can reuse them.
>>
>> We only require "LIFO" behavior when "flush" cache in order to make 
>> some room, so next free will be quick because it happens in our local 
>> cache, needn't access global ring.
>>
>> In short, we require a struct supports "FIFO" and "LIFO".
>
> Ok, thanks fro explanation.
> So you need a ring, but with an ability to revert prod N elements 
> back, right?

Yes, exactly!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20230820/9b3013c2/attachment.htm>