On 10/14/22 13:38, Simon Horman wrote: > From: Tianyu Yuan <[email protected]> > > This reverts commit dd9881ed55e6 ('tc: Fix stats dump when > using same meter table') > > This patch doesn't solve the tc flow stats update issue and > will lead to failure of system-offloads-traffic testsuite, it > only counts packets surviving after the tc filter, rather than > hitting the filter > > A following patch will come up to solve this flow stats update > issue > > Signed-off-by: Tianyu Yuan <[email protected]> > Signed-off-by: Simon Horman <[email protected]> > --- > lib/tc.c | 11 ----------- > 1 file changed, 11 deletions(-)
Hi. I'm not really sure what is worse - the undercounting or overcounting. If you think that revert is a good temporary solution then it's fine by me: Acked-by: Ilya Maximets <[email protected]> It might make sense to add a note to the 'Known TC flow offload limitations' in Documentation/howto/tc-offload.rst about this issue though. Best regards, Ilya Maximets. > > diff --git a/lib/tc.c b/lib/tc.c > index 94044cde6060..f8fbe44bf244 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -1904,8 +1904,6 @@ nl_parse_single_action(struct nlattr *action, struct > tc_flower *flower, > struct nlattr *act_cookie; > const char *act_kind; > struct nlattr *action_attrs[ARRAY_SIZE(act_policy)]; > - int act_index = flower->action_count; > - bool is_meter = false; > int err = 0; > > if (!nl_parse_nested(action, act_policy, action_attrs, > @@ -1943,7 +1941,6 @@ nl_parse_single_action(struct nlattr *action, struct > tc_flower *flower, > nl_parse_act_ct(act_options, flower); > } else if (!strcmp(act_kind, "police")) { > nl_parse_act_police(act_options, flower); > - is_meter = > tc_is_meter_index(flower->actions[act_index].police.index); > } else { > VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind); > err = EINVAL; > @@ -1958,14 +1955,6 @@ nl_parse_single_action(struct nlattr *action, struct > tc_flower *flower, > flower->act_cookie.len = nl_attr_get_size(act_cookie); > } > > - /* 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. */ > - if (is_meter) { > - return 0; > - } > - > return nl_parse_action_stats(action_attrs[TCA_ACT_STATS], > &flower->stats_sw, &flower->stats_hw, NULL); > } _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
