Re: [dpdk-dev] [PATCH v4 4/7] ethdev: make burst functions to use new flat array

[David Marchand <david.marchand@redhat.com>]

On Mon, Oct 4, 2021 at 3:59 PM Konstantin Ananyev
<konstantin.ananyev@intel.com> wrote:
>
> Rework 'fast' burst functions to use rte_eth_fp_ops[].
> While it is an API/ABI breakage, this change is intended to be
> transparent for both users (no changes in user app is required) and
> PMD developers (no changes in PMD is required).
> One extra thing to note - RX/TX callback invocation will cause extra
> function call with these changes. That might cause some insignificant
> slowdown for code-path where RX/TX callbacks are heavily involved.
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
>  lib/ethdev/ethdev_private.c |  31 +++++
>  lib/ethdev/rte_ethdev.h     | 242 ++++++++++++++++++++++++++----------
>  lib/ethdev/version.map      |   5 +
>  3 files changed, 210 insertions(+), 68 deletions(-)
>
> diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c
> index 3eeda6e9f9..27d29b2ac6 100644
> --- a/lib/ethdev/ethdev_private.c
> +++ b/lib/ethdev/ethdev_private.c
> @@ -226,3 +226,34 @@ eth_dev_fp_ops_setup(struct rte_eth_fp_ops *fpo,
>         fpo->txq.data = dev->data->tx_queues;
>         fpo->txq.clbk = (void **)(uintptr_t)dev->pre_tx_burst_cbs;
>  }
> +
> +uint16_t
> +__rte_eth_rx_epilog(uint16_t port_id, uint16_t queue_id,
> +       struct rte_mbuf **rx_pkts, uint16_t nb_rx, uint16_t nb_pkts,
> +       void *opaque)
> +{
> +       const struct rte_eth_rxtx_callback *cb = opaque;
> +
> +       while (cb != NULL) {
> +               nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
> +                               nb_pkts, cb->param);
> +               cb = cb->next;
> +       }
> +
> +       return nb_rx;
> +}

This helper name is ambiguous.
Maybe the intent was to have a generic place holder for updates in
future releases.
But in this series, __rte_eth_rx_epilog is invoked only if a rx
callback is registered, under #ifdef RTE_ETHDEV_RXTX_CALLBACKS.

I'd prefer we call it a spade, i.e. rte_eth_call_rx_callbacks, and it
does not need to be advertised as internal.


> +
> +uint16_t
> +__rte_eth_tx_prolog(uint16_t port_id, uint16_t queue_id,
> +       struct rte_mbuf **tx_pkts, uint16_t nb_pkts, void *opaque)
> +{
> +       const struct rte_eth_rxtx_callback *cb = opaque;
> +
> +       while (cb != NULL) {
> +               nb_pkts = cb->fn.tx(port_id, queue_id, tx_pkts, nb_pkts,
> +                               cb->param);
> +               cb = cb->next;
> +       }
> +
> +       return nb_pkts;
> +}

Idem, rte_eth_call_tx_callbacks.


-- 
David Marchand