Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> writes:
> 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)
Since you'll likely be spinning a v3 (to switch off the macroization),
this series seems to have some unit test failures:
24/82 DPDK:fast-tests / event_ring_autotest FAIL 0.12 s (exit status
255 or signal 127 SIGinvalid)
--- command ---
DPDK_TEST='event_ring_autotest'
/home/travis/build/ovsrobot/dpdk/build/app/test/dpdk-test -l 0-1
--file-prefix=event_ring_autotest
--- stdout ---
EAL: Probing VFIO support...
APP: HPET is not enabled, using TSC as default timer
RTE>>event_ring_autotest
RING: Requested number of elements is invalid, must be power of 2, and do
not exceed the limit 2147483647
Test detected odd count
Test detected NULL ring lookup
RING: Requested number of elements is invalid, must be power of 2, and do
not exceed the limit 2147483647
RING: Requested number of elements is invalid, must be power of 2, and do
not exceed the limit 2147483647
Error, status after enqueue is unexpected
Test Failed
RTE>>
--- stderr ---
EAL: Detected 2 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Multi-process socket /var/run/dpdk/event_ring_autotest/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: No available hugepages reported in hugepages-1048576kB
-------
Please double check. Seems to only happen with clang/llvm.
> Honnappa Nagarahalli (6):
> lib/ring: apis to support configurable element size
> lib/ring: add template to support different element sizes
> tools/checkpatch: relax constraints on __rte_experimental
> lib/ring: add ring APIs to support 32b ring elements
> lib/hash: use ring with 32b element size to save memory
> lib/eventdev: use ring templates for event rings
>
> devtools/checkpatches.sh | 11 +-
> lib/librte_eventdev/Makefile | 2 +
> lib/librte_eventdev/meson.build | 2 +
> lib/librte_eventdev/rte_event_ring.c | 146 +---------
> lib/librte_eventdev/rte_event_ring.h | 41 +--
> lib/librte_eventdev/rte_event_ring_128b.c | 19 ++
> lib/librte_eventdev/rte_event_ring_128b.h | 44 +++
> lib/librte_hash/rte_cuckoo_hash.c | 55 ++--
> lib/librte_hash/rte_cuckoo_hash.h | 2 +-
> lib/librte_ring/Makefile | 9 +-
> lib/librte_ring/meson.build | 11 +-
> lib/librte_ring/rte_ring.c | 34 ++-
> lib/librte_ring/rte_ring.h | 72 +++++
> lib/librte_ring/rte_ring_32.c | 19 ++
> lib/librte_ring/rte_ring_32.h | 36 +++
> lib/librte_ring/rte_ring_template.c | 46 +++
> lib/librte_ring/rte_ring_template.h | 330 ++++++++++++++++++++++
> lib/librte_ring/rte_ring_version.map | 4 +
> 18 files changed, 660 insertions(+), 223 deletions(-)
> create mode 100644 lib/librte_eventdev/rte_event_ring_128b.c
> create mode 100644 lib/librte_eventdev/rte_event_ring_128b.h
> create mode 100644 lib/librte_ring/rte_ring_32.c
> create mode 100644 lib/librte_ring/rte_ring_32.h
> create mode 100644 lib/librte_ring/rte_ring_template.c
> create mode 100644 lib/librte_ring/rte_ring_template.h
