mbox series

[ovs-dev,v5,0/5] dpif-netdev: Hash-based Tx packet steering

Message ID 20220105081926.613684-1-maxime.coquelin@redhat.com
Headers show
Series dpif-netdev: Hash-based Tx packet steering | expand

Message

Maxime Coquelin Jan. 5, 2022, 8:19 a.m. UTC
This series introduces a new hash-based Tx packets steering
mode alognside existing XPS and static modes. The goal is
to provide a mode where all the transmit queues are used,
whatever the number of PMD threads. This may be used with
Vhost-user ports, where the guest application driving the
Virtio device expects packets to be distributed on all the
queues.

As a preliminary step, in order to be able to validate the
feature at OVS level, the two first patches introduce
per-queue basic statistics for Vhost-user and dummy ports.
These patches are complementary to David's patch [0] adding
per-queue statistics to DPDK ports using xstats.

The series also introduces a dpif-netdev test for tx
steering, using dummy-pmd ports only, instead of the more
complex Vhost-user test in first revision.

Changes in v5:
==============
- Fix indents (David)
- Fix/remove comments (David)
- Fix grammar in documentation (David)
- Rebase and applied David's R-by's

Changes in v4:
==============
- Change config option from tx-steering=hash|default to tx-steering=hash|thread (Kevin, Ilya)
- Support case where packet's RSS is not valid (Ilya)
- Fix Txq packets counts check in hash test
- Add sub-test in Tx steering test with hw-offload=true,tx-steering=hash (Ilya)
- Revisit documentation (Ilya)
- Rebased on top of master branch

Changes in v3:
==============
- Free rxq_stats and txq_stats in netdev_dummy_destruct()
  caught by ASan (David)
- Fix line length in the documentation part (Ilya)
- Replace "\" with "dnl" in the dpif-netdev test (Ilya)
- Replace "xps-mode" with "tx-steering" in documenation (Ilya)

Changes in v2:
==============
- Use macros to generate Vhost custom stats (David)
- Change config option from hxps=true|false to tx-steering=hash|default (Ilya)
- Fix use-after-free of tx_port->txq_pkts (Kevin)
- Fix out-of-bound writes on Vhost-user rxq stats (David)
- Implement Tx steering test in dpif-netdev without fuzzing (Ilya)
- Remove Vhost-user tx-steering tests (Ilya)
- Fix various indents & other cosmetic issues (David)
- Move documenation out of dpdk/

Maxime Coquelin (5):
  netdev-dpdk: Introduce per rxq/txq Vhost-user statistics.
  netdev-dummy: Introduce per rxq/txq statistics.
  dpif-netdev: Introduce Tx queue mode.
  dpif-netdev: Introduce hash-based Tx packet steering mode.
  dpif-netdev.at: Add test for Tx packets steering.

 Documentation/automake.mk                     |   1 +
 Documentation/topics/index.rst                |   1 +
 .../topics/userspace-tx-steering.rst          |  73 +++++++++
 lib/dpif-netdev.c                             | 142 ++++++++++++++---
 lib/netdev-dpdk.c                             | 147 ++++++++++++++++--
 lib/netdev-dummy.c                            |  87 +++++++++--
 tests/dpif-netdev.at                          |  67 ++++++++
 tests/ofproto.at                              |   3 +-
 8 files changed, 477 insertions(+), 44 deletions(-)
 create mode 100644 Documentation/topics/userspace-tx-steering.rst

Comments

Ilya Maximets Jan. 17, 2022, 5:44 p.m. UTC | #1
On 1/5/22 09:19, Maxime Coquelin wrote:
> This series introduces a new hash-based Tx packets steering
> mode alognside existing XPS and static modes. The goal is
> to provide a mode where all the transmit queues are used,
> whatever the number of PMD threads. This may be used with
> Vhost-user ports, where the guest application driving the
> Virtio device expects packets to be distributed on all the
> queues.
> 
> As a preliminary step, in order to be able to validate the
> feature at OVS level, the two first patches introduce
> per-queue basic statistics for Vhost-user and dummy ports.
> These patches are complementary to David's patch [0] adding
> per-queue statistics to DPDK ports using xstats.
> 
> The series also introduces a dpif-netdev test for tx
> steering, using dummy-pmd ports only, instead of the more
> complex Vhost-user test in first revision.
> 
> Changes in v5:
> ==============
> - Fix indents (David)
> - Fix/remove comments (David)
> - Fix grammar in documentation (David)
> - Rebase and applied David's R-by's
> 
> Maxime Coquelin (5):
>   netdev-dpdk: Introduce per rxq/txq Vhost-user statistics.
>   netdev-dummy: Introduce per rxq/txq statistics.
>   dpif-netdev: Introduce Tx queue mode.
>   dpif-netdev: Introduce hash-based Tx packet steering mode.
>   dpif-netdev.at: Add test for Tx packets steering.
> 
>  Documentation/automake.mk                     |   1 +
>  Documentation/topics/index.rst                |   1 +
>  .../topics/userspace-tx-steering.rst          |  73 +++++++++
>  lib/dpif-netdev.c                             | 142 ++++++++++++++---
>  lib/netdev-dpdk.c                             | 147 ++++++++++++++++--
>  lib/netdev-dummy.c                            |  87 +++++++++--
>  tests/dpif-netdev.at                          |  67 ++++++++
>  tests/ofproto.at                              |   3 +-
>  8 files changed, 477 insertions(+), 44 deletions(-)
>  create mode 100644 Documentation/topics/userspace-tx-steering.rst
> 

Thanks, Maxime and David!

With some cosmetic changes I applied the series, except for the
first patch (vhost-user statistics), because it causes performance
degradation for me.  I'll reply with details to that patch.

And I just noticed that we're missing the documentation update
for vswitchd/vswitch.xml.  Could you, please, update it with
a new tx-steering option and send as a separate patch?

Best regards, Ilya Maximets.
Maxime Coquelin Jan. 17, 2022, 10:27 p.m. UTC | #2
On 1/17/22 18:44, Ilya Maximets wrote:
> On 1/5/22 09:19, Maxime Coquelin wrote:
>> This series introduces a new hash-based Tx packets steering
>> mode alognside existing XPS and static modes. The goal is
>> to provide a mode where all the transmit queues are used,
>> whatever the number of PMD threads. This may be used with
>> Vhost-user ports, where the guest application driving the
>> Virtio device expects packets to be distributed on all the
>> queues.
>>
>> As a preliminary step, in order to be able to validate the
>> feature at OVS level, the two first patches introduce
>> per-queue basic statistics for Vhost-user and dummy ports.
>> These patches are complementary to David's patch [0] adding
>> per-queue statistics to DPDK ports using xstats.
>>
>> The series also introduces a dpif-netdev test for tx
>> steering, using dummy-pmd ports only, instead of the more
>> complex Vhost-user test in first revision.
>>
>> Changes in v5:
>> ==============
>> - Fix indents (David)
>> - Fix/remove comments (David)
>> - Fix grammar in documentation (David)
>> - Rebase and applied David's R-by's
>>
>> Maxime Coquelin (5):
>>    netdev-dpdk: Introduce per rxq/txq Vhost-user statistics.
>>    netdev-dummy: Introduce per rxq/txq statistics.
>>    dpif-netdev: Introduce Tx queue mode.
>>    dpif-netdev: Introduce hash-based Tx packet steering mode.
>>    dpif-netdev.at: Add test for Tx packets steering.
>>
>>   Documentation/automake.mk                     |   1 +
>>   Documentation/topics/index.rst                |   1 +
>>   .../topics/userspace-tx-steering.rst          |  73 +++++++++
>>   lib/dpif-netdev.c                             | 142 ++++++++++++++---
>>   lib/netdev-dpdk.c                             | 147 ++++++++++++++++--
>>   lib/netdev-dummy.c                            |  87 +++++++++--
>>   tests/dpif-netdev.at                          |  67 ++++++++
>>   tests/ofproto.at                              |   3 +-
>>   8 files changed, 477 insertions(+), 44 deletions(-)
>>   create mode 100644 Documentation/topics/userspace-tx-steering.rst
>>
> 
> Thanks, Maxime and David!
> 
> With some cosmetic changes I applied the series, except for the
> first patch (vhost-user statistics), because it causes performance
> degradation for me.  I'll reply with details to that patch.

Thanks, I agree with skipping the Vhost-user stats patch for now.

> And I just noticed that we're missing the documentation update
> for vswitchd/vswitch.xml.  Could you, please, update it with
> a new tx-steering option and send as a separate patch?

I just sent the documentation patch adding the new PMD option.

Regards,
Maxime

> Best regards, Ilya Maximets.
>