Hi Eelco,
Please find replies inline.
> > +# Relative path for the pcap file location.
> > path = str(sys.argv[1]) + "/pcap/fuzzy.pcap"
>
> Would it be better to supply the full file name, rather than a fixed file name
> and directory being concatenated?
>
Sure, would make the script usable to more people .
Done directly passing the full patch via test-cases in v6.
> > +# 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.
> > +if (len(sys.argv) > 3):
>
> Just a nit, but int python those initial () are not needed, so it would
> become:
>
> if len(sys.argv) > 3:
Sure, taken in V6.
>
> > + traffic_opt = str(sys.argv[3])
> > +else:
> > + traffic_opt = ""
> > +
> > pktdump = PcapWriter(path, append=False, sync=True)
> >
> > -for i in range(0, 2000):
> > -
> > - # 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))
> > +pkt = []
> > +
> > +for i in range(0, size):
> > + if traffic_opt == "fuzzy":
> > +
> > + eth = Ether(src=RandMAC(), dst=RandMAC())
> > + vlan = Dot1Q()
> > + udp = UDP(dport=RandShort(), sport=RandShort())
> > + 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, 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:
> > + mac_addr_src = "52:54:00:FF:FF:%02x" % ((i % 0xff),)
> > + mac_addr_dst = "80:FF:FF:FF:FF:%02x" % ((i % 0xff),)
>
> Not sure why you have this odd formatting, this should work just fine:
>
> mac_addr_src = "52:54:00:FF:FF:%02X” % (i % 0xff)
>
> Also to be consistent use the .format() parameter as you do for IPv6, so:
>
> mac_addr_dst = "80:FF:FF:FF:FF:{:02X}".format(i % 0xff)
>
> NIT: you should probably use an X to make it caps :)
>
Sure, I am used to old school python 😊 , but kept the formatting same for all
the IPv4
And Ipv6 in V6.
> > + src_port = 200 + (i % 20)
> > + dst_port = 1000 + (i % 20)
> > + eth = Ether(src=mac_addr_src, dst=mac_addr_dst)
> > + vlan = Dot1Q(vlan=(i % 10))
> > + udp = UDP(dport=src_port, sport=dst_port)
> > + # IPv4 address range limits to 255 and IPv6 limit to 65535
> > + ipv4_addr_src = "192.168.150." + str((i % 255))
> > + ipv4_addr_dst = "200.100.198." + str((i % 255))
> > + ipv6_addr_src = "2001:0db8:85a3:0000:0000:8a2e:0370:{:04x}" \
> > + .format(i % 0xffff)
> > + ipv6_addr_dst = "3021:ffff:85a3:ffff:0000:8a2e:0480:{:04x}" \
> > + .format(i % 0xffff)
> > + ipv4 = IP(src=ipv4_addr_src, dst=ipv4_addr_dst)
> > + ipv6 = IPv6(src=ipv6_addr_src, dst=ipv6_addr_dst)
> > + tcp = TCP(dport=src_port, sport=dst_port, 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
> > 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..ac83e5a57 100644
> > --- a/tests/system-dpdk.at
> > +++ b/tests/system-dpdk.at
> > @@ -226,17 +226,19 @@ dnl
> > ----------------------------------------------------------------------
> > ----
> > dnl Add standard DPDK PHY port
> > AT_SETUP([OVS-DPDK - MFEX Autovalidator])
> > AT_KEYWORDS([dpdk])
> > -
> > +OVS_DPDK_PRE_CHECK()
> > 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 show], [],
> > [stdout])
> > -
> > 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], [], [stdout])
>
> Have you tested this patch on an MFEX machine on a clean branch? I do not
> have an AVX512 machine, but if I move this above the check I get errors due
> to missing tests/pcap/ directory.
>
> --- /dev/null 2022-04-25 09:31:38.866748964 +0200
> +++
> /home/echaudron/Documents/review/ovs_akumar_mfeximp/ovs_github/te
> sts/system-dpdk-testsuite.dir/at-groups/6/stderr 2022-05-25
> 11:17:40.116614366 +0200
> @@ -0,0 +1,6 @@
> +Traceback (most recent call last):
> + File
> "/home/echaudron/Documents/review/ovs_akumar_mfeximp/ovs_github/t
> ests/system-dpdk-testsuite.dir/6/../.././mfex_fuzzy.py", line 22, in <module>
> + pktdump = PcapWriter(path, append=False, sync=True)
> + File "/usr/lib/python3.10/site-packages/scapy/utils.py", line 1686, in
> __init__
> + self.f = open(filename, append and "ab" or "wb", bufsz)
> +FileNotFoundError: [Errno 2] No such file or directory:
> '../.././pcap/fuzzy.pcap'
> stdout:
>
Yes, I have tested on AVX512 machine.
If you compare this patch with the reorder you suggested in the first reply, I
had to do a bit of
Reorder in the first suggestion because I saw the same error:
We need to add a port to ovs to run the "get-mfex" command for skip to work
thus kept the
Add port before the AT_SKIP("mfex-get).
But kept the Pcap generation then subsequent netdev bridge addition later after
skip.
If you move the netdev bridge addition before pcap generation, you will get the
file missing error.
> > +
> > +dnl Add userspace bridge and attach it to OVS 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_CHECK([ovs-appctl dpif-netdev/dpif-impl-set dpif_avx512], [0],
> > [dnl DPIF implementation set to dpif_avx512.
> > ])
> > @@ -245,11 +247,12 @@ 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], [
> > +])")
>
> If you are not changing anything, you should keep the original line.
>
Somehow this was an old formatting and formatting was changed in the other
2 test-cases but this was missed and it's an improvement from David and keep
All the three test-case kinds of uniform.
Can remove it but would be good to keep it 😊
Regards
Amber
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
