On 12/14/21 16:38, David Marchand wrote:
> On Mon, Dec 6, 2021 at 11:16 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
>>
>> 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 <i.maxim...@ovn.org>
>>> Signed-off-by: David Marchand <david.march...@redhat.com>
>>> ---
>>> 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.
>
> I guess this comment is covered by your patch that refactors the
> documentation, correct?
Yep.
> I'll still update this part in my patch (and rebase if there is a
> conflict once your doc patch is merged).
OK. And we might not even need a rebase, as rebase would mean just
dropping the documentation changes, AFAICT. And that's not hard to
do while applying the patch.
>
>
>>
>>> 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 > $@.tmp && mv $@.tmp $@
>>>
>>> -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?
>
> That's what the unit test was doing before, I translated it in python.
> I don't get the intention of always testing with the same packets and
> I see little value in it.
> We could even remove the loop on receiving 1k packets.
>
>
>>
>>> + 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.
>
> Argh... yes this is a regression.
>
>
>>
>> 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)
>
>
> I'll try with your suggestion.
> Thanks Ilya.
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
