[v1,3/5] ethdev: add API for direct rearm mode
Checks
Commit Message
Add API for enabling direct rearm mode and for mapping RX and TX
queues. Currently, the API supports 1:1(txq : rxq) mapping.
Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
lib/ethdev/version.map | 1 +
4 files changed, 61 insertions(+)
Comments
> From: Feifei Wang [mailto:feifei.wang2@arm.com]
> Sent: Wednesday, 20 April 2022 10.17
>
> Add API for enabling direct rearm mode and for mapping RX and TX
> queues. Currently, the API supports 1:1(txq : rxq) mapping.
>
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
> lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
> lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
> lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
> lib/ethdev/version.map | 1 +
> 4 files changed, 61 insertions(+)
>
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 69d9dc21d8..22022f6da9 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct
> rte_eth_dev *dev,
> typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev *dev,
> uint16_t rx_queue_id);
>
> +/** @internal Enable direct rearm of a receive queue of an Ethernet
> device. */
> +typedef int (*eth_rx_direct_rearm_enable_t)(struct rte_eth_dev *dev,
> + uint16_t queue_id);
> +
> +/**< @internal map Rx/Tx queue of direct rearm mode */
> +typedef int (*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
> + uint16_t rx_queue_id,
> + uint16_t tx_port_id,
> + uint16_t tx_queue_id);
> +
> /** @internal Release memory resources allocated by given Rx/Tx queue.
> */
> typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> uint16_t queue_id);
> @@ -1152,6 +1162,11 @@ struct eth_dev_ops {
> /** Disable Rx queue interrupt */
> eth_rx_disable_intr_t rx_queue_intr_disable;
>
> + /** Enable Rx queue direct rearm mode */
> + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
A disable function seems to be missing.
> + /** Map Rx/Tx queue for direct rearm mode */
> + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
> +
> eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx
> queue */
> eth_queue_release_t tx_queue_release; /**< Release Tx
> queue */
> eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring
> mbufs */
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 29a3d80466..8e6f0284f4 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t port_id,
> uint16_t tx_queue_id,
> return eth_err(port_id, ret);
> }
>
> +int
> +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> + uint16_t tx_port_id, uint16_t tx_queue_id)
> +{
> + struct rte_eth_dev *dev;
> +
> + dev = &rte_eth_devices[rx_port_id];
> + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev, rx_queue_id);
> + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
> + tx_port_id, tx_queue_id);
Here you enable the mapping before you configure it. It could cause the driver to use an uninitialized map, if it processes packets between these two function calls.
Error handling is missing. Not all drivers support this feature, and the parameters should be validated.
Regarding driver support, the driver should also expose a capability flag to the application, similar to the RTE_ETH_DEV_CAPA_RXQ_SHARE or RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE flags. The documentation for this flag could include the description of all the restrictions to using it.
> +
> + return 0;
> +}
> +
> int
> rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port)
> {
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04cff8ee10..4a431fcbed 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5190,6 +5190,37 @@ __rte_experimental
> int rte_eth_dev_hairpin_capability_get(uint16_t port_id,
> struct rte_eth_hairpin_cap *cap);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Enable direct re-arm mode. In this mode the RX queue will be re-
> armed using
> + * buffers that have completed transmission on the transmit side.
> + *
> + * @note
> + * It is assumed that the buffers have completed transmission belong
> to the
> + * mempool used at the receive side, and have refcnt = 1.
> + *
> + * @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().
> + *
> + * @return
> + * - (0) if successful.
> + */
> +__rte_experimental
> +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t
> rx_queue_id,
> + uint16_t tx_port_id, uint16_t tx_queue_id);
> +
I agree with the parameters to your proposed API here. Since the relevant use case only needs 1:1 mapping, exposing an API function to take some sort of array with N:M mappings would be premature, and probably not ever come into play anyway.
How do you remove, disable and/or change a mapping?
> /**
> * @warning
> * @b EXPERIMENTAL: this structure may change without prior notice.
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 20391ab29e..68d664498c 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -279,6 +279,7 @@ EXPERIMENTAL {
> rte_flow_async_action_handle_create;
> rte_flow_async_action_handle_destroy;
> rte_flow_async_action_handle_update;
> + rte_eth_direct_rxrearm_map;
> };
>
> INTERNAL {
> --
> 2.25.1
>
On 4/20/22 11:16, Feifei Wang wrote:
> Add API for enabling direct rearm mode and for mapping RX and TX
> queues. Currently, the API supports 1:1(txq : rxq) mapping.
>
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
> lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
> lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
> lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
> lib/ethdev/version.map | 1 +
> 4 files changed, 61 insertions(+)
>
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 69d9dc21d8..22022f6da9 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct rte_eth_dev *dev,
> typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev *dev,
> uint16_t rx_queue_id);
>
> +/** @internal Enable direct rearm of a receive queue of an Ethernet device. */
> +typedef int (*eth_rx_direct_rearm_enable_t)(struct rte_eth_dev *dev,
> + uint16_t queue_id);
> +
> +/**< @internal map Rx/Tx queue of direct rearm mode */
> +typedef int (*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
> + uint16_t rx_queue_id,
> + uint16_t tx_port_id,
> + uint16_t tx_queue_id);
> +
> /** @internal Release memory resources allocated by given Rx/Tx queue. */
> typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> uint16_t queue_id);
> @@ -1152,6 +1162,11 @@ struct eth_dev_ops {
> /** Disable Rx queue interrupt */
> eth_rx_disable_intr_t rx_queue_intr_disable;
>
> + /** Enable Rx queue direct rearm mode */
> + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
> + /** Map Rx/Tx queue for direct rearm mode */
> + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
> +
> eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx queue */
> eth_queue_release_t tx_queue_release; /**< Release Tx queue */
> eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring mbufs */
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 29a3d80466..8e6f0284f4 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
> return eth_err(port_id, ret);
> }
>
> +int
> +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> + uint16_t tx_port_id, uint16_t tx_queue_id)
> +{
> + struct rte_eth_dev *dev;
> +
> + dev = &rte_eth_devices[rx_port_id];
I think it is rather control path. So:
We need standard checks that rx_port_id is valid.
tx_port_id must be checked as well.
rx_queue_id and tx_queue_id must be checked to be in the rate.
> + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev, rx_queue_id);
> + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
> + tx_port_id, tx_queue_id);
We must check that function pointers are not NULL as usual.
Return values must be checked.
Isn't is safe to setup map and than enable.
Otherwise we definitely need disable.
Also, what should happen on Tx port unplug? How to continue if
we still have Rx port up and running?
> +
> + return 0;
> +}
> +
> int
> rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port)
> {
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04cff8ee10..4a431fcbed 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5190,6 +5190,37 @@ __rte_experimental
> int rte_eth_dev_hairpin_capability_get(uint16_t port_id,
> struct rte_eth_hairpin_cap *cap);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Enable direct re-arm mode. In this mode the RX queue will be re-armed using
> + * buffers that have completed transmission on the transmit side.
> + *
> + * @note
> + * It is assumed that the buffers have completed transmission belong to the
> + * mempool used at the receive side, and have refcnt = 1.
I think it is possible to avoid such limitations, but
implementation will be less optimized - more checks.
> + *
> + * @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.
I guess there is an assumption that Rx and Tx ports are
serviced by the same driver. If so and if it is an API
limitation, ethdev layer must check it.
> + * @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().
> + *
> + * @return
> + * - (0) if successful.
> + */
> +__rte_experimental
> +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> + uint16_t tx_port_id, uint16_t tx_queue_id);
> +
> /**
> * @warning
> * @b EXPERIMENTAL: this structure may change without prior notice.
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 20391ab29e..68d664498c 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -279,6 +279,7 @@ EXPERIMENTAL {
> rte_flow_async_action_handle_create;
> rte_flow_async_action_handle_destroy;
> rte_flow_async_action_handle_update;
> + rte_eth_direct_rxrearm_map;
> };
>
> INTERNAL {
On Wed, Apr 20, 2022 at 1:47 PM Feifei Wang <feifei.wang2@arm.com> wrote:
>
> Add API for enabling direct rearm mode and for mapping RX and TX
> queues. Currently, the API supports 1:1(txq : rxq) mapping.
>
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
> + *
> + * @return
> + * - (0) if successful.
> + */
> +__rte_experimental
> +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> + uint16_t tx_port_id, uint16_t tx_queue_id);
Won't existing rte_eth_hairpin_* APIs work to achieve the same?
On Wed, 20 Apr 2022 16:16:48 +0800
Feifei Wang <feifei.wang2@arm.com> wrote:
> Add API for enabling direct rearm mode and for mapping RX and TX
> queues. Currently, the API supports 1:1(txq : rxq) mapping.
>
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
> lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
> lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
> lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
> lib/ethdev/version.map | 1 +
> 4 files changed, 61 insertions(+)
>
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 69d9dc21d8..22022f6da9 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct rte_eth_dev *dev,
> typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev *dev,
> uint16_t rx_queue_id);
>
> +/** @internal Enable direct rearm of a receive queue of an Ethernet device. */
> +typedef int (*eth_rx_direct_rearm_enable_t)(struct rte_eth_dev *dev,
> + uint16_t queue_id);
> +
> +/**< @internal map Rx/Tx queue of direct rearm mode */
> +typedef int (*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
> + uint16_t rx_queue_id,
> + uint16_t tx_port_id,
> + uint16_t tx_queue_id);
> +
> /** @internal Release memory resources allocated by given Rx/Tx queue. */
> typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> uint16_t queue_id);
> @@ -1152,6 +1162,11 @@ struct eth_dev_ops {
> /** Disable Rx queue interrupt */
> eth_rx_disable_intr_t rx_queue_intr_disable;
>
> + /** Enable Rx queue direct rearm mode */
> + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
> + /** Map Rx/Tx queue for direct rearm mode */
> + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
> +
> eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx queue */
> eth_queue_release_t tx_queue_release; /**< Release Tx queue */
> eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring mbufs */
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 29a3d80466..8e6f0284f4 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
> return eth_err(port_id, ret);
> }
>
> +int
> +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> + uint16_t tx_port_id, uint16_t tx_queue_id)
> +{
> + struct rte_eth_dev *dev;
> +
> + dev = &rte_eth_devices[rx_port_id];
> + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev, rx_queue_id);
> + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
> + tx_port_id, tx_queue_id);
> +
> + return 0;
> +}
> +
> int
> rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port)
> {
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04cff8ee10..4a431fcbed 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5190,6 +5190,37 @@ __rte_experimental
> int rte_eth_dev_hairpin_capability_get(uint16_t port_id,
> struct rte_eth_hairpin_cap *cap);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Enable direct re-arm mode. In this mode the RX queue will be re-armed using
> + * buffers that have completed transmission on the transmit side.
> + *
> + * @note
> + * It is assumed that the buffers have completed transmission belong to the
> + * mempool used at the receive side, and have refcnt = 1.
> + *
> + * @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().
> + *
> + * @return
> + * - (0) if successful.
> + */
> +__rte_experimental
> +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> + uint16_t tx_port_id, uint16_t tx_queue_id);
Just looking at this.
Why is this done via API call and not a flag as part of the receive config?
All the other offload and configuration happens via dev config.
Doing it this way doesn't follow the existing ethdev model.
> -----邮件原件-----
> 发件人: Morten Brørup <mb@smartsharesystems.com>
> 发送时间: Wednesday, April 20, 2022 5:59 PM
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>; thomas@monjalon.net;
> Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella <mdr@ashroe.eu>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>
> 主题: RE: [PATCH v1 3/5] ethdev: add API for direct rearm mode
>
> > From: Feifei Wang [mailto:feifei.wang2@arm.com]
> > Sent: Wednesday, 20 April 2022 10.17
> >
> > Add API for enabling direct rearm mode and for mapping RX and TX
> > queues. Currently, the API supports 1:1(txq : rxq) mapping.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> > lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
> > lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
> > lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
> > lib/ethdev/version.map | 1 +
> > 4 files changed, 61 insertions(+)
> >
> > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> > index 69d9dc21d8..22022f6da9 100644
> > --- a/lib/ethdev/ethdev_driver.h
> > +++ b/lib/ethdev/ethdev_driver.h
> > @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct
> > rte_eth_dev *dev, typedef int (*eth_rx_disable_intr_t)(struct
> > rte_eth_dev *dev,
> > uint16_t rx_queue_id);
> >
> > +/** @internal Enable direct rearm of a receive queue of an Ethernet
> > device. */
> > +typedef int (*eth_rx_direct_rearm_enable_t)(struct rte_eth_dev *dev,
> > + uint16_t queue_id);
> > +
> > +/**< @internal map Rx/Tx queue of direct rearm mode */ typedef int
> > +(*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
> > + uint16_t rx_queue_id,
> > + uint16_t tx_port_id,
> > + uint16_t tx_queue_id);
> > +
> > /** @internal Release memory resources allocated by given Rx/Tx queue.
> > */
> > typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> > uint16_t queue_id);
> > @@ -1152,6 +1162,11 @@ struct eth_dev_ops {
> > /** Disable Rx queue interrupt */
> > eth_rx_disable_intr_t rx_queue_intr_disable;
> >
> > + /** Enable Rx queue direct rearm mode */
> > + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
>
> A disable function seems to be missing.
[Feifei] I will try to use offload bits to enable direct-rearm mode, thus this enable function will be
removed and disable function will be unnecessary.
>
> > + /** Map Rx/Tx queue for direct rearm mode */
> > + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
> > +
> > eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx
> > queue */
> > eth_queue_release_t tx_queue_release; /**< Release Tx
> > queue */
> > eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring
> > mbufs */
> > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
> > 29a3d80466..8e6f0284f4 100644
> > --- a/lib/ethdev/rte_ethdev.c
> > +++ b/lib/ethdev/rte_ethdev.c
> > @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t
> > port_id, uint16_t tx_queue_id,
> > return eth_err(port_id, ret);
> > }
> >
> > +int
> > +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> > + uint16_t tx_port_id, uint16_t tx_queue_id) {
> > + struct rte_eth_dev *dev;
> > +
> > + dev = &rte_eth_devices[rx_port_id];
> > + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev,
> rx_queue_id);
> > + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
> > + tx_port_id, tx_queue_id);
>
> Here you enable the mapping before you configure it. It could cause the
> driver to use an uninitialized map, if it processes packets between these two
> function calls.
[Feifei] I agree with this and will change the code.
>
> Error handling is missing. Not all drivers support this feature, and the
> parameters should be validated.
[Feifei] You are right, I think after we use 'rxq->offload' bits, we can use some 'offload bits API'
to check if driver can support this.
For the parameters, I will add some check.
>
> Regarding driver support, the driver should also expose a capability flag to the
> application, similar to the RTE_ETH_DEV_CAPA_RXQ_SHARE or
> RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE flags. The documentation for this
> flag could include the description of all the restrictions to using it.
[Feifei] I will do like this by 'rxq->offload' bits, and add description to the documentation.
>
> > +
> > + return 0;
> > +}
> > +
> > int
> > rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port) { diff
> > --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > 04cff8ee10..4a431fcbed 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -5190,6 +5190,37 @@ __rte_experimental int
> > rte_eth_dev_hairpin_capability_get(uint16_t port_id,
> > struct rte_eth_hairpin_cap *cap);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> > notice
> > + *
> > + * Enable direct re-arm mode. In this mode the RX queue will be re-
> > armed using
> > + * buffers that have completed transmission on the transmit side.
> > + *
> > + * @note
> > + * It is assumed that the buffers have completed transmission belong
> > to the
> > + * mempool used at the receive side, and have refcnt = 1.
> > + *
> > + * @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().
> > + *
> > + * @return
> > + * - (0) if successful.
> > + */
> > +__rte_experimental
> > +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t
> > rx_queue_id,
> > + uint16_t tx_port_id, uint16_t tx_queue_id);
> > +
>
> I agree with the parameters to your proposed API here. Since the relevant
> use case only needs 1:1 mapping, exposing an API function to take some sort
> of array with N:M mappings would be premature, and probably not ever come
> into play anyway.
>
> How do you remove, disable and/or change a mapping?
[Feifei] It is not recommended that users change the map in the process of sending and receiving packets,
which may bring some error risks. If user want to change mapping, he needs to stop the device and call
' rte_eth_direct_rxrearm_map ' API to rewrite the mapping.
Furthermore, for 'rxq->offload', user needs to set it before dev starts. If user want to change it, dev needs to be restarted.
>
> > /**
> > * @warning
> > * @b EXPERIMENTAL: this structure may change without prior notice.
> > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map index
> > 20391ab29e..68d664498c 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -279,6 +279,7 @@ EXPERIMENTAL {
> > rte_flow_async_action_handle_create;
> > rte_flow_async_action_handle_destroy;
> > rte_flow_async_action_handle_update;
> > + rte_eth_direct_rxrearm_map;
> > };
> >
> > INTERNAL {
> > --
> > 2.25.1
> >
> -----邮件原件-----
> 发件人: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 发送时间: Wednesday, April 20, 2022 6:41 PM
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>; thomas@monjalon.net;
> Ferruh Yigit <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>
> 主题: Re: [PATCH v1 3/5] ethdev: add API for direct rearm mode
>
> On 4/20/22 11:16, Feifei Wang wrote:
> > Add API for enabling direct rearm mode and for mapping RX and TX
> > queues. Currently, the API supports 1:1(txq : rxq) mapping.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> > lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
> > lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
> > lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
> > lib/ethdev/version.map | 1 +
> > 4 files changed, 61 insertions(+)
> >
> > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> > index 69d9dc21d8..22022f6da9 100644
> > --- a/lib/ethdev/ethdev_driver.h
> > +++ b/lib/ethdev/ethdev_driver.h
> > @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct
> rte_eth_dev *dev,
> > typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev *dev,
> > uint16_t rx_queue_id);
> >
> > +/** @internal Enable direct rearm of a receive queue of an Ethernet
> > +device. */ typedef int (*eth_rx_direct_rearm_enable_t)(struct
> rte_eth_dev *dev,
> > + uint16_t queue_id);
> > +
> > +/**< @internal map Rx/Tx queue of direct rearm mode */ typedef int
> > +(*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
> > + uint16_t rx_queue_id,
> > + uint16_t tx_port_id,
> > + uint16_t tx_queue_id);
> > +
> > /** @internal Release memory resources allocated by given Rx/Tx queue.
> */
> > typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> > uint16_t queue_id);
> > @@ -1152,6 +1162,11 @@ struct eth_dev_ops {
> > /** Disable Rx queue interrupt */
> > eth_rx_disable_intr_t rx_queue_intr_disable;
> >
> > + /** Enable Rx queue direct rearm mode */
> > + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
> > + /** Map Rx/Tx queue for direct rearm mode */
> > + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
> > +
> > eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx
> queue */
> > eth_queue_release_t tx_queue_release; /**< Release Tx queue
> */
> > eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring mbufs
> */
> > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
> > 29a3d80466..8e6f0284f4 100644
> > --- a/lib/ethdev/rte_ethdev.c
> > +++ b/lib/ethdev/rte_ethdev.c
> > @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t
> port_id, uint16_t tx_queue_id,
> > return eth_err(port_id, ret);
> > }
> >
> > +int
> > +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> > + uint16_t tx_port_id, uint16_t tx_queue_id) {
> > + struct rte_eth_dev *dev;
> > +
> > + dev = &rte_eth_devices[rx_port_id];
>
> I think it is rather control path. So:
> We need standard checks that rx_port_id is valid.
> tx_port_id must be checked as well.
> rx_queue_id and tx_queue_id must be checked to be in the rate.
[Feifei] You are right, I will add check for these.
>
> > + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev,
> rx_queue_id);
> > + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
> > + tx_port_id, tx_queue_id);
>
> We must check that function pointers are not NULL as usual.
> Return values must be checked.
[Feifei] I agree with this, The check for pointer and return value will be added
> Isn't is safe to setup map and than enable.
> Otherwise we definitely need disable.
[Feifei] I will change code that map first and then set 'rxq->offload' to enable direct-rearm mode.
> Also, what should happen on Tx port unplug? How to continue if we still have
> Rx port up and running?
[Feifei] For direct rearm mode, if Tx port unplug, it means there is no buffer from Tx.
And then, Rx will put buffer from mempool as usual for rearm.
>
> > +
> > + return 0;
> > +}
> > +
> > int
> > rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port)
> > {
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > 04cff8ee10..4a431fcbed 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -5190,6 +5190,37 @@ __rte_experimental
> > int rte_eth_dev_hairpin_capability_get(uint16_t port_id,
> > struct rte_eth_hairpin_cap *cap);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> > +notice
> > + *
> > + * Enable direct re-arm mode. In this mode the RX queue will be
> > +re-armed using
> > + * buffers that have completed transmission on the transmit side.
> > + *
> > + * @note
> > + * It is assumed that the buffers have completed transmission belong to
> the
> > + * mempool used at the receive side, and have refcnt = 1.
>
> I think it is possible to avoid such limitations, but implementation will be less
> optimized - more checks.
[Feifei] For the first limitation: Rx and Tx buffers should be from the same mempool.
If we want to check this, we will add a check for each packet in the data-plane, this will
significantly reduce performance. So, it is better to note this for users rather than adding
check code.
For the second limitation: refcnt = 1. We have now add code to support 'refcnt = 1' in direct-rearm
mode, so this note can be removed in the next version.
>
> > + *
> > + * @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.
>
> I guess there is an assumption that Rx and Tx ports are serviced by the same
> driver. If so and if it is an API limitation, ethdev layer must check it.
[Feifei] I agree with this. For the check that Rx and Tx port should be the same driver,
I will add check for this.
>
> > + * @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().
> > + *
> > + * @return
> > + * - (0) if successful.
> > + */
> > +__rte_experimental
> > +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t
> rx_queue_id,
> > + uint16_t tx_port_id, uint16_t tx_queue_id);
> > +
> > /**
> > * @warning
> > * @b EXPERIMENTAL: this structure may change without prior notice.
> > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map index
> > 20391ab29e..68d664498c 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -279,6 +279,7 @@ EXPERIMENTAL {
> > rte_flow_async_action_handle_create;
> > rte_flow_async_action_handle_destroy;
> > rte_flow_async_action_handle_update;
> > + rte_eth_direct_rxrearm_map;
> > };
> >
> > INTERNAL {
> -----邮件原件-----
> 发件人: Stephen Hemminger <stephen@networkplumber.org>
> 发送时间: Thursday, April 21, 2022 10:58 PM
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>
> 抄送: thomas@monjalon.net; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; nd <nd@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>
> 主题: Re: [PATCH v1 3/5] ethdev: add API for direct rearm mode
>
> On Wed, 20 Apr 2022 16:16:48 +0800
> Feifei Wang <feifei.wang2@arm.com> wrote:
>
> > Add API for enabling direct rearm mode and for mapping RX and TX
> > queues. Currently, the API supports 1:1(txq : rxq) mapping.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> > lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
> > lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
> > lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
> > lib/ethdev/version.map | 1 +
> > 4 files changed, 61 insertions(+)
> >
> > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> > index 69d9dc21d8..22022f6da9 100644
> > --- a/lib/ethdev/ethdev_driver.h
> > +++ b/lib/ethdev/ethdev_driver.h
> > @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct
> > rte_eth_dev *dev, typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev
> *dev,
> > uint16_t rx_queue_id);
> >
> > +/** @internal Enable direct rearm of a receive queue of an Ethernet
> > +device. */ typedef int (*eth_rx_direct_rearm_enable_t)(struct
> rte_eth_dev *dev,
> > + uint16_t queue_id);
> > +
> > +/**< @internal map Rx/Tx queue of direct rearm mode */ typedef int
> > +(*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
> > + uint16_t rx_queue_id,
> > + uint16_t tx_port_id,
> > + uint16_t tx_queue_id);
> > +
> > /** @internal Release memory resources allocated by given Rx/Tx
> > queue. */ typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> > uint16_t queue_id);
> > @@ -1152,6 +1162,11 @@ struct eth_dev_ops {
> > /** Disable Rx queue interrupt */
> > eth_rx_disable_intr_t rx_queue_intr_disable;
> >
> > + /** Enable Rx queue direct rearm mode */
> > + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
> > + /** Map Rx/Tx queue for direct rearm mode */
> > + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
> > +
> > eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx
> queue */
> > eth_queue_release_t tx_queue_release; /**< Release Tx queue
> */
> > eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring mbufs
> */
> > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
> > 29a3d80466..8e6f0284f4 100644
> > --- a/lib/ethdev/rte_ethdev.c
> > +++ b/lib/ethdev/rte_ethdev.c
> > @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t
> port_id, uint16_t tx_queue_id,
> > return eth_err(port_id, ret);
> > }
> >
> > +int
> > +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> > + uint16_t tx_port_id, uint16_t tx_queue_id) {
> > + struct rte_eth_dev *dev;
> > +
> > + dev = &rte_eth_devices[rx_port_id];
> > + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev,
> rx_queue_id);
> > + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
> > + tx_port_id, tx_queue_id);
> > +
> > + return 0;
> > +}
> > +
> > int
> > rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port) { diff
> > --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > 04cff8ee10..4a431fcbed 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -5190,6 +5190,37 @@ __rte_experimental int
> > rte_eth_dev_hairpin_capability_get(uint16_t port_id,
> > struct rte_eth_hairpin_cap *cap);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> > +notice
> > + *
> > + * Enable direct re-arm mode. In this mode the RX queue will be
> > +re-armed using
> > + * buffers that have completed transmission on the transmit side.
> > + *
> > + * @note
> > + * It is assumed that the buffers have completed transmission belong to
> the
> > + * mempool used at the receive side, and have refcnt = 1.
> > + *
> > + * @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().
> > + *
> > + * @return
> > + * - (0) if successful.
> > + */
> > +__rte_experimental
> > +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t
> rx_queue_id,
> > + uint16_t tx_port_id, uint16_t tx_queue_id);
>
> Just looking at this.
>
> Why is this done via API call and not a flag as part of the receive config?
> All the other offload and configuration happens via dev config.
> Doing it this way doesn't follow the existing ethdev model.
[Feifei] Agree with this. I will remove direct-rearm enable function and
use "rxq->offload" bit to enable it.
For rte_eth_direct_rxrearm_map, I think it is necessary for users to call it to map Rx/Tx queue.
> -----邮件原件-----
> 发件人: Jerin Jacob <jerinjacobk@gmail.com>
> 发送时间: Wednesday, April 20, 2022 6:50 PM
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>
> 抄送: thomas@monjalon.net; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
> <mdr@ashroe.eu>; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>
> 主题: Re: [PATCH v1 3/5] ethdev: add API for direct rearm mode
>
> On Wed, Apr 20, 2022 at 1:47 PM Feifei Wang <feifei.wang2@arm.com>
> wrote:
> >
> > Add API for enabling direct rearm mode and for mapping RX and TX
> > queues. Currently, the API supports 1:1(txq : rxq) mapping.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
>
> > + *
> > + * @return
> > + * - (0) if successful.
> > + */
> > +__rte_experimental
> > +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t
> rx_queue_id,
> > + uint16_t tx_port_id, uint16_t
> > +tx_queue_id);
>
> Won't existing rte_eth_hairpin_* APIs work to achieve the same?
[Feifei] Thanks for the comment. Look at the hairpin feature which is enabled in MLX5 driver.
I think the most important difference is that hairpin just re-directs the packet from the Rx queue
to Tx queue in the same port, and Rx/Tx queue just can record the peer queue id.
For direct rearm, it can map Rx queue to the Tx queue which are from different ports. And this needs
Rx queue records paired port id and queue id.
Furthermore, hairpin needs to set up new hairpin queue and then it can bind Rx queue to Tx queue.
and direct-rearm just can use normal queue to map. This is due to direct rearm needs used buffers and
it doesn't care about packet.
<snip>
> >
> > On 4/20/22 11:16, Feifei Wang wrote:
> > > Add API for enabling direct rearm mode and for mapping RX and TX
> > > queues. Currently, the API supports 1:1(txq : rxq) mapping.
> > >
> > > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > ---
> > > lib/ethdev/ethdev_driver.h | 15 +++++++++++++++
> > > lib/ethdev/rte_ethdev.c | 14 ++++++++++++++
> > > lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
> > > lib/ethdev/version.map | 1 +
> > > 4 files changed, 61 insertions(+)
> > >
> > > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> > > index 69d9dc21d8..22022f6da9 100644
> > > --- a/lib/ethdev/ethdev_driver.h
> > > +++ b/lib/ethdev/ethdev_driver.h
> > > @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct
> > rte_eth_dev *dev,
> > > typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev *dev,
> > > uint16_t rx_queue_id);
> > >
> > > +/** @internal Enable direct rearm of a receive queue of an Ethernet
> > > +device. */ typedef int (*eth_rx_direct_rearm_enable_t)(struct
> > rte_eth_dev *dev,
> > > + uint16_t queue_id);
> > > +
> > > +/**< @internal map Rx/Tx queue of direct rearm mode */ typedef int
> > > +(*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
> > > + uint16_t rx_queue_id,
> > > + uint16_t tx_port_id,
> > > + uint16_t tx_queue_id);
> > > +
> > > /** @internal Release memory resources allocated by given Rx/Tx queue.
> > */
> > > typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
> > > uint16_t queue_id);
> > > @@ -1152,6 +1162,11 @@ struct eth_dev_ops {
> > > /** Disable Rx queue interrupt */
> > > eth_rx_disable_intr_t rx_queue_intr_disable;
> > >
> > > + /** Enable Rx queue direct rearm mode */
> > > + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
> > > + /** Map Rx/Tx queue for direct rearm mode */
> > > + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
> > > +
> > > eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx
> > queue */
> > > eth_queue_release_t tx_queue_release; /**< Release Tx queue
> > */
> > > eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring mbufs
> > */
> > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
> > > 29a3d80466..8e6f0284f4 100644
> > > --- a/lib/ethdev/rte_ethdev.c
> > > +++ b/lib/ethdev/rte_ethdev.c
> > > @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t
> > port_id, uint16_t tx_queue_id,
> > > return eth_err(port_id, ret);
> > > }
> > >
> > > +int
> > > +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
> > > + uint16_t tx_port_id, uint16_t tx_queue_id) {
> > > + struct rte_eth_dev *dev;
> > > +
> > > + dev = &rte_eth_devices[rx_port_id];
> >
> > I think it is rather control path. So:
> > We need standard checks that rx_port_id is valid.
> > tx_port_id must be checked as well.
> > rx_queue_id and tx_queue_id must be checked to be in the rate.
> [Feifei] You are right, I will add check for these.
>
> >
> > > + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev,
> > rx_queue_id);
> > > + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
> > > + tx_port_id, tx_queue_id);
> >
> > We must check that function pointers are not NULL as usual.
> > Return values must be checked.
> [Feifei] I agree with this, The check for pointer and return value will be added
>
> > Isn't is safe to setup map and than enable.
> > Otherwise we definitely need disable.
> [Feifei] I will change code that map first and then set 'rxq->offload' to enable
> direct-rearm mode.
>
> > Also, what should happen on Tx port unplug? How to continue if we
> > still have Rx port up and running?
> [Feifei] For direct rearm mode, if Tx port unplug, it means there is no buffer
> from Tx.
> And then, Rx will put buffer from mempool as usual for rearm.
Andrew, when you say 'TX port unplug', do you mean the 'rte_eth_dev_tx_queue_stop' is called? Is calling 'rte_eth_dev_tx_queue_stop' allowed when the device is running?
>
> >
> > > +
> > > + return 0;
> > > +}
> > > +
<snip>
On 5/11/22 01:49, Honnappa Nagarahalli wrote:
>>> On 4/20/22 11:16, Feifei Wang wrote:
>>>> Add API for enabling direct rearm mode and for mapping RX and TX
>>>> queues. Currently, the API supports 1:1(txq : rxq) mapping.
>>>>
>>>> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>> + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev,
>>> rx_queue_id);
>>>> + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
>>>> + tx_port_id, tx_queue_id);
>>>
>>> We must check that function pointers are not NULL as usual.
>>> Return values must be checked.
>> [Feifei] I agree with this, The check for pointer and return value will be added
>>
>>> Isn't is safe to setup map and than enable.
>>> Otherwise we definitely need disable.
>> [Feifei] I will change code that map first and then set 'rxq->offload' to enable
>> direct-rearm mode.
>>
>>> Also, what should happen on Tx port unplug? How to continue if we
>>> still have Rx port up and running?
>> [Feifei] For direct rearm mode, if Tx port unplug, it means there is no buffer
>> from Tx.
>> And then, Rx will put buffer from mempool as usual for rearm.
> Andrew, when you say 'TX port unplug', do you mean the 'rte_eth_dev_tx_queue_stop' is called? Is calling 'rte_eth_dev_tx_queue_stop' allowed when the device is running?
I think that deferred start and presence of
rte_eth_dev_tx_queue_stop() implies the possibility to stop
Tx queue. But, yes, application should care about conditions
to have no traffic running to Tx queue.
Anyway, I was talking about hot unplug of the entire
device used as Tx port in above config.
@@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct rte_eth_dev *dev,
typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev *dev,
uint16_t rx_queue_id);
+/** @internal Enable direct rearm of a receive queue of an Ethernet device. */
+typedef int (*eth_rx_direct_rearm_enable_t)(struct rte_eth_dev *dev,
+ uint16_t queue_id);
+
+/**< @internal map Rx/Tx queue of direct rearm mode */
+typedef int (*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev,
+ uint16_t rx_queue_id,
+ uint16_t tx_port_id,
+ uint16_t tx_queue_id);
+
/** @internal Release memory resources allocated by given Rx/Tx queue. */
typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev,
uint16_t queue_id);
@@ -1152,6 +1162,11 @@ struct eth_dev_ops {
/** Disable Rx queue interrupt */
eth_rx_disable_intr_t rx_queue_intr_disable;
+ /** Enable Rx queue direct rearm mode */
+ eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable;
+ /** Map Rx/Tx queue for direct rearm mode */
+ eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map;
+
eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx queue */
eth_queue_release_t tx_queue_release; /**< Release Tx queue */
eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring mbufs */
@@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
return eth_err(port_id, ret);
}
+int
+rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
+ uint16_t tx_port_id, uint16_t tx_queue_id)
+{
+ struct rte_eth_dev *dev;
+
+ dev = &rte_eth_devices[rx_port_id];
+ (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev, rx_queue_id);
+ (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id,
+ tx_port_id, tx_queue_id);
+
+ return 0;
+}
+
int
rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port)
{
@@ -5190,6 +5190,37 @@ __rte_experimental
int rte_eth_dev_hairpin_capability_get(uint16_t port_id,
struct rte_eth_hairpin_cap *cap);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Enable direct re-arm mode. In this mode the RX queue will be re-armed using
+ * buffers that have completed transmission on the transmit side.
+ *
+ * @note
+ * It is assumed that the buffers have completed transmission belong to the
+ * mempool used at the receive side, and have refcnt = 1.
+ *
+ * @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().
+ *
+ * @return
+ * - (0) if successful.
+ */
+__rte_experimental
+int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id,
+ uint16_t tx_port_id, uint16_t tx_queue_id);
+
/**
* @warning
* @b EXPERIMENTAL: this structure may change without prior notice.
@@ -279,6 +279,7 @@ EXPERIMENTAL {
rte_flow_async_action_handle_create;
rte_flow_async_action_handle_destroy;
rte_flow_async_action_handle_update;
+ rte_eth_direct_rxrearm_map;
};
INTERNAL {