Hi Ori,
On 10/29/19 10:39 PM, Ori Kam wrote:
On 10/28/19 9:44 PM, Ori Kam wrote:
On 10/27/19 3:24 PM, Ori Kam wrote:
+ if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
The condition should be more tricky if we resetup hairpin queue.
I.e. we should check if i is rx_queue_id and count it anyway.
+ count++;
+ }
+ if (count > cap.max_nb_queues) {
+ RTE_ETHDEV_LOG(ERR, "To many Rx hairpin queues
%d",
I think it would be useful to log max here as well to catch
unset max cases easier.
I'm not sure I understand.
If the question is about logging, the answer is simple:
if the user forget to initialize maximum number of hairpin queues
properly, it will be zero and setup will fail here. So, it would be
good to log maximum value here just to make it clear which
limit is exceeded.
Maybe I'm missing something but the PMD sets the max number of hairpin queues.
Yes, it is just my paranoia to simplify debugging the case if PMD
forgets to do it.
But in any case I agree we should log what the user requested and what is the
max
that the PMD reports.
If the question is about above check, let's consider the case when
maximum is one and one hairpin queue is already setup, but
user tries to setup one more. Above loop will count only one since
hairpin state for current queue is set below. So, the condition will
allow to setup the second hairpin queue.
In theory, we could initialize cound=1 to count this one, but
it would break the case when we call setup once again for the
queue which is already hairpin. API allows and handles it.
Nice catch. I think the best solution is to compare the count to
cap.max_nb_queues - 1.
and even before this comparison check if the current queue is already hairpin
queue if so
we can skip this check.
What do you think?
I think the right solution is always count current queue since it is either
becoming hairpin or already hairpin, i.e.
if (i == rx_queue_id || rte_eth_dev_is_rx_hairpin_queue(dev, i))
So, the result will be always total number of hairpin queues if
this one is hairpin.
Andrew.
