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

[Vladimir Oltean <vladimir.oltean@nxp.com>]

[-- 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