Re: [PATCH net-next v2 2/2] flow_offload: reject offload for all drivers with invalid police parameters

Vladimir Oltean
2022-02-22T10:10:06Z

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]

Hi Jianbo,

On Tue, Feb 22, 2022 at 01:58:23AM +0000, Jianbo Liu wrote:
> Hi Vladimir,
>
> I'd love to hear your suggestion regarding where this validate function
> to be placed for drivers/net/ethernet/mscc, as it will be used by both
> ocelot_net.c and ocelot_flower.c.
>
> Thanks!
> Jianbo

Try the attached patch on top of yours.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ocelot-policer-validate.patch --]
[-- Type: text/x-diff; name="0001-ocelot-policer-validate.patch", Size: 7837 bytes --]

From d401f55a34390c27b35ae647d260e2b900528517 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Tue, 22 Feb 2022 12:05:59 +0200
Subject: [PATCH] ocelot policer validate

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ethernet/mscc/ocelot_flower.c | 38 ++++-----------------
drivers/net/ethernet/mscc/ocelot_net.c | 37 +++-----------------
drivers/net/ethernet/mscc/ocelot_police.c | 41 +++++++++++++++++++++++
drivers/net/ethernet/mscc/ocelot_police.h | 5 +++
4 files changed, 57 insertions(+), 64 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index 4680a62b4d9a..b3f5418dc622 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c
@@ -6,6 +6,7 @@
#include <net/pkt_cls.h>
#include <net/tc_act/tc_gact.h>
#include <soc/mscc/ocelot_vcap.h>
+#include "ocelot_police.h"
#include "ocelot_vcap.h"

/* Arbitrarily chosen constants for encoding the VCAP block and lookup number
@@ -217,6 +218,7 @@ static int ocelot_flower_parse_action(struct ocelot *ocelot, int port,
bool ingress, struct flow_cls_offload *f,
struct ocelot_vcap_filter *filter)
{
+ const struct flow_action *action = &f->rule->action;
struct netlink_ext_ack *extack = f->common.extack;
bool allow_missing_goto_target = false;
const struct flow_action_entry *a;
@@ -244,7 +246,7 @@ static int ocelot_flower_parse_action(struct ocelot *ocelot, int port,
filter->goto_target = -1;
filter->type = OCELOT_VCAP_FILTER_DUMMY;

- flow_action_for_each(i, a, &f->rule->action) {
+ flow_action_for_each(i, a, action) {
switch (a->id) {
case FLOW_ACTION_DROP:
if (filter->block_id != VCAP_IS2) {
@@ -298,38 +300,10 @@ static int ocelot_flower_parse_action(struct ocelot *ocelot, int port,
return -EOPNOTSUPP;
}

- if (a->police.exceed.act_id != FLOW_ACTION_DROP) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the exceed action is not drop");
- return -EOPNOTSUPP;
- }
-
- if (a->police.notexceed.act_id != FLOW_ACTION_PIPE &&
- a->police.notexceed.act_id != FLOW_ACTION_ACCEPT) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the conform action is not pipe or ok");
- return -EOPNOTSUPP;
- }
-
- if (a->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
- !flow_action_is_last_entry(&f->rule->action, a)) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the conform action is ok, but police action is not last");
- return -EOPNOTSUPP;
- }
-
- if (a->police.peakrate_bytes_ps ||
- a->police.avrate || a->police.overhead) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when peakrate/avrate/overhead is configured");
- return -EOPNOTSUPP;
- }
+ err = ocelot_policer_validate(action, a, extack);
+ if (err)
+ return err;

- if (a->police.rate_pkt_ps) {
- NL_SET_ERR_MSG_MOD(extack,
- "QoS offload not support packets per second");
- return -EOPNOTSUPP;
- }
filter->action.police_ena = true;

pol_ix = a->hw_index + ocelot->vcap_pol.base;
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 4e38d41dba29..5767e38c0c5a 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -14,6 +14,7 @@
#include <linux/phy/phy.h>
#include <net/pkt_cls.h>
#include "ocelot.h"
+#include "ocelot_police.h"
#include "ocelot_vcap.h"
#include "ocelot_fdma.h"

@@ -258,38 +259,10 @@ static int ocelot_setup_tc_cls_matchall(struct ocelot_port_private *priv,
return -EEXIST;
}

- if (action->police.exceed.act_id != FLOW_ACTION_DROP) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the exceed action is not drop");
- return -EOPNOTSUPP;
- }
-
- if (action->police.notexceed.act_id != FLOW_ACTION_PIPE &&
- action->police.notexceed.act_id != FLOW_ACTION_ACCEPT) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the conform action is not pipe or ok");
- return -EOPNOTSUPP;
- }
-
- if (action->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
- !flow_action_is_last_entry(&f->rule->action, action)) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the conform action is ok, but police action is not last");
- return -EOPNOTSUPP;
- }
-
- if (action->police.peakrate_bytes_ps ||
- action->police.avrate || action->police.overhead) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when peakrate/avrate/overhead is configured");
- return -EOPNOTSUPP;
- }
-
- if (action->police.rate_pkt_ps) {
- NL_SET_ERR_MSG_MOD(extack,
- "QoS offload not support packets per second");
- return -EOPNOTSUPP;
- }
+ err = ocelot_policer_validate(&f->rule->action, action,
+ extack);
+ if (err)
+ return err;

pol.rate = (u32)div_u64(action->police.rate_bytes_ps, 1000) * 8;
pol.burst = action->police.burst;
diff --git a/drivers/net/ethernet/mscc/ocelot_police.c b/drivers/net/ethernet/mscc/ocelot_police.c
index 6f5068c1041a..a65606bb84a0 100644
--- a/drivers/net/ethernet/mscc/ocelot_police.c
+++ b/drivers/net/ethernet/mscc/ocelot_police.c
@@ -154,6 +154,47 @@ int qos_policer_conf_set(struct ocelot *ocelot, int port, u32 pol_ix,
return 0;
}

+int ocelot_policer_validate(const struct flow_action *action,
+ const struct flow_action_entry *a,
+ struct netlink_ext_ack *extack)
+{
+ if (a->police.exceed.act_id != FLOW_ACTION_DROP) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when exceed action is not drop");
+ return -EOPNOTSUPP;
+ }
+
+ if (a->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+ a->police.notexceed.act_id != FLOW_ACTION_ACCEPT) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when conform action is not pipe or ok");
+ return -EOPNOTSUPP;
+ }
+
+ if (a->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+ !flow_action_is_last_entry(action, a)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when conform action is ok, but police action is not last");
+ return -EOPNOTSUPP;
+ }
+
+ if (a->police.peakrate_bytes_ps ||
+ a->police.avrate || a->police.overhead) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when peakrate/avrate/overhead is configured");
+ return -EOPNOTSUPP;
+ }
+
+ if (a->police.rate_pkt_ps) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload does not support packets per second");
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(ocelot_policer_validate);
+
int ocelot_port_policer_add(struct ocelot *ocelot, int port,
struct ocelot_policer *pol)
{
diff --git a/drivers/net/ethernet/mscc/ocelot_police.h b/drivers/net/ethernet/mscc/ocelot_police.h
index 7adb05f71999..7552995f8b17 100644
--- a/drivers/net/ethernet/mscc/ocelot_police.h
+++ b/drivers/net/ethernet/mscc/ocelot_police.h
@@ -8,6 +8,7 @@
#define _MSCC_OCELOT_POLICE_H_

#include "ocelot.h"
+#include <net/flow_offload.h>

enum mscc_qos_rate_mode {
MSCC_QOS_RATE_MODE_DISABLED, /* Policer/shaper disabled */
@@ -33,4 +34,8 @@ struct qos_policer_conf {
int qos_policer_conf_set(struct ocelot *ocelot, int port, u32 pol_ix,
struct qos_policer_conf *conf);

+int ocelot_policer_validate(const struct flow_action *action,
+ const struct flow_action_entry *a,
+ struct netlink_ext_ack *extack);
+
#endif /* _MSCC_OCELOT_POLICE_H_ */
--
2.25.1