On 5/10/2023 10:52 PM, Eelco Chaudron wrote:
It's a little different. In this case sample rate is 1, we can't receive more than 1000 packets.On 26 Apr 2023, at 4:47, Chris Mi wrote:<SNIP>+ +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 ]]]])Did not verify, but this looks right. Do we need a -le check, as you have below?
So checking less and equal than 1000 seems unnecessary.
<SNIP>+ +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 ]]]])^^^^^^^^^^^^^^^^^^
In this case, sample rate is 2, we may receive more than 1000 packets.
Not the same, there are some differences, I'll write that in the tc-offload.rst as known issues:<SNIP>+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])Did not verify, but it looks good. Can you make sure it also passes with the kernel datapath, i.e. it has the same output?
+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
+ Except this, other info are the same.
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.Ok, thanks for looking into this and fixing it.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. -ChrisTC: 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.Did you add this check to the tests?
Yes. dnl Vni is 0, test-sflow doesn't show itout_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"
ifindex="in_ifindex=$br0_ifindex in_format=0 out_ifindex=$vxlan_sys_ifindex" out_count=`grep "$out_tunnel_hdr" sflow.log | grep "$ifindex" | 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"
ifindex="in_ifindex=$vxlan_sys_ifindex in_format=0 out_ifindex=$br0_ifindex" in_count=`grep "$in_tunnel_hdr" sflow.log | grep "$ifindex" | wc -l` AT_CHECK([[[[ $in_count -ge 999 ]]]])
+ +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
