[PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[PATCH v6 net-next 0/5] 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
ip link set dev lan1 up
ip link set dev lan2 up
ip a add 192.168.0.1/24 dev hsr0
ip link set dev hsr0 up

To remove HSR network device:
ip link del hsr0

To test if one can adjust MAC address:
ip link set lan2 address 00:01:02:AA:BB:CC

It is also possible to create another HSR interface, but it will
only support HSR is software - e.g.
ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision 45 version 1

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

Performance SW used:
nuttcp -S --nofork
nuttcp -vv -T 60 -r 192.168.0.2
nuttcp -vv -T 60 -t 192.168.0.2

Code: v6.6.0-rc2+ Linux net-next repository
SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172

Tested HSR v0 and v1
Results:
With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
With no offloading 		       RX: 63 Mbps  TX: 63 Mbps


Lukasz Majewski (2):
  net: dsa: tag_ksz: Extend ksz9477_xmit() for HSR frame duplication
  net: dsa: microchip: Enable HSR offloading for KSZ9477

Vladimir Oltean (3):
  net: dsa: propagate extack to ds->ops->port_hsr_join()
  net: dsa: notify drivers of MAC address changes on user ports
  net: dsa: microchip: move REG_SW_MAC_ADDR to dev->info->regs[]

 drivers/net/dsa/microchip/ksz8795_reg.h |   7 --
 drivers/net/dsa/microchip/ksz9477.c     |  77 ++++++++++++
 drivers/net/dsa/microchip/ksz9477.h     |   2 +
 drivers/net/dsa/microchip/ksz9477_reg.h |   7 --
 drivers/net/dsa/microchip/ksz_common.c  | 149 ++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h  |  10 ++
 drivers/net/dsa/xrs700x/xrs700x.c       |  18 ++-
 include/net/dsa.h                       |  13 ++-
 net/dsa/port.c                          |   5 +-
 net/dsa/port.h                          |   3 +-
 net/dsa/slave.c                         |   9 +-
 net/dsa/tag_ksz.c                       |   8 ++
 12 files changed, 283 insertions(+), 25 deletions(-)

-- 
2.20.1

[PATCH v6 net-next 1/5] net: dsa: propagate extack to ds->ops->port_hsr_join()

[Lukasz Majewski]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Drivers can provide meaningful error messages which state a reason why
they can't perform an offload, and dsa_slave_changeupper() already has
the infrastructure to propagate these over netlink rather than printing
to the kernel log. So pass the extack argument and modify the xrs700x
driver's port_hsr_join() prototype.

Also take the opportunity and use the extack for the 2 -EOPNOTSUPP cases
from xrs700x_hsr_join().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
Changes for v5:
- New patch
Changes for v6:
- None
---
 drivers/net/dsa/xrs700x/xrs700x.c | 18 ++++++++++++------
 include/net/dsa.h                 |  3 ++-
 net/dsa/port.c                    |  5 +++--
 net/dsa/port.h                    |  3 ++-
 net/dsa/slave.c                   |  2 +-
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 753fef757f11..5b02e9e426fd 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -548,7 +548,8 @@ static void xrs700x_bridge_leave(struct dsa_switch *ds, int port,
 }
 
 static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
-			    struct net_device *hsr)
+			    struct net_device *hsr,
+			    struct netlink_ext_ack *extack)
 {
 	unsigned int val = XRS_HSR_CFG_HSR_PRP;
 	struct dsa_port *partner = NULL, *dp;
@@ -562,16 +563,21 @@ static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
 	if (ret)
 		return ret;
 
-	/* Only ports 1 and 2 can be HSR/PRP redundant ports. */
-	if (port != 1 && port != 2)
+	if (port != 1 && port != 2) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Only ports 1 and 2 can offload HSR/PRP");
 		return -EOPNOTSUPP;
+	}
 
-	if (ver == HSR_V1)
+	if (ver == HSR_V1) {
 		val |= XRS_HSR_CFG_HSR;
-	else if (ver == PRP_V1)
+	} else if (ver == PRP_V1) {
 		val |= XRS_HSR_CFG_PRP;
-	else
+	} else {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Only HSR v1 and PRP v1 can be offloaded");
 		return -EOPNOTSUPP;
+	}
 
 	dsa_hsr_foreach_port(dp, ds, hsr) {
 		if (dp->index != port) {
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 0b9c6aa27047..426724808e76 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1198,7 +1198,8 @@ struct dsa_switch_ops {
 	 * HSR integration
 	 */
 	int	(*port_hsr_join)(struct dsa_switch *ds, int port,
-				 struct net_device *hsr);
+				 struct net_device *hsr,
+				 struct netlink_ext_ack *extack);
 	int	(*port_hsr_leave)(struct dsa_switch *ds, int port,
 				  struct net_device *hsr);
 
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 37ab238e8304..5f01bd4f9dec 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -2024,7 +2024,8 @@ void dsa_shared_port_link_unregister_of(struct dsa_port *dp)
 		dsa_shared_port_setup_phy_of(dp, false);
 }
 
-int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
+int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr,
+		      struct netlink_ext_ack *extack)
 {
 	struct dsa_switch *ds = dp->ds;
 	int err;
@@ -2034,7 +2035,7 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
 
 	dp->hsr_dev = hsr;
 
-	err = ds->ops->port_hsr_join(ds, dp->index, hsr);
+	err = ds->ops->port_hsr_join(ds, dp->index, hsr, extack);
 	if (err)
 		dp->hsr_dev = NULL;
 
diff --git a/net/dsa/port.h b/net/dsa/port.h
index dc812512fd0e..334879964e2c 100644
--- a/net/dsa/port.h
+++ b/net/dsa/port.h
@@ -103,7 +103,8 @@ int dsa_port_phylink_create(struct dsa_port *dp);
 void dsa_port_phylink_destroy(struct dsa_port *dp);
 int dsa_shared_port_link_register_of(struct dsa_port *dp);
 void dsa_shared_port_link_unregister_of(struct dsa_port *dp);
-int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr);
+int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr,
+		      struct netlink_ext_ack *extack);
 void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
 int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
 void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 48db91b33390..2b3d89b77121 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2862,7 +2862,7 @@ static int dsa_slave_changeupper(struct net_device *dev,
 		}
 	} else if (is_hsr_master(info->upper_dev)) {
 		if (info->linking) {
-			err = dsa_port_hsr_join(dp, info->upper_dev);
+			err = dsa_port_hsr_join(dp, info->upper_dev, extack);
 			if (err == -EOPNOTSUPP) {
 				NL_SET_ERR_MSG_WEAK_MOD(extack,
 							"Offloading not supported");
-- 
2.20.1

Re: [PATCH v6 net-next 1/5] net: dsa: propagate extack to ds->ops->port_hsr_join()

[Vladimir Oltean]

On Fri, Sep 22, 2023 at 03:31:04PM +0200, Lukasz Majewski wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Drivers can provide meaningful error messages which state a reason why
> they can't perform an offload, and dsa_slave_changeupper() already has
> the infrastructure to propagate these over netlink rather than printing
> to the kernel log. So pass the extack argument and modify the xrs700x
> driver's port_hsr_join() prototype.
> 
> Also take the opportunity and use the extack for the 2 -EOPNOTSUPP cases
> from xrs700x_hsr_join().
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> Changes for v5:
> - New patch
> Changes for v6:
> - None
> ---
>  drivers/net/dsa/xrs700x/xrs700x.c | 18 ++++++++++++------
>  include/net/dsa.h                 |  3 ++-
>  net/dsa/port.c                    |  5 +++--
>  net/dsa/port.h                    |  3 ++-
>  net/dsa/slave.c                   |  2 +-
>  5 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
> index 753fef757f11..5b02e9e426fd 100644
> --- a/drivers/net/dsa/xrs700x/xrs700x.c
> +++ b/drivers/net/dsa/xrs700x/xrs700x.c
> @@ -548,7 +548,8 @@ static void xrs700x_bridge_leave(struct dsa_switch *ds, int port,
>  }
>  
>  static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
> -			    struct net_device *hsr)
> +			    struct net_device *hsr,
> +			    struct netlink_ext_ack *extack)
>  {
>  	unsigned int val = XRS_HSR_CFG_HSR_PRP;
>  	struct dsa_port *partner = NULL, *dp;
> @@ -562,16 +563,21 @@ static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
>  	if (ret)
>  		return ret;
>  
> -	/* Only ports 1 and 2 can be HSR/PRP redundant ports. */
> -	if (port != 1 && port != 2)
> +	if (port != 1 && port != 2) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Only ports 1 and 2 can offload HSR/PRP");
>  		return -EOPNOTSUPP;
> +	}
>  
> -	if (ver == HSR_V1)
> +	if (ver == HSR_V1) {
>  		val |= XRS_HSR_CFG_HSR;
> -	else if (ver == PRP_V1)
> +	} else if (ver == PRP_V1) {
>  		val |= XRS_HSR_CFG_PRP;
> -	else
> +	} else {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Only HSR v1 and PRP v1 can be offloaded");
>  		return -EOPNOTSUPP;
> +	}
>  
>  	dsa_hsr_foreach_port(dp, ds, hsr) {
>  		if (dp->index != port) {
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 0b9c6aa27047..426724808e76 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -1198,7 +1198,8 @@ struct dsa_switch_ops {
>  	 * HSR integration
>  	 */
>  	int	(*port_hsr_join)(struct dsa_switch *ds, int port,
> -				 struct net_device *hsr);
> +				 struct net_device *hsr,
> +				 struct netlink_ext_ack *extack);
>  	int	(*port_hsr_leave)(struct dsa_switch *ds, int port,
>  				  struct net_device *hsr);
>  
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 37ab238e8304..5f01bd4f9dec 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -2024,7 +2024,8 @@ void dsa_shared_port_link_unregister_of(struct dsa_port *dp)
>  		dsa_shared_port_setup_phy_of(dp, false);
>  }
>  
> -int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
> +int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr,
> +		      struct netlink_ext_ack *extack)
>  {
>  	struct dsa_switch *ds = dp->ds;
>  	int err;
> @@ -2034,7 +2035,7 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
>  
>  	dp->hsr_dev = hsr;
>  
> -	err = ds->ops->port_hsr_join(ds, dp->index, hsr);
> +	err = ds->ops->port_hsr_join(ds, dp->index, hsr, extack);
>  	if (err)
>  		dp->hsr_dev = NULL;
>  
> diff --git a/net/dsa/port.h b/net/dsa/port.h
> index dc812512fd0e..334879964e2c 100644
> --- a/net/dsa/port.h
> +++ b/net/dsa/port.h
> @@ -103,7 +103,8 @@ int dsa_port_phylink_create(struct dsa_port *dp);
>  void dsa_port_phylink_destroy(struct dsa_port *dp);
>  int dsa_shared_port_link_register_of(struct dsa_port *dp);
>  void dsa_shared_port_link_unregister_of(struct dsa_port *dp);
> -int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr);
> +int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr,
> +		      struct netlink_ext_ack *extack);
>  void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
>  int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
>  void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast);
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 48db91b33390..2b3d89b77121 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -2862,7 +2862,7 @@ static int dsa_slave_changeupper(struct net_device *dev,
>  		}
>  	} else if (is_hsr_master(info->upper_dev)) {
>  		if (info->linking) {
> -			err = dsa_port_hsr_join(dp, info->upper_dev);
> +			err = dsa_port_hsr_join(dp, info->upper_dev, extack);
>  			if (err == -EOPNOTSUPP) {
>  				NL_SET_ERR_MSG_WEAK_MOD(extack,
>  							"Offloading not supported");
> -- 
> 2.20.1
> 

It would have been good to Cc George here, for the driver-side patch.
But anyway, it looks ok.

[PATCH v6 net-next 2/5] net: dsa: notify drivers of MAC address changes on user ports

[Lukasz Majewski]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

In some cases, drivers may need to veto the changing of a MAC address on
a user port. Such is the case with KSZ9477 when it offloads a HSR device,
because it programs the MAC address of multiple ports to a shared
hardware register. Those ports need to have equal MAC addresses for the
lifetime of the HSR offload.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
Changes for v5:
- New patch
Changes for v6:
- None
---
 include/net/dsa.h | 10 ++++++++++
 net/dsa/slave.c   |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 426724808e76..d98439ea6146 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -969,6 +969,16 @@ struct dsa_switch_ops {
 			       struct phy_device *phy);
 	void	(*port_disable)(struct dsa_switch *ds, int port);
 
+
+	/*
+	 * Notification for MAC address changes on user ports. Drivers can
+	 * currently only veto operations. They should not use the method to
+	 * program the hardware, since the operation is not rolled back in case
+	 * of other errors.
+	 */
+	int	(*port_set_mac_address)(struct dsa_switch *ds, int port,
+					const unsigned char *addr);
+
 	/*
 	 * Compatibility between device trees defining multiple CPU ports and
 	 * drivers which are not OK to use by default the numerically smallest
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2b3d89b77121..4c3e502d7e16 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -457,6 +457,13 @@ static int dsa_slave_set_mac_address(struct net_device *dev, void *a)
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
 
+	if (ds->ops->port_set_mac_address) {
+		err = ds->ops->port_set_mac_address(ds, dp->index,
+						    addr->sa_data);
+		if (err)
+			return err;
+	}
+
 	/* If the port is down, the address isn't synced yet to hardware or
 	 * to the DSA master, so there is nothing to change.
 	 */
-- 
2.20.1

[PATCH v6 net-next 3/5] net: dsa: tag_ksz: Extend ksz9477_xmit() for HSR frame 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.

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>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
Changes for v2:
- Use ksz_hsr_get_ports() to obtain the bits values corresponding to
  HSR aware ports

Changes for v3:
- None

Changes for v4:
- Iterate over switch ports to find ones supporting HSR. Comparing to v3,
  where caching of egress tag bits were used, no noticeable performance
  regression has been observed.

Changes for v5:
- None

Changes for v6:
- None
---
 net/dsa/tag_ksz.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index ea100bd25939..3632e47dea9e 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -293,6 +293,14 @@ 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) {
+		struct net_device *hsr_dev = dp->hsr_dev;
+		struct dsa_port *other_dp;
+
+		dsa_hsr_foreach_port(other_dp, dp->ds, hsr_dev)
+			val |= BIT(other_dp->index);
+	}
+
 	*tag = cpu_to_be16(val);
 
 	return ksz_defer_xmit(dp, skb);
-- 
2.20.1

[PATCH v6 net-next 4/5] net: dsa: microchip: move REG_SW_MAC_ADDR to dev->info->regs[]

[Lukasz Majewski]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Defining macros which have the same name but different values is bad
practice, because it makes it hard to avoid code duplication. The same
code does different things, depending on the file it's placed in.
Case in point, we want to access REG_SW_MAC_ADDR from ksz_common.c, but
currently we can't, because we don't know which kszXXXX_reg.h to include
from the common code.

Remove the REG_SW_MAC_ADDR_{0..5} macros from ksz8795_reg.h and
ksz9477_reg.h, and re-add this register offset to the dev->info->regs[]
array.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
Changes for v5:
- New patch
Changes for v6:
- None
---
 drivers/net/dsa/microchip/ksz8795_reg.h | 7 -------
 drivers/net/dsa/microchip/ksz9477_reg.h | 7 -------
 drivers/net/dsa/microchip/ksz_common.c  | 2 ++
 drivers/net/dsa/microchip/ksz_common.h  | 1 +
 4 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index d33db4f86c64..3c9dae53e4d8 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -323,13 +323,6 @@
 	((addr) + REG_PORT_1_CTRL_0 + (port) *	\
 		(REG_PORT_2_CTRL_0 - REG_PORT_1_CTRL_0))
 
-#define REG_SW_MAC_ADDR_0		0x68
-#define REG_SW_MAC_ADDR_1		0x69
-#define REG_SW_MAC_ADDR_2		0x6A
-#define REG_SW_MAC_ADDR_3		0x6B
-#define REG_SW_MAC_ADDR_4		0x6C
-#define REG_SW_MAC_ADDR_5		0x6D
-
 #define TABLE_EXT_SELECT_S		5
 #define TABLE_EEE_V			1
 #define TABLE_ACL_V			2
diff --git a/drivers/net/dsa/microchip/ksz9477_reg.h b/drivers/net/dsa/microchip/ksz9477_reg.h
index 504e085aab52..f3a205ee483f 100644
--- a/drivers/net/dsa/microchip/ksz9477_reg.h
+++ b/drivers/net/dsa/microchip/ksz9477_reg.h
@@ -153,13 +153,6 @@
 #define SW_DOUBLE_TAG			BIT(7)
 #define SW_RESET			BIT(1)
 
-#define REG_SW_MAC_ADDR_0		0x0302
-#define REG_SW_MAC_ADDR_1		0x0303
-#define REG_SW_MAC_ADDR_2		0x0304
-#define REG_SW_MAC_ADDR_3		0x0305
-#define REG_SW_MAC_ADDR_4		0x0306
-#define REG_SW_MAC_ADDR_5		0x0307
-
 #define REG_SW_MTU__2			0x0308
 #define REG_SW_MTU_MASK			GENMASK(13, 0)
 
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 173ad8f04671..6c31d51410e3 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -364,6 +364,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
 };
 
 static const u16 ksz8795_regs[] = {
+	[REG_SW_MAC_ADDR]		= 0x68,
 	[REG_IND_CTRL_0]		= 0x6E,
 	[REG_IND_DATA_8]		= 0x70,
 	[REG_IND_DATA_CHECK]		= 0x72,
@@ -492,6 +493,7 @@ static u8 ksz8863_shifts[] = {
 };
 
 static const u16 ksz9477_regs[] = {
+	[REG_SW_MAC_ADDR]		= 0x0302,
 	[P_STP_CTRL]			= 0x0B04,
 	[S_START_CTRL]			= 0x0300,
 	[S_BROADCAST_CTRL]		= 0x0332,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index d180c8a34e27..07c7723dbc37 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -212,6 +212,7 @@ enum ksz_chip_id {
 };
 
 enum ksz_regs {
+	REG_SW_MAC_ADDR,
 	REG_IND_CTRL_0,
 	REG_IND_DATA_8,
 	REG_IND_DATA_CHECK,
-- 
2.20.1

[PATCH v6 net-next 5/5] net: dsa: microchip: Enable HSR offloading for KSZ9477

[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 - described in AN3474).
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.

Additionally, the KSZ9477 switch has been configured to forward frames
between HSR ports (e.g. 1,2) members to provide support for
NETIF_F_HW_HSR_FWD flag.

Join and leave functions are written in a way, that are executed with
single port - i.e. configuration is NOT done only when second HSR port
is configured.

Co-developed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes for v2:
- Use struct ksz_device to store hsr ports information (not struct dsa)

Changes for v3:
- Enable in-switch forwarding of frames between HSR ports (i.e. enable
  bridging of those two ports)

- The NETIF_F_HW_HSR_FWD flag has been marked as supported by the HSR
  network device

- Remove ETH MAC address validity check as it is done earlier in the net
  driver

- Add comment regarding adding support for NETIF_F_HW_HSR_FWD flag

Changes for v4:
- Merge patches for ksz_common.c and ksz9477.c

- Remove code to set global self-address filtering as this bit has
  already been set at ksz9477_reset_switch() function. Update also
  comment.

- Use already available ksz9477_cfg_port_member() instead of ksz_prmw32()

- Add description about chip limitations

- Allow having only one offloaded hsr interface (e.g. hsr0). Other ones
  (like hsr1) will have only SW HSR support

- Add check if MAC address of HSR device is equal to one from DSA master

- Rewrite the code to support per port join (i.e. not making init only
  when second HSR port is join)

- Add check to allow only one HSR port HW offloading

- Add hsr_ports to ksz_device struct to allow clean removal of network
  interfaces composing hsr device

Changes for v5:
- Add implementation of .port_set_mac_address callback for KSZ switch
  to prevent MAC address change when HSR HW offloading is used

- Use NL_SET_ERR_MSG_MOD() to propagate error messages to user

- Implement functions to use reference counter to allow in-HW MAC
  address change only when no other functionality requires it.

Changes for v6:
- Properly set REG_PORT_VLAN_MEMBERSHIP__4 register for each HSR
  supporting port. Expected value for HSR ports supporting the
  offloading is 0x23, not 0x2[12].
---
 drivers/net/dsa/microchip/ksz9477.c    |  77 +++++++++++++
 drivers/net/dsa/microchip/ksz9477.h    |   2 +
 drivers/net/dsa/microchip/ksz_common.c | 147 +++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h |   9 ++
 4 files changed, 235 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 9e63ed820c92..cde8ef33d029 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1143,6 +1143,83 @@ 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 - to be more specific; there is a race
+ * condition in hardware such that, when duplicate packets are received
+ * on member ports very close in time to each other, the hardware fails
+ * to detect that they are duplicates.
+ *
+ * Hence, the SW needs to handle those special cases. However, the speed
+ * up gain is considerable when above features are used.
+ *
+ * Moreover, the NETIF_F_HW_HSR_FWD feature is also enabled, as HSR frames
+ * can be forwarded in the switch fabric between HSR ports.
+ */
+#define KSZ9477_SUPPORTED_HSR_FEATURES (NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_FWD)
+
+void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr)
+{
+	struct ksz_device *dev = ds->priv;
+	struct net_device *slave;
+	struct dsa_port *hsr_dp;
+	u8 data, hsr_ports = 0;
+
+	/* Program which port(s) shall support HSR */
+	ksz_rmw32(dev, REG_HSR_PORT_MAP__4, BIT(port), BIT(port));
+
+	/* Forward frames between HSR ports (i.e. bridge together HSR ports) */
+	if (dev->hsr_ports) {
+		dsa_hsr_foreach_port(hsr_dp, ds, hsr)
+			hsr_ports |= BIT(hsr_dp->index);
+
+		hsr_ports |= BIT(dsa_upstream_port(ds, port));
+		dsa_hsr_foreach_port(hsr_dp, ds, hsr)
+			ksz9477_cfg_port_member(dev, hsr_dp->index, hsr_ports);
+	}
+
+	if (!dev->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);
+	}
+
+	/* Enable per port self-address filtering.
+	 * The global self-address filtering has already been enabled in the
+	 * ksz9477_reset_switch() function.
+	 */
+	ksz_port_cfg(dev, port, 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;
+}
+
+void ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr)
+{
+	struct ksz_device *dev = ds->priv;
+
+	/* Clear port HSR support */
+	ksz_rmw32(dev, REG_HSR_PORT_MAP__4, BIT(port), 0);
+
+	/* Disable forwarding frames between HSR ports */
+	ksz9477_cfg_port_member(dev, port, BIT(dsa_upstream_port(ds, port)));
+
+	/* Disable per port self-address filtering */
+	ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, false);
+}
+
 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 d7bbd9bd8893..f90e2e8ebe80 100644
--- a/drivers/net/dsa/microchip/ksz9477.h
+++ b/drivers/net/dsa/microchip/ksz9477.h
@@ -56,6 +56,8 @@ int ksz9477_reset_switch(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);
+void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr);
+void ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr);
 
 int ksz9477_port_acl_init(struct ksz_device *dev, int port);
 void ksz9477_port_acl_free(struct ksz_device *dev, int port);
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 6c31d51410e3..b800ace40ce1 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.h>
@@ -3541,6 +3542,149 @@ static int ksz_setup_tc(struct dsa_switch *ds, int port,
 	}
 }
 
+static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
+				    const unsigned char *addr)
+{
+	struct dsa_port *dp = dsa_to_port(ds, port);
+
+	if (dp->hsr_dev) {
+		dev_err(ds->dev,
+			"Cannot change MAC address on port %d with active HSR offload\n",
+			port);
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+/* Program the switch's MAC address register with the MAC address of the
+ * requesting user port. This single address is used by the switch for multiple
+ * features, like HSR self-address filtering and WoL. Other user ports are
+ * allowed to share ownership of this address as long as their MAC address is
+ * the same. The user ports' MAC addresses must not change while they have
+ * ownership of the switch MAC address.
+ */
+static int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
+				  struct netlink_ext_ack *extack)
+{
+	struct net_device *slave = dsa_to_port(ds, port)->slave;
+	const unsigned char *addr = slave->dev_addr;
+	struct ksz_switch_macaddr *switch_macaddr;
+	struct ksz_device *dev = ds->priv;
+	const u16 *regs = dev->info->regs;
+	int i;
+
+	/* Make sure concurrent MAC address changes are blocked */
+	ASSERT_RTNL();
+
+	switch_macaddr = dev->switch_macaddr;
+	if (switch_macaddr) {
+		if (!ether_addr_equal(switch_macaddr->addr, addr)) {
+			NL_SET_ERR_MSG_FMT_MOD(extack,
+					       "Switch already configured for MAC address %pM",
+					       switch_macaddr->addr);
+			return -EBUSY;
+		}
+
+		refcount_inc(&switch_macaddr->refcount);
+		return 0;
+	}
+
+	switch_macaddr = kzalloc(sizeof(*switch_macaddr), GFP_KERNEL);
+	if (!switch_macaddr)
+		return -ENOMEM;
+
+	ether_addr_copy(switch_macaddr->addr, addr);
+	refcount_set(&switch_macaddr->refcount, 1);
+	dev->switch_macaddr = switch_macaddr;
+
+	/* Program the switch MAC address to hardware */
+	for (i = 0; i < ETH_ALEN; i++)
+		ksz_write8(dev, regs[REG_SW_MAC_ADDR] + i, addr[i]);
+
+	return 0;
+}
+
+static void ksz_switch_macaddr_put(struct dsa_switch *ds)
+{
+	struct ksz_switch_macaddr *switch_macaddr;
+	struct ksz_device *dev = ds->priv;
+	const u16 *regs = dev->info->regs;
+	int i;
+
+	/* Make sure concurrent MAC address changes are blocked */
+	ASSERT_RTNL();
+
+	switch_macaddr = dev->switch_macaddr;
+	if (!refcount_dec_and_test(&switch_macaddr->refcount))
+		return;
+
+	for (i = 0; i < ETH_ALEN; i++)
+		ksz_write8(dev, regs[REG_SW_MAC_ADDR] + i, 0);
+
+	dev->switch_macaddr = NULL;
+	kfree(switch_macaddr);
+}
+
+static int ksz_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr,
+			struct netlink_ext_ack *extack)
+{
+	struct ksz_device *dev = ds->priv;
+	enum hsr_version ver;
+	int ret;
+
+	ret = hsr_get_version(hsr, &ver);
+	if (ret)
+		return ret;
+
+	if (dev->chip_id != KSZ9477_CHIP_ID) {
+		NL_SET_ERR_MSG_MOD(extack, "Chip does not support HSR offload");
+		return -EOPNOTSUPP;
+	}
+
+	/* KSZ9477 can support HW offloading of only 1 HSR device */
+	if (dev->hsr_dev && hsr != dev->hsr_dev) {
+		NL_SET_ERR_MSG_MOD(extack, "Offload supported for a single HSR");
+		return -EOPNOTSUPP;
+	}
+
+	/* KSZ9477 only supports HSR v0 and v1 */
+	if (!(ver == HSR_V0 || ver == HSR_V1)) {
+		NL_SET_ERR_MSG_MOD(extack, "Only HSR v0 and v1 supported");
+		return -EOPNOTSUPP;
+	}
+
+	/* Self MAC address filtering, to avoid frames traversing
+	 * the HSR ring more than once.
+	 */
+	ret = ksz_switch_macaddr_get(ds, port, extack);
+	if (ret)
+		return ret;
+
+	ksz9477_hsr_join(ds, port, hsr);
+	dev->hsr_dev = hsr;
+	dev->hsr_ports |= BIT(port);
+
+	return 0;
+}
+
+static int ksz_hsr_leave(struct dsa_switch *ds, int port,
+			 struct net_device *hsr)
+{
+	struct ksz_device *dev = ds->priv;
+
+	WARN_ON(dev->chip_id != KSZ9477_CHIP_ID);
+
+	ksz9477_hsr_leave(ds, port, hsr);
+	dev->hsr_ports &= ~BIT(port);
+	if (!dev->hsr_ports)
+		dev->hsr_dev = NULL;
+
+	ksz_switch_macaddr_put(ds);
+
+	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,
@@ -3560,6 +3704,9 @@ 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_set_mac_address	= ksz_port_set_mac_address,
 	.port_stp_state_set	= ksz_port_stp_state_set,
 	.port_teardown		= ksz_port_teardown,
 	.port_pre_bridge_flags	= ksz_port_pre_bridge_flags,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 07c7723dbc37..8842efca0871 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -101,6 +101,11 @@ struct ksz_ptp_irq {
 	int num;
 };
 
+struct ksz_switch_macaddr {
+	unsigned char addr[ETH_ALEN];
+	refcount_t refcount;
+};
+
 struct ksz_port {
 	bool remove_tag;		/* Remove Tag flag set, for ksz8795 only */
 	bool learning;
@@ -170,6 +175,10 @@ struct ksz_device {
 	struct mutex lock_irq;		/* IRQ Access */
 	struct ksz_irq girq;
 	struct ksz_ptp_data ptp_data;
+
+	struct ksz_switch_macaddr *switch_macaddr;
+	struct net_device *hsr_dev;     /* HSR */
+	u8 hsr_ports;
 };
 
 /* List of supported models */
-- 
2.20.1

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Vladimir Oltean]

On Fri, Sep 22, 2023 at 03:31:03PM +0200, Lukasz Majewski wrote:
> 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
> ip link set dev lan1 up
> ip link set dev lan2 up
> ip a add 192.168.0.1/24 dev hsr0
> ip link set dev hsr0 up
> 
> To remove HSR network device:
> ip link del hsr0
> 
> To test if one can adjust MAC address:
> ip link set lan2 address 00:01:02:AA:BB:CC
> 
> It is also possible to create another HSR interface, but it will
> only support HSR is software - e.g.
> ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision 45 version 1
> 
> Test HW:
> Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".
> 
> Performance SW used:
> nuttcp -S --nofork
> nuttcp -vv -T 60 -r 192.168.0.2
> nuttcp -vv -T 60 -t 192.168.0.2
> 
> Code: v6.6.0-rc2+ Linux net-next repository
> SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
> 
> Tested HSR v0 and v1
> Results:
> With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
> With no offloading 		       RX: 63 Mbps  TX: 63 Mbps

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Thanks!

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Lukasz Majewski]

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

Hi Vladimir,

> On Fri, Sep 22, 2023 at 03:31:03PM +0200, Lukasz Majewski wrote:
> > 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 ip link set dev lan1 up
> > ip link set dev lan2 up
> > ip a add 192.168.0.1/24 dev hsr0
> > ip link set dev hsr0 up
> > 
> > To remove HSR network device:
> > ip link del hsr0
> > 
> > To test if one can adjust MAC address:
> > ip link set lan2 address 00:01:02:AA:BB:CC
> > 
> > It is also possible to create another HSR interface, but it will
> > only support HSR is software - e.g.
> > ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision
> > 45 version 1
> > 
> > Test HW:
> > Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".
> > 
> > Performance SW used:
> > nuttcp -S --nofork
> > nuttcp -vv -T 60 -r 192.168.0.2
> > nuttcp -vv -T 60 -t 192.168.0.2
> > 
> > Code: v6.6.0-rc2+ Linux net-next repository
> > SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
> > 
> > Tested HSR v0 and v1
> > Results:
> > With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
> > With no offloading 		       RX: 63 Mbps  TX: 63 Mbps  
> 
> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Thanks!

I hope, that it will find its way to net-next soon :-).

Thanks for your help and patience.


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

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Lukasz Majewski]

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

Hi Vladimir, Andrew, Woojung,

> Hi Vladimir,
> 
> > On Fri, Sep 22, 2023 at 03:31:03PM +0200, Lukasz Majewski wrote:  
> > > 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 ip link set dev lan1 up
> > > ip link set dev lan2 up
> > > ip a add 192.168.0.1/24 dev hsr0
> > > ip link set dev hsr0 up
> > > 
> > > To remove HSR network device:
> > > ip link del hsr0
> > > 
> > > To test if one can adjust MAC address:
> > > ip link set lan2 address 00:01:02:AA:BB:CC
> > > 
> > > It is also possible to create another HSR interface, but it will
> > > only support HSR is software - e.g.
> > > ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision
> > > 45 version 1
> > > 
> > > Test HW:
> > > Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".
> > > 
> > > Performance SW used:
> > > nuttcp -S --nofork
> > > nuttcp -vv -T 60 -r 192.168.0.2
> > > nuttcp -vv -T 60 -t 192.168.0.2
> > > 
> > > Code: v6.6.0-rc2+ Linux net-next repository
> > > SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
> > > 
> > > Tested HSR v0 and v1
> > > Results:
> > > With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
> > > With no offloading 		       RX: 63 Mbps  TX: 63
> > > Mbps    
> > 
> > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > Thanks!  
> 
> I hope, that it will find its way to net-next soon :-).
> 

I'm a bit puzzled with this patch series - will it be pulled directly
to net-next [1] or is there any other (KSZ maintainer's?) tree to which
it will be first pulled and then PR will be send to net-next?

Thanks in advance for the clarification.

> Thanks for your help and patience.
> 

Links:

[1] -
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/log/

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

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

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Vladimir Oltean]

On Tue, Oct 03, 2023 at 09:58:32AM +0200, Lukasz Majewski wrote:
> I'm a bit puzzled with this patch series - will it be pulled directly
> to net-next [1] or is there any other (KSZ maintainer's?) tree to which
> it will be first pulled and then PR will be send to net-next?
> 
> Thanks in advance for the clarification.
> 
> Links:
> 
> [1] -
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/log/

No, there's no other tree than net-next. I see your patch was marked as
"Changes requested", let me see if I can transition it back to "Under review"
so that it gains the netdev maintainers' attention again:

https://patchwork.kernel.org/project/netdevbpf/cover/20230922133108.2090612-1-lukma@denx.de/

pw-bot: under-review

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Lukasz Majewski]

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

Hi Vladimir,

> On Tue, Oct 03, 2023 at 09:58:32AM +0200, Lukasz Majewski wrote:
> > I'm a bit puzzled with this patch series - will it be pulled
> > directly to net-next [1] or is there any other (KSZ maintainer's?)
> > tree to which it will be first pulled and then PR will be send to
> > net-next?
> > 
> > Thanks in advance for the clarification.
> > 
> > Links:
> > 
> > [1] -
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/log/
> >  
> 
> No, there's no other tree than net-next. I see your patch was marked
> as "Changes requested", let me see if I can transition it back to
> "Under review" so that it gains the netdev maintainers' attention
> again:
> 
> https://patchwork.kernel.org/project/netdevbpf/cover/20230922133108.2090612-1-lukma@denx.de/
> 
> pw-bot: under-review

Thanks!

I've just noticed that there is a WARNING:
https://patchwork.kernel.org/project/netdevbpf/patch/20230922133108.2090612-6-lukma@denx.de/

but then on the newest kernel checkpatch.pl is silent:
./scripts/checkpatch.pl
0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch total: 0
errors, 0 warnings, 0 checks, 277 lines checked

0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch has no
obvious style problems and is ready for submission.

Does the checkpatch for patchwork differs in any way from mainline?


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

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Jakub Kicinski]

On Tue, 3 Oct 2023 14:51:06 +0200 Lukasz Majewski wrote:
> I've just noticed that there is a WARNING:
> https://patchwork.kernel.org/project/netdevbpf/patch/20230922133108.2090612-6-lukma@denx.de/
> 
> but then on the newest kernel checkpatch.pl is silent:
> ./scripts/checkpatch.pl
> 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch total: 0
> errors, 0 warnings, 0 checks, 277 lines checked
> 
> 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch has no
> obvious style problems and is ready for submission.
> 
> Does the checkpatch for patchwork differs in any way from mainline?

We run:

checkpatch with --strict --max-line-length=80

https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh

The "multiple new lines" warning on patch 2 looks legit, no?

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[Lukasz Majewski]

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

On Tue, 3 Oct 2023 06:42:13 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Tue, 3 Oct 2023 14:51:06 +0200 Lukasz Majewski wrote:
> > I've just noticed that there is a WARNING:
> > https://patchwork.kernel.org/project/netdevbpf/patch/20230922133108.2090612-6-lukma@denx.de/
> > 
> > but then on the newest kernel checkpatch.pl is silent:
> > ./scripts/checkpatch.pl
> > 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch
> > total: 0 errors, 0 warnings, 0 checks, 277 lines checked
> > 
> > 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch has
> > no obvious style problems and is ready for submission.
> > 
> > Does the checkpatch for patchwork differs in any way from mainline?
> >  
> 
> We run:
> 
> checkpatch with --strict --max-line-length=80
> 
> https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh
> 
> The "multiple new lines" warning on patch 2 looks legit, no?

Indeed - the:

'--strict --max-line-length=80'

makes the difference...

If I may ask - why it is added? Or to ask in other way - why the
"vanila" checkpatch is not enough for net-dev ?


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

[net][hsr] Question regarding HSR RedBox functionality implementation (preferably on KSZ9477)

[Lukasz Majewski]

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

Dear Community,

I would like to ask you for some help regarding HSR mainline
implementation.

As of now for KSZ9477 we do have working hsr0 (as offloading HW) for
HSR ring operation and some other ports for this IC (like lan3,4,5).

With current setup it is possible to forward packets from HSR ring to
non-HSR network (i.e. plain ethernet) with L3 routing.

However, I'm wondering how the mainline Linux kernel could handle HSR
RedBox functionality (on document [1], Figure 2. we do have "bridge" -
OSI L2).

To be more interesting - br0 can be created between hsr0 and e.g. lan3.
But as expected communication breaks on both directions (to SAN and to
HSR ring).

Is there a similar functionality already present in the Linux kernel
(so this approach could be reused)?

My (very rough idea) would be to extend KSZ9477 bridge join functions
to check if HSR capable interface is "bridged" and then handle frames
in a special way.

However, I would like to first ask for as much input as possible - to
avoid any unnecessary work.

Thanks in advance for help :-)

Link:

[1] -
https://ww1.microchip.com/downloads/en/Appnotes/AN3474-KSZ9477-High-Availability-Seamless-Redundancy-Application-Note-00003474A.pdf


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

Re: [net][hsr] Question regarding HSR RedBox functionality implementation (preferably on KSZ9477)

[Vladimir Oltean]

Hi Lukasz,

On Tue, Jan 09, 2024 at 01:32:34PM +0100, Lukasz Majewski wrote:
> However, I'm wondering how the mainline Linux kernel could handle HSR
> RedBox functionality (on document [1], Figure 2. we do have "bridge" -
> OSI L2).
> 
> To be more interesting - br0 can be created between hsr0 and e.g. lan3.
> But as expected communication breaks on both directions (to SAN and to
> HSR ring).

Yes, I suppose this is how a RedBox should be modeled. In principle it's
identical to how bridging with LAG ports (bond, team) works - either in
software or offloaded. The trouble is that the HSR driver seems to only
work with the DANH/DANP roles (as also mentioned in
Documentation/networking/dsa/dsa.rst). I don't remember what doesn't
work (or if I ever knew at all). It might be the address substitution
from hsr_xmit() that masks the MAC address of the SAN side device?

> Is there a similar functionality already present in the Linux kernel
> (so this approach could be reused)?
> 
> My (very rough idea) would be to extend KSZ9477 bridge join functions
> to check if HSR capable interface is "bridged" and then handle frames
> in a special way.
> 
> However, I would like to first ask for as much input as possible - to
> avoid any unnecessary work.

First I'd figure out why the software data path isn't working, and if it
can be fixed. Then, fix that if possible, and add a new selftest to
tools/testing/selftests/net/forwarding/, that should pass using veth
interfaces as lower ports.

Then, offloading something that has a clear model in software should be
relatively easy, though you might need to add some logic to DSA. This is
one place that needs to be edited, there may be others.

	/* dsa_port_pre_hsr_leave is not yet necessary since hsr devices cannot
	 * meaningfully placed under a bridge yet
	 */

> 
> Thanks in advance for help :-)
> 
> Link:
> 
> [1] -
> https://ww1.microchip.com/downloads/en/Appnotes/AN3474-KSZ9477-High-Availability-Seamless-Redundancy-Application-Note-00003474A.pdf
> 
> 
> Best regards,
> 
> Lukasz Majewski

Re: [net][hsr] Question regarding HSR RedBox functionality implementation (preferably on KSZ9477)

[Lukasz Majewski]

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

Hi Vladimir,

> Hi Lukasz,
> 
> On Tue, Jan 09, 2024 at 01:32:34PM +0100, Lukasz Majewski wrote:
> > However, I'm wondering how the mainline Linux kernel could handle
> > HSR RedBox functionality (on document [1], Figure 2. we do have
> > "bridge" - OSI L2).
> > 
> > To be more interesting - br0 can be created between hsr0 and e.g.
> > lan3. But as expected communication breaks on both directions (to
> > SAN and to HSR ring).  
> 
> Yes, I suppose this is how a RedBox should be modeled. In principle
> it's identical to how bridging with LAG ports (bond, team) works -
> either in software or offloaded. 
> The trouble is that the HSR driver
> seems to only work with the DANH/DANP roles (as also mentioned in
> Documentation/networking/dsa/dsa.rst). I don't remember what doesn't
> work (or if I ever knew at all).

In the newest net-next only PRP_TLV_REDBOX_MAC is defined, which seems
to be REDBOX for DAN P (PRP).

> It might be the address substitution
> from hsr_xmit() that masks the MAC address of the SAN side device?
> 

This needs to be further investigated.

> > Is there a similar functionality already present in the Linux kernel
> > (so this approach could be reused)?
> > 
> > My (very rough idea) would be to extend KSZ9477 bridge join
> > functions to check if HSR capable interface is "bridged" and then
> > handle frames in a special way.
> > 
> > However, I would like to first ask for as much input as possible -
> > to avoid any unnecessary work.  
> 
> First I'd figure out why the software data path isn't working, and if
> it can be fixed. 

+1

> Then, fix that if possible, and add a new selftest to
> tools/testing/selftests/net/forwarding/, that should pass using veth
> interfaces as lower ports.
> 
> Then, offloading something that has a clear model in software should
> be relatively easy, though you might need to add some logic to DSA.
> This is one place that needs to be edited, there may be others.
> 
> 	/* dsa_port_pre_hsr_leave is not yet necessary since hsr
> devices cannot
> 	 * meaningfully placed under a bridge yet
> 	 */
> 

Ok, the LAG approach in /net/dsa/user.c can be used as an example.

Thanks for shedding some light on this issue :-)

> > 
> > Thanks in advance for help :-)
> > 
> > Link:
> > 
> > [1] -
> > https://ww1.microchip.com/downloads/en/Appnotes/AN3474-KSZ9477-High-Availability-Seamless-Redundancy-Application-Note-00003474A.pdf
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski  




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

Re: [net][hsr] Question regarding HSR RedBox functionality implementation (preferably on KSZ9477)

[Lukasz Majewski]

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

Hi Vladimir, Andrew,

> Hi Vladimir,
> 
> > Hi Lukasz,
> > 
> > On Tue, Jan 09, 2024 at 01:32:34PM +0100, Lukasz Majewski wrote:  
> > > However, I'm wondering how the mainline Linux kernel could handle
> > > HSR RedBox functionality (on document [1], Figure 2. we do have
> > > "bridge" - OSI L2).
> > > 
> > > To be more interesting - br0 can be created between hsr0 and e.g.
> > > lan3. But as expected communication breaks on both directions (to
> > > SAN and to HSR ring).    
> > 
> > Yes, I suppose this is how a RedBox should be modeled. In principle
> > it's identical to how bridging with LAG ports (bond, team) works -
> > either in software or offloaded. 
> > The trouble is that the HSR driver
> > seems to only work with the DANH/DANP roles (as also mentioned in
> > Documentation/networking/dsa/dsa.rst). I don't remember what doesn't
> > work (or if I ever knew at all).  
> 
> In the newest net-next only PRP_TLV_REDBOX_MAC is defined, which seems
> to be REDBOX for DAN P (PRP).
> 
> > It might be the address substitution
> > from hsr_xmit() that masks the MAC address of the SAN side device?
> >   
> 
> This needs to be further investigated.

I've looked into the HSR and Bridge drivers internals.

The creation of hsrX device from command line ends with
hsr_dev_finalize(), which then calls hsr_add_port() for HSR_PT_MASTER,
HSR_PT_SLAVE_A and HSR_PT_SLAVE_B.

Those three ports allows HSR DANH operation.

For Redbox (SANH) it looks like the HSR_PT_INTERLINK shall be used.

When calling:
ip link add name br0 type bridge; ip link set br0 up;
ip link set hsr1 master br0; ip link set lan3 master br0;

(the hsr1 is the SW HSR - NO offloading).

The br_add_if() is called, which calls:
netdev_rx_handler_register(dev, br_get_rx_handler(dev), p);

This setup of RX handler is problematic, as for HSR INTERLINK port the
hsr_handle_frame() shall be used (so proper port->type =
HSR_PT_INTERLINK would be used in the hsr driver).

When the bridge handler is used, all the incoming frames are set as
HSR_PT_MASTER type (and only some frames with "reserved" MAC address
are passed to HSR network).


The problem:
############
Proper setup of rx_handler for network device, so L2 frames are handled
by HSR driver.


I've tried:
###########
1. In dsa_user_prechangeupper() (or ksz_port_bridge_join)
if (netdev_is_rx_handler_busy(dp->user)) {
	rtnl_trylock();
	netdev_rx_handler_unregister(dp->user);
	rtnl_unlock();
}

hsr_add_interlink_port(dev->hsr_dev, dp->user, extack);

But it looks like it is too low-level code to play with it by hand as
several WARN_ONs are triggered.

2. Stop/unlink the bridge slave port (lan3) and then call
hsr_add_interlink_port() for it.

However, there are several warnings as well and this approach may harm
the "bridging" modelling approach as I would use 'unlinked' device for
normal operation.


Idea:
#####

1. In the br_get_rx_handler() I could add check if "sibling" port is the
HSR master one and then set the RX handler for lan3 to this one.

However, this would require "bridge" driver modification for HSR
operation.

2. Maybe the way of HSR port creation shall be augmented [*]?
For example from:
ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 supervision 45

to:
ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink lan3
supervision 45

In that way I could modify only the hsr_dev_finalize() and everything
would be managed by hsr driver?




Or maybe I've overlooked something, and there is easier solution for
this problem?



Note:

[*] - this approach solves also another problem - when we do have e.g 5
ports in the switch how one can know which lanX port shall be added to
HSR network? With the current approach the hsr1 device first needs to
be created and then it is implicitly assumed that the next bridged port
(lan3) shall be the Interlink one for HSR.

> 
> > > Is there a similar functionality already present in the Linux
> > > kernel (so this approach could be reused)?
> > > 
> > > My (very rough idea) would be to extend KSZ9477 bridge join
> > > functions to check if HSR capable interface is "bridged" and then
> > > handle frames in a special way.
> > > 
> > > However, I would like to first ask for as much input as possible -
> > > to avoid any unnecessary work.    
> > 
> > First I'd figure out why the software data path isn't working, and
> > if it can be fixed.   
> 
> +1
> 
> > Then, fix that if possible, and add a new selftest to
> > tools/testing/selftests/net/forwarding/, that should pass using veth
> > interfaces as lower ports.
> > 
> > Then, offloading something that has a clear model in software should
> > be relatively easy, though you might need to add some logic to DSA.
> > This is one place that needs to be edited, there may be others.
> > 
> > 	/* dsa_port_pre_hsr_leave is not yet necessary since hsr
> > devices cannot
> > 	 * meaningfully placed under a bridge yet
> > 	 */
> >   
> 
> Ok, the LAG approach in /net/dsa/user.c can be used as an example.
> 
> Thanks for shedding some light on this issue :-)
> 
> > > 
> > > Thanks in advance for help :-)
> > > 
> > > Link:
> > > 
> > > [1] -
> > > https://ww1.microchip.com/downloads/en/Appnotes/AN3474-KSZ9477-High-Availability-Seamless-Redundancy-Application-Note-00003474A.pdf
> > > 
> > > 
> > > Best regards,
> > > 
> > > Lukasz Majewski    
> 
> 
> 
> 
> 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




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

Re: [net][hsr] Question regarding HSR RedBox functionality implementation (preferably on KSZ9477)

[Lukasz Majewski]

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

Dear Community,

> Hi Vladimir, Andrew,
> 
> > Hi Vladimir,
> >   
> > > Hi Lukasz,
> > > 
> > > On Tue, Jan 09, 2024 at 01:32:34PM +0100, Lukasz Majewski wrote:
> > >   
> > > > However, I'm wondering how the mainline Linux kernel could
> > > > handle HSR RedBox functionality (on document [1], Figure 2. we
> > > > do have "bridge" - OSI L2).
> > > > 
> > > > To be more interesting - br0 can be created between hsr0 and
> > > > e.g. lan3. But as expected communication breaks on both
> > > > directions (to SAN and to HSR ring).      
> > > 
> > > Yes, I suppose this is how a RedBox should be modeled. In
> > > principle it's identical to how bridging with LAG ports (bond,
> > > team) works - either in software or offloaded. 
> > > The trouble is that the HSR driver
> > > seems to only work with the DANH/DANP roles (as also mentioned in
> > > Documentation/networking/dsa/dsa.rst). I don't remember what
> > > doesn't work (or if I ever knew at all).    
> > 
> > In the newest net-next only PRP_TLV_REDBOX_MAC is defined, which
> > seems to be REDBOX for DAN P (PRP).
> >   
> > > It might be the address substitution
> > > from hsr_xmit() that masks the MAC address of the SAN side device?
> > >     
> > 
> > This needs to be further investigated.  
> 
> I've looked into the HSR and Bridge drivers internals.
> 
> The creation of hsrX device from command line ends with
> hsr_dev_finalize(), which then calls hsr_add_port() for HSR_PT_MASTER,
> HSR_PT_SLAVE_A and HSR_PT_SLAVE_B.
> 
> Those three ports allows HSR DANH operation.
> 
> For Redbox (SANH) it looks like the HSR_PT_INTERLINK shall be used.
> 
> When calling:
> ip link add name br0 type bridge; ip link set br0 up;
> ip link set hsr1 master br0; ip link set lan3 master br0;
> 
> (the hsr1 is the SW HSR - NO offloading).
> 
> The br_add_if() is called, which calls:
> netdev_rx_handler_register(dev, br_get_rx_handler(dev), p);
> 
> This setup of RX handler is problematic, as for HSR INTERLINK port the
> hsr_handle_frame() shall be used (so proper port->type =
> HSR_PT_INTERLINK would be used in the hsr driver).
> 
> When the bridge handler is used, all the incoming frames are set as
> HSR_PT_MASTER type (and only some frames with "reserved" MAC address
> are passed to HSR network).
> 
> 
> The problem:
> ############
> Proper setup of rx_handler for network device, so L2 frames are
> handled by HSR driver.
> 
> 
> I've tried:
> ###########
> 1. In dsa_user_prechangeupper() (or ksz_port_bridge_join)
> if (netdev_is_rx_handler_busy(dp->user)) {
> 	rtnl_trylock();
> 	netdev_rx_handler_unregister(dp->user);
> 	rtnl_unlock();
> }
> 
> hsr_add_interlink_port(dev->hsr_dev, dp->user, extack);
> 
> But it looks like it is too low-level code to play with it by hand as
> several WARN_ONs are triggered.
> 
> 2. Stop/unlink the bridge slave port (lan3) and then call
> hsr_add_interlink_port() for it.
> 
> However, there are several warnings as well and this approach may harm
> the "bridging" modelling approach as I would use 'unlinked' device for
> normal operation.
> 
> 
> Idea:
> #####
> 
> 1. In the br_get_rx_handler() I could add check if "sibling" port is
> the HSR master one and then set the RX handler for lan3 to this one.
> 
> However, this would require "bridge" driver modification for HSR
> operation.
> 
> 2. Maybe the way of HSR port creation shall be augmented [*]?
> For example from:
> ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 supervision 45
> 
> to:
> ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink lan3
> supervision 45

Please find the draft proposal of iproute2 patch to add support for HSR
interlink passing network device:

https://github.com/lmajewski/iproute2/commit/5edf2ab77786ab49419712a4defa42a600fe47c2

> 
> In that way I could modify only the hsr_dev_finalize() and everything
> would be managed by hsr driver?
> 
> 
> 
> 
> Or maybe I've overlooked something, and there is easier solution for
> this problem?
> 
> 
> 
> Note:
> 
> [*] - this approach solves also another problem - when we do have e.g
> 5 ports in the switch how one can know which lanX port shall be added
> to HSR network? With the current approach the hsr1 device first needs
> to be created and then it is implicitly assumed that the next bridged
> port (lan3) shall be the Interlink one for HSR.
> 
> >   
> > > > Is there a similar functionality already present in the Linux
> > > > kernel (so this approach could be reused)?
> > > > 
> > > > My (very rough idea) would be to extend KSZ9477 bridge join
> > > > functions to check if HSR capable interface is "bridged" and
> > > > then handle frames in a special way.
> > > > 
> > > > However, I would like to first ask for as much input as
> > > > possible - to avoid any unnecessary work.      
> > > 
> > > First I'd figure out why the software data path isn't working, and
> > > if it can be fixed.     
> > 
> > +1
> >   
> > > Then, fix that if possible, and add a new selftest to
> > > tools/testing/selftests/net/forwarding/, that should pass using
> > > veth interfaces as lower ports.
> > > 
> > > Then, offloading something that has a clear model in software
> > > should be relatively easy, though you might need to add some
> > > logic to DSA. This is one place that needs to be edited, there
> > > may be others.
> > > 
> > > 	/* dsa_port_pre_hsr_leave is not yet necessary since hsr
> > > devices cannot
> > > 	 * meaningfully placed under a bridge yet
> > > 	 */
> > >     
> > 
> > Ok, the LAG approach in /net/dsa/user.c can be used as an example.
> > 
> > Thanks for shedding some light on this issue :-)
> >   
> > > > 
> > > > Thanks in advance for help :-)
> > > > 
> > > > Link:
> > > > 
> > > > [1] -
> > > > https://ww1.microchip.com/downloads/en/Appnotes/AN3474-KSZ9477-High-Availability-Seamless-Redundancy-Application-Note-00003474A.pdf
> > > > 
> > > > 
> > > > Best regards,
> > > > 
> > > > Lukasz Majewski      
> > 
> > 
> > 
> > 
> > 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  
> 
> 
> 
> 
> 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




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

Re: [net][hsr] Question regarding HSR RedBox functionality implementation (preferably on KSZ9477)

[Stephen Hemminger]

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

On Thu, 15 Feb 2024 12:51:56 +0100
Lukasz Majewski <lukma@denx.de> wrote:

> > to:
> > ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink lan3
> > supervision 45  
> 
> Please find the draft proposal of iproute2 patch to add support for HSR
> interlink passing network device:
> 
> https://github.com/lmajewski/iproute2/commit/5edf2ab77786ab49419712a4defa42a600fe47c2


Please post patches to mailing list for review directly.

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

Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 22 Sep 2023 15:31:03 +0200 you wrote:
> 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
> ip link set dev lan1 up
> ip link set dev lan2 up
> ip a add 192.168.0.1/24 dev hsr0
> ip link set dev hsr0 up
> 
> [...]

Here is the summary with links:
  - [v6,net-next,1/5] net: dsa: propagate extack to ds->ops->port_hsr_join()
    https://git.kernel.org/netdev/net-next/c/fefe5dc4afea
  - [v6,net-next,2/5] net: dsa: notify drivers of MAC address changes on user ports
    https://git.kernel.org/netdev/net-next/c/6715042cd112
  - [v6,net-next,3/5] net: dsa: tag_ksz: Extend ksz9477_xmit() for HSR frame duplication
    https://git.kernel.org/netdev/net-next/c/5e5db71a92c5
  - [v6,net-next,4/5] net: dsa: microchip: move REG_SW_MAC_ADDR to dev->info->regs[]
    https://git.kernel.org/netdev/net-next/c/e5de2ad163e7
  - [v6,net-next,5/5] net: dsa: microchip: Enable HSR offloading for KSZ9477
    https://git.kernel.org/netdev/net-next/c/2d61298fdd7b

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