On 9/13/22 09:51, Simon Horman wrote: > On Mon, Sep 12, 2022 at 03:02:06PM +0200, Eelco Chaudron wrote: >> >> >> On 7 Sep 2022, at 11:36, Simon Horman wrote: >> >>> From: Tianyu Yuan <[email protected]> >>> >>> When we apply meter police on both directions of TCP traffic, the >>> dumped stats is shown same (as shown below). This issue is introduced >>> by modifying the stats update strategy. >>> >>> ...,in_port(6),eth(),eth_type(0x0800),ipv4(frag=no), packets:1488557, >>> bytes:2089059644, used:0.040s, actions:meter(0),9 >>> ...,in_port(9),eth(),eth_type(0x0800),ipv4(frag=no), packets:1488557, >>> bytes:2089059644, used:0.040s, actions:meter(0),6 >>> >>> In previous patch, after parsing police action, the flower stats will >>> be updated by dumped meter table stats, which will result in the issue >>> above. >>> >>> Thus, the stats of meter table should not be used when dumping flow >>> stats. Ignore the stats update when police.index belongs to meter. >>> >>> Fixes: a9b8cdde69de ("tc: Add support parsing tc police action") >>> Signed-off-by: Tianyu Yuan <[email protected]> >>> Reviewed-by: Baowen Zheng <[email protected]> >>> Signed-off-by: Simon Horman <[email protected]> > > ... > >>> + /* >>> + * Skip the stats update when act_police is meter, since there are >>> always >>> + * some other actions following meter. For other potential kind of >>> police, >>> + * whose stats could not be skipped (e.g. filter has only one police >>> + * action), update the action stats to flow rule >>> + */ >> >> Small nit: >> >> >> + /* Skip the stats update when act_police is meter since there are >> always >> + * some other actions following meter. For other potential kinds of >> + * act_police actions, whose stats could not be skipped (e.g. filter >> has >> + * only one police action), update the action stats to the flow rule. >> */ >> >> The rest looks good to me. >> >> Acked-by: Eelco Chaudron <[email protected]> > > Thanks Eelco, > > I've made the change suggested above and applied the patch to master and > branch-3.0.
Hi, Simon and Eelco. This commit made offload tests consistently fail due to incorrectly counted flow stats: # make check-offloads TESTSUITEFLAGS='8' 8. system-offloads-traffic.at:230: testing offloads - check interface meter offloading - offloads enabled ... ./system-offloads-traffic.at:266: ovs-appctl dpctl/dump-flows | grep "meter" | sed -e 's/used:\([0-9.]*s\|never\)/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 --- - 2022-09-13 06:37:19.306764602 -0400 +++ /root/ovs/tests/system-offloads-testsuite.dir/at-groups/8/stdout 2022-09-13 06:37:19.304198787 -0400 @@ -1,2 +1,2 @@ -in_port(2),eth(macs),eth_type(0x0800),ipv4(proto=17,frag=no), packets:10, bytes:330, used:0.001s, actions:meter(0),3 +in_port(2),eth(macs),eth_type(0x0800),ipv4(proto=17,frag=no), packets:1, bytes:33, used:0.001s, actions:meter(0),3 Could you, please, take a look? I see this on my rhel8 VM. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
