Hi Eelco, Please find my replies Inline.
> -----Original Message----- > From: Eelco Chaudron <[email protected]> > Sent: Thursday, May 12, 2022 2:53 PM > To: Amber, Kumar <[email protected]> > Cc: [email protected]; Ferriter, Cian <[email protected]>; > Stokes, Ian <[email protected]>; Van Haaren, Harry > <[email protected]>; Ilya Maximets <[email protected]> > Subject: Re: [PATCH v2] tests/mfex: Improve pcap script for mfex tests. > > > > On 11 May 2022, at 18:28, Kumar Amber wrote: > > > The mfex pcap generation script is improved for varied length traffic > > and also removes the hard coded mfex_pcap and instead uses the script > > itself to generate complex traffic patterns for testing. > > I did not follow the discussion around the fuzzy testing being part of the > unit > tests, so I’ll only express my concerns below. > The discussion on this led to a patch-set which introduces a new way to test in an integrated way to include flow based testing of AVX512 mfex as it happens for the scalar mfex in the testing setup, the new method has its own merits and these .at based tests have their own merits and we feel having both will strengthen the AVX512 data-path testing both at functional level and in an integrated level as well. The new Testing patch is under review and can be found here: http://patchwork.ozlabs.org/project/openvswitch/list/?series=297158 On the Sidelines I feel its good to have the infrastructure ready to enable the test in CI, even if we choose Not to enable it and considering its just adding some warnings which can be ignored. > > Signed-off-by: Kumar Amber <[email protected]> > > Acked-by: Cian Ferriter <[email protected]> > > > > --- > > v2: > > - Add huge page test-skip. > > - Change core id to 3 to 0 to allow the mfex config test-case > > to run on any system. > > --- > > --- > > tests/automake.mk | 1 - > > tests/mfex_fuzzy.py | 66 +++++++++++++++++++++++++++----------- > > tests/pcap/mfex_test.pcap | Bin 416 -> 0 bytes > > tests/system-dpdk.at | 44 +++++++++++++++++-------- > > 4 files changed, 77 insertions(+), 34 deletions(-) delete mode > > 100644 tests/pcap/mfex_test.pcap > > > > diff --git a/tests/automake.mk b/tests/automake.mk index > > 34ddda6aa..204e86fac 100644 > > --- a/tests/automake.mk > > +++ b/tests/automake.mk > > @@ -146,7 +146,6 @@ $(srcdir)/tests/fuzz-regression-list.at: > > tests/automake.mk > > > > EXTRA_DIST += $(MFEX_AUTOVALIDATOR_TESTS) > MFEX_AUTOVALIDATOR_TESTS = > > \ > > - tests/pcap/mfex_test.pcap \ > > tests/mfex_fuzzy.py > > > > OVSDB_CLUSTER_TESTSUITE_AT = \ > > diff --git a/tests/mfex_fuzzy.py b/tests/mfex_fuzzy.py index > > 3efe1152d..dbde5fe1b 100755 > > --- a/tests/mfex_fuzzy.py > > +++ b/tests/mfex_fuzzy.py > > @@ -3,30 +3,58 @@ > > import sys > > > > from scapy.all import RandMAC, RandIP, PcapWriter, RandIP6, > > RandShort, fuzz -from scapy.all import IPv6, Dot1Q, IP, Ether, UDP, > > TCP > > +from scapy.all import IPv6, Dot1Q, IP, Ether, UDP, TCP, random > > > > +# Relative path for the pcap file location. > > path = str(sys.argv[1]) + "/pcap/fuzzy.pcap" > > +# The number of packets generated will be size * 8. > > +size = int(sys.argv[2]) > > +# Traffic option is used to choose between fuzzy or simple packet type. > > +traffic_opt = str(sys.argv[3]) > > Think here we should check for the existence of sys.argv[3], and if not just > initialize traffic_opt as “” so we do not have to supply a dummy “0” option. > Done in V3. > > + > > pktdump = PcapWriter(path, append=False, sync=True) > > > > -for i in range(0, 2000): > > +pkt = [] > > + > > +for i in range(0, size): > > > > - # 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 > > - pktdump.write(fuzz(eth / ipv4 / udp)) > > - pktdump.write(fuzz(eth / ipv4 / tcp)) > > - pktdump.write(fuzz(eth / vlan / ipv4 / udp)) > > - pktdump.write(fuzz(eth / vlan / ipv4 / tcp)) > > - > > - # IPv6 packets with fuzzing > > - pktdump.write(fuzz(eth / ipv6 / udp)) > > - pktdump.write(fuzz(eth / ipv6 / tcp)) > > - pktdump.write(fuzz(eth / vlan / ipv6 / udp)) > > - pktdump.write(fuzz(eth / vlan / ipv6 / tcp)) > > + > > + if traffic_opt == "fuzzy": > > + > > + ipv4 = IP(src=RandIP(), dst=RandIP(), len=random.randint(0, 100)) > > + ipv6 = IPv6(src=RandIP6(), dst=RandIP6(), plen=random.randint(0, > 100)) > > + tcp = TCP(dport=RandShort(), sport=RandShort(), flags='S', > > + dataofs=random.randint(0, 20)) > > dataof is 4 bits, why go to 20? Do we want more of the lower values? > Reduced the value to 0-15. > > + # IPv4 packets with fuzzing > > + pkt.append(fuzz(eth / ipv4 / udp)) > > + pkt.append(fuzz(eth / ipv4 / tcp)) > > + pkt.append(fuzz(eth / vlan / ipv4 / udp)) > > + pkt.append(fuzz(eth / vlan / ipv4 / tcp)) > > + > > + # IPv6 packets with fuzzing > > + pkt.append(fuzz(eth / ipv6 / udp)) > > + pkt.append(fuzz(eth / ipv6 / tcp)) > > + pkt.append(fuzz(eth / vlan / ipv6 / udp)) > > + pkt.append(fuzz(eth / vlan / ipv6 / tcp)) > > + > > + else: > > + > > + ipv4 = IP(src=RandIP(), dst=RandIP()) > > + ipv6 = IPv6(src=RandIP6(), dst=RandIP6()) > > + tcp = TCP(dport=RandShort(), sport=RandShort(), flags='S') > > + # IPv4 packets > > + pkt.append(eth / ipv4 / udp) > > + pkt.append(eth / ipv4 / tcp) > > + pkt.append(eth / vlan / ipv4 / udp) > > + pkt.append(eth / vlan / ipv4 / tcp) > > + > > + # IPv6 packets > > + pkt.append(eth / ipv6 / udp) > > + pkt.append(eth / ipv6 / tcp) > > + pkt.append(eth / vlan / ipv6 / udp) > > + pkt.append(eth / vlan / ipv6 / tcp) > > + > > +pktdump.write(pkt) > > diff --git a/tests/pcap/mfex_test.pcap b/tests/pcap/mfex_test.pcap > > The thing I do not like about creating a random dynamic file is that it will > be > hard to reproduce. If someone reports a failure and we do not have the pcap > file it might be hard to reproduce. Is there a specific reason for using > random > Mac/ip/ports? Can we use a fixed range for non-fuzz testing? > Fixed ranges and values for IP, MAC, Ports. > > deleted file mode 100644 > > index > > > 1aac67b8d643ecb016c758cba4cc32212a80f52a..000000000000000000000000 > 0000 > > 000000000000 > > GIT binary patch > > literal 0 > > HcmV?d00001 > > > > literal 416 > > > zcmca|c+)~A1{MYw`2U}Qff2}Q<eHVR>K`M68ITRa|G@yFii5$Gfk6YL%z>@uY& > }o| > > > z2s4N<1VH2&7y^V87$)XGOtD~MV$cFgfG~zBGGJ2#YtF$<F=a4i;9x8Q*<ZrSM6 > Ufz > > xK>KST_NTIwYriok6N4Vm)gX- > Q@<yO<!C`>c^{cp<7_5LgK^UuU{2>VS0RZ!RQ+EIW > > > > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at index > > 7d2715c4a..27ba42954 100644 > > --- a/tests/system-dpdk.at > > +++ b/tests/system-dpdk.at > > @@ -226,12 +226,14 @@ dnl > > ---------------------------------------------------------------------- > > ---- > > dnl Add standard DPDK PHY port > > AT_SETUP([OVS-DPDK - MFEX Autovalidator]) > > AT_KEYWORDS([dpdk]) > > - > > +OVS_DPDK_PRE_CHECK() > > +AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], []) > > +AT_CHECK([$PYTHON3 $srcdir/mfex_fuzzy.py $srcdir 2000 0], [], > > +[stdout]) > > See the comment above, we could remove the 0 here. > Done. > > OVS_DPDK_START() > > > > dnl Add userspace bridge and attach it to OVS AT_CHECK([ovs-vsctl > > add-br br0 -- set bridge br0 datapath_type=netdev]) > > -AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dpdk > > options:dpdk- > devargs=net_pcap1,rx_pcap=$srcdir/pcap/mfex_test.pcap,inf > > inite_rx=1], [], [stdout], [stderr]) > > +AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dpdk > > +options:dpdk-devargs=net_pcap1,rx_pcap=$srcdir/pcap/fuzzy.pcap,infini > > +te_rx=1], [], [stdout], [stderr]) > > AT_CHECK([ovs-vsctl show], [], [stdout]) > > > > AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | > > grep "True"], [], [dnl > > Can we hustle this code around a bit, because else we wait a long time > generating the pcap file if we do not need this (also for the fuzzy test > below). > Something like: > Done needed a few more tweaks to make it work. > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at index > 27ba42954..a6447ecc4 100644 > --- a/tests/system-dpdk.at > +++ b/tests/system-dpdk.at > @@ -227,18 +227,19 @@ dnl Add standard DPDK PHY port AT_SETUP([OVS- > DPDK - MFEX Autovalidator]) > AT_KEYWORDS([dpdk]) > OVS_DPDK_PRE_CHECK() > +OVS_DPDK_START() > + > +AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | > +grep "True"], [], [dnl > +]) > + > AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], []) > AT_CHECK([$PYTHON3 $srcdir/mfex_fuzzy.py $srcdir 2000 0], [], [stdout]) > -OVS_DPDK_START() > > dnl Add userspace bridge and attach it to OVS AT_CHECK([ovs-vsctl add-br > br0 -- set bridge br0 datapath_type=netdev]) AT_CHECK([ovs-vsctl add-port > br0 p1 -- set Interface p1 type=dpdk options:dpdk- > devargs=net_pcap1,rx_pcap=$srcdir/pcap/fuzzy.pcap,infinite_rx=1], [], > [stdout], [stderr]) AT_CHECK([ovs-vsctl show], [], [stdout]) > > -AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | grep > "True"], [], [dnl > -]) > - > AT_CHECK([ovs-appctl dpif-netdev/dpif-impl-set dpif_avx512], [0], [dnl DPIF > implementation set to dpif_avx512. > ]) > @@ -264,18 +265,19 @@ dnl Add standard DPDK PHY port AT_SETUP([OVS- > DPDK - MFEX Autovalidator Fuzzy]) > AT_KEYWORDS([dpdk]) > OVS_DPDK_PRE_CHECK() > +OVS_DPDK_START() > + > +AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | > +grep "True"], [], [dnl > +]) > + > AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], []) > AT_CHECK([$PYTHON3 $srcdir/mfex_fuzzy.py $srcdir 2000 fuzzy], [], > [stdout]) > -OVS_DPDK_START() > > dnl Add userspace bridge and attach it to OVS AT_CHECK([ovs-vsctl add-br > br0 -- set bridge br0 datapath_type=netdev]) AT_CHECK([ovs-vsctl add-port > br0 p1 -- set Interface p1 type=dpdk options:dpdk- > devargs=net_pcap1,rx_pcap=$srcdir/pcap/fuzzy.pcap,infinite_rx=1], [], > [stdout], [stderr]) AT_CHECK([ovs-vsctl show], [], [stdout]) > > -AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | grep > "True"], [], [dnl > -]) > - > AT_CHECK([ovs-appctl dpif-netdev/dpif-impl-set dpif_avx512], [0], [dnl DPIF > implementation set to dpif_avx512. > ]) > > > @@ -245,11 +247,15 @@ AT_CHECK([ovs-appctl > > dpif-netdev/miniflow-parser-set autovalidator], [0], [dnl Miniflow extract > implementation set to autovalidator. > > ]) > > > > -OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep > > -oP 'rx_packets=\s*\K\d+'` -ge 1000]) > > +OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep > > +-oP 'rx_packets=\s*\K\d+'` -ge 16000]) > > > > dnl Clean up > > AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr]) > > -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]") > > +OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [ > > +\@upcall: datapath reached the dynamic limit of .* flows.@d > > +\@received packet on unknown port .* on bridge br0 while processing@d > > +\@upcall_cb failure: ukey installation fails@d > > +])") > > AT_CLEANUP > > dnl > > ---------------------------------------------------------------------- > > ---- > > > > @@ -257,8 +263,9 @@ dnl > > ---------------------------------------------------------------------- > > ---- > > dnl Add standard DPDK PHY port > > AT_SETUP([OVS-DPDK - MFEX Autovalidator Fuzzy]) > > AT_KEYWORDS([dpdk]) > > +OVS_DPDK_PRE_CHECK() > > AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], []) > > -AT_CHECK([$PYTHON3 $srcdir/mfex_fuzzy.py $srcdir], [], [stdout]) > > +AT_CHECK([$PYTHON3 $srcdir/mfex_fuzzy.py $srcdir 2000 fuzzy], [], > > +[stdout]) > > OVS_DPDK_START() > > > > dnl Add userspace bridge and attach it to OVS @@ -277,12 +284,18 @@ > > AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], > > [0], [dnl Miniflow extract implementation set to autovalidator. > > ]) > > > > -OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep > > -oP 'rx_packets=\s*\K\d+'` -ge 100000]) > > +OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep > > +-oP 'rx_packets=\s*\K\d+'` -ge 16000]) > > > > dnl Clean up > > AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr]) > > OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [ > > \@upcall: datapath reached the dynamic limit of .* flows.@d > > +\@received packet on unknown port .* on bridge br0 while processing@d > > +\@upcall_cb failure: ukey installation fails@d \@Unreasonably long .* > > +poll interval@d \@context switches: .* voluntary, .* involuntary@d > > +\@faults: .* minor, .* major@d > > +\@upcall_cb failure: ukey installation fails@d > > There was some discussion about adding these in the past with Ilya if I > remember correctly? Was this settled, and ACKed? > Explained above. Patch v3: http://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ Regards, Amber _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
