Re: [dpdk-dev] [RFC] eventdev: uninline inline API functions

On 2021-08-31 14:28, Jerin Jacob wrote:

On Mon, Aug 30, 2021 at 9:30 PM Mattias Rönnblom
[off-list ref] wrote:

quoted

Replace the inline functions in the eventdev user application API with
regular non-inline API calls. This allows for a cleaner and more
simple API/ABI, but might well also cause performance regressions.

The purpose of this RFC patch is to allow for performance testing.

The rte_eventdev struct declaration should be moved off the public
API.

Signed-off-by: Mattias Rönnblom <redacted>

I think we need to align all DPDK subsystems to a similar scheme.[1]

That makes perfect sense.

I see -5% kind of regression-based on workload.

[1]
https://protect2.fireeye.com/v1/url?k=50900b74-0f0b325f-50904bef-866038973a15-e3d1ddd2888a6b58&q=1&e=96184cd6-f4b1-4ed9-98a3-ae921c747c7e&u=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20210820162834.12544-2-konstantin.ananyev%40intel.com%2F

quoted

---
  drivers/net/octeontx/octeontx_ethdev.h  |  1 +
  lib/eventdev/rte_event_eth_rx_adapter.h |  1 +
  lib/eventdev/rte_event_eth_tx_adapter.c | 31 ++++++++
  lib/eventdev/rte_event_eth_tx_adapter.h | 35 ++-------
  lib/eventdev/rte_eventdev.c             | 82 +++++++++++++++++++++
  lib/eventdev/rte_eventdev.h             | 94 +++----------------------
  lib/eventdev/version.map                |  4 ++
  7 files changed, 134 insertions(+), 114 deletions(-)

diff --git a/drivers/net/octeontx/octeontx_ethdev.h b/drivers/net/octeontx/octeontx_ethdev.h
index b73515de37..9402105fcf 100644
--- a/drivers/net/octeontx/octeontx_ethdev.h
+++ b/drivers/net/octeontx/octeontx_ethdev.h

@@ -9,6 +9,7 @@

  #include <rte_common.h>
  #include <ethdev_driver.h>
+#include <eventdev_pmd.h>
  #include <rte_eventdev.h>
  #include <rte_mempool.h>
  #include <rte_memory.h>

diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h b/lib/eventdev/rte_event_eth_rx_adapter.h
index 182dd2e5dd..79f4822fb0 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.h
+++ b/lib/eventdev/rte_event_eth_rx_adapter.h

@@ -84,6 +84,7 @@ extern "C" {
  #include <rte_service.h>

  #include "rte_eventdev.h"
+#include "eventdev_pmd.h"

  #define RTE_EVENT_ETH_RX_ADAPTER_MAX_INSTANCE 32

diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
index 18c0359db7..74f88e6147 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.c
+++ b/lib/eventdev/rte_event_eth_tx_adapter.c

@@ -1154,6 +1154,37 @@ rte_event_eth_tx_adapter_start(uint8_t id)
         return ret;
  }

+uint16_t
+rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
+                                uint8_t port_id,
+                                struct rte_event ev[],
+                                uint16_t nb_events,
+                                const uint8_t flags)
+{
+       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+
+#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
+       if (dev_id >= RTE_EVENT_MAX_DEVS ||
+               !rte_eventdevs[dev_id].attached) {
+               rte_errno = EINVAL;
+               return 0;
+       }
+
+       if (port_id >= dev->data->nb_ports) {
+               rte_errno = EINVAL;
+               return 0;
+       }
+#endif
+       rte_eventdev_trace_eth_tx_adapter_enqueue(dev_id, port_id, ev,
+               nb_events, flags);
+       if (flags)
+               return dev->txa_enqueue_same_dest(dev->data->ports[port_id],
+                                                 ev, nb_events);
+       else
+               return dev->txa_enqueue(dev->data->ports[port_id], ev,
+                                       nb_events);
+}
+
  int
  rte_event_eth_tx_adapter_stats_get(uint8_t id,
                                 struct rte_event_eth_tx_adapter_stats *stats)

diff --git a/lib/eventdev/rte_event_eth_tx_adapter.h b/lib/eventdev/rte_event_eth_tx_adapter.h
index 8c59547165..3cd65e8a09 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.h
+++ b/lib/eventdev/rte_event_eth_tx_adapter.h

@@ -79,6 +79,7 @@ extern "C" {
  #include <rte_mbuf.h>

  #include "rte_eventdev.h"
+#include "eventdev_pmd.h"

  /**
   * Adapter configuration structure

@@ -348,36 +349,12 @@ rte_event_eth_tx_adapter_event_port_get(uint8_t id, uint8_t *event_port_id);
   *              one or more events. This error code is only applicable to
   *              closed systems.
   */
-static inline uint16_t
+uint16_t
  rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
-                               uint8_t port_id,
-                               struct rte_event ev[],
-                               uint16_t nb_events,
-                               const uint8_t flags)
-{
-       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
-
-#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
-       if (dev_id >= RTE_EVENT_MAX_DEVS ||
-               !rte_eventdevs[dev_id].attached) {
-               rte_errno = EINVAL;
-               return 0;
-       }
-
-       if (port_id >= dev->data->nb_ports) {
-               rte_errno = EINVAL;
-               return 0;
-       }
-#endif
-       rte_eventdev_trace_eth_tx_adapter_enqueue(dev_id, port_id, ev,
-               nb_events, flags);
-       if (flags)
-               return dev->txa_enqueue_same_dest(dev->data->ports[port_id],
-                                                 ev, nb_events);
-       else
-               return dev->txa_enqueue(dev->data->ports[port_id], ev,
-                                       nb_events);
-}
+                                uint8_t port_id,
+                                struct rte_event ev[],
+                                uint16_t nb_events,
+                                const uint8_t flags);

  /**
   * Retrieve statistics for an adapter

diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 594dd5e759..e2dad8a838 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c

@@ -1119,6 +1119,65 @@ rte_event_port_links_get(uint8_t dev_id, uint8_t port_id,
         return count;
  }

+static __rte_always_inline uint16_t
+__rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
+                       const struct rte_event ev[], uint16_t nb_events,
+                       const event_enqueue_burst_t fn)
+{
+       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+
+#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
+       if (dev_id >= RTE_EVENT_MAX_DEVS || !rte_eventdevs[dev_id].attached) {
+               rte_errno = EINVAL;
+               return 0;
+       }
+
+       if (port_id >= dev->data->nb_ports) {
+               rte_errno = EINVAL;
+               return 0;
+       }
+#endif
+       rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, fn);
+       /*
+        * Allow zero cost non burst mode routine invocation if application
+        * requests nb_events as const one
+        */
+       if (nb_events == 1)
+               return (*dev->enqueue)(dev->data->ports[port_id], ev);
+       else
+               return fn(dev->data->ports[port_id], ev, nb_events);
+}
+
+uint16_t
+rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
+                       const struct rte_event ev[], uint16_t nb_events)
+{
+       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+
+       return __rte_event_enqueue_burst(dev_id, port_id, ev, nb_events,
+                                        dev->enqueue_burst);
+}
+
+uint16_t
+rte_event_enqueue_new_burst(uint8_t dev_id, uint8_t port_id,
+                           const struct rte_event ev[], uint16_t nb_events)
+{
+       const struct rte_eventdev *dev = &rte_event_devices[dev_id];
+
+       return __rte_event_enqueue_burst(dev_id, port_id, ev, nb_events,
+                                        dev->enqueue_new_burst);
+}
+
+uint16_t
+rte_event_enqueue_forward_burst(uint8_t dev_id, uint8_t port_id,
+                               const struct rte_event ev[], uint16_t nb_events)
+{
+       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+
+       return __rte_event_enqueue_burst(dev_id, port_id, ev, nb_events,
+                       dev->enqueue_forward_burst);
+}
+
  int
  rte_event_dequeue_timeout_ticks(uint8_t dev_id, uint64_t ns,
                                  uint64_t *timeout_ticks)

@@ -1135,6 +1194,29 @@ rte_event_dequeue_timeout_ticks(uint8_t dev_id, uint64_t ns,
         return (*dev->dev_ops->timeout_ticks)(dev, ns, timeout_ticks);
  }

+uint16_t
+rte_event_dequeue_burst(uint8_t dev_id, uint8_t port_id, struct rte_event ev[],
+                       uint16_t nb_events, uint64_t timeout_ticks)
+{
+       struct rte_eventdev *dev = &rte_event_devices[dev_id];
+
+#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
+       if (dev_id >= RTE_EVENT_MAX_DEVS || !rte_eventdevs[dev_id].attached) {
+               rte_errno = EINVAL;
+               return 0;
+       }
+
+       if (port_id >= dev->data->nb_ports) {
+               rte_errno = EINVAL;
+               return 0;
+       }
+#endif
+       rte_eventdev_trace_deq_burst(dev_id, port_id, ev, nb_events);
+
+       return (*dev->dequeue_burst)(dev->data->ports[port_id], ev, nb_events,
+                                    timeout_ticks);
+}
+
  int
  rte_event_dev_service_id_get(uint8_t dev_id, uint32_t *service_id)
  {

diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index a9c496fb62..451e9fb0a0 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h

@@ -1445,38 +1445,6 @@ struct rte_eventdev {
         void *reserved_ptrs[3];   /**< Reserved for future fields */
  } __rte_cache_aligned;

-extern struct rte_eventdev *rte_eventdevs;
-/** @internal The pool of rte_eventdev structures. */
-
-static __rte_always_inline uint16_t
-__rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
-                       const struct rte_event ev[], uint16_t nb_events,
-                       const event_enqueue_burst_t fn)
-{
-       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
-
-#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
-       if (dev_id >= RTE_EVENT_MAX_DEVS || !rte_eventdevs[dev_id].attached) {
-               rte_errno = EINVAL;
-               return 0;
-       }
-
-       if (port_id >= dev->data->nb_ports) {
-               rte_errno = EINVAL;
-               return 0;
-       }
-#endif
-       rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, fn);
-       /*
-        * Allow zero cost non burst mode routine invocation if application
-        * requests nb_events as const one
-        */
-       if (nb_events == 1)
-               return (*dev->enqueue)(dev->data->ports[port_id], ev);
-       else
-               return fn(dev->data->ports[port_id], ev, nb_events);
-}
-
  /**
   * Enqueue a burst of events objects or an event object supplied in *rte_event*
   * structure on an  event device designated by its *dev_id* through the event

@@ -1520,15 +1488,9 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
   *              closed systems.
   * @see rte_event_port_attr_get(), RTE_EVENT_PORT_ATTR_ENQ_DEPTH
   */
-static inline uint16_t
+uint16_t
  rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
-                       const struct rte_event ev[], uint16_t nb_events)
-{
-       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
-
-       return __rte_event_enqueue_burst(dev_id, port_id, ev, nb_events,
-                       dev->enqueue_burst);
-}
+                       const struct rte_event ev[], uint16_t nb_events);

  /**
   * Enqueue a burst of events objects of operation type *RTE_EVENT_OP_NEW* on

@@ -1571,15 +1533,9 @@ rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
   * @see rte_event_port_attr_get(), RTE_EVENT_PORT_ATTR_ENQ_DEPTH
   * @see rte_event_enqueue_burst()
   */
-static inline uint16_t
+uint16_t
  rte_event_enqueue_new_burst(uint8_t dev_id, uint8_t port_id,
-                       const struct rte_event ev[], uint16_t nb_events)
-{
-       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
-
-       return __rte_event_enqueue_burst(dev_id, port_id, ev, nb_events,
-                       dev->enqueue_new_burst);
-}
+                           const struct rte_event ev[], uint16_t nb_events);

  /**
   * Enqueue a burst of events objects of operation type *RTE_EVENT_OP_FORWARD*

@@ -1622,15 +1578,10 @@ rte_event_enqueue_new_burst(uint8_t dev_id, uint8_t port_id,
   * @see rte_event_port_attr_get(), RTE_EVENT_PORT_ATTR_ENQ_DEPTH
   * @see rte_event_enqueue_burst()
   */
-static inline uint16_t
+uint16_t
  rte_event_enqueue_forward_burst(uint8_t dev_id, uint8_t port_id,
-                       const struct rte_event ev[], uint16_t nb_events)
-{
-       const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
-
-       return __rte_event_enqueue_burst(dev_id, port_id, ev, nb_events,
-                       dev->enqueue_forward_burst);
-}
+                               const struct rte_event ev[],
+                               uint16_t nb_events);

  /**
   * Converts nanoseconds to *timeout_ticks* value for rte_event_dequeue_burst()

@@ -1727,36 +1678,9 @@ rte_event_dequeue_timeout_ticks(uint8_t dev_id, uint64_t ns,
   *
   * @see rte_event_port_dequeue_depth()
   */
-static inline uint16_t
+uint16_t
  rte_event_dequeue_burst(uint8_t dev_id, uint8_t port_id, struct rte_event ev[],
-                       uint16_t nb_events, uint64_t timeout_ticks)
-{
-       struct rte_eventdev *dev = &rte_eventdevs[dev_id];
-
-#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
-       if (dev_id >= RTE_EVENT_MAX_DEVS || !rte_eventdevs[dev_id].attached) {
-               rte_errno = EINVAL;
-               return 0;
-       }
-
-       if (port_id >= dev->data->nb_ports) {
-               rte_errno = EINVAL;
-               return 0;
-       }
-#endif
-       rte_eventdev_trace_deq_burst(dev_id, port_id, ev, nb_events);
-       /*
-        * Allow zero cost non burst mode routine invocation if application
-        * requests nb_events as const one
-        */
-       if (nb_events == 1)
-               return (*dev->dequeue)(
-                       dev->data->ports[port_id], ev, timeout_ticks);
-       else
-               return (*dev->dequeue_burst)(
-                       dev->data->ports[port_id], ev, nb_events,
-                               timeout_ticks);
-}
+                       uint16_t nb_events, uint64_t timeout_ticks);

  /**
   * Link multiple source event queues supplied in *queues* to the destination

diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
index 88625621ec..8da79cbdc0 100644
--- a/lib/eventdev/version.map
+++ b/lib/eventdev/version.map

@@ -13,7 +13,11 @@ DPDK_22 {
         rte_event_crypto_adapter_stats_get;
         rte_event_crypto_adapter_stats_reset;
         rte_event_crypto_adapter_stop;
+       rte_event_enqueue_burst;
+       rte_event_enqueue_new_burst;
+       rte_event_enqueue_forward_burst;
         rte_event_dequeue_timeout_ticks;
+       rte_event_dequeue_burst;
         rte_event_dev_attr_get;
         rte_event_dev_close;
         rte_event_dev_configure;
--

2.17.1