On 10/8/2021 2:47 PM, Eelco Chaudron wrote:
Hi Chris,I hope you enjoyed the holiday. And don't worry about the delay, as I try to only spend time reviewing stuff on Fridays. Thanks for fixing the comments, and I will re-review once it's integrated into the sflow patchset.
Sure. Thanks, Chris
//Eelco On 8 Oct 2021, at 8:23, Chris Mi wrote:Hi Eelco, Sorry for the late reply due to the long National Day holiday in China. On 10/1/2021 10:34 PM, Eelco Chaudron wrote:Thanks for working on the test cases. I have two small requests below to make the tests also verify the datapath actions. And as you already discussed with Simon this should be part of the generic TC sFlow patchset. //Eelco On 16 Sep 2021, at 10:38, Chris Mi wrote:Add two sFlow offload test caes: 3: sflow offloads with sampling=1 - ping between two ports - offloads enabled ok 4: sflow offloads with sampling=2 - ping between two ports - offloads enabled ok Signed-off-by: Chris Mi <[email protected]> --- tests/system-offloads-traffic.at | 84 ++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index be63057bb..f263c9183 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -5,6 +5,7 @@ AT_BANNER([datapath offloads]) # Normilizes output ports, recirc_id, packets and macs. # m4_define([DUMP_CLEAN_SORTED], [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/actions:[[0-9,]]*/actions:output/;s/recirc_id(0),//' | sort]) +m4_define([DUMP_CLEAN_SORTED_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/actions:.*$/actions:output/;s/recirc_id(0),//' | sort]) AT_SETUP([offloads - ping between two ports - offloads disabled]) OVS_TRAFFIC_VSWITCHD_START() @@ -71,6 +72,89 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows : [[1-9]]"], [0], [i OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([sflow offloads with sampling=1 - ping between two ports - offloads enabled]) +OVS_TRAFFIC_VSWITCHD_START() + +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-vsctl set Open_vSwitch . other_config:hw-offload=true]) + +AT_CHECK([ovs-appctl -t ovsdb-server exit]) +AT_CHECK([ovs-appctl -t ovs-vswitchd exit]) +AT_CHECK([rm -f ovsdb-server.pid]) +AT_CHECK([rm -f ovs-vswitchd.pid]) +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file --remote=punix:$OVS_RUNDIR/db.sock], [0], [], [stderr]) +AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) +on_exit "kill `cat ovsdb-server.pid`" +on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`" + +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-appctl dpctl/dump-flows], [0], [ignore]) + +NS_CHECK_EXEC([at_ns0], [ping -c 10 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +10 packets transmitted, 10 received, 0% packet loss, time 0ms +]) +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 -c 10 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl +10 packets transmitted, 10 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "actions:userspace" |grep "sFlow" | grep "eth_type(0x0800)" | DUMP_CLEAN_SORTED_SFLOW], [0], [dnl +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:840, used:0.001s, actions:output +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:840, used:0.001s, actions:output +])I think your filter to much in this case, as we would like to verify the correct datapath format for actions, so I think it should be something like: actions:userspace(pid=XXX,sFlow(vid=0,pcp=0,output=341),actions),3 Where you can ignore/replace the actual pid value, but you need to find a way to map the correct if_index.Done.+ +OVS_TRAFFIC_VSWITCHD_STOP +OVS_APP_EXIT_AND_WAIT([test-sflow]) +count=`cat sflow.log | wc -l` +AT_CHECK([[[[ $count -le 22 && $count -ge 18 ]]]]) +AT_CLEANUP + +AT_SETUP([sflow offloads with sampling=2 - ping between two ports - offloads enabled]) +OVS_TRAFFIC_VSWITCHD_START() + +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-vsctl set Open_vSwitch . other_config:hw-offload=true]) + +AT_CHECK([ovs-appctl -t ovsdb-server exit]) +AT_CHECK([ovs-appctl -t ovs-vswitchd exit]) +AT_CHECK([rm -f ovsdb-server.pid]) +AT_CHECK([rm -f ovs-vswitchd.pid]) +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file --remote=punix:$OVS_RUNDIR/db.sock], [0], [], [stderr]) +AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) +on_exit "kill `cat ovsdb-server.pid`" +on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`" + +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-appctl dpctl/dump-flows], [0], [ignore]) + +AT_CHECK([ovs-vsctl -- --id=@sflow create sflow agent=lo target=\"127.0.0.1:$SFLOW_PORT\" header=128 sampling=2 polling=100 -- set bridge br0 sflow=@sflow], [0], [ignore]) +NS_CHECK_EXEC([at_ns0], [ping -c 1000 -i 0.01 -w 12 10.1.1.2], [0], [ignore]) +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "actions:sample" | grep "sFlow" | grep "eth_type(0x0800)"], [0], [ignore])Why is this ignore? It should match flow/action,see also above: sample(sample=50.0%,actions(userspace(pid=XXX,sFlow(vid=0,pcp=0,output=361),actions))),2Because I think we have enough grep, so I ignored it. Anyway, it's done now. I will address your other comments in generic TC sFlow patchset and put this patch at the end of it. Thanks, Chris+ +OVS_TRAFFIC_VSWITCHD_STOP +OVS_APP_EXIT_AND_WAIT([test-sflow]) +count=`cat sflow.log | wc -l` +AT_CHECK([[[[ $count -le 1100 && $count -ge 900 ]]]]) +AT_CLEANUP + AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - offloads disabled]) AT_KEYWORDS([ingress_policing]) AT_SKIP_IF([test $HAVE_TC = "no"]) -- 2.27.0
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
