[PATCH net-next 0/5] Ocelot VCAP cleanups

[PATCH net-next 0/5] Ocelot VCAP cleanups

[Vladimir Oltean]

This is a series of minor code cleanups brought to the Ocelot switch
driver logic for VCAP filters.

- don't use list_for_each_safe() in ocelot_vcap_filter_add_to_block
- don't use magic numbers for OCELOT_POLICER_DISCARD

Vladimir Oltean (5):
  net: mscc: ocelot: use list_add_tail in
    ocelot_vcap_filter_add_to_block()
  net: mscc: ocelot: add to tail of empty list in
    ocelot_vcap_filter_add_to_block
  net: mscc: ocelot: use list_for_each_entry in
    ocelot_vcap_filter_add_to_block
  net: mscc: ocelot: drop port argument from qos_policer_conf_set
  net: mscc: ocelot: don't use magic numbers for OCELOT_POLICER_DISCARD

 drivers/net/ethernet/mscc/ocelot_police.c | 26 +++++++++-------
 drivers/net/ethernet/mscc/ocelot_police.h |  2 +-
 drivers/net/ethernet/mscc/ocelot_vcap.c   | 38 +++++++++--------------
 3 files changed, 31 insertions(+), 35 deletions(-)

-- 
2.25.1

[PATCH net-next 1/5] net: mscc: ocelot: use list_add_tail in ocelot_vcap_filter_add_to_block()

[Vladimir Oltean]

list_add(..., pos->prev) and list_add_tail(..., pos) are equivalent, use
the later form to unify with the case where the list is empty later.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_vcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index e98e7527da21..cba42566edc3 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -1012,7 +1012,7 @@ static int ocelot_vcap_filter_add_to_block(struct ocelot *ocelot,
 		if (filter->prio < tmp->prio)
 			break;
 	}
-	list_add(&filter->list, pos->prev);
+	list_add_tail(&filter->list, pos);
 
 	return 0;
 }
-- 
2.25.1

[PATCH net-next 2/5] net: mscc: ocelot: add to tail of empty list in ocelot_vcap_filter_add_to_block

[Vladimir Oltean]

This makes no functional difference but helps in minimizing the delta
for a future change.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_vcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index cba42566edc3..3f73d4790532 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -1003,7 +1003,7 @@ static int ocelot_vcap_filter_add_to_block(struct ocelot *ocelot,
 	block->count++;
 
 	if (list_empty(&block->rules)) {
-		list_add(&filter->list, &block->rules);
+		list_add_tail(&filter->list, &block->rules);
 		return 0;
 	}
 
-- 
2.25.1

[PATCH net-next 3/5] net: mscc: ocelot: use list_for_each_entry in ocelot_vcap_filter_add_to_block

[Vladimir Oltean]

Unify the code paths for adding to an empty list and to a list with
elements by keeping a "pos" list_head element that indicates where to
insert. Initialize "pos" with the list head itself in case
list_for_each_entry() doesn't iterate over any element.

Note that list_for_each_safe() isn't needed because no element is
removed from the list while iterating.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_vcap.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index 3f73d4790532..e8445d78a168 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -992,8 +992,8 @@ static int ocelot_vcap_filter_add_to_block(struct ocelot *ocelot,
 					   struct ocelot_vcap_filter *filter,
 					   struct netlink_ext_ack *extack)
 {
+	struct list_head *pos = &block->rules;
 	struct ocelot_vcap_filter *tmp;
-	struct list_head *pos, *n;
 	int ret;
 
 	ret = ocelot_vcap_filter_add_aux_resources(ocelot, filter, extack);
@@ -1002,15 +1002,11 @@ static int ocelot_vcap_filter_add_to_block(struct ocelot *ocelot,
 
 	block->count++;
 
-	if (list_empty(&block->rules)) {
-		list_add_tail(&filter->list, &block->rules);
-		return 0;
-	}
-
-	list_for_each_safe(pos, n, &block->rules) {
-		tmp = list_entry(pos, struct ocelot_vcap_filter, list);
-		if (filter->prio < tmp->prio)
+	list_for_each_entry(tmp, &block->rules, list) {
+		if (filter->prio < tmp->prio) {
+			pos = &tmp->list;
 			break;
+		}
 	}
 	list_add_tail(&filter->list, pos);
 
-- 
2.25.1

[PATCH net-next 4/5] net: mscc: ocelot: drop port argument from qos_policer_conf_set

[Vladimir Oltean]

The "port" argument is used for nothing else except printing on the
error path. Print errors on behalf of the policer index, which is less
confusing anyway.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_police.c | 26 +++++++++++++----------
 drivers/net/ethernet/mscc/ocelot_police.h |  2 +-
 drivers/net/ethernet/mscc/ocelot_vcap.c   |  4 ++--
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_police.c b/drivers/net/ethernet/mscc/ocelot_police.c
index a65606bb84a0..7e1f67be38f5 100644
--- a/drivers/net/ethernet/mscc/ocelot_police.c
+++ b/drivers/net/ethernet/mscc/ocelot_police.c
@@ -20,7 +20,7 @@
 /* Default policer order */
 #define POL_ORDER 0x1d3 /* Ocelot policer order: Serial (QoS -> Port -> VCAP) */
 
-int qos_policer_conf_set(struct ocelot *ocelot, int port, u32 pol_ix,
+int qos_policer_conf_set(struct ocelot *ocelot, u32 pol_ix,
 			 struct qos_policer_conf *conf)
 {
 	u32 cf = 0, cir_ena = 0, frm_mode = POL_MODE_LINERATE;
@@ -102,26 +102,30 @@ int qos_policer_conf_set(struct ocelot *ocelot, int port, u32 pol_ix,
 
 	/* Check limits */
 	if (pir > GENMASK(15, 0)) {
-		dev_err(ocelot->dev, "Invalid pir for port %d: %u (max %lu)\n",
-			port, pir, GENMASK(15, 0));
+		dev_err(ocelot->dev,
+			"Invalid pir for policer %u: %u (max %lu)\n",
+			pol_ix, pir, GENMASK(15, 0));
 		return -EINVAL;
 	}
 
 	if (cir > GENMASK(15, 0)) {
-		dev_err(ocelot->dev, "Invalid cir for port %d: %u (max %lu)\n",
-			port, cir, GENMASK(15, 0));
+		dev_err(ocelot->dev,
+			"Invalid cir for policer %u: %u (max %lu)\n",
+			pol_ix, cir, GENMASK(15, 0));
 		return -EINVAL;
 	}
 
 	if (pbs > pbs_max) {
-		dev_err(ocelot->dev, "Invalid pbs for port %d: %u (max %u)\n",
-			port, pbs, pbs_max);
+		dev_err(ocelot->dev,
+			"Invalid pbs for policer %u: %u (max %u)\n",
+			pol_ix, pbs, pbs_max);
 		return -EINVAL;
 	}
 
 	if (cbs > cbs_max) {
-		dev_err(ocelot->dev, "Invalid cbs for port %d: %u (max %u)\n",
-			port, cbs, cbs_max);
+		dev_err(ocelot->dev,
+			"Invalid cbs for policer %u: %u (max %u)\n",
+			pol_ix, cbs, cbs_max);
 		return -EINVAL;
 	}
 
@@ -211,7 +215,7 @@ int ocelot_port_policer_add(struct ocelot *ocelot, int port,
 	dev_dbg(ocelot->dev, "%s: port %u pir %u kbps, pbs %u bytes\n",
 		__func__, port, pp.pir, pp.pbs);
 
-	err = qos_policer_conf_set(ocelot, port, POL_IX_PORT + port, &pp);
+	err = qos_policer_conf_set(ocelot, POL_IX_PORT + port, &pp);
 	if (err)
 		return err;
 
@@ -235,7 +239,7 @@ int ocelot_port_policer_del(struct ocelot *ocelot, int port)
 
 	pp.mode = MSCC_QOS_RATE_MODE_DISABLED;
 
-	err = qos_policer_conf_set(ocelot, port, POL_IX_PORT + port, &pp);
+	err = qos_policer_conf_set(ocelot, POL_IX_PORT + port, &pp);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/mscc/ocelot_police.h b/drivers/net/ethernet/mscc/ocelot_police.h
index 7552995f8b17..0749f23684f2 100644
--- a/drivers/net/ethernet/mscc/ocelot_police.h
+++ b/drivers/net/ethernet/mscc/ocelot_police.h
@@ -31,7 +31,7 @@ struct qos_policer_conf {
 	u8   ipg; /* Size of IPG when MSCC_QOS_RATE_MODE_LINE is chosen */
 };
 
-int qos_policer_conf_set(struct ocelot *ocelot, int port, u32 pol_ix,
+int qos_policer_conf_set(struct ocelot *ocelot, u32 pol_ix,
 			 struct qos_policer_conf *conf);
 
 int ocelot_policer_validate(const struct flow_action *action,
diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index e8445d78a168..30e25d45f08d 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -913,7 +913,7 @@ int ocelot_vcap_policer_add(struct ocelot *ocelot, u32 pol_ix,
 	if (!tmp)
 		return -ENOMEM;
 
-	ret = qos_policer_conf_set(ocelot, 0, pol_ix, &pp);
+	ret = qos_policer_conf_set(ocelot, pol_ix, &pp);
 	if (ret) {
 		kfree(tmp);
 		return ret;
@@ -944,7 +944,7 @@ int ocelot_vcap_policer_del(struct ocelot *ocelot, u32 pol_ix)
 
 	if (z) {
 		pp.mode = MSCC_QOS_RATE_MODE_DISABLED;
-		return qos_policer_conf_set(ocelot, 0, pol_ix, &pp);
+		return qos_policer_conf_set(ocelot, pol_ix, &pp);
 	}
 
 	return 0;
-- 
2.25.1

[PATCH net-next 5/5] net: mscc: ocelot: don't use magic numbers for OCELOT_POLICER_DISCARD

[Vladimir Oltean]

OCELOT_POLICER_DISCARD helps "kill dropped packets dead" since a
PERMIT/DENY mask mode with a port mask of 0 isn't enough to stop the CPU
port from receiving packets removed from the forwarding path.

The hardcoded initialization done for it in ocelot_vcap_init() is
confusing. All we need from it is to have a rate and a burst size of 0.

Reuse qos_policer_conf_set() for that purpose.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_vcap.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index 30e25d45f08d..40afab2c076a 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -1403,22 +1403,18 @@ static void ocelot_vcap_detect_constants(struct ocelot *ocelot,
 
 int ocelot_vcap_init(struct ocelot *ocelot)
 {
-	int i;
+	struct qos_policer_conf cpu_drop = {
+		.mode = MSCC_QOS_RATE_MODE_DATA,
+	};
+	int ret, i;
 
 	/* Create a policer that will drop the frames for the cpu.
 	 * This policer will be used as action in the acl rules to drop
 	 * frames.
 	 */
-	ocelot_write_gix(ocelot, 0x299, ANA_POL_MODE_CFG,
-			 OCELOT_POLICER_DISCARD);
-	ocelot_write_gix(ocelot, 0x1, ANA_POL_PIR_CFG,
-			 OCELOT_POLICER_DISCARD);
-	ocelot_write_gix(ocelot, 0x3fffff, ANA_POL_PIR_STATE,
-			 OCELOT_POLICER_DISCARD);
-	ocelot_write_gix(ocelot, 0x0, ANA_POL_CIR_CFG,
-			 OCELOT_POLICER_DISCARD);
-	ocelot_write_gix(ocelot, 0x3fffff, ANA_POL_CIR_STATE,
-			 OCELOT_POLICER_DISCARD);
+	ret = qos_policer_conf_set(ocelot, OCELOT_POLICER_DISCARD, &cpu_drop);
+	if (ret)
+		return ret;
 
 	for (i = 0; i < OCELOT_NUM_VCAP_BLOCKS; i++) {
 		struct ocelot_vcap_block *block = &ocelot->block[i];
-- 
2.25.1

Re: [PATCH net-next 0/5] Ocelot VCAP cleanups

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  3 May 2022 15:01:45 +0300 you wrote:
> This is a series of minor code cleanups brought to the Ocelot switch
> driver logic for VCAP filters.
> 
> - don't use list_for_each_safe() in ocelot_vcap_filter_add_to_block
> - don't use magic numbers for OCELOT_POLICER_DISCARD
> 
> Vladimir Oltean (5):
>   net: mscc: ocelot: use list_add_tail in
>     ocelot_vcap_filter_add_to_block()
>   net: mscc: ocelot: add to tail of empty list in
>     ocelot_vcap_filter_add_to_block
>   net: mscc: ocelot: use list_for_each_entry in
>     ocelot_vcap_filter_add_to_block
>   net: mscc: ocelot: drop port argument from qos_policer_conf_set
>   net: mscc: ocelot: don't use magic numbers for OCELOT_POLICER_DISCARD
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] net: mscc: ocelot: use list_add_tail in ocelot_vcap_filter_add_to_block()
    https://git.kernel.org/netdev/net-next/c/0a448bba5009
  - [net-next,2/5] net: mscc: ocelot: add to tail of empty list in ocelot_vcap_filter_add_to_block
    https://git.kernel.org/netdev/net-next/c/3825a0d02748
  - [net-next,3/5] net: mscc: ocelot: use list_for_each_entry in ocelot_vcap_filter_add_to_block
    https://git.kernel.org/netdev/net-next/c/09fd1e0d1481
  - [net-next,4/5] net: mscc: ocelot: drop port argument from qos_policer_conf_set
    https://git.kernel.org/netdev/net-next/c/8e90c499bd68
  - [net-next,5/5] net: mscc: ocelot: don't use magic numbers for OCELOT_POLICER_DISCARD
    https://git.kernel.org/netdev/net-next/c/91d350d661bf

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html