On 12/13/21 18:19, David Marchand wrote:
> On Fri, Dec 10, 2021 at 8:36 PM Ilya Maximets <[email protected]> wrote:
>>
>> Few problems with a current documentation:
>>
>> 1. bridge.rst is the high-level documentation for the end user.
>> Unit testing and complex implementation details are for developers,
>> hence should not be there. Testing instructions for developers
>> should be in testing.rst. Words in the doc should be understandable
>> for the user who doesn't know OVS internals.
>>
>> 2. Some paragraphs in the current documentation are repeating each
>> other almost to the word.
>>
>> 3. Some paragraphs are incorrectly formatted. That affects the
>> rendering.
>>
>> 4. There is no point describing every separate test of a system-dpdk
>> testsuite.
>>
>> What is done:
>>
>> 1. All the testing related paragraphs are consolidated and moved
>> to the testing.rst.
>>
>> 2. Most of abbreviations replaced with more readable and understandable
>> for the end user words.
>>
>> 3. Meaning or the purpose of several sentences I failed to understand,
>> therefore just deleted.
>>
>> 4. Fixed formatting and a few typos along the way.
>>
>> IMO, some parts of the doc still needs some re-wording, but this change
>> provides at least a starting point for improvement setting a better
>> structure for the document.
>
> +1.
>
>>
>> Signed-off-by: Ilya Maximets <[email protected]>
>
> I don't mind if you drop some of my suggestion / nits (see inline ),
> this patch enhances the documentation anyway.
>
> Reviewed-by: David Marchand <[email protected]>
Thanks, David! See replies inline.
>
>
>> ---
>> Documentation/topics/dpdk/bridge.rst | 248 ++++++---------------------
>> Documentation/topics/testing.rst | 59 ++++++-
>> 2 files changed, 113 insertions(+), 194 deletions(-)
>>
>> diff --git a/Documentation/topics/dpdk/bridge.rst
>> b/Documentation/topics/dpdk/bridge.rst
>> index f645b9ade..d9cc86268 100644
>> --- a/Documentation/topics/dpdk/bridge.rst
>> +++ b/Documentation/topics/dpdk/bridge.rst
>> @@ -153,7 +153,7 @@ In OVS v2.14 runtime CPU detection was introduced to
>> enable identifying if
>> these CPU ISA additions are available, and to allow the user to enable them.
>>
>> OVS provides multiple implementations of dpcls. The following command
>> enables
>> -the user to check what implementations are available in a running instance
>> ::
>> +the user to check what implementations are available in a running instance::
>>
>> $ ovs-appctl dpif-netdev/subtable-lookup-prio-get
>> Available lookup functions (priority : name)
>> @@ -161,7 +161,7 @@ the user to check what implementations are available in
>> a running instance ::
>> 1 : generic
>> 0 : avx512_gather
>>
>> -To set the priority of a lookup function, run the ``prio-set`` command ::
>> +To set the priority of a lookup function, run the ``prio-set`` command::
>>
>> $ ovs-appctl dpif-netdev/subtable-lookup-prio-set avx512_gather 5
>> Lookup priority change affected 1 dpcls ports and 1 subtables.
>> @@ -169,7 +169,7 @@ To set the priority of a lookup function, run the
>> ``prio-set`` command ::
>> The highest priority lookup function is used for classification, and the
>> output
>> above indicates that one subtable of one DPCLS port is has changed its
>> lookup
>> function due to the command being run. To verify the prioritization, re-run
>> the
>> -get command, note the updated priority of the ``avx512_gather`` function ::
>> +get command, note the updated priority of the ``avx512_gather`` function::
>>
>> $ ovs-appctl dpif-netdev/subtable-lookup-prio-get
>> Available lookup functions (priority : name)
>> @@ -185,24 +185,24 @@ best candidate.
>> Optimizing Specific Subtable Search
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> -During the packet classification, the datapath can use specialized
>> -lookup tables to optimize the search. However, not all situations
>> -are optimized. If you see a message like the following one in the OVS
>> -logs, it means that there is no specialized implementation available
>> -for the current networking traffic. In this case, OVS will continue
>> -to process the traffic normally using a more generic lookup table."
>> +During the packet classification, the datapath can use specialized lookup
>> +tables to optimize the search. However, not all situations are optimized.
>> If
>
> Nit: I would add a space before However.
ok.
>
>
>> +you see a message like the following one in the OVS logs, it means that
>> there
>> +is no specialized implementation available for the current networking
>> traffic.
>> +In this case, OVS will continue to process the traffic normally using a more
>> +generic lookup table.::
>
> I don't really get the intention for this part.
>
> This is rendered as "generic lookup table.:" and I don't get the
> relation with the sentence below.
> So I would simply end this sentence above with a .
I think, the sentences should be swapped. Something like this:
...
If you see a message like the following one in the OVS logs, it means that there
is no specialized implementation available for the current network traffic::
Using non-specialized AVX512 lookup for subtable (X,Y) and possibly others.
In this case, OVS will continue to process the traffic normally using a more
generic lookup table.
...
And the (Note ...) part below can actually be removed.
What do you think?
>
>
>> -"Using non-specialized AVX512 lookup for subtable (4,1) and possibly
>> others."
>> + Using non-specialized AVX512 lookup for subtable (4,1) and possibly
>> others.
>
> And make this a sub paragraph title instead.
>
>
>>
>> (Note that the numbers 4 and 1 will likely be different in your logs)
>>
>> -Additional specialized lookups can be added to OVS if the user
>> -provides that log message along with the command output as show
>> -below to the OVS mailing list. Note that the numbers in the log
>> -message ("subtable (X,Y)") need to match with the numbers in
>> -the provided command output ("dp-extra-info:miniflow_bits(X,Y)").
>> +Additional specialized lookups can be added to OVS if the user provides that
>> +log message along with the command output as show below to the OVS mailing
>> +list. Note that the numbers in the log message (``subtable (X,Y)``) need to
>> +match with the numbers in the provided command output
>> +(``dp-extra-info:miniflow_bits(X,Y)``).
>>
>> -"ovs-appctl dpctl/dump-flows -m", which results in output like this:
>> +``ovs-appctl dpctl/dump-flows -m``, which results in output like this::
>>
>> ufid:82770b5d-ca38-44ff-8283-74ba36bd1ca5,
>> skb_priority(0/0),skb_mark(0/0)
>> ,ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),
>> @@ -214,62 +214,28 @@ the provided command output
>> ("dp-extra-info:miniflow_bits(X,Y)").
>> actions:vhostuserclient0, dp-extra-info:miniflow_bits(4,1)
>>
>> Please send an email to the OVS mailing list [email protected] with
>> -the output of the "dp-extra-info:miniflow_bits(4,1)" values.
>> -
>> -CPU ISA Testing and Validation
>> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> -
>> -As multiple versions of DPCLS can co-exist, each with different CPU ISA
>> -optimizations, it is important to validate that they all give the exact same
>> -results. To easily test all DPCLS implementations, an ``autovalidator``
>> -implementation of the DPCLS exists. This implementation runs all other
>> -available DPCLS implementations, and verifies that the results are
>> identical.
>> -
>> -Running the OVS unit tests with the autovalidator enabled ensures all
>> -implementations provide the same results. Note that the performance of the
>> -autovalidator is lower than all other implementations, as it tests the
>> scalar
>> -implementation against itself, and against all other enabled DPCLS
>> -implementations.
>> -
>> -To adjust the DPCLS autovalidator priority, use this command ::
>> -
>> - $ ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 7
>> -
>> -Running Unit Tests with Autovalidator
>> -+++++++++++++++++++++++++++++++++++++
>> -
>> -To run the OVS unit test suite with the DPCLS autovalidator as the default
>> -implementation, it is required to recompile OVS. During the recompilation,
>> -the default priority of the `autovalidator` implementation is set to the
>> -maximum priority, ensuring every test will be run with every lookup
>> -implementation ::
>> -
>> - $ ./configure --enable-autovalidator
>> -
>> -Compile OVS in debug mode to have `ovs_assert` statements error out if
>> -there is a mis-match in the DPCLS lookup implementation.
>> +the output of the ``dp-extra-info:miniflow_bits(4,1)`` values.
>>
>> Datapath Interface Performance
>> ------------------------------
>>
>> -The datapath interface (DPIF) or dp_netdev_input() is responsible for taking
>> -packets through the major components of the userspace datapath; such as
>> -miniflow_extract, EMC, SMC and DPCLS lookups, and a lot of the performance
>> -stats associated with the datapath.
>> +The datapath interface (DPIF) is responsible for taking packets through the
>> +major components of the userspace datapath; such as packet parsing, caches
>> and
>> +datapath classifier lookups.
>>
>> -Just like with the SIMD DPCLS feature above, SIMD can be applied to the
>> DPIF to
>> -improve performance.
>> +Just like with the datapath classifier, SIMD instructions can be applied to
>> the
>> +datapath interface implementation to improve performance.
>>
>> -OVS provides multiple implementations of the DPIF. The available
>> -implementations can be listed with the following command ::
>> +OVS provides multiple implementations of the userspace datapath interface.
>> +Available implementations can be listed with the following command::
>>
>> $ ovs-appctl dpif-netdev/dpif-impl-get
>> Available DPIF implementations:
>> dpif_scalar (pmds: none)
>> dpif_avx512 (pmds: 1,2,6,7)
>>
>> -By default, dpif_scalar is used. The DPIF implementation can be selected by
>> -name ::
>> +By default, ``dpif_scalar`` is used. Implementations can be selected by
>> +name::
>>
>> $ ovs-appctl dpif-netdev/dpif-impl-set dpif_avx512
>> DPIF implementation set to dpif_avx512.
>> @@ -277,38 +243,23 @@ name ::
>> $ ovs-appctl dpif-netdev/dpif-impl-set dpif_scalar
>> DPIF implementation set to dpif_scalar.
>>
>> -Running Unit Tests with AVX512 DPIF
>> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> -
>> -Since the AVX512 DPIF is disabled by default, a compile time option is
>> -available in order to test it with the OVS unit test suite. When building
>> with
>> -a CPU that supports AVX512, use the following configure option ::
>> +Packet parsing performance
>> +--------------------------
>>
>> - $ ./configure --enable-dpif-default-avx512
>> +Open vSwitch performs parsing of the raw packets and extracts the important
>> +header information into a compressed miniflow structure. This miniflow is
>> +composed of bits and blocks where the bits signify which blocks are set or
>> have
>> +values where as the blocks hold the metadata, ip, udp, vlan, etc. These
>> values
>> +are used by the datapath for switching decisions later.
>>
>> -The following line should be seen in the configure output when the above
>> option
>> -is used ::
>> +Most modern CPUs have some SIMD (single instruction, mutiple data)
>> +capabilities. These SIMD instructions are able to process a vector rather
>> than
>> +act on one variable. OVS provides multiple implementations of packet
>> parsing
>> +functions. This allows the user to take advantage of SIMD instructions like
>> +AVX512 to gain additional performance.
>>
>> - checking whether DPIF AVX512 is default implementation... yes
>> -
>> -Miniflow Extract
>> -----------------
>> -
>> -Miniflow extract (MFEX) performs parsing of the raw packets and extracts the
>> -important header information into a compressed miniflow. This miniflow is
>> -composed of bits and blocks where the bits signify which blocks are set or
>> -have values where as the blocks hold the metadata, ip, udp, vlan, etc. These
>> -values are used by the datapath for switching decisions later. The Optimized
>> -miniflow extract is traffic specific to speed up the lookup, whereas the
>> -scalar works for ALL traffic patterns
>> -
>> -Most modern CPUs have SIMD capabilities. These SIMD instructions are able
>> -to process a vector rather than act on one variable. OVS provides multiple
>> -implementations of miniflow extract. This allows the user to take advantage
>> -of SIMD instructions like AVX512 to gain additional performance.
>> -
>> -A list of implementations can be obtained by the following command. The
>> -command also shows whether the CPU supports each implementation ::
>> +A list of implementations can be obtained by the following command. The
>> +command also shows whether the CPU supports each implementation::
>>
>> $ ovs-appctl dpif-netdev/miniflow-parser-get
>> Available Optimized Miniflow Extracts:
>> @@ -316,123 +267,34 @@ command also shows whether the CPU supports each
>> implementation ::
>> scalar (available: True, pmds: 1,15)
>> study (available: True, pmds: none)
>>
>> -An implementation can be selected manually by the following command ::
>> +An implementation can be selected manually by the following command::
>>
>> $ ovs-appctl dpif-netdev/miniflow-parser-set [-pmd core_id] name \
>> [study_cnt]
>>
>> -The above command has two optional parameters: study_cnt and core_id.
>> -The core_id sets a particular miniflow extract function to a specific
>> -pmd thread on the core. The third parameter study_cnt, which is specific
>> -to study and ignored by other implementations, means how many packets
>> +The above command has two optional parameters: ``study_cnt`` and
>> ``core_id``.
>> +The ``core_id`` sets a particular packet parsing function to a specific
>> +PMD thread on the core. The third parameter ``study_cnt``, which is
>> specific
>> +to ``study`` and ignored by other implementations, means how many packets
>> are needed to choose the best implementation.
>>
>> -Also user can select the study implementation which studies the traffic for
>> +Also user can select the ``study`` implementation which studies the traffic
>> for
>> a specific number of packets by applying all available implementations of
>> -miniflow extract and then chooses the one with the most optimal result for
>> -that traffic pattern. The user can optionally provide an packet count
>> -[study_cnt] parameter which is the minimum number of packets that OVS must
>> -study before choosing an optimal implementation. If no packet count is
>> -provided, then the default value, 128 is chosen. Also, as there is no
>> -synchronization point between threads, one PMD thread might still be running
>> -a previous round, and can now decide on earlier data.
>> -
>> -The per packet count is a global value, and parallel study executions with
>> -differing packet counts will use the most recent count value provided by
>> user.
>> +the packet parsing function and then chooses the one with the most optimal
>> +result for that traffic pattern. The user can optionally provide a packet
>
> Nit: there is one extra space before The user.
ok.
>
>
>> +count ``study_cnt`` parameter which is the minimum number of packets that
>> OVS
>> +must study before choosing an optimal implementation. If no packet count is
>> +provided, then the default value, ``128`` is chosen.
>>
>> -Study can be selected with packet count by the following command ::
>> +``study`` can be selected with packet count by the following command::
>>
>> $ ovs-appctl dpif-netdev/miniflow-parser-set study 1024
>>
>> -Study can be selected with packet count and explicit PMD selection
>> -by the following command ::
>> +``study`` can be selected with packet count and explicit PMD selection by
>> the
>> +following command::
>>
>> $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 study 1024
>>
>> -In the above command the first parameter is the CORE ID of the PMD
>> -thread and this can also be used to explicitly set the miniflow
>> -extraction function pointer on different PMD threads.
>> -
>> -Scalar can be selected on core 3 by the following command where
>> -study count should not be provided for any implementation other
>> -than study ::
>> +``scalar`` can be selected on core ``3`` by the following command::
>>
>> $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 scalar
>> -
>> -Miniflow Extract Validation
>> -~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> -
>> -As multiple versions of miniflow extract can co-exist, each with different
>> -CPU ISA optimizations, it is important to validate that they all give the
>> -exact same results. To easily test all miniflow implementations, an
>> -``autovalidator`` implementation of the miniflow exists. This implementation
>> -runs all other available miniflow extract implementations, and verifies that
>> -the results are identical.
>> -
>> -Running the OVS unit tests with the autovalidator enabled ensures all
>> -implementations provide the same results.
>> -
>> -To set the Miniflow autovalidator, use this command ::
>> -
>> - $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator
>> -
>> -A compile time option is available in order to test it with the OVS unit
>> -test suite. Use the following configure option ::
>> -
>> - $ ./configure --enable-mfex-default-autovalidator
>> -
>> -Unit Test Miniflow Extract
>> -++++++++++++++++++++++++++
>> -
>> -Unit test can also be used to test the workflow mentioned above by running
>> -the following test-case in tests/system-dpdk.at ::
>> -
>> - make check-dpdk TESTSUITEFLAGS='-k MFEX'
>> - OVS-DPDK - MFEX Autovalidator
>> -
>> -The unit test uses mulitple traffic type to test the correctness of the
>> -implementaions.
>> -
>> -The MFEX commands can also be tested for negative and positive cases to
>> -verify that the MFEX set command does not allow for incorrect parameters.
>> -A user can directly run the following configuration test case in
>> -tests/system-dpdk.at ::
>> -
>> - make check-dpdk TESTSUITEFLAGS='-k MFEX'
>> - OVS-DPDK - MFEX Configuration
>> -
>> -Running Fuzzy test with Autovalidator
>> -+++++++++++++++++++++++++++++++++++++
>> -
>> -Fuzzy tests can also be done on miniflow extract with the help of
>> -auto-validator and Scapy. The steps below describes the steps to
>> -reproduce the setup with IP being fuzzed to generate packets.
>> -
>> -Scapy is used to create fuzzy IP packets and save them into a PCAP ::
>> -
>> - pkt = fuzz(Ether()/IP()/TCP())
>> -
>> -Set the miniflow extract to autovalidator using ::
>> -
>> - $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator
>> -
>> -OVS is configured to receive the generated packets ::
>> -
>> - $ ovs-vsctl add-port br0 pcap0 -- \
>> - set Interface pcap0 type=dpdk options:dpdk-devargs=net_pcap0
>> - "rx_pcap=fuzzy.pcap"
>> -
>> -With this workflow, the autovalidator will ensure that all MFEX
>> -implementations are classifying each packet in exactly the same way.
>> -If an optimized MFEX implementation causes a different miniflow to be
>> -generated, the autovalidator has ovs_assert and logging statements that
>> -will inform about the issue.
>> -
>> -Unit Fuzzy test with Autovalidator
>> -+++++++++++++++++++++++++++++++++++++
>> -
>> -Unit test can also be used to test the workflow mentioned above by running
>> -the following test-case in tests/system-dpdk.at ::
>> -
>> - make check-dpdk TESTSUITEFLAGS='-k MFEX'
>> - OVS-DPDK - MFEX Autovalidator Fuzzy
>> diff --git a/Documentation/topics/testing.rst
>> b/Documentation/topics/testing.rst
>> index ea11700e3..85797df07 100644
>> --- a/Documentation/topics/testing.rst
>> +++ b/Documentation/topics/testing.rst
>> @@ -356,7 +356,64 @@ The phy test will skip if no compatible physical device
>> is available.
>> .. _Configure hugepages:
>> https://doc.dpdk.org/guides-21.11/linux_gsg/sys_reqs.html
>>
>> All the features documented under `Unit Tests`_ are available for the DPDK
>> -datapath testsuite.
>> +testsuite.
>> +
>> +Userspace datapath: CPU ISA Testing and Validation
>> +''''''''''''''''''''''''''''''''''''''''''''''''''
>
> It could be worth rewording (or in a followup patch?): this block is
> not about testing CPU ISA, but about testing optimisations of some
> parts of OVS.
Userspace datapath: Testing and Validation of CPU-specific Optimizations
?
>
>
>> +
>> +As multiple versions of the datapath classifier and packet parsing functions
>> +can co-exist, each with different CPU ISA optimizations, it is important to
>> +validate that they all give the exact same results. To easily test all the
>> +implementations, an ``autovalidator`` implementations of them exists. This
>
> Nit: implementation*
ok.
>
>
>
>
>> +implementation runs all other available implementations, and verifies that
>> the
>> +results are identical.
>> +
>> +Running the OVS unit tests with the autovalidator enabled ensures all
>> +implementations provide the same results. Note that the performance of the
>> +autovalidator is lower than all other implementations, as it tests the
>> scalar
>> +implementation against itself, and against all other enabled
>> implementations.
>> +
>> +To adjust the autovalidator priority for a datapath classifier, use this
>> +command::
>> +
>> + $ ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 7
>> +
>> +To set the autovalidator for the packet parser, use this command::
>> +
>> + $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator
>> +
>> +To run the OVS unit test suite with the autovalidator as the default
>> +implementation, it is required to recompile OVS. During the recompilation,
>> +the default priority of the `autovalidator` implementation is set to the
>> +maximum priority, ensuring every test will be run with every
>> implementation::
>> +
>> + $ ./configure --enable-autovalidator --enable-mfex-default-autovalidator
>> +
>> +The following line should be seen in the configuration log when the above
>> +options are used::
>> +
>> + checking whether DPCLS Autovalidator is default implementation... yes
>> + checking whether MFEX Autovalidator is default implementation... yes
>> +
>> +Compile OVS in debug mode to have `ovs_assert` statements error out if
>> +there is a mis-match in the datapath classifier lookup or packet parser
>> +implementations.
>> +
>> +Since the AVX512 implementation of the datapath interface is disabled by
>> +default, a compile time option is available in order to test it with the OVS
>> +unit test suite::
>> +
>> + $ ./configure --enable-dpif-default-avx512
>> +
>> +The following line should be seen in the configuration log when the above
>> +option is used::
>> +
>> + checking whether DPIF AVX512 is default implementation... yes
>> +
>> +.. note::
>> + Run all the available testsuites including `make check`,
>> + `make check-system-userspace` and `make check-dpdk` to ensure the optimal
>> + test coverage.
>>
>> Kernel datapath
>> '''''''''''''''
>> --
>> 2.31.1
>>
>
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
