[dpdk-dev] [PATCH v2 0/6] lib/ring: templates to support custom element size

[David Marchand]

On Fri, Sep 6, 2019 at 9:05 PM Honnappa Nagarahalli
<honnappa.nagarahalli at arm.com> wrote:
>
> The current rte_ring hard-codes the type of the ring element to 'void *',
> hence the size of the element is hard-coded to 32b/64b. Since the ring
> element type is not an input to rte_ring APIs, it results in couple
> of issues:
>
> 1) If an application requires to store an element which is not 64b, it
>    needs to write its own ring APIs similar to rte_event_ring APIs. This
>    creates additional burden on the programmers, who end up making
>    work-arounds and often waste memory.
> 2) If there are multiple libraries that store elements of the same
>    type, currently they would have to write their own rte_ring APIs. This
>    results in code duplication.
>
> This patch consists of several parts:
> 1) New APIs to support configurable ring element size
>    These will help reduce code duplication in the templates. I think these
>    can be made internal (do not expose to DPDK applications, but expose to
>    DPDK libraries), feedback needed.
>
> 2) rte_ring templates
>    The templates provide an easy way to add new APIs for different ring
>    element types/sizes which can be used by multiple libraries. These
>    also allow for creating APIs to store elements of custom types
>    (for ex: a structure)
>
>    The template needs 4 parameters:
>    a) RTE_RING_TMPLT_API_SUFFIX - This is used as a suffix to the
>       rte_ring APIs.
>       For ex: if RTE_RING_TMPLT_API_SUFFIX is '32b', the API name will be
>       rte_ring_create_32b
>    b) RTE_RING_TMPLT_ELEM_SIZE - Size of the ring element in bytes.
>       For ex: sizeof(uint32_t)
>    c) RTE_RING_TMPLT_ELEM_TYPE - Type of the ring element.
>       For ex: uint32_t. If a common ring library does not use a standard
>       data type, it should create its own type by defining a structure
>       with standard data type. For ex: for an elment size of 96b, one
>       could define a structure
>
>       struct s_96b {
>           uint32_t a[3];
>       }
>       The common library can use this structure to define
>       RTE_RING_TMPLT_ELEM_TYPE.
>
>       The application using this common ring library should define its
>       element type as a union with the above structure.
>
>       union app_element_type {
>           struct s_96b v;
>           struct app_element {
>               uint16_t a;
>               uint16_t b;
>               uint32_t c;
>               uint32_t d;
>           }
>       }
>    d) RTE_RING_TMPLT_EXPERIMENTAL - Indicates if the new APIs being defined
>       are experimental. Should be set to empty to remove the experimental
>       tag.
>
>    The ring library consists of some APIs that are defined as inline
>    functions and some APIs that are non-inline functions. The non-inline
>    functions are in rte_ring_template.c. However, this file needs to be
>    included in other .c files. Any feedback on how to handle this is
>    appreciated.
>
>    Note that the templates help create the APIs that are dependent on the
>    element size (for ex: rte_ring_create, enqueue/dequeue etc). Other APIs
>    that do NOT depend on the element size do not need to be part of the
>    template (for ex: rte_ring_dump, rte_ring_count, rte_ring_free_count
>    etc).
>
> 3) APIs for 32b ring element size
>    This uses the templates to create APIs to enqueue/dequeue elements of
>    size 32b.
>
> 4) rte_hash libray is changed to use 32b ring APIs
>    The 32b APIs are used in rte_hash library to store the free slot index
>    and free bucket index.
>
> 5) Event Dev changed to use ring templates
>    Event Dev defines its own 128b ring APIs using the templates. This helps
>    in keeping the 'struct rte_event' as is. If Event Dev has to use generic
>    128b ring APIs, it requires 'struct rte_event' to change to
>    'union rte_event' to include a generic data type such as '__int128_t'.
>    This breaks the API compatibility and results in large number of
>    changes.
>    With this change, the event rings are stored on rte_ring's tailq.
>    Event Dev specific ring list is NOT available. IMO, this does not have
>    any impact to the user.
>
> This patch results in following checkpatch issue:
> WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
>
> However, this patch is following the rules in the existing code. Please
> let me know if this needs to be fixed.
>
> v2
>  - Change Event Ring implementation to use ring templates
>    (Jerin, Pavan)

I expect a v3 on this series:
- Bruce/Stephen were not happy with using macros,
- Aaron caught test issues,
- from my side, if patch 3 still applies after your changes, I prefer
we drop this patch on the check script, we can live with these
warnings,


Thanks.

-- 
David Marchand