On 2021-08-31 14:28, Jerin Jacob wrote: > On Mon, Aug 30, 2021 at 9:30 PM Mattias Rönnblom > <mattias.ronnb...@ericsson.com> wrote: >> 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 <mattias.ronnb...@ericsson.com> > 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 > >> --- >> 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 >>
