<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.nagaraha...@arm.com>
> > > Signed-off-by: Feifei Wang <feifei.wa...@arm.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.w...@arm.com>
> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@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>
