[ovs-dev] [PATCH] system-offloads-traffic.at: Add sFlow offload test cases

Eelco Chaudron echaudro at redhat.com
Fri Oct 1 14:34:22 UTC 2021


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 <cmi at nvidia.com>
> ---
>  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.

> +
> +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))),2

> +
> +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



More information about the dev mailing list