On 11/30/21 16:00, David Marchand wrote:
> net_pcap is not always available in DPDK (like, in a dev
> environment when you forgot to install the libpcap-devel).
> On the other hand, OVS already has its own way to inject packets into a
> bridge. Let's make use of it.
> 
> This solution is slower than net/pcap DPDK, so lower the number of
> expected packets so that it runs in OVS_CTL_TIMEOUT seconds.
> 
> While at it, convert "known" packets from pcap to scapy so that the
> injected packets can be updated without having to read/write a pcap file.
> 
> Note: this change also (avoids/) fixes a python exception in PcapWriter
> with scapy 2.4.3 that comes from EPEL.
> 
> Suggested-by: Ilya Maximets <[email protected]>
> Signed-off-by: David Marchand <[email protected]>
> ---
> Changes since v2:
> - updated documentation,
> - cleaned tests/automake.mk,
> - fixed shebang in python script,
> - added missing check for scapy availability,
> 
> Changes since v1:
> - renamed generator script,
> - decreased packet count for fuzzy test,
> - simplified wait expression for packet count,
> 
> ---
>  Documentation/topics/dpdk/bridge.rst |   7 ++--
>  tests/automake.mk                    |   7 +---
>  tests/genpkts.py                     |  56 +++++++++++++++++++++++++++
>  tests/mfex_fuzzy.py                  |  32 ---------------
>  tests/pcap/mfex_test.pcap            | Bin 416 -> 0 bytes
>  tests/system-dpdk-macros.at          |   4 +-
>  tests/system-dpdk.at                 |  33 +++++++++++++---
>  7 files changed, 89 insertions(+), 50 deletions(-)
>  create mode 100755 tests/genpkts.py
>  delete mode 100755 tests/mfex_fuzzy.py
>  delete mode 100644 tests/pcap/mfex_test.pcap
> 
> diff --git a/Documentation/topics/dpdk/bridge.rst 
> b/Documentation/topics/dpdk/bridge.rst
> index f645b9ade5..648ce203eb 100644
> --- a/Documentation/topics/dpdk/bridge.rst
> +++ b/Documentation/topics/dpdk/bridge.rst
> @@ -408,7 +408,7 @@ 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 ::
> +Scapy is used to create fuzzy IP packets (see tests/genpkts.py) ::
>  
>      pkt = fuzz(Ether()/IP()/TCP())
>  
> @@ -418,9 +418,8 @@ Set the miniflow extract to autovalidator using ::
>  
>  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"
> +    $ ovs-vsctl add-port br0 p1 -- \
> +        set Interface p1 type=dummy-pmd
>  
>  With this workflow, the autovalidator will ensure that all MFEX
>  implementations are classifying each packet in exactly the same way.

I'm actually getting a hard time understanding a value of this section
of the doc for the end user, who is the target audience for this doc.

And the next section "Unit Fuzzy test with Autovalidator" is incorrectly
formatted and also almost to the word equal to the "Unit Test Miniflow
Extract" section.

It doesn't make a lot of sense talking that functionality is covered by
unit tests.  Everything should be covered by unit tests by default.
And this is definitely not something that should be part of an end-user
high-level documentation.

It's not a problem of a current patch set, but I'd suggest to just
remove the last 3 sections of dpdk/bridge.rst.  They should not be there.

> diff --git a/tests/automake.mk b/tests/automake.mk
> index 43731d0973..3bc74a5aff 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -143,11 +143,6 @@ $(srcdir)/tests/fuzz-regression-list.at: 
> tests/automake.mk
>           echo "TEST_FUZZ_REGRESSION([$$basename])"; \
>       done > [email protected] && mv [email protected] $@
>  
> -EXTRA_DIST += $(MFEX_AUTOVALIDATOR_TESTS)
> -MFEX_AUTOVALIDATOR_TESTS = \
> -     tests/pcap/mfex_test.pcap \
> -     tests/mfex_fuzzy.py
> -
>  OVSDB_CLUSTER_TESTSUITE_AT = \
>       tests/ovsdb-cluster-testsuite.at \
>       tests/ovsdb-execution.at \
> @@ -518,7 +513,7 @@ tests_test_type_props_SOURCES = tests/test-type-props.c
>  CHECK_PYFILES = \
>       tests/appctl.py \
>       tests/flowgen.py \
> -     tests/mfex_fuzzy.py \
> +     tests/genpkts.py \
>       tests/ovsdb-monitor-sort.py \
>       tests/test-daemon.py \
>       tests/test-json.py \
> diff --git a/tests/genpkts.py b/tests/genpkts.py
> new file mode 100755
> index 0000000000..f64f786ccb
> --- /dev/null
> +++ b/tests/genpkts.py
> @@ -0,0 +1,56 @@
> +#!/usr/bin/env python3
> +
> +import sys
> +
> +from scapy.all import RandMAC, RandIP, RandIP6, RandShort, fuzz
> +from scapy.all import IPv6, Dot1Q, IP, Ether, UDP, TCP
> +
> +if len(sys.argv) < 2:
> +    print('usage: {} packets_count [fuzz]'.format(sys.argv[0]))
> +    sys.exit(1)
> +
> +tmpl = []
> +
> +if len(sys.argv) == 2:
> +    eth = Ether(dst='ff:ff:ff:ff:ff:ff')
> +    vlan = eth / Dot1Q(vlan=1)
> +    p = eth / IP() / TCP(sport=20, dport=80, flags='SA', window=8192)
> +    tmpl += [p.build().hex()]
> +    p = eth / IP() / UDP(sport=53, dport=53)
> +    tmpl += [p.build().hex()]
> +    p = eth / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> +    tmpl += [p.build().hex()]
> +    p = eth / IP() / UDP(sport=53, dport=53)

We already have the same packet.  Is it intended behavior?

> +    tmpl += [p.build().hex()]
> +    p = vlan / IP() / UDP(sport=53, dport=53)
> +    tmpl += [p.build().hex()]
> +    p = vlan / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> +    tmpl += [p.build().hex()]
> +elif sys.argv[2] == 'fuzz':
> +    # Generate random protocol bases, use a fuzz() over the combined packet
> +    # for full fuzzing.
> +    eth = Ether(src=RandMAC(), dst=RandMAC())
> +    vlan = Dot1Q()
> +    ipv4 = IP(src=RandIP(), dst=RandIP())
> +    ipv6 = IPv6(src=RandIP6(), dst=RandIP6())
> +    udp = UDP(dport=RandShort(), sport=RandShort())
> +    tcp = TCP(dport=RandShort(), sport=RandShort())
> +
> +    # IPv4 packets with fuzzing
> +    tmpl += [fuzz(eth / ipv4 / udp).build().hex()]
> +    tmpl += [fuzz(eth / ipv4 / tcp).build().hex()]
> +    tmpl += [fuzz(eth / vlan / ipv4 / udp).build().hex()]
> +    tmpl += [fuzz(eth / vlan / ipv4 / tcp).build().hex()]
> +
> +    # IPv6 packets with fuzzing
> +    tmpl += [fuzz(eth / ipv6 / udp).build().hex()]
> +    tmpl += [fuzz(eth / ipv6 / tcp).build().hex()]
> +    tmpl += [fuzz(eth / vlan / ipv6 / udp).build().hex()]
> +    tmpl += [fuzz(eth / vlan / ipv6 / tcp).build().hex()]
> +
> +i = 0
> +count = int(sys.argv[1])
> +while count != 0:
> +    print(tmpl[i % len(tmpl)])
> +    i += 1
> +    count -= 1

While it's OK to reduce the number of packets for the fuzzy test
in order to seed it up, we should still generate N different
fuzzy packets.  This script seems to print out the same 8 packets
again and again.

Maybe something like this:

def simple():
    tmpl = []
    <generate a few packets>
    return tmpl

def fuzzy():
    tmpl = []
    <generate a few fuzzy packets>
    return tmpl

while True:
    for packet in simple() if len(sys.argv) == 2 else fuzzy():
        print(packet)
        count -= 1
        if count == 0:
            exit(0)

---

What do you think?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to