Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 6/24/2023 4:18 AM, Ilya Maximets wrote: On 6/19/23 07:05, Chris Mi wrote: Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron --- Documentation/howto/tc-offload.rst | 24 + tests/system-common-macros.at | 13 +++ tests/system-offloads-traffic.at | 160 + 3 files changed, 197 insertions(+) diff --git a/Documentation/howto/tc-offload.rst b/Documentation/howto/tc-offload.rst index 681dff13e..a50dc3c58 100644 --- a/Documentation/howto/tc-offload.rst +++ b/Documentation/howto/tc-offload.rst @@ -88,6 +88,30 @@ As you can see above the none-offload case reports 140 bytes more, which is 14 bytes per packet. This represents the L2 header, in this case, 2 * *Ethernet address* + *Ethertype*. +Tunnel offload +++ + +Current tunnel offload ignores DF and CSUM flags configuration requested by +the user. TC for now has no way to pass these flags in a flower key and their +masks are set by default. To make tunnel offload work, DF and CSUM flags +are cleared. So please be aware of the following differences. + +Dumping vxlan decap match without offload, it shows:: + + recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,flags(-df+csum+key)),in_port(vxlan_sys_4789) + +Dumping vxlan decap match with offload, it shows:: + + recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,tp_dst=4789,flags(+key)),in_port(vxlan_sys_4789) + +Dumping vxlan encap action without offload, it shows:: + + actions:set(tunnel(tun_id=0x4,dst=192.168.1.1,ttl=64,tp_dst=4789,flags(df|key))),vxlan_sys_4789 + +Dumping vxlan encap action with offload, it shows:: + + actions:set(tunnel(tun_id=0x4,dst=192.168.1.64,ttl=64,tp_dst=4789,flags(key))),vxlan_sys_4789 + TC Meter Offload diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 0077a8609..55ec0bf1e 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -359,3 +359,16 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP], # OVS_CHECK_CT_CLEAR() m4_define([OVS_CHECK_CT_CLEAR], [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" ovs-vswitchd.log])]) + +# LOAD_MODULE([name]) +# +# Tries to load specified kernel module and removes it after +# if it wasn't loaded before this call. +# +m4_define([LOAD_MODULE], +[if ! lsmod | grep -q $1; then + on_exit 'modprobe -q -r $1' + fi + AT_CHECK([modprobe $1]) +] +) \ No newline at end of file diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index ae302a294..db287a86d 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -93,6 +93,166 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows : [[1-9]]"], [0], [i OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([offloads - sflow with sampling=1 - offloads enabled]) +LOAD_MODULE([psample]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +on_exit 'kill `cat test-sflow.pid`' +AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 0:127.0.0.1 > sflow.log], [0], [], [ignore]) Please, wrap long lines. At least move the error codes and output streams to a new line. Indented to the first argument, of course. It's also possible to break the argument with either 'dnl' or a line continuation '\' (new versions of atotest seems to handle them better). Done. +AT_CAPTURE_FILE([sflow.log]) +PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +AT_CHECK([ovs-vsctl -- --id=@sflow create sflow agent=lo target=\"127.0.0.1:$SFLOW_PORT\" header=128 sampling=1 polling=100 -- set bridge br0 sflow=@sflow], [0], [ignore]) Wrap. Done. +NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +1000 packets transmitted, 1000 received, 0% packet loss, time 0ms This check fails when I run tests with ASAN. It's unable to finish in 12 seconds: ./system-offloads-traffic.at:113: ip netns exec at_ns0 sh << NS_EXEC_HEREDOC ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | grep "transmitted" | sed 's/time.*ms$/time 0ms/' NS_EXEC_HEREDOC --- - 2023-06-23 18:53:18.127071711 + +++ /root/ovs/tests/system-offloads-testsuite.dir/at-groups/3/stdout 2023-06-23 18:53:18.12000 + @@ -1,2 +1,2 @@ -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +750 packets transmitted, 750 received, 0% packet loss, time 0ms Same for the other ping in these tests. Done. +]) + +p1_ifindex=$(cat
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 2/29/2024 9:04 PM, Ilya Maximets wrote: On 2/29/24 01:56, Chris Mi wrote: On 2/28/2024 8:51 PM, Ilya Maximets wrote: On 2/28/24 13:26, Chris Mi wrote: Hi Ilya, On 2/28/2024 7:56 PM, Ilya Maximets wrote: On 2/28/24 11:03, Eelco Chaudron wrote: On 28 Feb 2024, at 8:06, Chris Mi wrote: Hi Eelco, I did a rebase and pushed to github. Nothing changed, but the actions always reported the following failure: -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +744 packets transmitted, 744 received, 0% packet loss, time 0ms I can't reproduce the failure locally. Not sure if the test machine is a little weak. So how about change the test like this to make test robost: -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +200 packets transmitted, 200 received, 0% packet loss, time 0ms It looks like the machine might be too slow to handle the 1000 pings in 12 seconds. Scaling it down should not be a problem. Maybe back to 500, and see if this is enough? 500 is also not stable. Is it necessary to send hundreds of packets? Can we just send 3-10 like other tests do? 200 seems stable. But why 5 or 10 is not enough? If ratio is 1, 5 or 10 is enough. I can change it. If ratio is 2, we need send more packets. But maybe we only need to check the range of the sampled packets. For the ratio of 2 we could send 100 packets and check that the number is within 30-70 range. This gives us less than 0.01% failure rate. With 200 we'll need to check the 70-130 range, which isn't a significantly narrower range and a chance to successfully send 100 pings is much higher than with 200. So, maybe 100 is a good number to test ratio of 2 with? Also, please, use -W instead of -w in pings as -w doesn't stop ping from sending more packets than requested leading to random failures. OK, thanks for your suggestion. I'll try it. I see, there is an existing test that sends 1000 packets, but it probably should be canged as well. Not as part of this patch set, but in general. In case you missed my previous review for this patch where I pointed out the problem with the slow ping: https://patchwork.ozlabs.org/project/openvswitch/patch/20230619050557.310690-9-...@nvidia.com/#3136718 OK, I'll address other comments. It should be reproducible locally under ASAN.OK. Ping is not a reliable tool when we're trying to send a particular number of packets, especially with intervals under 100ms. OK, I see. Thanks, Chris Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 2/29/24 01:56, Chris Mi wrote: > On 2/28/2024 8:51 PM, Ilya Maximets wrote: >> On 2/28/24 13:26, Chris Mi wrote: >>> Hi Ilya, >>> >>> On 2/28/2024 7:56 PM, Ilya Maximets wrote: On 2/28/24 11:03, Eelco Chaudron wrote: > > > On 28 Feb 2024, at 8:06, Chris Mi wrote: > >> Hi Eelco, >> >> I did a rebase and pushed to github. Nothing changed, but the actions >> always reported the following failure: >> >> -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms >> +744 packets transmitted, 744 received, 0% packet loss, time 0ms >> >> I can't reproduce the failure locally. Not sure if the test machine >> is a little weak. So how about change the test like this to make test >> robost: >> >> -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | >> FORMAT_PING], [0], [dnl >> -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | >> FORMAT_PING], [0], [dnl >> +200 packets transmitted, 200 received, 0% packet loss, time 0ms > > It looks like the machine might be too slow to handle the 1000 pings in > 12 seconds. Scaling it down should not be a problem. Maybe back to 500, > and see if this is enough? >>> 500 is also not stable. Is it necessary to send hundreds of packets? Can we just send 3-10 like other tests do? >>> 200 seems stable. >> >> But why 5 or 10 is not enough? > If ratio is 1, 5 or 10 is enough. I can change it. > If ratio is 2, we need send more packets. But maybe we only need > to check the range of the sampled packets. For the ratio of 2 we could send 100 packets and check that the number is within 30-70 range. This gives us less than 0.01% failure rate. With 200 we'll need to check the 70-130 range, which isn't a significantly narrower range and a chance to successfully send 100 pings is much higher than with 200. So, maybe 100 is a good number to test ratio of 2 with? Also, please, use -W instead of -w in pings as -w doesn't stop ping from sending more packets than requested leading to random failures. >> I see, there is an existing test that sends 1000 packets, but it probably should be canged as well. Not as part of this patch set, but in general. In case you missed my previous review for this patch where I pointed out the problem with the slow ping: https://patchwork.ozlabs.org/project/openvswitch/patch/20230619050557.310690-9-...@nvidia.com/#3136718 >>> OK, I'll address other comments. It should be reproducible locally under ASAN.OK. Ping is not a reliable tool when we're trying to send a particular number of packets, especially with intervals under 100ms. >>> OK, I see. >>> >>> Thanks, >>> Chris Best regards, Ilya Maximets. >> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 2/28/2024 8:51 PM, Ilya Maximets wrote: On 2/28/24 13:26, Chris Mi wrote: Hi Ilya, On 2/28/2024 7:56 PM, Ilya Maximets wrote: On 2/28/24 11:03, Eelco Chaudron wrote: On 28 Feb 2024, at 8:06, Chris Mi wrote: Hi Eelco, I did a rebase and pushed to github. Nothing changed, but the actions always reported the following failure: -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +744 packets transmitted, 744 received, 0% packet loss, time 0ms I can't reproduce the failure locally. Not sure if the test machine is a little weak. So how about change the test like this to make test robost: -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +200 packets transmitted, 200 received, 0% packet loss, time 0ms It looks like the machine might be too slow to handle the 1000 pings in 12 seconds. Scaling it down should not be a problem. Maybe back to 500, and see if this is enough? 500 is also not stable. Is it necessary to send hundreds of packets? Can we just send 3-10 like other tests do? 200 seems stable. But why 5 or 10 is not enough? If ratio is 1, 5 or 10 is enough. I can change it. If ratio is 2, we need send more packets. But maybe we only need to check the range of the sampled packets. I see, there is an existing test that sends 1000 packets, but it probably should be canged as well. Not as part of this patch set, but in general. In case you missed my previous review for this patch where I pointed out the problem with the slow ping: https://patchwork.ozlabs.org/project/openvswitch/patch/20230619050557.310690-9-...@nvidia.com/#3136718 OK, I'll address other comments. It should be reproducible locally under ASAN.OK. Ping is not a reliable tool when we're trying to send a particular number of packets, especially with intervals under 100ms. OK, I see. Thanks, Chris Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 2/28/24 13:26, Chris Mi wrote: > Hi Ilya, > > On 2/28/2024 7:56 PM, Ilya Maximets wrote: >> On 2/28/24 11:03, Eelco Chaudron wrote: >>> >>> >>> On 28 Feb 2024, at 8:06, Chris Mi wrote: >>> Hi Eelco, I did a rebase and pushed to github. Nothing changed, but the actions always reported the following failure: -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +744 packets transmitted, 744 received, 0% packet loss, time 0ms I can't reproduce the failure locally. Not sure if the test machine is a little weak. So how about change the test like this to make test robost: -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +200 packets transmitted, 200 received, 0% packet loss, time 0ms >>> >>> It looks like the machine might be too slow to handle the 1000 pings in 12 >>> seconds. Scaling it down should not be a problem. Maybe back to 500, and >>> see if this is enough? > 500 is also not stable. >> >> Is it necessary to send hundreds of packets? Can we just send 3-10 like >> other tests do? > 200 seems stable. But why 5 or 10 is not enough? >> >> I see, there is an existing test that sends 1000 packets, but it probably >> should be canged as well. Not as part of this patch set, but in general. >> >> In case you missed my previous review for this patch where I pointed out >> the problem with the slow ping: >> >> https://patchwork.ozlabs.org/project/openvswitch/patch/20230619050557.310690-9-...@nvidia.com/#3136718 > OK, I'll address other comments. >> It should be reproducible locally under ASAN.OK. >> >> Ping is not a reliable tool when we're trying to send a particular number >> of packets, especially with intervals under 100ms. > OK, I see. > > Thanks, > Chris >> >> Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
Hi Ilya, On 2/28/2024 7:56 PM, Ilya Maximets wrote: On 2/28/24 11:03, Eelco Chaudron wrote: On 28 Feb 2024, at 8:06, Chris Mi wrote: Hi Eelco, I did a rebase and pushed to github. Nothing changed, but the actions always reported the following failure: -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +744 packets transmitted, 744 received, 0% packet loss, time 0ms I can't reproduce the failure locally. Not sure if the test machine is a little weak. So how about change the test like this to make test robost: -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +200 packets transmitted, 200 received, 0% packet loss, time 0ms It looks like the machine might be too slow to handle the 1000 pings in 12 seconds. Scaling it down should not be a problem. Maybe back to 500, and see if this is enough? 500 is also not stable. Is it necessary to send hundreds of packets? Can we just send 3-10 like other tests do? 200 seems stable. I see, there is an existing test that sends 1000 packets, but it probably should be canged as well. Not as part of this patch set, but in general. In case you missed my previous review for this patch where I pointed out the problem with the slow ping: https://patchwork.ozlabs.org/project/openvswitch/patch/20230619050557.310690-9-...@nvidia.com/#3136718 OK, I'll address other comments. It should be reproducible locally under ASAN.OK. Ping is not a reliable tool when we're trying to send a particular number of packets, especially with intervals under 100ms. OK, I see. Thanks, Chris Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 2/28/24 11:03, Eelco Chaudron wrote: > > > On 28 Feb 2024, at 8:06, Chris Mi wrote: > >> Hi Eelco, >> >> I did a rebase and pushed to github. Nothing changed, but the actions always >> reported the following failure: >> >> -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms >> +744 packets transmitted, 744 received, 0% packet loss, time 0ms >> >> I can't reproduce the failure locally. Not sure if the test machine >> is a little weak. So how about change the test like this to make test robost: >> >> -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | >> FORMAT_PING], [0], [dnl >> -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | >> FORMAT_PING], [0], [dnl >> +200 packets transmitted, 200 received, 0% packet loss, time 0ms > > It looks like the machine might be too slow to handle the 1000 pings in 12 > seconds. Scaling it down should not be a problem. Maybe back to 500, and see > if this is enough? Is it necessary to send hundreds of packets? Can we just send 3-10 like other tests do? I see, there is an existing test that sends 1000 packets, but it probably should be canged as well. Not as part of this patch set, but in general. In case you missed my previous review for this patch where I pointed out the problem with the slow ping: https://patchwork.ozlabs.org/project/openvswitch/patch/20230619050557.310690-9-...@nvidia.com/#3136718 It should be reproducible locally under ASAN. Ping is not a reliable tool when we're trying to send a particular number of packets, especially with intervals under 100ms. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 28 Feb 2024, at 8:06, Chris Mi wrote: > Hi Eelco, > > I did a rebase and pushed to github. Nothing changed, but the actions always > reported the following failure: > > -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms > +744 packets transmitted, 744 received, 0% packet loss, time 0ms > > I can't reproduce the failure locally. Not sure if the test machine > is a little weak. So how about change the test like this to make test robost: > > -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | > FORMAT_PING], [0], [dnl > -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms > +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | > FORMAT_PING], [0], [dnl > +200 packets transmitted, 200 received, 0% packet loss, time 0ms It looks like the machine might be too slow to handle the 1000 pings in 12 seconds. Scaling it down should not be a problem. Maybe back to 500, and see if this is enough? //Eelco > Thanks, > Chris > > On 6/19/2023 1:05 PM, Chris Mi wrote: >> Add three sFlow offload test cases: >> >>3: offloads - sflow with sampling=1 - offloads enabled ok >>4: offloads - sflow with sampling=2 - offloads enabled ok >>5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok >> >> Signed-off-by: Chris Mi >> Reviewed-by: Roi Dayan >> Acked-by: Eelco Chaudron >> --- >> Documentation/howto/tc-offload.rst | 24 + >> tests/system-common-macros.at | 13 +++ >> tests/system-offloads-traffic.at | 160 + >> 3 files changed, 197 insertions(+) >> >> diff --git a/Documentation/howto/tc-offload.rst >> b/Documentation/howto/tc-offload.rst >> index 681dff13e..a50dc3c58 100644 >> --- a/Documentation/howto/tc-offload.rst >> +++ b/Documentation/howto/tc-offload.rst >> @@ -88,6 +88,30 @@ As you can see above the none-offload case reports 140 >> bytes more, which is 14 >> bytes per packet. This represents the L2 header, in this case, 2 * >> *Ethernet >> address* + *Ethertype*. >> +Tunnel offload >> +++ >> + >> +Current tunnel offload ignores DF and CSUM flags configuration requested by >> +the user. TC for now has no way to pass these flags in a flower key and >> their >> +masks are set by default. To make tunnel offload work, DF and CSUM flags >> +are cleared. So please be aware of the following differences. >> + >> +Dumping vxlan decap match without offload, it shows:: >> + >> + >> recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,flags(-df+csum+key)),in_port(vxlan_sys_4789) >> + >> +Dumping vxlan decap match with offload, it shows:: >> + >> + >> recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,tp_dst=4789,flags(+key)),in_port(vxlan_sys_4789) >> + >> +Dumping vxlan encap action without offload, it shows:: >> + >> + >> actions:set(tunnel(tun_id=0x4,dst=192.168.1.1,ttl=64,tp_dst=4789,flags(df|key))),vxlan_sys_4789 >> + >> +Dumping vxlan encap action with offload, it shows:: >> + >> + >> actions:set(tunnel(tun_id=0x4,dst=192.168.1.64,ttl=64,tp_dst=4789,flags(key))),vxlan_sys_4789 >> + >> TC Meter Offload >> >> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >> index 0077a8609..55ec0bf1e 100644 >> --- a/tests/system-common-macros.at >> +++ b/tests/system-common-macros.at >> @@ -359,3 +359,16 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP], >> # OVS_CHECK_CT_CLEAR() >> m4_define([OVS_CHECK_CT_CLEAR], >> [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" >> ovs-vswitchd.log])]) >> + >> +# LOAD_MODULE([name]) >> +# >> +# Tries to load specified kernel module and removes it after >> +# if it wasn't loaded before this call. >> +# >> +m4_define([LOAD_MODULE], >> +[if ! lsmod | grep -q $1; then >> + on_exit 'modprobe -q -r $1' >> + fi >> + AT_CHECK([modprobe $1]) >> +] >> +) >> \ No newline at end of file >> diff --git a/tests/system-offloads-traffic.at >> b/tests/system-offloads-traffic.at >> index ae302a294..db287a86d 100644 >> --- a/tests/system-offloads-traffic.at >> +++ b/tests/system-offloads-traffic.at >> @@ -93,6 +93,166 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded >> flows : [[1-9]]"], [0], [i >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> + >> +AT_SETUP([offloads - sflow with sampling=1 - offloads enabled]) >> +LOAD_MODULE([psample]) >> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . >> other_config:hw-offload=true]) >> + >> +on_exit 'kill `cat test-sflow.pid`' >> +AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile >> 0:127.0.0.1 > sflow.log], [0], [], [ignore]) >> +AT_CAPTURE_FILE([sflow.log]) >> +PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT]) >> + >> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) >> + >> +ADD_NAMESPACES(at_ns0, at_ns1) >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") >> + >> +AT_CHECK([ovs-vsctl -- --id=@sflow create sflow
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 28 Feb 2024, at 4:38, Chris Mi wrote: > On 2/28/2024 9:20 AM, Chris Mi via dev wrote: >> On 2/27/2024 6:54 PM, Eelco Chaudron wrote: >>> >>> >>> On 9 Jan 2024, at 11:40, Eelco Chaudron wrote: >>> On 9 Jan 2024, at 10:22, Chris Mi wrote: > Hi Roi and Eelco, > > Sorry for the late reply. We are still busy with other task. So maybe > we'll have to postpone it. Thanks for the update and I guess we are too late for the upcoming release unless you can submit it this week. >>> >>> Now that 3.3 has been released, do you have an idea when you can submit a >>> new revision? This so we have time to review before the next release? >> I'm working on it now. Hopefully I can submit a new revision this week. >> But I can't reproduce the memory leak Ilya found. Not sure if I miss >> anything. Do you have any idea? >> >> Anyway, I found something wrong. Maybe we should introduce >> offload_sample_uninit() to free sample->userspace_actions if something wrong >> happens. > I attached a possible fix, could you please review it? Hi Chris, I do not have the full context of these series clear anymore, but the fix seems to fix a potential memory leak. I guess Ilya can reply, or we can follow this up in the next revision. //Eelco >> Thanks, >> Chris >> >>> //Eelco FYI, We got an inquiry from Jim Michelson (Nvidia) directly about the status. > Regards, > Chris > > On 1/9/2024 4:52 PM, Roi Dayan wrote: >> >> On 02/11/2023 15:24, Eelco Chaudron wrote: >>> >>> On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: >>> On 19 Jun 2023, at 12:18, Chris Mi wrote: > On 6/19/2023 6:04 PM, Eelco Chaudron wrote: >> On 19 Jun 2023, at 7:05, Chris Mi wrote: >> >>> Add three sFlow offload test cases: >>> >>> 3: offloads - sflow with sampling=1 - offloads enabled ok >>> 4: offloads - sflow with sampling=2 - offloads enabled ok >>> 5: offloads - ping over vxlan tunnel with sflow - offloads >>> enabled ok >>> >>> Signed-off-by: Chris Mi >>> Reviewed-by: Roi Dayan >>> Acked-by: Eelco Chaudron >> Thanks for making all the suggested changes to this series. This is >> my final ack, which should conclude the series :) >> >> Acked-by: Eelco Chaudron >> >> //Eelco >> > Sorry for missing your Acked-by for previous patches :( > And thanks a lot for acking this patchset. That's so great :) Hi Chris, Any update on this patchset? It has some open questions/comments from Ilya, and it would be nice to get the patch in before the next release. Cheers, Eelco >>> Hi Roi, >>> >>> As Chris does not seem to reply, do you have any update on this? >>> >>> Thanks, >>> >>> Eelco >>> >> Hi Eelco, >> I don't have an update on this patchset. I'll try to ping Chris about it. >> >> Thanks, >> Roi >> >>> >> ___ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
Hi Eelco, I did a rebase and pushed to github. Nothing changed, but the actions always reported the following failure: -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +744 packets transmitted, 744 received, 0% packet loss, time 0ms I can't reproduce the failure locally. Not sure if the test machine is a little weak. So how about change the test like this to make test robost: -NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +NS_CHECK_EXEC([at_ns0], [ping -q -c 200 -i 0.05 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +200 packets transmitted, 200 received, 0% packet loss, time 0ms Thanks, Chris On 6/19/2023 1:05 PM, Chris Mi wrote: Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron --- Documentation/howto/tc-offload.rst | 24 + tests/system-common-macros.at | 13 +++ tests/system-offloads-traffic.at | 160 + 3 files changed, 197 insertions(+) diff --git a/Documentation/howto/tc-offload.rst b/Documentation/howto/tc-offload.rst index 681dff13e..a50dc3c58 100644 --- a/Documentation/howto/tc-offload.rst +++ b/Documentation/howto/tc-offload.rst @@ -88,6 +88,30 @@ As you can see above the none-offload case reports 140 bytes more, which is 14 bytes per packet. This represents the L2 header, in this case, 2 * *Ethernet address* + *Ethertype*. +Tunnel offload +++ + +Current tunnel offload ignores DF and CSUM flags configuration requested by +the user. TC for now has no way to pass these flags in a flower key and their +masks are set by default. To make tunnel offload work, DF and CSUM flags +are cleared. So please be aware of the following differences. + +Dumping vxlan decap match without offload, it shows:: + + recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,flags(-df+csum+key)),in_port(vxlan_sys_4789) + +Dumping vxlan decap match with offload, it shows:: + + recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,tp_dst=4789,flags(+key)),in_port(vxlan_sys_4789) + +Dumping vxlan encap action without offload, it shows:: + + actions:set(tunnel(tun_id=0x4,dst=192.168.1.1,ttl=64,tp_dst=4789,flags(df|key))),vxlan_sys_4789 + +Dumping vxlan encap action with offload, it shows:: + + actions:set(tunnel(tun_id=0x4,dst=192.168.1.64,ttl=64,tp_dst=4789,flags(key))),vxlan_sys_4789 + TC Meter Offload diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 0077a8609..55ec0bf1e 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -359,3 +359,16 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP], # OVS_CHECK_CT_CLEAR() m4_define([OVS_CHECK_CT_CLEAR], [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" ovs-vswitchd.log])]) + +# LOAD_MODULE([name]) +# +# Tries to load specified kernel module and removes it after +# if it wasn't loaded before this call. +# +m4_define([LOAD_MODULE], +[if ! lsmod | grep -q $1; then + on_exit 'modprobe -q -r $1' + fi + AT_CHECK([modprobe $1]) +] +) \ No newline at end of file diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index ae302a294..db287a86d 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -93,6 +93,166 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows : [[1-9]]"], [0], [i OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([offloads - sflow with sampling=1 - offloads enabled]) +LOAD_MODULE([psample]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +on_exit 'kill `cat test-sflow.pid`' +AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 0:127.0.0.1 > sflow.log], [0], [], [ignore]) +AT_CAPTURE_FILE([sflow.log]) +PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +AT_CHECK([ovs-vsctl -- --id=@sflow create sflow agent=lo target=\"127.0.0.1:$SFLOW_PORT\" header=128 sampling=1 polling=100 -- set bridge br0 sflow=@sflow], [0], [ignore]) +NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +]) + +p1_ifindex=$(cat /sys/class/net/ovs-p1/ifindex) +m4_define([DUMP_SFLOW], [sed -e "s/used:[[0-9]].[[0-9]]*s/used:0.001s/;s/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/pid=[[0-9]]*/pid=1/;s/output=$p1_ifindex/output=1/"]) +AT_CHECK([ovs-appctl dpctl/dump-flows
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 2/28/2024 9:20 AM, Chris Mi via dev wrote: On 2/27/2024 6:54 PM, Eelco Chaudron wrote: On 9 Jan 2024, at 11:40, Eelco Chaudron wrote: On 9 Jan 2024, at 10:22, Chris Mi wrote: Hi Roi and Eelco, Sorry for the late reply. We are still busy with other task. So maybe we'll have to postpone it. Thanks for the update and I guess we are too late for the upcoming release unless you can submit it this week. Now that 3.3 has been released, do you have an idea when you can submit a new revision? This so we have time to review before the next release? I'm working on it now. Hopefully I can submit a new revision this week. But I can't reproduce the memory leak Ilya found. Not sure if I miss anything. Do you have any idea? Anyway, I found something wrong. Maybe we should introduce offload_sample_uninit() to free sample->userspace_actions if something wrong happens. I attached a possible fix, could you please review it? Thanks, Chris Thanks, Chris //Eelco FYI, We got an inquiry from Jim Michelson (Nvidia) directly about the status. Regards, Chris On 1/9/2024 4:52 PM, Roi Dayan wrote: On 02/11/2023 15:24, Eelco Chaudron wrote: On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: On 19 Jun 2023, at 12:18, Chris Mi wrote: On 6/19/2023 6:04 PM, Eelco Chaudron wrote: On 19 Jun 2023, at 7:05, Chris Mi wrote: Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco Sorry for missing your Acked-by for previous patches :( And thanks a lot for acking this patchset. That's so great :) Hi Chris, Any update on this patchset? It has some open questions/comments from Ilya, and it would be nice to get the patch in before the next release. Cheers, Eelco Hi Roi, As Chris does not seem to reply, do you have any update on this? Thanks, Eelco Hi Eelco, I don't have an update on this patchset. I'll try to ping Chris about it. Thanks, Roi ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-devdiff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 2c86e8102..61374ced5 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -2197,6 +2197,12 @@ parse_sample_actions_attribute(const struct nlattr *actions, return err; } +static void +offload_sample_uninit(struct offload_sample *sample) +{ +free(sample->userspace_actions); +} + static void offload_sample_init(struct offload_sample *sample, const struct nlattr *next_actions, @@ -2239,13 +2245,15 @@ parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action, } else if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_PROBABILITY) { rate = UINT32_MAX / nl_attr_get_u32(nla); } else { -return EINVAL; +ret = EINVAL; +goto err_out; } } /* This check makes sure both attributes above were present and valid. */ if (!rate || ret) { -return EINVAL; +ret = EINVAL; +goto err_out; } sgid = sgid_alloc(); @@ -2260,6 +2268,10 @@ parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action, flower->action_count++; return 0; + +err_out: +offload_sample_uninit(); +return ret; } static int @@ -2281,6 +2293,7 @@ parse_userspace_action(struct tc_flower *flower, struct tc_action *tc_action, * only offload userspace actions for sFlow. */ err = parse_userspace_attributes(userspace_action, ); if (err) { +offload_sample_uninit(); return err; } sample.action = (struct nlattr *) userspace_action; ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 2/27/2024 6:54 PM, Eelco Chaudron wrote: On 9 Jan 2024, at 11:40, Eelco Chaudron wrote: On 9 Jan 2024, at 10:22, Chris Mi wrote: Hi Roi and Eelco, Sorry for the late reply. We are still busy with other task. So maybe we'll have to postpone it. Thanks for the update and I guess we are too late for the upcoming release unless you can submit it this week. Now that 3.3 has been released, do you have an idea when you can submit a new revision? This so we have time to review before the next release? I'm working on it now. Hopefully I can submit a new revision this week. But I can't reproduce the memory leak Ilya found. Not sure if I miss anything. Do you have any idea? Anyway, I found something wrong. Maybe we should introduce offload_sample_uninit() to free sample->userspace_actions if something wrong happens. Thanks, Chris //Eelco FYI, We got an inquiry from Jim Michelson (Nvidia) directly about the status. Regards, Chris On 1/9/2024 4:52 PM, Roi Dayan wrote: On 02/11/2023 15:24, Eelco Chaudron wrote: On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: On 19 Jun 2023, at 12:18, Chris Mi wrote: On 6/19/2023 6:04 PM, Eelco Chaudron wrote: On 19 Jun 2023, at 7:05, Chris Mi wrote: Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco Sorry for missing your Acked-by for previous patches :( And thanks a lot for acking this patchset. That's so great :) Hi Chris, Any update on this patchset? It has some open questions/comments from Ilya, and it would be nice to get the patch in before the next release. Cheers, Eelco Hi Roi, As Chris does not seem to reply, do you have any update on this? Thanks, Eelco Hi Eelco, I don't have an update on this patchset. I'll try to ping Chris about it. Thanks, Roi ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 9 Jan 2024, at 11:40, Eelco Chaudron wrote: > On 9 Jan 2024, at 10:22, Chris Mi wrote: > >> Hi Roi and Eelco, >> >> Sorry for the late reply. We are still busy with other task. So maybe we'll >> have to postpone it. > > Thanks for the update and I guess we are too late for the upcoming release > unless you can submit it this week. Now that 3.3 has been released, do you have an idea when you can submit a new revision? This so we have time to review before the next release? > //Eelco > > FYI, We got an inquiry from Jim Michelson (Nvidia) directly about the status. > >> Regards, >> Chris >> >> On 1/9/2024 4:52 PM, Roi Dayan wrote: >>> >>> On 02/11/2023 15:24, Eelco Chaudron wrote: On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: > On 19 Jun 2023, at 12:18, Chris Mi wrote: > >> On 6/19/2023 6:04 PM, Eelco Chaudron wrote: >>> On 19 Jun 2023, at 7:05, Chris Mi wrote: >>> Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron >>> Thanks for making all the suggested changes to this series. This is my >>> final ack, which should conclude the series :) >>> >>> Acked-by: Eelco Chaudron >>> >>> //Eelco >>> >> Sorry for missing your Acked-by for previous patches :( >> And thanks a lot for acking this patchset. That's so great :) > Hi Chris, > > Any update on this patchset? It has some open questions/comments from > Ilya, and it would be nice to get the patch in before the next release. > > Cheers, > > Eelco Hi Roi, As Chris does not seem to reply, do you have any update on this? Thanks, Eelco >>> Hi Eelco, >>> I don't have an update on this patchset. I'll try to ping Chris about it. >>> >>> Thanks, >>> Roi >>> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 9 Jan 2024, at 10:22, Chris Mi wrote: > Hi Roi and Eelco, > > Sorry for the late reply. We are still busy with other task. So maybe we'll > have to postpone it. Thanks for the update and I guess we are too late for the upcoming release unless you can submit it this week. //Eelco FYI, We got an inquiry from Jim Michelson (Nvidia) directly about the status. > Regards, > Chris > > On 1/9/2024 4:52 PM, Roi Dayan wrote: >> >> On 02/11/2023 15:24, Eelco Chaudron wrote: >>> >>> On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: >>> On 19 Jun 2023, at 12:18, Chris Mi wrote: > On 6/19/2023 6:04 PM, Eelco Chaudron wrote: >> On 19 Jun 2023, at 7:05, Chris Mi wrote: >> >>> Add three sFlow offload test cases: >>> >>> 3: offloads - sflow with sampling=1 - offloads enabled ok >>> 4: offloads - sflow with sampling=2 - offloads enabled ok >>> 5: offloads - ping over vxlan tunnel with sflow - offloads enabled >>> ok >>> >>> Signed-off-by: Chris Mi >>> Reviewed-by: Roi Dayan >>> Acked-by: Eelco Chaudron >> Thanks for making all the suggested changes to this series. This is my >> final ack, which should conclude the series :) >> >> Acked-by: Eelco Chaudron >> >> //Eelco >> > Sorry for missing your Acked-by for previous patches :( > And thanks a lot for acking this patchset. That's so great :) Hi Chris, Any update on this patchset? It has some open questions/comments from Ilya, and it would be nice to get the patch in before the next release. Cheers, Eelco >>> Hi Roi, >>> >>> As Chris does not seem to reply, do you have any update on this? >>> >>> Thanks, >>> >>> Eelco >>> >> Hi Eelco, >> I don't have an update on this patchset. I'll try to ping Chris about it. >> >> Thanks, >> Roi >> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
Hi Roi and Eelco, Sorry for the late reply. We are still busy with other task. So maybe we'll have to postpone it. Regards, Chris On 1/9/2024 4:52 PM, Roi Dayan wrote: On 02/11/2023 15:24, Eelco Chaudron wrote: On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: On 19 Jun 2023, at 12:18, Chris Mi wrote: On 6/19/2023 6:04 PM, Eelco Chaudron wrote: On 19 Jun 2023, at 7:05, Chris Mi wrote: Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco Sorry for missing your Acked-by for previous patches :( And thanks a lot for acking this patchset. That's so great :) Hi Chris, Any update on this patchset? It has some open questions/comments from Ilya, and it would be nice to get the patch in before the next release. Cheers, Eelco Hi Roi, As Chris does not seem to reply, do you have any update on this? Thanks, Eelco Hi Eelco, I don't have an update on this patchset. I'll try to ping Chris about it. Thanks, Roi ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 02/11/2023 15:24, Eelco Chaudron wrote: > > > On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: > >> On 19 Jun 2023, at 12:18, Chris Mi wrote: >> >>> On 6/19/2023 6:04 PM, Eelco Chaudron wrote: On 19 Jun 2023, at 7:05, Chris Mi wrote: > Add three sFlow offload test cases: > >3: offloads - sflow with sampling=1 - offloads enabled ok >4: offloads - sflow with sampling=2 - offloads enabled ok >5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan > Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco >>> Sorry for missing your Acked-by for previous patches :( >>> And thanks a lot for acking this patchset. That's so great :) >> >> Hi Chris, >> >> Any update on this patchset? It has some open questions/comments from Ilya, >> and it would be nice to get the patch in before the next release. >> >> Cheers, >> >> Eelco > > Hi Roi, > > As Chris does not seem to reply, do you have any update on this? > > Thanks, > > Eelco > Hi Eelco, I don't have an update on this patchset. I'll try to ping Chris about it. Thanks, Roi ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 3 Oct 2023, at 17:03, Eelco Chaudron wrote: > On 19 Jun 2023, at 12:18, Chris Mi wrote: > >> On 6/19/2023 6:04 PM, Eelco Chaudron wrote: >>> >>> On 19 Jun 2023, at 7:05, Chris Mi wrote: >>> Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron >>> >>> Thanks for making all the suggested changes to this series. This is my >>> final ack, which should conclude the series :) >>> >>> Acked-by: Eelco Chaudron >>> >>> //Eelco >>> >> Sorry for missing your Acked-by for previous patches :( >> And thanks a lot for acking this patchset. That's so great :) > > Hi Chris, > > Any update on this patchset? It has some open questions/comments from Ilya, > and it would be nice to get the patch in before the next release. > > Cheers, > > Eelco Hi Roi, As Chris does not seem to reply, do you have any update on this? Thanks, Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 19 Jun 2023, at 12:18, Chris Mi wrote: > On 6/19/2023 6:04 PM, Eelco Chaudron wrote: >> >> On 19 Jun 2023, at 7:05, Chris Mi wrote: >> >>> Add three sFlow offload test cases: >>> >>>3: offloads - sflow with sampling=1 - offloads enabled ok >>>4: offloads - sflow with sampling=2 - offloads enabled ok >>>5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok >>> >>> Signed-off-by: Chris Mi >>> Reviewed-by: Roi Dayan >>> Acked-by: Eelco Chaudron >> >> Thanks for making all the suggested changes to this series. This is my final >> ack, which should conclude the series :) >> >> Acked-by: Eelco Chaudron >> >> //Eelco >> > Sorry for missing your Acked-by for previous patches :( > And thanks a lot for acking this patchset. That's so great :) Hi Chris, Any update on this patchset? It has some open questions/comments from Ilya, and it would be nice to get the patch in before the next release. Cheers, Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 6/19/23 07:05, Chris Mi wrote: > Add three sFlow offload test cases: > > 3: offloads - sflow with sampling=1 - offloads enabled ok > 4: offloads - sflow with sampling=2 - offloads enabled ok > 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan > Acked-by: Eelco Chaudron > --- > Documentation/howto/tc-offload.rst | 24 + > tests/system-common-macros.at | 13 +++ > tests/system-offloads-traffic.at | 160 + > 3 files changed, 197 insertions(+) > > diff --git a/Documentation/howto/tc-offload.rst > b/Documentation/howto/tc-offload.rst > index 681dff13e..a50dc3c58 100644 > --- a/Documentation/howto/tc-offload.rst > +++ b/Documentation/howto/tc-offload.rst > @@ -88,6 +88,30 @@ As you can see above the none-offload case reports 140 > bytes more, which is 14 > bytes per packet. This represents the L2 header, in this case, 2 * *Ethernet > address* + *Ethertype*. > > +Tunnel offload > +++ > + > +Current tunnel offload ignores DF and CSUM flags configuration requested by > +the user. TC for now has no way to pass these flags in a flower key and their > +masks are set by default. To make tunnel offload work, DF and CSUM flags > +are cleared. So please be aware of the following differences. > + > +Dumping vxlan decap match without offload, it shows:: > + > + > recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,flags(-df+csum+key)),in_port(vxlan_sys_4789) > + > +Dumping vxlan decap match with offload, it shows:: > + > + > recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,tp_dst=4789,flags(+key)),in_port(vxlan_sys_4789) > + > +Dumping vxlan encap action without offload, it shows:: > + > + > actions:set(tunnel(tun_id=0x4,dst=192.168.1.1,ttl=64,tp_dst=4789,flags(df|key))),vxlan_sys_4789 > + > +Dumping vxlan encap action with offload, it shows:: > + > + > actions:set(tunnel(tun_id=0x4,dst=192.168.1.64,ttl=64,tp_dst=4789,flags(key))),vxlan_sys_4789 > + > TC Meter Offload > > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 0077a8609..55ec0bf1e 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -359,3 +359,16 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP], > # OVS_CHECK_CT_CLEAR() > m4_define([OVS_CHECK_CT_CLEAR], > [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" > ovs-vswitchd.log])]) > + > +# LOAD_MODULE([name]) > +# > +# Tries to load specified kernel module and removes it after > +# if it wasn't loaded before this call. > +# > +m4_define([LOAD_MODULE], > +[if ! lsmod | grep -q $1; then > + on_exit 'modprobe -q -r $1' > + fi > + AT_CHECK([modprobe $1]) > +] > +) > \ No newline at end of file > diff --git a/tests/system-offloads-traffic.at > b/tests/system-offloads-traffic.at > index ae302a294..db287a86d 100644 > --- a/tests/system-offloads-traffic.at > +++ b/tests/system-offloads-traffic.at > @@ -93,6 +93,166 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded > flows : [[1-9]]"], [0], [i > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > + > +AT_SETUP([offloads - sflow with sampling=1 - offloads enabled]) > +LOAD_MODULE([psample]) > +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . > other_config:hw-offload=true]) > + > +on_exit 'kill `cat test-sflow.pid`' > +AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile > 0:127.0.0.1 > sflow.log], [0], [], [ignore]) Please, wrap long lines. At least move the error codes and output streams to a new line. Indented to the first argument, of course. It's also possible to break the argument with either 'dnl' or a line continuation '\' (new versions of atotest seems to handle them better). > +AT_CAPTURE_FILE([sflow.log]) > +PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT]) > + > +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) > + > +ADD_NAMESPACES(at_ns0, at_ns1) > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > + > +AT_CHECK([ovs-vsctl -- --id=@sflow create sflow agent=lo > target=\"127.0.0.1:$SFLOW_PORT\" header=128 sampling=1 polling=100 -- set > bridge br0 sflow=@sflow], [0], [ignore]) Wrap. > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | > FORMAT_PING], [0], [dnl > +1000 packets transmitted, 1000 received, 0% packet loss, time 0ms This check fails when I run tests with ASAN. It's unable to finish in 12 seconds: ./system-offloads-traffic.at:113: ip netns exec at_ns0 sh << NS_EXEC_HEREDOC ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | grep "transmitted" | sed 's/time.*ms$/time 0ms/' NS_EXEC_HEREDOC --- - 2023-06-23 18:53:18.127071711 + +++ /root/ovs/tests/system-offloads-testsuite.dir/at-groups/3/stdout 2023-06-23 18:53:18.12000 + @@ -1,2 +1,2 @@ -1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +750 packets transmitted, 750
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 6/19/2023 6:04 PM, Eelco Chaudron wrote: On 19 Jun 2023, at 7:05, Chris Mi wrote: Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco Sorry for missing your Acked-by for previous patches :( And thanks a lot for acking this patchset. That's so great :) Thanks, Chris ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 19 Jun 2023, at 7:05, Chris Mi wrote: > Add three sFlow offload test cases: > > 3: offloads - sflow with sampling=1 - offloads enabled ok > 4: offloads - sflow with sampling=2 - offloads enabled ok > 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan > Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
Bleep bloop. Greetings Chris Mi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 111 characters long (recommended limit is 79) #39 FILE: Documentation/howto/tc-offload.rst:101: recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,flags(-df+csum+key)),in_port(vxlan_sys_4789) WARNING: Line is 115 characters long (recommended limit is 79) #43 FILE: Documentation/howto/tc-offload.rst:105: recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,tp_dst=4789,flags(+key)),in_port(vxlan_sys_4789) WARNING: Line is 99 characters long (recommended limit is 79) #47 FILE: Documentation/howto/tc-offload.rst:109: actions:set(tunnel(tun_id=0x4,dst=192.168.1.1,ttl=64,tp_dst=4789,flags(df|key))),vxlan_sys_4789 WARNING: Line is 97 characters long (recommended limit is 79) #51 FILE: Documentation/howto/tc-offload.rst:113: actions:set(tunnel(tun_id=0x4,dst=192.168.1.64,ttl=64,tp_dst=4789,flags(key))),vxlan_sys_4789 Lines checked: 252, Warnings: 4, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron --- Documentation/howto/tc-offload.rst | 24 + tests/system-common-macros.at | 13 +++ tests/system-offloads-traffic.at | 160 + 3 files changed, 197 insertions(+) diff --git a/Documentation/howto/tc-offload.rst b/Documentation/howto/tc-offload.rst index 681dff13e..a50dc3c58 100644 --- a/Documentation/howto/tc-offload.rst +++ b/Documentation/howto/tc-offload.rst @@ -88,6 +88,30 @@ As you can see above the none-offload case reports 140 bytes more, which is 14 bytes per packet. This represents the L2 header, in this case, 2 * *Ethernet address* + *Ethertype*. +Tunnel offload +++ + +Current tunnel offload ignores DF and CSUM flags configuration requested by +the user. TC for now has no way to pass these flags in a flower key and their +masks are set by default. To make tunnel offload work, DF and CSUM flags +are cleared. So please be aware of the following differences. + +Dumping vxlan decap match without offload, it shows:: + + recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,flags(-df+csum+key)),in_port(vxlan_sys_4789) + +Dumping vxlan decap match with offload, it shows:: + + recirc_id(0),tunnel(tun_id=0x4,src=192.168.1.1,dst=192.168.1.2,tp_dst=4789,flags(+key)),in_port(vxlan_sys_4789) + +Dumping vxlan encap action without offload, it shows:: + + actions:set(tunnel(tun_id=0x4,dst=192.168.1.1,ttl=64,tp_dst=4789,flags(df|key))),vxlan_sys_4789 + +Dumping vxlan encap action with offload, it shows:: + + actions:set(tunnel(tun_id=0x4,dst=192.168.1.64,ttl=64,tp_dst=4789,flags(key))),vxlan_sys_4789 + TC Meter Offload diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 0077a8609..55ec0bf1e 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -359,3 +359,16 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP], # OVS_CHECK_CT_CLEAR() m4_define([OVS_CHECK_CT_CLEAR], [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" ovs-vswitchd.log])]) + +# LOAD_MODULE([name]) +# +# Tries to load specified kernel module and removes it after +# if it wasn't loaded before this call. +# +m4_define([LOAD_MODULE], +[if ! lsmod | grep -q $1; then + on_exit 'modprobe -q -r $1' + fi + AT_CHECK([modprobe $1]) +] +) \ No newline at end of file diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index ae302a294..db287a86d 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -93,6 +93,166 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows : [[1-9]]"], [0], [i OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([offloads - sflow with sampling=1 - offloads enabled]) +LOAD_MODULE([psample]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +on_exit 'kill `cat test-sflow.pid`' +AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 0:127.0.0.1 > sflow.log], [0], [], [ignore]) +AT_CAPTURE_FILE([sflow.log]) +PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +AT_CHECK([ovs-vsctl -- --id=@sflow create sflow agent=lo target=\"127.0.0.1:$SFLOW_PORT\" header=128 sampling=1 polling=100 -- set bridge br0 sflow=@sflow], [0], [ignore]) +NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +]) + +p1_ifindex=$(cat /sys/class/net/ovs-p1/ifindex) +m4_define([DUMP_SFLOW], [sed -e "s/used:[[0-9]].[[0-9]]*s/used:0.001s/;s/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/pid=[[0-9]]*/pid=1/;s/output=$p1_ifindex/output=1/"]) +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "in_port(2)" | grep "eth_type(0x0800)" | grep "actions:userspace" | grep "sFlow" | DUMP_SFLOW], [0], [dnl +recirc_id(0),in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:999, bytes:83916, used:0.001s, actions:userspace(pid=1,sFlow(vid=0,pcp=0,output=1),actions),3 +]) + +p0_ifindex=$(cat /sys/class/net/ovs-p0/ifindex) +m4_define([DUMP_SFLOW], [sed -e "s/used:[[0-9]].[[0-9]]*s/used:0.001s/;s/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/pid=[[0-9]]*/pid=1/;s/output=$p0_ifindex/output=1/"]) +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "in_port(3)" | grep "eth_type(0x0800)" | grep "actions:userspace" | grep "sFlow" | DUMP_SFLOW], [0], [dnl +recirc_id(0),in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:999, bytes:83916,