| Message ID | 20211124212400.70613-1-maxime.coquelin@redhat.com |
|---|---|
| Headers | show |
| Series | dpif-netdev: Hash-based Tx packet steering | expand |
Hi, On 11/24/21 22:23, Maxime Coquelin wrote: > This series introduces a new HXPS Tx 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 first patch introduces per-queue > basic statistics for Vhost-user ports. This patch is > complementary to David's patch [0] adding per-queue > statistics to DPDK ports using xstats. [0]: http://patchwork.ozlabs.org/project/openvswitch/patch/20211015150406.12949-1-david.marchand@redhat.com/ > > The series also introduces two DPDK tests for Vhost-user > multiqueue, with and without HXPS enabled. I also forgot to mention that this series depends on David's system-dpdk tests refactoring series: http://patchwork.ozlabs.org/project/openvswitch/list/?series=273438 > Maxime Coquelin (4): > netdev-dpdk: Introduce per rxq/txq Vhost-user statistics > dpif-netdev: Introduce Tx queue mode > dpif-netdev: Add HXPS Tx queue mode > system-dpdk: Add tests for HXPS > > Documentation/automake.mk | 1 + > Documentation/topics/dpdk/hxps.rst | 51 ++++++++++ > Documentation/topics/dpdk/index.rst | 1 + > lib/dpif-netdev.c | 95 ++++++++++++++---- > lib/netdev-dpdk.c | 143 ++++++++++++++++++++++++++-- > tests/system-dpdk.at | 135 ++++++++++++++++++++++++++ > 6 files changed, 399 insertions(+), 27 deletions(-) > create mode 100644 Documentation/topics/dpdk/hxps.rst >
On 11/24/21 22:23, Maxime Coquelin wrote: > This series introduces a new HXPS Tx 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 first patch introduces per-queue > basic statistics for Vhost-user ports. This patch is > complementary to David's patch [0] adding per-queue > statistics to DPDK ports using xstats. > > The series also introduces two DPDK tests for Vhost-user > multiqueue, with and without HXPS enabled. > > Maxime Coquelin (4): > netdev-dpdk: Introduce per rxq/txq Vhost-user statistics > dpif-netdev: Introduce Tx queue mode > dpif-netdev: Add HXPS Tx queue mode > system-dpdk: Add tests for HXPS > > Documentation/automake.mk | 1 + > Documentation/topics/dpdk/hxps.rst | 51 ++++++++++ > Documentation/topics/dpdk/index.rst | 1 + > lib/dpif-netdev.c | 95 ++++++++++++++---- > lib/netdev-dpdk.c | 143 ++++++++++++++++++++++++++-- > tests/system-dpdk.at | 135 ++++++++++++++++++++++++++ > 6 files changed, 399 insertions(+), 27 deletions(-) > create mode 100644 Documentation/topics/dpdk/hxps.rst > Hi, Maxime. Thanks for working on this. I agree that this feature might be very useful for some deployments. I didn't read the code any carefully, just glanced at it. But I have a couple of high level comments: 1. I don't think that the test for the actual XPS mode implementation should be part of a system-dpdk testsuite, as it's not actually related to DPDK and doesn't require any system HW/ports/non-OVS applications running in order to test it. So, I think, that we should be able to do practically the same testing, but with dummy interfaces, with a test placed in the dpif-netdev.at or pmd.at (probably, latter). To achieve that you'll need per-queue stats in netdev-dummy, but implementation of these will be practically the same or even a bit simpler as you did for vhost-user ports. Per-queue stats for vhost-user ports might be good to have in general, so that patch along with some simple test in system-dpdk.at for them could be split from this patch set and sent separately. Or dropped, if you think they are not valuable (?). 2. The test itself doesn't need a packet generator script, AFAICT. You may just send some number of packets via dummy port changing source or destination udp/tcp port affecting the packet hash this way. For example, see the SEND_TCP_BOND_PKTS macro in the tests/ofproto-dpif.at and how bonding rebalancing tests are using it. 3. Might be better instead of introduction of a specialized config knob (other_config:hxps=true), to have a multi-choice knob like other_config:xps-mode with 2 options 'default' and 'hash', where 'default' is a current way of tx queue distribution and it will be a default value. 'hash' will be a new mode that uses packet hash to choose the tx queue (what's implemented in this patch set). What do you think? Best regards, Ilya Maximets.
Hi Ilya, On 12/8/21 00:23, Ilya Maximets wrote: > On 11/24/21 22:23, Maxime Coquelin wrote: >> This series introduces a new HXPS Tx 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 first patch introduces per-queue >> basic statistics for Vhost-user ports. This patch is >> complementary to David's patch [0] adding per-queue >> statistics to DPDK ports using xstats. >> >> The series also introduces two DPDK tests for Vhost-user >> multiqueue, with and without HXPS enabled. >> >> Maxime Coquelin (4): >> netdev-dpdk: Introduce per rxq/txq Vhost-user statistics >> dpif-netdev: Introduce Tx queue mode >> dpif-netdev: Add HXPS Tx queue mode >> system-dpdk: Add tests for HXPS >> >> Documentation/automake.mk | 1 + >> Documentation/topics/dpdk/hxps.rst | 51 ++++++++++ >> Documentation/topics/dpdk/index.rst | 1 + >> lib/dpif-netdev.c | 95 ++++++++++++++---- >> lib/netdev-dpdk.c | 143 ++++++++++++++++++++++++++-- >> tests/system-dpdk.at | 135 ++++++++++++++++++++++++++ >> 6 files changed, 399 insertions(+), 27 deletions(-) >> create mode 100644 Documentation/topics/dpdk/hxps.rst >> > > Hi, Maxime. Thanks for working on this. > I agree that this feature might be very useful for some deployments. > > I didn't read the code any carefully, just glanced at it. But I have > a couple of high level comments: > > 1. I don't think that the test for the actual XPS mode implementation > should be part of a system-dpdk testsuite, as it's not actually > related to DPDK and doesn't require any system HW/ports/non-OVS > applications running in order to test it. > > So, I think, that we should be able to do practically the same > testing, but with dummy interfaces, with a test placed in the > dpif-netdev.at or pmd.at (probably, latter). > > To achieve that you'll need per-queue stats in netdev-dummy, but > implementation of these will be practically the same or even a > bit simpler as you did for vhost-user ports. > > Per-queue stats for vhost-user ports might be good to have in > general, so that patch along with some simple test in system-dpdk.at > for them could be split from this patch set and sent separately. > Or dropped, if you think they are not valuable (?). OK, I will work on adding per-queue stats to netdev-dummy. I agree it is better to decouple the testing of this feature from DPDK, and it will make the test being executed more often. > 2. The test itself doesn't need a packet generator script, AFAICT. > You may just send some number of packets via dummy port changing > source or destination udp/tcp port affecting the packet hash this > way. For example, see the SEND_TCP_BOND_PKTS macro in the > tests/ofproto-dpif.at and how bonding rebalancing tests are using it. Thanks for the pointer on SEND_TCP_BOND_PKTS, I agree this is better this way. Also, using fuzzing as I did may have the drawback of seldomly generate packets with hashes that would make them all ending into a single queue. > 3. Might be better instead of introduction of a specialized config > knob (other_config:hxps=true), to have a multi-choice knob like > other_config:xps-mode with 2 options 'default' and 'hash', where > 'default' is a current way of tx queue distribution and it will > be a default value. 'hash' will be a new mode that uses packet > hash to choose the tx queue (what's implemented in this patch set). Ok, makes sense. > What do you think? Those are valid points, thanks for your suggestions. Maxime > Best regards, Ilya Maximets. >