[PATCH net-next 0/7] Cleanups for ocelot/felix drivers

[PATCH net-next 0/7] Cleanups for ocelot/felix drivers

[Vladimir Oltean]

This patch set is an assorted collection of minor cleanups brought to
the felix DSA driver and ocelot switch library.

Vladimir Oltean (7):
  net: mscc: ocelot: use list_for_each_entry in
    ocelot_vcap_block_remove_filter
  net: mscc: ocelot: use pretty names for IPPROTO_UDP and IPPROTO_TCP
  net: dsa: felix: remove ocelot->npi assignment from
    felix_8021q_cpu_port_init
  net: dsa: felix: drop the ptp_type argument from felix_check_xtr_pkt()
  net: dsa: felix: initialize "err" to 0 in felix_check_xtr_pkt()
  net: dsa: felix: print error message in felix_check_xtr_pkt()
  net: dsa: felix: remove redundant assignment in
    felix_8021q_cpu_port_deinit

 drivers/net/dsa/ocelot/felix.c          | 17 ++++++++---------
 drivers/net/ethernet/mscc/ocelot_vcap.c | 12 +++++-------
 2 files changed, 13 insertions(+), 16 deletions(-)

-- 
2.25.1

[PATCH net-next 1/7] net: mscc: ocelot: use list_for_each_entry in ocelot_vcap_block_remove_filter

[Vladimir Oltean]

Simplify ocelot_vcap_block_remove_filter by using list_for_each_entry
instead of list_for_each.

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

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index 852054da9db9..0972c6d58c48 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -1195,18 +1195,16 @@ static void ocelot_vcap_block_remove_filter(struct ocelot *ocelot,
 					    struct ocelot_vcap_block *block,
 					    struct ocelot_vcap_filter *filter)
 {
-	struct ocelot_vcap_filter *tmp;
-	struct list_head *pos, *q;
+	struct ocelot_vcap_filter *tmp, *n;
 
-	list_for_each_safe(pos, q, &block->rules) {
-		tmp = list_entry(pos, struct ocelot_vcap_filter, list);
+	list_for_each_entry_safe(tmp, n, &block->rules, list) {
 		if (ocelot_vcap_filter_equal(filter, tmp)) {
 			if (tmp->block_id == VCAP_IS2 &&
 			    tmp->action.police_ena)
 				ocelot_vcap_policer_del(ocelot,
 							tmp->action.pol_ix);
 
-			list_del(pos);
+			list_del(&tmp->list);
 			kfree(tmp);
 		}
 	}
-- 
2.25.1

[PATCH net-next 2/7] net: mscc: ocelot: use pretty names for IPPROTO_UDP and IPPROTO_TCP

[Vladimir Oltean]

Hardcoding these IP protocol numbers in is2_entry_set() obscures the
purpose of the code, so replace the magic numbers with the definitions
from linux/in.h.

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

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index 0972c6d58c48..b976d480aeb3 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -564,9 +564,9 @@ static void is2_entry_set(struct ocelot *ocelot, int ix,
 		val = proto.value[0];
 		msk = proto.mask[0];
 		type = IS2_TYPE_IP_UDP_TCP;
-		if (msk == 0xff && (val == 6 || val == 17)) {
+		if (msk == 0xff && (val == IPPROTO_TCP || val == IPPROTO_UDP)) {
 			/* UDP/TCP protocol match */
-			tcp = (val == 6 ?
+			tcp = (val == IPPROTO_TCP ?
 			       OCELOT_VCAP_BIT_1 : OCELOT_VCAP_BIT_0);
 			vcap_key_bit_set(vcap, &data, VCAP_IS2_HK_TCP, tcp);
 			vcap_key_l4_port_set(vcap, &data,
-- 
2.25.1

[PATCH net-next 3/7] net: dsa: felix: remove ocelot->npi assignment from felix_8021q_cpu_port_init

[Vladimir Oltean]

This assignment is redundant, since ocelot->npi has already been set to
-1 by felix_npi_port_deinit(). Call path:

felix_change_tag_protocol
-> felix_del_tag_protocol(DSA_TAG_PROTO_OCELOT)
   -> felix_teardown_tag_npi
      -> felix_npi_port_deinit
-> felix_set_tag_protocol(DSA_TAG_PROTO_OCELOT_8021Q)
   -> felix_setup_tag_8021q
      -> felix_8021q_cpu_port_init

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index badb5b9ba790..6ddfe6fb43c0 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -235,7 +235,6 @@ static void felix_8021q_cpu_port_init(struct ocelot *ocelot, int port)
 	mutex_lock(&ocelot->fwd_domain_lock);
 
 	ocelot_port_set_dsa_8021q_cpu(ocelot, port);
-	ocelot->npi = -1;
 
 	/* Overwrite PGID_CPU with the non-tagging port */
 	ocelot_write_rix(ocelot, BIT(port), ANA_PGID_PGID, PGID_CPU);
-- 
2.25.1

[PATCH net-next 4/7] net: dsa: felix: drop the ptp_type argument from felix_check_xtr_pkt()

[Vladimir Oltean]

The DSA ->port_rxtstamp() function is never called for PTP_CLASS_NONE:

dsa_skb_defer_rx_timestamp:

	if (type == PTP_CLASS_NONE)
		return false;

	if (likely(ds->ops->port_rxtstamp))
		return ds->ops->port_rxtstamp(ds, p->dp->index, skb, type);

So practically, the argument is unused, so remove it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 6ddfe6fb43c0..24963335f17e 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1331,7 +1331,7 @@ static int felix_hwtstamp_set(struct dsa_switch *ds, int port,
 	return felix_update_trapping_destinations(ds, using_tag_8021q);
 }
 
-static bool felix_check_xtr_pkt(struct ocelot *ocelot, unsigned int ptp_type)
+static bool felix_check_xtr_pkt(struct ocelot *ocelot)
 {
 	struct felix *felix = ocelot_to_felix(ocelot);
 	int err, grp = 0;
@@ -1342,9 +1342,6 @@ static bool felix_check_xtr_pkt(struct ocelot *ocelot, unsigned int ptp_type)
 	if (!felix->info->quirk_no_xtr_irq)
 		return false;
 
-	if (ptp_type == PTP_CLASS_NONE)
-		return false;
-
 	while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp)) {
 		struct sk_buff *skb;
 		unsigned int type;
@@ -1395,7 +1392,7 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int port,
 	 * MMIO in the CPU port module, and inject that into the stack from
 	 * ocelot_xtr_poll().
 	 */
-	if (felix_check_xtr_pkt(ocelot, type)) {
+	if (felix_check_xtr_pkt(ocelot)) {
 		kfree_skb(skb);
 		return true;
 	}
-- 
2.25.1

[PATCH net-next 5/7] net: dsa: felix: initialize "err" to 0 in felix_check_xtr_pkt()

[Vladimir Oltean]

Automated tools complain that felix_check_xtr_pkt() has logic to drain
the CPU queue on the reception of a PTP packet over Ethernet, yet it
returns an uninitialized error code in the case where the CPU queue was
empty.

This is not likely to happen (/possible if hardware works correctly),
but it isn't a fatal condition either. The PTP packet will be dequeued
from the CPU queue when the next PTP packet arrives. So initialize "err"
to 0 for the case where nothing was dequeued during this iteration.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 24963335f17e..5c4eecf3994a 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1334,7 +1334,7 @@ static int felix_hwtstamp_set(struct dsa_switch *ds, int port,
 static bool felix_check_xtr_pkt(struct ocelot *ocelot)
 {
 	struct felix *felix = ocelot_to_felix(ocelot);
-	int err, grp = 0;
+	int err = 0, grp = 0;
 
 	if (felix->tag_proto != DSA_TAG_PROTO_OCELOT_8021Q)
 		return false;
-- 
2.25.1

[PATCH net-next 6/7] net: dsa: felix: print error message in felix_check_xtr_pkt()

[Vladimir Oltean]

Packet extraction failures over register-based MMIO are silent, and
difficult to pinpoint. Add an error message to remedy this.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 5c4eecf3994a..25eb57058ce0 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1371,8 +1371,12 @@ static bool felix_check_xtr_pkt(struct ocelot *ocelot)
 	}
 
 out:
-	if (err < 0)
+	if (err < 0) {
+		dev_err_ratelimited(ocelot->dev,
+				    "Error during packet extraction: %pe\n",
+				    ERR_PTR(err));
 		ocelot_drain_cpu_queue(ocelot, 0);
+	}
 
 	return true;
 }
-- 
2.25.1

[PATCH net-next 7/7] net: dsa: felix: remove redundant assignment in felix_8021q_cpu_port_deinit

[Vladimir Oltean]

Due to an apparently incorrect conflict resolution on my part in commit
54c319846086 ("net: mscc: ocelot: enforce FDB isolation when
VLAN-unaware"), "ocelot->ports[port]->is_dsa_8021q_cpu = false" was
supposed to be replaced by "ocelot_port_unset_dsa_8021q_cpu(ocelot, port)"
which does the same thing, and more. But now we have both, so the direct
assignment is redundant. Remove it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 25eb57058ce0..5a6f83fa4d96 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -248,7 +248,6 @@ static void felix_8021q_cpu_port_deinit(struct ocelot *ocelot, int port)
 {
 	mutex_lock(&ocelot->fwd_domain_lock);
 
-	ocelot->ports[port]->is_dsa_8021q_cpu = false;
 	ocelot_port_unset_dsa_8021q_cpu(ocelot, port);
 
 	/* Restore PGID_CPU */
-- 
2.25.1

Re: [PATCH net-next 0/7] Cleanups for ocelot/felix drivers

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu,  3 Mar 2022 16:01:19 +0200 you wrote:
> This patch set is an assorted collection of minor cleanups brought to
> the felix DSA driver and ocelot switch library.
> 
> Vladimir Oltean (7):
>   net: mscc: ocelot: use list_for_each_entry in
>     ocelot_vcap_block_remove_filter
>   net: mscc: ocelot: use pretty names for IPPROTO_UDP and IPPROTO_TCP
>   net: dsa: felix: remove ocelot->npi assignment from
>     felix_8021q_cpu_port_init
>   net: dsa: felix: drop the ptp_type argument from felix_check_xtr_pkt()
>   net: dsa: felix: initialize "err" to 0 in felix_check_xtr_pkt()
>   net: dsa: felix: print error message in felix_check_xtr_pkt()
>   net: dsa: felix: remove redundant assignment in
>     felix_8021q_cpu_port_deinit
> 
> [...]

Here is the summary with links:
  - [net-next,1/7] net: mscc: ocelot: use list_for_each_entry in ocelot_vcap_block_remove_filter
    https://git.kernel.org/netdev/net-next/c/c5a0edaeb9e1
  - [net-next,2/7] net: mscc: ocelot: use pretty names for IPPROTO_UDP and IPPROTO_TCP
    https://git.kernel.org/netdev/net-next/c/c3cde44f3c6e
  - [net-next,3/7] net: dsa: felix: remove ocelot->npi assignment from felix_8021q_cpu_port_init
    https://git.kernel.org/netdev/net-next/c/28c1305b0b72
  - [net-next,4/7] net: dsa: felix: drop the ptp_type argument from felix_check_xtr_pkt()
    https://git.kernel.org/netdev/net-next/c/d219b4b674e9
  - [net-next,5/7] net: dsa: felix: initialize "err" to 0 in felix_check_xtr_pkt()
    https://git.kernel.org/netdev/net-next/c/dbd032856ba3
  - [net-next,6/7] net: dsa: felix: print error message in felix_check_xtr_pkt()
    https://git.kernel.org/netdev/net-next/c/5d3bb7dda43a
  - [net-next,7/7] net: dsa: felix: remove redundant assignment in felix_8021q_cpu_port_deinit
    https://git.kernel.org/netdev/net-next/c/162fbf6a2f95

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