| Message ID | 1593423078-27413-1-git-send-email-wenxu@ucloud.cn |
|---|---|
| State | Accepted |
| Commit | 0fc38297ba3830631e4a6ad3f9fbbe364e8a3b6b |
| Headers | show |
| Series | [ovs-dev] lib/tc: only update the stats for non-empty counter | expand |
On Mon, Jun 29, 2020 at 05:31:18PM +0800, wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > A packet with first frag and execute act_ct action. > The packet will stole by defrag. So the stats counter > for "gact action goto chain" will always 0. The openvswitch > update each action in order. So the flower stats finally > alway be zero. The rule will be delete adter max-idle time > even there are packet executing the action. > > ovs-appctl dpctl/dump-flows > recirc_id(0),in_port(1),eth_type(0x0800),ipv4(dst=11.0.0.7,frag=first), packets:0, bytes:0, used:5.390s, actions:ct(zone=1,nat),recirc(0x4) > > filter protocol ip pref 2 flower chain 0 handle 0x2 > eth_type ipv4 > dst_ip 1.1.1.1 > ip_flags frag/firstfrag > skip_hw > not_in_hw > action order 1: ct zone 1 nat pipe > index 2 ref 1 bind 1 installed 11 sec used 1 sec > Action statistics: > Sent 15000 bytes 11 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > cookie e04106c2ac41769b278edaa9b5309960 > > action order 2: gact action goto chain 1 > random type none pass val 0 > index 2 ref 1 bind 1 installed 11 sec used 11 sec > Action statistics: > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > cookie e04106c2ac41769b278edaa9b5309960 > > Signed-off-by: wenxu <wenxu@ucloud.cn> Thanks, this patch looks good to me. I've added it to Travis CI for it to check with a view to pushing this change. https://travis-ci.org/github/horms2/ovs/builds/703600018
On Tue, Jun 30, 2020 at 06:33:11PM +0200, Simon Horman wrote: > On Mon, Jun 29, 2020 at 05:31:18PM +0800, wenxu@ucloud.cn wrote: > > From: wenxu <wenxu@ucloud.cn> > > > > A packet with first frag and execute act_ct action. > > The packet will stole by defrag. So the stats counter > > for "gact action goto chain" will always 0. The openvswitch > > update each action in order. So the flower stats finally > > alway be zero. The rule will be delete adter max-idle time > > even there are packet executing the action. > > > > ovs-appctl dpctl/dump-flows > > recirc_id(0),in_port(1),eth_type(0x0800),ipv4(dst=11.0.0.7,frag=first), packets:0, bytes:0, used:5.390s, actions:ct(zone=1,nat),recirc(0x4) > > > > filter protocol ip pref 2 flower chain 0 handle 0x2 > > eth_type ipv4 > > dst_ip 1.1.1.1 > > ip_flags frag/firstfrag > > skip_hw > > not_in_hw > > action order 1: ct zone 1 nat pipe > > index 2 ref 1 bind 1 installed 11 sec used 1 sec > > Action statistics: > > Sent 15000 bytes 11 pkt (dropped 0, overlimits 0 requeues 0) > > backlog 0b 0p requeues 0 > > cookie e04106c2ac41769b278edaa9b5309960 > > > > action order 2: gact action goto chain 1 > > random type none pass val 0 > > index 2 ref 1 bind 1 installed 11 sec used 11 sec > > Action statistics: > > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > > backlog 0b 0p requeues 0 > > cookie e04106c2ac41769b278edaa9b5309960 > > > > Signed-off-by: wenxu <wenxu@ucloud.cn> > > Thanks, > > this patch looks good to me. > > I've added it to Travis CI for it to check with a view to pushing this > change. > > https://travis-ci.org/github/horms2/ovs/builds/703600018 Hi again, The above build was bogus because I accidently applied the patch to an old revision of the master branch. A follow-up builds worked fine. master: https://travis-ci.org/github/horms2/ovs/builds/703820337 branch-2.13: https://travis-ci.org/github/horms2/ovs/builds/703821260 branch-2.12: https://travis-ci.org/github/horms2/ovs/builds/703821346 branch-2.11: https://travis-ci.org/github/horms2/ovs/builds/703821222 branch-2.10: https://travis-ci.org/github/horms2/ovs/builds/703821288 branch-2.9: https://travis-ci.org/github/horms2/ovs/builds/703821405 branch-2.8: https://travis-ci.org/github/horms2/ovs/builds/703821316 I have gone ahead and pushed this change to master and branch-2.13. As use of for the goto action seems to have been introduced in v2.13 I did not push this change to branches for older releases (although as per the above it applied cleanly). Let me know if you think it is appropriate to push to branches for releases earlier than v2.13.
diff --git a/lib/tc.c b/lib/tc.c index c2ab775..c96d095 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -1726,8 +1726,10 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower, } bs = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs); - put_32aligned_u64(&stats->n_packets, bs->packets); - put_32aligned_u64(&stats->n_bytes, bs->bytes); + if (bs->packets) { + put_32aligned_u64(&stats->n_packets, bs->packets); + put_32aligned_u64(&stats->n_bytes, bs->bytes); + } return 0; }