On 25 May 2022, at 12:59, Amber, Kumar wrote:
> 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.
This is not the problem, you removed the only test/pcap/… file which will
result in the pcap directory being removed from the GitHub repo, so it no
longer exist, hence you get the error.
Whoever reviewed your patches should have run into this if he applied the
patches thought git am.
>>> +
>>> +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 😊
I would remove it, as doing m4_join() with an empty string looks odd.
>
> Regards
> Amber
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
