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

Eelco Chaudron echaudro at redhat.com
Fri Oct 8 06:47:15 UTC 2021


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.

//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 <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.
> 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))),2
> Because 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



More information about the dev mailing list