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?
<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 ]]]])
^^^^^^^^^^^^^^^^^^
<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?
>> 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.
>
> -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.
Did you add this check to the tests?
>>> +
>>> +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
