[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    RE: net/e1000: Test issues following change in max rx/tx queues
From:       Morten Brørup <mb () smartsharesystems ! com>
Date:       2023-11-30 12:01:59
Message-ID: 98CBD80474FA8B44BF855DF32C47DC35E9F079 () smartserver ! smartshare ! dk
[Download RAW message or body]

+TO: Qiming Yang, author of the patch.
+CC: stable

> From: Edwin Brossette [mailto:edwin.brossette@6wind.com] 
> Sent: Thursday, 30 November 2023 12.25
> 
> Hello, 
> 
> We recently started to observe concerning behaviour on our continuous integration \
> platform  following this commit, which set the maximum amount of tx/rx queues at 2 \
> for the e1000 pmd: 
> net/e1000: fix queue number initialization
> https://git.dpdk.org/dpdk/commit/?id=c1a42d646472fd3477429bf016f682e0865b77f0
> 
> Reverting this change locally on our side was enough to fix our test issues.
> 
> There is a considerately long explanation in a comment just above the change \
> explaining why the number of rx/tx queues is limited at 1, yet it was changed at 2 \
> anyway. 

For reference, here is the explanation in the source code:

<quote>
Starting with 631xESB hw supports 2 TX/RX queues per port.
Unfortunatelly, all these nics have just one TX context.
So we have few choises for TX:
- Use just one TX queue.
- Allow cksum offload only for one TX queue.
- Don't allow TX cksum offload at all.
For now, option #1 was chosen.
To use second RX queue we have to use extended RX descriptor
(Multiple Receive Queues are mutually exclusive with UDP
fragmentation and are not supported when a legacy receive
descriptor format is used).
Which means separate RX routinies - as legacy nics (82540, 82545)
don't support extended RXD.
To avoid it we support just one RX queue for now (no RSS).
</quote>

> 
> Since I couldn't see any mention in logs about theses restrictions being removed, I \
> wished to inquire whether this change was truly intended and if there was any \
> problem which motivated it.

Qiming, what is the explanation, please?

> Maybe the max number of rx/tx queues should reverted back to 1? Or maybe the \
> comment should be updated if limitations are no longer true?

Your CI platform flagging problems is a strong indicator of something being wrong. So \
I would guess the limitations are still true, and the max number of rx/tx queues \
should be reverted back to 1.

If someone really needs 2 queues, perhaps it could be a build time option to enable 2 \
queues without offloads. Qiming, what do you think?

> 
> Regards,
> Edwin Brossette.


[prev in list] [next in list] [prev in thread] [next in thread]