On 24 May 2022, at 12:10, Amber, Kumar wrote:
> Hi Eelco, > > Thanks again replies Ilnie. > >> -----Original Message----- >> From: Eelco Chaudron <[email protected]> >> Sent: Tuesday, May 24, 2022 3:15 PM >> To: Amber, Kumar <[email protected]> >> Cc: [email protected]; Ferriter, Cian <[email protected]>; >> [email protected]; Stokes, Ian <[email protected]>; Van Haaren, Harry >> <[email protected]> >> Subject: Re: [PATCH v3] tests/mfex: Improve pcap script for mfex tests. >> >> >> >> On 24 May 2022, at 7:40, Amber, Kumar wrote: >> >>> Hi Eelco, >>> >>> Thanks again for reviews . Please find my replies inline. >>> >>> <Snip> >>> >>>>> + tcp = TCP(dport=RandShort(), sport=RandShort(), flags='S', >>>>> + dataofs=random.randint(0, 15)) >>>> >>>> Maybe a new line before the command, as it was before. >>>> >>> >>> Fixed in next version. >>> >>>>> + # 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 = "52:54:00:FF:FF:%02x" % (random.randint(0, 255),) >>>>> + src_port = random.randrange(600, 800) >>>>> + dst_port = random.randrange(800, 1000) >>>> >>>> MAC and ports are still random, they should be fixed also for >>>> replication purposes. >>>> >>> >>> Have a fixed mac and l4 ports in next version. >>> >>>>> + eth = Ether(src=mac_addr, dst=mac_addr) >>>>> + vlan = Dot1Q(vlan=random.randrange(1, 20)) >>>> >>>> Should be a fixed range, not random. >>>> >>> >>> Done something like that if this is what you expect: >>> vlan = Dot1Q(vlan=(i % 10)) >> >> Yes, something that assures, each time you run the script you get the same >> pcap. >> > > Will send the V4. > >>>>> + udp = UDP(dport=src_port, sport=dst_port) >>>>> + ipv4 = IP(src=RandIP()._fix(), dst=RandIP()._fix()) >>>>> + ipv6 = IPv6(src=RandIP6()._fix(), dst=RandIP6()._fix()) >>>> >>>> Why use _fix()? We should use fixed IP ranges, _fix to me sound like >>>> we are using a private function? >>>> You should build the src/dst using the i variable. >>> >>> Same as vlan: >>> >>> ipv4_addr = "192.168.150." + str((i % 255)) >>> ipv6_addr = "2001:0db8:85a3:0000:0000:8a2e:0370:" + str(i) >> >> Yes, this looks good, with the note that this will limit the IPv4 addresses >> to >> 255 unique ones. >> >> For IPv6 you also need to make sure you print hex with max 4 leading zero’s >> and do an (i % 0xffff). >> > > Thanks, included in V4. > >>>> >>>>> + tcp = TCP(dport=src_port, sport=dst_port, flags='S') >>>> >>>> Maybe a new line before the command, as it was before. >>>> >>>>> + # 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..6b7d73a68 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]) >>>>> + >>>>> +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,infi >>>>> +ni 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,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 bra while >>>>> +processing@d \@upcall_cb failure: ukey installation fails@d >>>> >>>> Still not convinced we need these additional messages. I remember >>>> there was an earlier discussion around these (I think with Ilya), can >>>> you point me to the conclusion on this? >>>> >>> The discussion was for the "MFEX-fuzzy " which experiences context >>> changes on CPU Mentioned below. >>> >>> Sure the discussion are here : >>> https://mail.openvswitch.org/pipermail/ovs-dev/2022-March/392374.html >>> >>> But I will remove these changes in fuzzy test for now as this patch >>> holds importance for us for testing IPv6 Patches and fuzzy topic can be >> delayed for now. >> >> I’m not talking about the fuzzy topic, but about the error messages: >> >> \@upcall: datapath reached the dynamic limit of .* flows.@d \@received >> packet on unknown port .* on bridge bra while processing@d \@upcall_cb >> failure: ukey installation fails@d >> >> And the ones below: >> >> \@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 >> >> Do we fully understand why they are generated? I can see some are due to >> limited CPU availability, and the number of entries: >> >> \@upcall: datapath reached the dynamic limit of .* flows.@d \@context >> switches: .* voluntary, .* involuntary@d \@Unreasonably long .* poll >> interval@d >> \@faults: .* minor, .* major@d >> >> But I do not understand the following (and I do not have a system to >> replicate this): >> >> \@received packet on unknown port .* on bridge bra while processing@d >> \@upcall_cb failure: ukey installation fails@d >> > > Yes I guess I should have explained about each warning in a detailed commit > message. > So to clarify all the warnings I will publish a separate patch and will put > up explanation for > Each of them in the commit message. > > Until then I will remove the following from this patch. Sound like a plan, as I know Ilya commented on them before, but I could not quickly find it. So it will be good to know why they are there. Would also be nice to confirm you also see these messages without AVX512 enabled doing the same tests. //Eelco _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
