On October 21, 2022 5:11 PM, Simon 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.
>
>To fix this issue, 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.
>
Hi all,
gentle ping for review, thanks.
>Fixes: 5c039ddc64ff ("netdev-linux: Add functions to manipulate tc police
>action")
>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 | 2 ++
> 5 files changed, 47 insertions(+), 13 deletions(-)
>
> v2:
> * Address review of v1
> - Update patch description: problem depends on setting tc-policy
> - Correct typo in comment: "independently"
> - Reformat nl_msg_put_tc_action_flag() declaration
> - Rename helper as nl_msg_put_act_tc_policy_flag() and correct
> line wrapping of declaration.
>
>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..7ea4070c23ab 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 independently from filter, we need to
>+ * add action flag according to tc-policy. */
>+ if (single_action) {
>+ nl_msg_put_act_tc_policy_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..66dc4208888c 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_act_tc_policy_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..161f438124b0 100644
>--- a/lib/tc.h
>+++ b/lib/tc.h
>@@ -399,4 +399,6 @@ 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_act_tc_policy_flag(struct ofpbuf *request);
>+
> #endif /* tc.h */
>--
>2.30.2
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
