On Tue, Oct 11, 2022 at 11:35:27AM +0200, Simon Horman wrote:
> From: Baowen Zheng <[email protected]>
> 
> Add tc action flags when adding police action to offload meter table.
> 
> There is a restriction that the flag of skip_sw/skip_hw should be same for
> filter rule and the independent created tc actions the rule uses. In this
> case, if we configure the tc-policy as skip_hw, filter rule will be created
> with skip_hw flag and the police action according to meter table will have
> no action flag, then flower rule will fail to add to tc kernel system.
> 
> In this patch, we will add tc action flag when adding police action to
> offload a meter table, so it will allow meter table to work in tc software
> datapath.

Gentle ping for review of this.

I believe this issue is independent from other discussions
around metering that are ongoing.

> Signed-off-by: Baowen Zheng <[email protected]>
> Signed-off-by: Simon Horman <[email protected]>
> ---
>  acinclude.m4            |  6 +++---
>  include/linux/pkt_cls.h | 11 +++++++----
>  lib/netdev-linux.c      | 20 ++++++++++++++------
>  lib/tc.c                | 21 +++++++++++++++++++++
>  lib/tc.h                |  3 +++
>  5 files changed, 48 insertions(+), 13 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index ad07989ac29c..aa9af55062f0 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -163,10 +163,10 @@ dnl Configure Linux tc compat.
>  AC_DEFUN([OVS_CHECK_LINUX_TC], [
>    AC_COMPILE_IFELSE([
>      AC_LANG_PROGRAM([#include <linux/pkt_cls.h>], [
> -        int x = TCA_POLICE_PKTRATE64;
> +        int x = TCA_ACT_FLAGS_SKIP_HW;
>      ])],
> -    [AC_DEFINE([HAVE_TCA_POLICE_PKTRATE64], [1],
> -               [Define to 1 if TCA_POLICE_PKTRATE64 is available.])])
> +    [AC_DEFINE([HAVE_TCA_ACT_FLAGS_SKIP_HW], [1],
> +               [Define to 1 if TCA_ACT_FLAGS_SKIP_HW is available.])])
>  
>    AC_CHECK_MEMBERS([struct tcf_t.firstuse], [], [], [#include 
> <linux/pkt_cls.h>])
>  
> diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
> index ba82e690eba9..a8cd8db5bf88 100644
> --- a/include/linux/pkt_cls.h
> +++ b/include/linux/pkt_cls.h
> @@ -1,7 +1,7 @@
>  #ifndef __LINUX_PKT_CLS_WRAPPER_H
>  #define __LINUX_PKT_CLS_WRAPPER_H 1
>  
> -#if defined(__KERNEL__) || defined(HAVE_TCA_POLICE_PKTRATE64)
> +#if defined(__KERNEL__) || defined(HAVE_TCA_ACT_FLAGS_SKIP_HW)
>  #include_next <linux/pkt_cls.h>
>  #else
>  
> @@ -21,9 +21,12 @@ enum {
>       __TCA_ACT_MAX
>  };
>  
> -#define TCA_ACT_FLAGS_NO_PERCPU_STATS 1 /* Don't use percpu allocator for
> -                                         * actions stats.
> -                                         */
> +/* See other TCA_ACT_FLAGS_ * flags in include/net/act_api.h. */
> +#define TCA_ACT_FLAGS_NO_PERCPU_STATS (1 << 0) /* Don't use percpu allocator 
> for
> +                                             * actions stats.
> +                                             */
> +#define TCA_ACT_FLAGS_SKIP_HW        (1 << 1) /* don't offload action to HW 
> */
> +#define TCA_ACT_FLAGS_SKIP_SW        (1 << 2) /* don't use action in SW */
>  
>  #define TCA_ACT_MAX __TCA_ACT_MAX
>  #define TCA_OLD_COMPAT (TCA_ACT_MAX+1)
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index cdc66246ced3..c6422aacefeb 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2623,10 +2623,17 @@ tc_matchall_fill_police(uint32_t kbits_rate, uint32_t 
> kbits_burst)
>  
>  static void
>  nl_msg_act_police_start_nest(struct ofpbuf *request, uint32_t prio,
> -                             size_t *offset, size_t *act_offset)
> +                             size_t *offset, size_t *act_offset,
> +                             bool single_action)
>  {
>      *act_offset = nl_msg_start_nested(request, prio);
>      nl_msg_put_string(request, TCA_ACT_KIND, "police");
> +
> +    /* If police action is added independ from filter, we need to
> +     * add action flag according to tc-policy. */
> +    if (single_action) {
> +        nl_msg_put_tc_action_flag(request);
> +    }
>      *offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
>  }
>  
> @@ -2642,7 +2649,7 @@ nl_msg_act_police_end_nest(struct ofpbuf *request, 
> size_t offset,
>  static void
>  nl_msg_put_act_police(struct ofpbuf *request, struct tc_police *police,
>                        uint64_t pkts_rate, uint64_t pkts_burst,
> -                      uint32_t notexceed_act)
> +                      uint32_t notexceed_act, bool single_action)
>  {
>      size_t offset, act_offset;
>      uint32_t prio = 0;
> @@ -2651,7 +2658,8 @@ nl_msg_put_act_police(struct ofpbuf *request, struct 
> tc_police *police,
>          return;
>      }
>  
> -    nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset);
> +    nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset,
> +                                 single_action);
>      if (police->rate.rate) {
>          tc_put_rtab(request, TCA_POLICE_RATE, &police->rate);
>      }
> @@ -2698,7 +2706,7 @@ tc_add_matchall_policer(struct netdev *netdev, uint32_t 
> kbits_rate,
>      basic_offset = nl_msg_start_nested(&request, TCA_OPTIONS);
>      action_offset = nl_msg_start_nested(&request, TCA_MATCHALL_ACT);
>      nl_msg_put_act_police(&request, &pol_act, kpkts_rate * 1000,
> -                          kpkts_burst * 1000, TC_ACT_UNSPEC);
> +                          kpkts_burst * 1000, TC_ACT_UNSPEC, false);
>      nl_msg_end_nested(&request, action_offset);
>      nl_msg_end_nested(&request, basic_offset);
>  
> @@ -5667,7 +5675,7 @@ tc_add_policer(struct netdev *netdev, uint32_t 
> kbits_rate,
>      police_offset = nl_msg_start_nested(&request, TCA_BASIC_ACT);
>      tc_policer_init(&tc_police, kbits_rate, kbits_burst);
>      nl_msg_put_act_police(&request, &tc_police, kpkts_rate * 1000ULL,
> -                          kpkts_burst * 1000ULL, TC_ACT_UNSPEC);
> +                          kpkts_burst * 1000ULL, TC_ACT_UNSPEC, false);
>      nl_msg_end_nested(&request, police_offset);
>      nl_msg_end_nested(&request, basic_offset);
>  
> @@ -5702,7 +5710,7 @@ tc_add_policer_action(uint32_t index, uint32_t 
> kbits_rate,
>  
>      offset = nl_msg_start_nested(&request, TCA_ACT_TAB);
>      nl_msg_put_act_police(&request, &tc_police, pkts_rate, pkts_burst,
> -                          TC_ACT_PIPE);
> +                          TC_ACT_PIPE, true);
>      nl_msg_end_nested(&request, offset);
>  
>      error = tc_transact(&request, NULL);
> diff --git a/lib/tc.c b/lib/tc.c
> index 94044cde6060..e46f5cc73c8e 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -3808,3 +3808,24 @@ tc_set_policy(const char *policy)
>  
>      VLOG_INFO("tc: Using policy '%s'", policy);
>  }
> +
> +void
> +nl_msg_put_tc_action_flag(struct ofpbuf *request)
> +{
> +    int flag = 0;
> +
> +    if (!request) {
> +        return;
> +    }
> +
> +    if (tc_policy == TC_POLICY_SKIP_HW) {
> +        flag = TCA_ACT_FLAGS_SKIP_HW;
> +    } else if (tc_policy == TC_POLICY_SKIP_SW) {
> +        flag = TCA_ACT_FLAGS_SKIP_SW;
> +    }
> +
> +    if (flag) {
> +        struct nla_bitfield32 flags = { flag, flag };
> +        nl_msg_put_unspec(request, TCA_ACT_FLAGS, &flags, sizeof flags);
> +    }
> +}
> diff --git a/lib/tc.h b/lib/tc.h
> index 2e64ad372592..8e3226e89bd4 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -399,4 +399,7 @@ int tc_parse_action_stats(struct nlattr *action,
>  int tc_dump_tc_action_start(char *name, struct nl_dump *dump);
>  int parse_netlink_to_tc_policer(struct ofpbuf *reply, uint32_t police_idx[]);
>  
> +void
> +nl_msg_put_tc_action_flag(struct ofpbuf *request);
> +
>  #endif /* tc.h */
> -- 
> 2.30.2
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to