Hi Ori,

thanks for review notes applied. Please, see below.

On 10/23/19 4:37 PM, Ori Kam wrote:
This commit introduce hairpin queue type.

The hairpin queue in build from Rx queue binded to Tx queue.
It is used to offload traffic coming from the wire and redirect it back
to the wire.

There are 3 new functions:
- rte_eth_dev_hairpin_capability_get
- rte_eth_rx_hairpin_queue_setup
- rte_eth_tx_hairpin_queue_setup

In order to use the queue, there is a need to create rte_flow
with queue / RSS action that targets one or more of the Rx queues.

Signed-off-by: Ori Kam <or...@mellanox.com>

Just a bit below
Reviewed-by: Andrew Rybchenko <arybche...@solarflare.com>

[snip]

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 78da293..199e96e 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -923,6 +923,13 @@ struct rte_eth_dev *
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_start, -ENOTSUP); + if (rte_eth_dev_is_rx_hairpin_queue(dev, rx_queue_id) == 1) {

I think the function should return bool and it results should be
used as is without == 1 or something like this.
Everyrwhere in the patch.

[snip]

diff --git a/lib/librte_ethdev/rte_ethdev_driver.h 
b/lib/librte_ethdev/rte_ethdev_driver.h
index c404f17..98023d7 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -26,6 +26,50 @@
   */
  #define RTE_ETH_QUEUE_STATE_STOPPED 0
  #define RTE_ETH_QUEUE_STATE_STARTED 1
+#define RTE_ETH_QUEUE_STATE_HAIRPIN 2
+
+/**
+ * @internal
+ * Check if the selected Rx queue is hairpin queue.
+ *
+ * @param dev
+ *  Pointer to the selected device.
+ * @param queue_id
+ *  The selected queue.
+ *
+ * @return
+ *   - (1) if the queue is hairpin queue, 0 otherwise.
+ */
+static inline int

I think the function should return bool (and stdbool.h should be included).
Return value description should be updated.

+rte_eth_dev_is_rx_hairpin_queue(struct rte_eth_dev *dev, uint16_t queue_id)
+{
+       if (dev->data->rx_queue_state[queue_id] ==
+           RTE_ETH_QUEUE_STATE_HAIRPIN)
+               return 1;
+       return 0;
+}
+
+
+/**
+ * @internal
+ * Check if the selected Tx queue is hairpin queue.
+ *
+ * @param dev
+ *  Pointer to the selected device.
+ * @param queue_id
+ *  The selected queue.
+ *
+ * @return
+ *   - (1) if the queue is hairpin queue, 0 otherwise.
+ */
+static inline int

Same here.

+rte_eth_dev_is_tx_hairpin_queue(struct rte_eth_dev *dev, uint16_t queue_id)
+{
+       if (dev->data->tx_queue_state[queue_id] ==
+           RTE_ETH_QUEUE_STATE_HAIRPIN)
+               return 1;
+       return 0;
+}
/**
   * @internal

[snip]

Reply via email to