Hi Ori,
On 10/16/19 10:36 PM, Ori Kam wrote:
Hi Andrew,
Thanks again for your time.
PSB,
Ori
-----Original Message-----
From: Andrew Rybchenko <arybche...@solarflare.com>
Sent: Tuesday, October 15, 2019 1:12 PM
To: Ori Kam <or...@mellanox.com>; Thomas Monjalon
<tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>
Cc: dev@dpdk.org; jingjing...@intel.com; step...@networkplumber.org
Subject: Re: [PATCH v3 01/14] ethdev: add support for hairpin queue
Hi Ori,
On 10/15/19 12:04 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>
[snip]
@@ -746,9 +769,9 @@ struct rte_eth_dev_data {
dev_started : 1, /**< Device state: STARTED(1) / STOPPED(0).
*/
lro : 1; /**< RX LRO is ON(1) / OFF(0) */
uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT];
- /**< Queues state: STARTED(1) / STOPPED(0). */
+ /**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT];
- /**< Queues state: STARTED(1) / STOPPED(0). */
+ /**< Queues state: HAIRPIN(2) STARTED(1) / STOPPED(0). */
/ is missing above after HAIRPIN(2).
In fact there is no point to duplicate values in parenthesis, but it is
out of scope of the review.
I will add the missing /
I'm not 100% happy that it makes impossible to mark hairpin queues
as started/stopped. It is not that important right now, but may be it is
better to use state as bit field. Bit 0 - stopped/started,
bit 1 - regular/hairpin. Anyway, it is internal interface.
Your idea is very nice, but there are some things to consider.
For example if converted to bits it will take more memory.
We can just it is flags for the U8 but this will mean that we could have
both hairpin and stopped / started in the same time. Which I'm not sure if
it is good or bad. Like you say it is internal so let's keep the current
implementation,
and discuss your idea after this patch set, and after we will see how the
community uses
the hairpin feature. Is that O.K. for you?
Yes.
