Hi, Morten
> -----Original Message-----
> From: Morten Brørup <m...@smartsharesystems.com>
> Sent: Thursday, March 23, 2023 7:42 PM
> To: Feifei Wang <feifei.wa...@arm.com>; tho...@monjalon.net; Ferruh
> Yigit <ferruh.yi...@amd.com>; Andrew Rybchenko
> <andrew.rybche...@oktetlabs.ru>
> Cc: dev@dpdk.org; konstantin.v.anan...@yandex.ru; nd <n...@arm.com>;
> Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; Ruifeng Wang
> <ruifeng.w...@arm.com>
> Subject: RE: [PATCH v4 1/3] ethdev: add API for buffer recycle mode
>
> > From: Feifei Wang [mailto:feifei.wa...@arm.com]
> > Sent: Thursday, 23 March 2023 11.43
> >
>
> [...]
>
> > +static inline uint16_t rte_eth_rx_descriptors_refill(uint16_t port_id,
> > + uint16_t queue_id, uint16_t nb)
> > +{
> > + struct rte_eth_fp_ops *p;
> > + void *qd;
> > +
> > +#ifdef RTE_ETHDEV_DEBUG_RX
> > + if (port_id >= RTE_MAX_ETHPORTS ||
> > + queue_id >= RTE_MAX_QUEUES_PER_PORT) {
> > + RTE_ETHDEV_LOG(ERR,
> > + "Invalid port_id=%u or queue_id=%u\n",
> > + port_id, queue_id);
> > + rte_errno = ENODEV;
> > + return 0;
> > + }
> > +#endif
> > +
> > + p = &rte_eth_fp_ops[port_id];
> > + qd = p->rxq.data[queue_id];
> > +
> > +#ifdef RTE_ETHDEV_DEBUG_RX
> > + if (!rte_eth_dev_is_valid_port(port_id)) {
> > + RTE_ETHDEV_LOG(ERR, "Invalid Rx port_id=%u\n", port_id);
> > + rte_errno = ENODEV;
> > + return 0;
> > +
> > + if (qd == NULL) {
> > + RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u for
> port_id=%u\n",
> > + queue_id, port_id);
> > + rte_errno = ENODEV;
> > + return 0;
> > + }
> > +#endif
> > +
> > + if (!p->rx_descriptors_refill)
>
> Compare to NULL instead: if (p->rx_descriptors_refill == NULL)
>
Ack.
> > + return 0;
> > +
> > + return p->rx_descriptors_refill(qd, nb); }
> > +
> > /**@{@name Rx hardware descriptor states
> > * @see rte_eth_rx_descriptor_status
> > */
> > @@ -6483,6 +6597,122 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
> queue_id,
> > return rte_eth_tx_buffer_flush(port_id, queue_id, buffer); }
> >
> > +/**
> > + * @internal
> > + * Tx routine for rte_eth_dev_buf_recycle().
> > + * Stash Tx used buffers into Rx buffer ring in buffer recycle mode.
> > + *
> > + * @note
> > + * This API can only be called by rte_eth_dev_buf_recycle().
> > + * After calling this API, rte_eth_rx_descriptors_refill() should be
> > + * called to refill Rx ring descriptors.
> > + *
> > + * When this functionality is not implemented in the driver, the
> > +return
> > + * buffer number is 0.
> > + *
> > + * @param port_id
> > + * The port identifier of the Ethernet device.
> > + * @param queue_id
> > + * The index of the transmit queue.
> > + * The value must be in the range [0, nb_tx_queue - 1] previously
> supplied
> > + * to rte_eth_dev_configure().
> > + * @param rxq_buf_recycle_info
> > + * A pointer to a structure of Rx queue buffer ring information in buffer
> > + * recycle mode.
> > + *
> > + * @return
> > + * The number buffers correct to be filled in the Rx buffer ring.
> > + * - ENODEV: bad port or queue (only if compiled with debug).
> > + */
> > +static inline uint16_t rte_eth_tx_buf_stash(uint16_t port_id,
> > +uint16_t
> > queue_id,
> > + struct rte_eth_rxq_buf_recycle_info *rxq_buf_recycle_info)
> {
> > + struct rte_eth_fp_ops *p;
> > + void *qd;
> > +
> > +#ifdef RTE_ETHDEV_DEBUG_TX
> > + if (port_id >= RTE_MAX_ETHPORTS ||
> > + queue_id >= RTE_MAX_QUEUES_PER_PORT) {
> > + RTE_ETHDEV_LOG(ERR,
> > + "Invalid port_id=%u or queue_id=%u\n",
> > + port_id, queue_id);
> > + rte_errno = ENODEV;
> > + return 0;
> > + }
> > +#endif
> > +
> > + p = &rte_eth_fp_ops[port_id];
> > + qd = p->txq.data[queue_id];
> > +
> > +#ifdef RTE_ETHDEV_DEBUG_TX
> > + if (!rte_eth_dev_is_valid_port(port_id)) {
> > + RTE_ETHDEV_LOG(ERR, "Invalid Tx port_id=%u\n", port_id);
> > + rte_errno = ENODEV;
> > + return 0;
> > +
> > + if (qd == NULL) {
> > + RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u for
> port_id=%u\n",
> > + queue_id, port_id);
> > + rte_erno = ENODEV;
> > + return 0;
> > + }
> > +#endif
> > +
> > + if (p->tx_buf_stash == NULL)
> > + return 0;
> > +
> > + return p->tx_buf_stash(qd, rxq_buf_recycle_info); }
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> > +notice
> > + *
> > + * Buffer recycle mode can let Tx queue directly put used buffers
> > +into Rx
> > buffer
> > + * ring. This avoids freeing buffers into mempool and allocating
> > +buffers from
> > + * mempool.
> > + *
> > + * @param rx_port_id
> > + * Port identifying the receive side.
> > + * @param rx_queue_id
> > + * The index of the receive queue identifying the receive side.
> > + * The value must be in the range [0, nb_rx_queue - 1] previously
> supplied
> > + * to rte_eth_dev_configure().
> > + * @param tx_port_id
> > + * Port identifying the transmit side.
> > + * @param tx_queue_id
> > + * The index of the transmit queue identifying the transmit side.
> > + * The value must be in the range [0, nb_tx_queue - 1] previously
> supplied
> > + * to rte_eth_dev_configure().
> > + * @param rxq_recycle_info
> > + * A pointer to a structure of type *rte_eth_txq_rearm_data* to be
> > filled.
> > + * @return
> > + * - (0) on success or no recycling buffer.
> > + * - (-EINVAL) rxq_recycle_info is NULL.
> > + */
> > +__rte_experimental
> > +static inline int
> > +rte_eth_dev_buf_recycle(uint16_t rx_port_id, uint16_t rx_queue_id,
> > + uint16_t tx_port_id, uint16_t tx_queue_id,
> > + struct rte_eth_rxq_buf_recycle_info *rxq_buf_recycle_info)
> {
> > + /* The number of recycling buffers. */
> > + uint16_t nb_buf;
> > +
> > + if (!rxq_buf_recycle_info)
> > + return -EINVAL;
>
> Compare to NULL instead: if (rxq_buf_recycle_info == NULL)
>
Ack.
> Alternatively: Consider RTE_ASSERT() and require rxq_buf_recycle_info to
> be valid.
>
Good comments. This ensure users to create a rxq_buf_recycle_info variable
before
call this info_get API.
> > +
> > + /* Stash Tx used buffers into Rx buffer ring */
> > + nb_buf = rte_eth_tx_buf_stash(tx_port_id, tx_queue_id,
> > + rxq_buf_recycle_info);
> > + /* If there are recycling buffers, refill Rx queue descriptors. */
> > + if (nb_buf)
> > + rte_eth_rx_descriptors_refill(rx_port_id, rx_queue_id,
> > + nb_buf);
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * @warning
> > * @b EXPERIMENTAL: this API may change without prior notice diff
> > --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h
> > index dcf8adab92..10f9d5cbe7 100644
> > --- a/lib/ethdev/rte_ethdev_core.h
> > +++ b/lib/ethdev/rte_ethdev_core.h
> > @@ -56,6 +56,13 @@ typedef int (*eth_rx_descriptor_status_t)(void
> > *rxq, uint16_t offset);
> > /** @internal Check the status of a Tx descriptor */ typedef int
> > (*eth_tx_descriptor_status_t)(void *txq, uint16_t offset);
> >
> > +/** @internal Stash Tx used buffers into RX ring in buffer recycle
> > +mode */ typedef uint16_t (*eth_tx_buf_stash_t)(void *txq,
> > + struct rte_eth_rxq_buf_recycle_info *rxq_buf_recycle_info);
> > +
> > +/** @internal Refill Rx descriptors in buffer recycle mode */ typedef
> > +uint16_t (*eth_rx_descriptors_refill_t)(void *rxq, uint16_t nb);
> > +
> > /**
> > * @internal
> > * Structure used to hold opaque pointers to internal ethdev Rx/Tx @@
> > -90,6 +97,8 @@ struct rte_eth_fp_ops {
> > eth_rx_queue_count_t rx_queue_count;
> > /** Check the status of a Rx descriptor. */
> > eth_rx_descriptor_status_t rx_descriptor_status;
> > + /** Refill Rx descriptors in buffer recycle mode */
> > + eth_rx_descriptors_refill_t rx_descriptors_refill;
> > /** Rx queues data. */
> > struct rte_ethdev_qdata rxq;
> > uintptr_t reserved1[3];
>
> In order to keep 64 B alignment, the reserved1 array must be reduced from 3
> to 2.
Agree.
>
> > @@ -106,6 +115,8 @@ struct rte_eth_fp_ops {
> > eth_tx_prep_t tx_pkt_prepare;
> > /** Check the status of a Tx descriptor. */
> > eth_tx_descriptor_status_t tx_descriptor_status;
> > + /** Stash Tx used buffers into RX ring in buffer recycle mode */
> > + eth_tx_buf_stash_t tx_buf_stash;
> > /** Tx queues data. */
> > struct rte_ethdev_qdata txq;
> > uintptr_t reserved2[3];
>
> In order to keep 64 B alignment, the reserved2 array must be reduced from 3
> to 2.
Agree.
>
> > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map index
> > 357d1a88c0..8a4b1dac80 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -299,6 +299,10 @@ EXPERIMENTAL {
> > rte_flow_action_handle_query_update;
> > rte_flow_async_action_handle_query_update;
> > rte_flow_async_create_by_index;
> > +
> > + # added in 23.07
> > + rte_eth_dev_buf_recycle;
> > + rte_eth_rx_queue_buf_recycle_info_get;
> > };
> >
> > INTERNAL {
> > @@ -328,4 +332,6 @@ INTERNAL {
> > rte_eth_representor_id_get;
> > rte_eth_switch_domain_alloc;
> > rte_eth_switch_domain_free;
> > + rte_eth_tx_buf_stash;
> > + rte_eth_rx_descriptors_refill;
> > };
> > --
> > 2.25.1
> >
