net/e1000: Test issues following change in max rx/tx queues

net/e1000: Test issues following change in max rx/tx queues

[Edwin Brossette]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

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.

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.
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?

Regards,
Edwin Brossette.

[-- Attachment #2: Type: text/html, Size: 1271 bytes --]

RE: net/e1000: Test issues following change in max rx/tx queues

[Morten Brørup]

+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.