On 4/12/2023 10:10 PM, Eelco Chaudron wrote:
On 29 Mar 2023, at 13:42, 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
See some comments inline. This concluded the v26 review...

//Eelco

Signed-off-by: Chris Mi<[email protected]>
Reviewed-by: Roi Dayan<[email protected]>
Acked-by: Eelco Chaudron<[email protected]>
---
  tests/system-offloads-traffic.at | 125 +++++++++++++++++++++++++++++++
  1 file changed, 125 insertions(+)

diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index eb331d6ce..3a596eb19 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -93,6 +93,131 @@ 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])
+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, used:0.001s, 
actions:userspace(pid=1,sFlow(vid=0,pcp=0,output=1),actions),2
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+OVS_APP_EXIT_AND_WAIT([test-sflow])
There is no check to verify the content of the sflow log?
How about the following check?

+hdr="in_ifindex=$p0_ifindex in_format=0 out_ifindex=$p1_ifindex out_format=0 hdr_prot=1 pkt_len=102 stripped=4 hdr_len=98"
+count=`grep "$hdr" sflow.log | wc -l`
+AT_CHECK([[[[ $count -ge 996 ]]]])
+hdr="in_ifindex=$p1_ifindex in_format=0 out_ifindex=$p0_ifindex out_format=0 hdr_prot=1 pkt_len=102 stripped=4 hdr_len=98"
+count=`grep "$hdr" sflow.log | wc -l`
+AT_CHECK([[[[ $count -ge 996 ]]]])
+AT_CLEANUP
+
+AT_SETUP([offloads - sflow with sampling=2 - offloads enabled])
+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-appctl dpctl/dump-flows], [0], [ignore])
Guess this dump can be removed?
Done.
+
+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 -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:sample" | 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:sample(sample=50.0%,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:sample" | grep "sFlow" | DUMP_SFLOW], [0], [dnl
+recirc_id(0),in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:999, 
bytes:83916, used:0.001s, 
actions:sample(sample=50.0%,actions(userspace(pid=1,sFlow(vid=0,pcp=0,output=1),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 - ping over vxlan tunnel with sflow - offloads enabled])
+OVS_CHECK_TUNNEL_TSO()
+OVS_CHECK_VXLAN()
+
+OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
other_config:hw-offload=true])
+ADD_BR([br-underlay])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and with a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], [10.1.1.100/24])
+ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100], 
[10.1.1.1/24],
+                  [id 0 dstport 4789])
+
+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 -- --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])
+
+dnl First, check the underlay
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, now check the overlay with different packet sizes
Guess this comment needs an update.
Done.
+NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 10.1.1.100 | FORMAT_PING], 
[0], [dnl
+1000 packets transmitted, 1000 received, 0% packet loss, time 0ms
+])
+
Can we also add dpctl/dumpflows checking here (3x), as the tunnels stuff is 
complex?
How about the following check?

+dnl Check encap flow
+match="recirc_id(0),in_port(br0),eth(src=$br0_mac,dst=$vxlan1_mac),eth_type(0x0800),ipv4(tos=0/0x3,frag=no)"
+action="actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=$vxlan_sys_ifindex),actions),set(tunnel(dst=172.31.1.1,ttl=64,tp_dst=4789,flags(0))),vxlan_sys_4789"
+AT_CHECK([ovs-appctl dpctl/dump-flows --names | grep $match | grep $action > /dev/null])
+
+dnl Check decap flow
+match="recirc_id(0),tunnel(tun_id=0x0,src=172.31.1.1,dst=172.31.1.100,tp_dst=4789,flags(+key)),in_port(vxlan_sys_4789),eth(src=$vxlan1_mac,dst=$br0_mac),eth_type(0x0800),ipv4(frag=no)"
+action="actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=$br0_ifindex),actions),br0"
+AT_CHECK([ovs-appctl dpctl/dump-flows --names | grep $match | grep $action> /dev/null])
recirc_id(0),in_port(1),eth(src=06:9c:de:63:c9:40,dst=01:00:5e:00:00:fb),eth_type(0x0800),ipv4(tos=0/0x3,frag=no),
 packets:13, bytes:1883, used:5.700s, 
actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=20360),actions),set(tunnel(dst=172.31.1.1,ttl=64,tp_dst=4789,flags(0))),4
recirc_id(0),in_port(1),eth(src=06:9c:de:63:c9:40,dst=1e:51:4f:cd:c3:0e),eth_type(0x0800),ipv4(tos=0/0x3,frag=no),
 packets:999, bytes:97902, used:0.010s, 
actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=20360),actions),set(tunnel(dst=172.31.1.1,ttl=64,tp_dst=4789,flags(0))),4
recirc_id(0),tunnel(tun_id=0x0,src=172.31.1.1,dst=172.31.1.100,tp_dst=4789,flags(+key)),in_port(4),eth(src=1e:51:4f:cd:c3:0e,dst=06:9c:de:63:c9:40),eth_type(0x0800),ipv4(frag=no),
 packets:999, bytes:83916, used:0.010s, 
actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=20356),actions),1

I did notice a little difference between TC and Kernel, below is the kernel and 
it has flags as (df) where tc does not have this. Is this a bug in tc?

recirc_id(0),in_port(1),eth(src=52:3c:6f:15:be:40,dst=01:00:5e:00:00:fb),eth_type(0x0800),ipv4(tos=0/0x3,frag=no),
 packets:13, bytes:1883, used:5.324s, 
actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=20369),actions),set(tunnel(dst=172.31.1.1,ttl=64,tp_dst=4789,flags(df))),4
recirc_id(0),in_port(1),eth(src=52:3c:6f:15:be:40,dst=0a:54:8d:56:5f:e0),eth_type(0x0800),ipv4(tos=0/0x3,frag=no),
 packets:999, bytes:97902, used:0.017s, 
actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=20369),actions),set(tunnel(dst=172.31.1.1,ttl=64,tp_dst=4789,flags(df))),4
recirc_id(0),tunnel(tun_id=0x0,src=172.31.1.1,dst=172.31.1.100,flags(-df+csum+key)),in_port(4),eth(src=0a:54:8d:56:5f:e0,dst=52:3c:6f:15:be:40),eth_type(0x0800),ipv4(frag=no),
 packets:999, bytes:97902, used:0.017s, 
actions:userspace(pid=4294967295,sFlow(vid=0,pcp=0,output=20365),actions),1

+dnl Vni is 0, test-sflow doesn't show it
+out_tunnel_hdr="tunnel4_out_protocol=17 tunnel4_out_src=0.0.0.0 
tunnel4_out_dst=172.31.1.1 tunnel4_out_src_port=0 tunnel4_out_dst_port=46354"
+out_count=`grep "$out_tunnel_hdr" sflow.log | wc -l`
+AT_CHECK([[[[ $out_count -ge 999 ]]]])
+
+in_tunnel_hdr="tunnel4_in_protocol=17 tunnel4_in_src=172.31.1.1 
tunnel4_in_dst=172.31.1.100"
+in_count=`grep "$in_tunnel_hdr" sflow.log | wc -l`
+AT_CHECK([[[[ $in_count -ge 999 ]]]])
Note there seems to be a bug in tc, as it's missing the in ifindex.
I think that's because tunnel vtep is in a namespace, so ovs/tc can't get in_ifindex from namespace.

I changed the code to only get sample group id and sampled packets from tc. All other information, like input ifindex, output tunnel and so on, will be got from group id mapping.
After this change, we'll not have such error.

Eelco,

Now I finished all your comments in v26. Will send a new version after you ack all of them.
Thanks again for the review.

-Chris
TC:


HEADER dgramSeqNo=345 ds=127.0.0.1>2:1000 fsSeqNo=2025 tunnel4_out_length=0 
tunnel4_out_protocol=17 \
   tunnel4_out_src=0.0.0.0 tunnel4_out_dst=172.31.1.1 tunnel4_out_src_port=0 
tunnel4_out_dst_port=46354 \
   tunnel4_out_tcp_flags=0 tunnel4_out_tos=0 in_vlan=0 in_priority=0 out_vlan=0 
out_priority=0 meanSkip=1 \
   samplePool=2025 dropEvents=0 in_ifindex=0 in_format=0 out_ifindex=20351 
out_format=0 hdr_prot=1 pkt_len=102...
                                ^^^^^^^^^^^^
Kernel:

HEADER dgramSeqNo=348 ds=127.0.0.1>2:1000 fsSeqNo=2027 tunnel4_out_length=0 
tunnel4_out_protocol=17 \
   tunnel4_out_src=0.0.0.0 tunnel4_out_dst=172.31.1.1 tunnel4_out_src_port=0 
tunnel4_out_dst_port=46354 \
   tunnel4_out_tcp_flags=0 tunnel4_out_tos=0 in_vlan=0 in_priority=0 out_vlan=0 
out_priority=0 meanSkip=1 \
   samplePool=2027 dropEvents=0 in_ifindex=20365 in_format=0 out_ifindex=20369 
out_format=0 hdr_prot=1 pkt_len=102...
                                ^^^^^^^^^^^^^^^^

Guess this needs to be fixed, and we should also add a check to make sure no 
in_ifindex=0 or out_ifindex=0 exists.

+
+OVS_TRAFFIC_VSWITCHD_STOP
+OVS_APP_EXIT_AND_WAIT([test-sflow])
+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.26.3
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to