On Wed, Sep 07, 2022 at 11:36:06AM +0200, Simon Horman wrote:
> From: Tianyu Yuan <tianyu.y...@corigine.com>
> 
> 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 <tianyu.y...@corigine.com>
> Reviewed-by: Baowen Zheng <baowen.zh...@corigine.com>
> Signed-off-by: Simon Horman <simon.hor...@corigine.com>
> ---
> 
> Changes between v2 and v3
> * Add comment to explain why we skip stats update only when act_police is 
> meter
> 
> Changes between v1 and v2
> * Fix checkpatch errors

Sorry, I forgot to mark this patch as v3 when posting it.

> 
> ---
>  lib/tc.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/lib/tc.c b/lib/tc.c
> index f8fbe44bf244..0cf6817d3ca9 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -1904,6 +1904,8 @@ 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,
> @@ -1941,6 +1943,7 @@ 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;
> @@ -1955,6 +1958,16 @@ 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 kind of 
> police,
> +     * whose stats could not be skipped (e.g. filter has only one police
> +     * action), update the action stats to flow rule
> +     */
> +    if (is_meter) {
> +        return 0;
> +    }
> +
>      return nl_parse_action_stats(action_attrs[TCA_ACT_STATS],
>                                   &flower->stats_sw, &flower->stats_hw, NULL);
>  }
> -- 
> 2.30.2
> 
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to