[PATCH 0/4] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[PATCH 0/4] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Lukasz Majewski]

This patch series provides support for HSR HW offloading in KSZ9477
switch IC.

To test this feature:
ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
ifconfig lan1 up;ifconfig lan2 up
ifconfig hsr0 192.168.0.1 up

To remove HSR network device:
ip link del hsr0

Test HW:
Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".

Lukasz Majewski (4):
  net: dsa: Extend the dsa_switch structure to hold info about HSR ports
  net: dsa: Extend ksz9477 TAG setup to support HSR frames duplication
  net: dsa: hsr: Enable in KSZ9477 switch HW HSR offloading
  net: dsa: hsr: Provide generic HSR ksz_hsr_{join|leave} functions

 drivers/net/dsa/microchip/ksz9477.c    | 96 ++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz9477.h    |  4 ++
 drivers/net/dsa/microchip/ksz_common.c | 69 ++++++++++++++++++
 include/net/dsa.h                      |  3 +
 net/dsa/tag_ksz.c                      |  5 ++
 5 files changed, 177 insertions(+)

-- 
2.20.1

[PATCH 1/4] net: dsa: Extend the dsa_switch structure to hold info about HSR ports

[Lukasz Majewski]

Information about HSR aware ports in a DSA switch can be helpful when
one needs tags to be adjusted before the HSR frame is sent.

For example - with ksz9477 switch - the TAG needs to be adjusted to have
both HSR ports marked in tag to allow execution of HW based frame
duplication.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 include/net/dsa.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index d309ee7ed04b..15274afc42bb 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -470,6 +470,9 @@ struct dsa_switch {
 	/* Number of switch port queues */
 	unsigned int		num_tx_queues;
 
+	/* Bitmask indicating ports supporting HSR */
+	u16                     hsr_ports;
+
 	/* Drivers that benefit from having an ID associated with each
 	 * offloaded LAG should set this to the maximum number of
 	 * supported IDs. DSA will then maintain a mapping of _at
-- 
2.20.1

Re: [PATCH 1/4] net: dsa: Extend the dsa_switch structure to hold info about HSR ports

[Paolo Abeni]

On Tue, 2023-08-29 at 14:11 +0200, Lukasz Majewski wrote:
> Information about HSR aware ports in a DSA switch can be helpful when
> one needs tags to be adjusted before the HSR frame is sent.
> 
> For example - with ksz9477 switch - the TAG needs to be adjusted to have
> both HSR ports marked in tag to allow execution of HW based frame
> duplication.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
>  include/net/dsa.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index d309ee7ed04b..15274afc42bb 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -470,6 +470,9 @@ struct dsa_switch {
>  	/* Number of switch port queues */
>  	unsigned int		num_tx_queues;
>  
> +	/* Bitmask indicating ports supporting HSR */
> +	u16                     hsr_ports;
> +
>  	/* Drivers that benefit from having an ID associated with each
>  	 * offloaded LAG should set this to the maximum number of
>  	 * supported IDs. DSA will then maintain a mapping of _at

Out of sheer ignorance, I think this new field does not belong to
dsa_switch, at least not in this form. AFAICS there is no current hard
limitation on the number of ports a DSA switch can handle at the API
level, and this will introduce an hard one.

I think you are better off keeping this field in the KSZ-specific
struct. If you really want to keep it here you should remove the above
limitation somehow (possibly a query op to check if a given port is HSR
aware???)

In any case this series looks like net-next material, does not apply
correctly to net-next and net-next is currently closed. You can share a
new version as RFC or wait for net-next to re-open in ~2w.

Cheers,

Paolo

Re: [PATCH 1/4] net: dsa: Extend the dsa_switch structure to hold info about HSR ports

[Lukasz Majewski]

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

Hi Paolo,

> On Tue, 2023-08-29 at 14:11 +0200, Lukasz Majewski wrote:
> > Information about HSR aware ports in a DSA switch can be helpful
> > when one needs tags to be adjusted before the HSR frame is sent.
> > 
> > For example - with ksz9477 switch - the TAG needs to be adjusted to
> > have both HSR ports marked in tag to allow execution of HW based
> > frame duplication.
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> >  include/net/dsa.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/include/net/dsa.h b/include/net/dsa.h
> > index d309ee7ed04b..15274afc42bb 100644
> > --- a/include/net/dsa.h
> > +++ b/include/net/dsa.h
> > @@ -470,6 +470,9 @@ struct dsa_switch {
> >  	/* Number of switch port queues */
> >  	unsigned int		num_tx_queues;
> >  
> > +	/* Bitmask indicating ports supporting HSR */
> > +	u16                     hsr_ports;
> > +
> >  	/* Drivers that benefit from having an ID associated with
> > each
> >  	 * offloaded LAG should set this to the maximum number of
> >  	 * supported IDs. DSA will then maintain a mapping of _at  
> 
> Out of sheer ignorance, I think this new field does not belong to
> dsa_switch, at least not in this form. AFAICS there is no current hard
> limitation on the number of ports a DSA switch can handle at the API
> level, and this will introduce an hard one.
> 
> I think you are better off keeping this field in the KSZ-specific
> struct.

That was mine first idea - to move it to struct ksz_device from
./drivers/net/dsa/ksz_common.h

However, this file and this struct is not easily accessible from
net/dsa/tag_ksz.c

One idea was to use an exported function - e.g. ksz_get_hsr_ports() and
in it I would read the hsr_ports member ?

Another option would be to loop through all switch ports with
hsr_for_each_port(), but this would affect overall network performance.

> If you really want to keep it here you should remove the above
> limitation somehow (possibly a query op to check if a given port is
> HSR aware???)

I will use the idea of exported helper function to get HSR members
ports.

> 
> In any case this series looks like net-next material, does not apply
> correctly to net-next and net-next is currently closed. You can share
> a new version as RFC or wait for net-next to re-open in ~2w.
> 

I will prepare v2 and then adjust it to net-next when required.

> Cheers,
> 
> Paolo
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 2/4] net: dsa: Extend ksz9477 TAG setup to support HSR frames duplication

[Lukasz Majewski]

The KSZ9477 has support for HSR (High-Availability Seamless Redundancy).
One of its offloading (i.e. performed in the switch IC hardware) features
is to duplicate received frame to both HSR aware switch ports.

To achieve this goal - the tail TAG needs to be modified. To be more
specific, both ports must be marked as destination (egress) ones.

Moreover, according to AN3474 application note, the lookup bit (10)
should not be set in the tail tag.

Last but not least - the NETIF_F_HW_HSR_DUP flag indicates that the device
supports HSR and assures (in HSR core code) that frame is sent only once
from HOST to switch with tail tag indicating both ports.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 net/dsa/tag_ksz.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index ea100bd25939..1410e7ed0f9a 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -293,6 +293,11 @@ static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
 	if (is_link_local_ether_addr(hdr->h_dest))
 		val |= KSZ9477_TAIL_TAG_OVERRIDE;
 
+	if (dev->features & NETIF_F_HW_HSR_DUP) {
+		val &= ~KSZ9477_TAIL_TAG_LOOKUP;
+		val |= dp->ds->hsr_ports;
+	}
+
 	*tag = cpu_to_be16(val);
 
 	return ksz_defer_xmit(dp, skb);
-- 
2.20.1

[PATCH 3/4] net: dsa: hsr: Enable in KSZ9477 switch HW HSR offloading

[Lukasz Majewski]

This patch adds functions for providing in KSZ9477 switch HSR
(High-availability Seamless Redundancy) hardware offloading.

According to AN3474 application note following features are provided:
- TX packet duplication from host to switch (NETIF_F_HW_HSR_DUP)
- RX packet duplication discarding
- Prevention of packet loop

For last two ones - there is a probability that some packets will not
be filtered in HW (in some special cases). Hence, the HSR core code
shall be used to discard those not caught frames.

Moreover, some switch registers adjustments are required - like setting
MAC address of HSR network interface.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/net/dsa/microchip/ksz9477.c | 96 +++++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz9477.h |  4 ++
 2 files changed, 100 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 2bf88c2e32a0..bf84c70e675d 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1207,6 +1207,102 @@ int ksz9477_tc_cbs_set_cinc(struct ksz_device *dev, int port, u32 val)
 	return ksz_pwrite16(dev, port, REG_PORT_MTI_CREDIT_INCREMENT, val);
 }
 
+/* The KSZ9477 provides following HW features to accelerate
+ * HSR frames handling:
+ *
+ * 1. TX PACKET DUPLICATION FROM HOST TO SWITCH
+ * 2. RX PACKET DUPLICATION DISCARDING
+ * 3. PREVENTING PACKET LOOP IN THE RING BY SELF-ADDRESS FILTERING
+ *
+ * Only one from point 1. has the NETIF_F* flag available.
+ *
+ * Ones from point 2 and 3 are "best effort" - i.e. those will
+ * work correctly most of the time, but it may happen that some
+ * frames will not be caught. Hence, the SW needs to handle those
+ * special cases. However, the speed up gain is considerable when
+ * above features are used.
+ */
+#define KSZ9477_SUPPORTED_HSR_FEATURES NETIF_F_HW_HSR_DUP
+
+int ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr,
+		     struct dsa_port *partner)
+{
+	struct ksz_device *dev = ds->priv;
+	struct net_device *slave;
+	u8 i, data;
+	int ret;
+
+	/* Program which ports shall support HSR */
+	ds->hsr_ports = BIT(port) | BIT(partner->index);
+	ksz_write32(dev, REG_HSR_PORT_MAP__4, ds->hsr_ports);
+
+	/* Enable discarding of received HSR frames */
+	ksz_read8(dev, REG_HSR_ALU_CTRL_0__1, &data);
+	data |= HSR_DUPLICATE_DISCARD;
+	data &= ~HSR_NODE_UNICAST;
+	ksz_write8(dev, REG_HSR_ALU_CTRL_0__1, data);
+
+	/* Self MAC address filtering for HSR frames to avoid
+	 * traverse of the HSR ring more than once.
+	 *
+	 * The HSR port (i.e. hsr0) MAC address is used.
+	 */
+	if (!is_valid_ether_addr(hsr->dev_addr)) {
+		dev_err(dev->dev,
+			"Set valid MAC address to %s for HSR operation!",
+			hsr->name);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ETH_ALEN; i++) {
+		ret = ksz_write8(dev, REG_SW_MAC_ADDR_0 + i, hsr->dev_addr[i]);
+		if (ret)
+			return ret;
+	}
+
+	/* Enable global self-address filtering if not yet done during switch
+	 * start
+	 */
+	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data);
+	if (!(data & SW_SRC_ADDR_FILTER)) {
+		data |= SW_SRC_ADDR_FILTER;
+		ksz_write8(dev, REG_SW_LUE_CTRL_1, data);
+	}
+
+	/* Enable per port self-address filtering */
+	ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, true);
+	ksz_port_cfg(dev, partner->index, REG_PORT_LUE_CTRL,
+		     PORT_SRC_ADDR_FILTER, true);
+
+	/* Setup HW supported features for lan HSR ports */
+	slave = dsa_to_port(ds, port)->slave;
+	slave->features |= KSZ9477_SUPPORTED_HSR_FEATURES;
+
+	slave = dsa_to_port(ds, partner->index)->slave;
+	slave->features |= KSZ9477_SUPPORTED_HSR_FEATURES;
+
+	pr_debug("%s: HSR join port: %d partner: %d port_map: 0x%x\n", __func__,
+		 port, partner->index, ds->hsr_ports);
+
+	return 0;
+}
+
+int ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr,
+		      struct dsa_port *partner)
+{
+	struct ksz_device *dev = ds->priv;
+
+	/* Clear ports HSR support */
+	ksz_write32(dev, REG_HSR_PORT_MAP__4, 0);
+
+	/* Disable per port self-address filtering */
+	ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, false);
+	ksz_port_cfg(dev, partner->index, REG_PORT_LUE_CTRL,
+		     PORT_SRC_ADDR_FILTER, false);
+
+	return 0;
+}
+
 int ksz9477_switch_init(struct ksz_device *dev)
 {
 	u8 data8;
diff --git a/drivers/net/dsa/microchip/ksz9477.h b/drivers/net/dsa/microchip/ksz9477.h
index b6f7e3c46e3f..634262efb73c 100644
--- a/drivers/net/dsa/microchip/ksz9477.h
+++ b/drivers/net/dsa/microchip/ksz9477.h
@@ -58,5 +58,9 @@ int ksz9477_dsa_init(struct ksz_device *dev);
 int ksz9477_switch_init(struct ksz_device *dev);
 void ksz9477_switch_exit(struct ksz_device *dev);
 void ksz9477_port_queue_split(struct ksz_device *dev, int port);
+int ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr,
+		     struct dsa_port *partner);
+int ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr,
+		      struct dsa_port *partner);
 
 #endif
-- 
2.20.1

[PATCH 4/4] net: dsa: hsr: Provide generic HSR ksz_hsr_{join|leave} functions

[Lukasz Majewski]

This patch provides the common KSZ (i.e. Microchip) DSA code with support
for HSR aware devices.

To be more specific - generic ksz_hsr_{join|leave} functions are provided,
now only supporting KSZ9477 IC.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/net/dsa/microchip/ksz_common.c | 69 ++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 6c0623f88654..d53ca28583cb 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -16,6 +16,7 @@
 #include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
 #include <linux/if_vlan.h>
+#include <linux/if_hsr.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/of_mdio.h>
@@ -3407,6 +3408,72 @@ static int ksz_setup_tc(struct dsa_switch *ds, int port,
 	}
 }
 
+static int ksz_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr)
+{
+	struct dsa_port *partner = NULL, *dp;
+	struct ksz_device *dev = ds->priv;
+	enum hsr_version ver;
+	int ret;
+
+	ret = hsr_get_version(hsr, &ver);
+	if (ret)
+		return ret;
+
+	switch (dev->chip_id) {
+	case KSZ9477_CHIP_ID:
+		if (ver == PRP_V1)
+			return -EOPNOTSUPP;
+	}
+
+	/* We can't enable redundancy on the switch until both
+	 * redundant ports have signed up.
+	 */
+	dsa_hsr_foreach_port(dp, ds, hsr) {
+		if (dp->index != port) {
+			partner = dp;
+			break;
+		}
+	}
+
+	if (!partner)
+		return 0;
+
+	switch (dev->chip_id) {
+	case KSZ9477_CHIP_ID:
+		return ksz9477_hsr_join(ds, port, hsr, partner);
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int ksz_hsr_leave(struct dsa_switch *ds, int port,
+			 struct net_device *hsr)
+{
+	struct dsa_port *partner = NULL, *dp;
+	struct ksz_device *dev = ds->priv;
+
+	dsa_hsr_foreach_port(dp, ds, hsr) {
+		if (dp->index != port) {
+			partner = dp;
+			break;
+		}
+	}
+
+	if (!partner)
+		return 0;
+
+	switch (dev->chip_id) {
+	case KSZ9477_CHIP_ID:
+		return ksz9477_hsr_leave(ds, port, hsr, partner);
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static const struct dsa_switch_ops ksz_switch_ops = {
 	.get_tag_protocol	= ksz_get_tag_protocol,
 	.connect_tag_protocol   = ksz_connect_tag_protocol,
@@ -3426,6 +3493,8 @@ static const struct dsa_switch_ops ksz_switch_ops = {
 	.get_sset_count		= ksz_sset_count,
 	.port_bridge_join	= ksz_port_bridge_join,
 	.port_bridge_leave	= ksz_port_bridge_leave,
+	.port_hsr_join		= ksz_hsr_join,
+	.port_hsr_leave		= ksz_hsr_leave,
 	.port_stp_state_set	= ksz_port_stp_state_set,
 	.port_pre_bridge_flags	= ksz_port_pre_bridge_flags,
 	.port_bridge_flags	= ksz_port_bridge_flags,
-- 
2.20.1