[RFC PATCH net-next] net: hsr: create an API to get hsr port type

[RFC PATCH net-next] net: hsr: create an API to get hsr port type

[Xiaoliang Yang]

If a switch device has HSR hardware ability and HSR configuration
offload to hardware. The device driver needs to get the HSR port type
when joining the port to HSR. Different port types require different
settings for the hardware, like HSR_PT_SLAVE_A, HSR_PT_SLAVE_B, and
HSR_PT_INTERLINK. Create the API hsr_get_port_type() and export it.

When the hsr_get_port_type() is called in the device driver, if the port
can be found in the HSR port list, the HSR port type can be obtained.
Therefore, before calling the device driver, we need to first add the
hsr_port to the HSR port list.

Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
 include/linux/if_hsr.h |  8 ++++++++
 net/hsr/hsr_device.c   | 20 ++++++++++++++++++++
 net/hsr/hsr_slave.c    |  7 ++++---
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
index d7941fd88032..4d6452ca2ac8 100644
--- a/include/linux/if_hsr.h
+++ b/include/linux/if_hsr.h
@@ -43,6 +43,8 @@ extern bool is_hsr_master(struct net_device *dev);
 extern int hsr_get_version(struct net_device *dev, enum hsr_version *ver);
 struct net_device *hsr_get_port_ndev(struct net_device *ndev,
 				     enum hsr_port_type pt);
+extern int hsr_get_port_type(struct net_device *hsr_dev, struct net_device *dev,
+			     enum hsr_port_type *type);
 #else
 static inline bool is_hsr_master(struct net_device *dev)
 {
@@ -59,6 +61,12 @@ static inline struct net_device *hsr_get_port_ndev(struct net_device *ndev,
 {
 	return ERR_PTR(-EINVAL);
 }
+
+static inline int hsr_get_port_type(struct net_device *hsr_dev, struct net_device *dev,
+				    enum hsr_port_type *type)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_HSR */
 
 #endif /*_LINUX_IF_HSR_H_*/
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 88657255fec1..d4bea847527c 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -679,6 +679,26 @@ struct net_device *hsr_get_port_ndev(struct net_device *ndev,
 }
 EXPORT_SYMBOL(hsr_get_port_ndev);
 
+/* Get hsr port type, return -EINVAL if not get.
+ */
+int hsr_get_port_type(struct net_device *hsr_dev, struct net_device *dev, enum hsr_port_type *type)
+{
+	struct hsr_priv *hsr;
+	struct hsr_port *port;
+
+	hsr = netdev_priv(hsr_dev);
+
+	hsr_for_each_port(hsr, port) {
+		if (port->dev == dev) {
+			*type = port->type;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(hsr_get_port_type);
+
 /* Default multicast address for HSR Supervision frames */
 static const unsigned char def_multicast_addr[ETH_ALEN] __aligned(2) = {
 	0x01, 0x15, 0x4e, 0x00, 0x01, 0x00
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index b87b6a6fe070..e11ab1ed3320 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -198,14 +198,14 @@ int hsr_add_port(struct hsr_priv *hsr, struct net_device *dev,
 	port->type = type;
 	ether_addr_copy(port->original_macaddress, dev->dev_addr);
 
+	list_add_tail_rcu(&port->port_list, &hsr->ports);
+
 	if (type != HSR_PT_MASTER) {
 		res = hsr_portdev_setup(hsr, dev, port, extack);
 		if (res)
 			goto fail_dev_setup;
 	}
 
-	list_add_tail_rcu(&port->port_list, &hsr->ports);
-
 	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
 	netdev_update_features(master->dev);
 	dev_set_mtu(master->dev, hsr_get_max_mtu(hsr));
@@ -213,7 +213,8 @@ int hsr_add_port(struct hsr_priv *hsr, struct net_device *dev,
 	return 0;
 
 fail_dev_setup:
-	kfree(port);
+	list_del_rcu(&port->port_list);
+	kfree_rcu(port, rcu);
 	return res;
 }
 
-- 
2.17.1

Re: [RFC PATCH net-next] net: hsr: create an API to get hsr port type

[Lukasz Majewski]

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

Hi Xiaoliang,

> If a switch device has HSR hardware ability and HSR configuration
> offload to hardware. The device driver needs to get the HSR port type
> when joining the port to HSR. Different port types require different
> settings for the hardware, like HSR_PT_SLAVE_A, HSR_PT_SLAVE_B, and
> HSR_PT_INTERLINK. Create the API hsr_get_port_type() and export it.
> 

Could you describe the use case in more detail - as pointed out by
Vladimir?

In my use case - when I use the KSZ9477 switch I just provide correct
arguments for the iproute2 configuration:

# Configuration - RedBox (EVB-KSZ9477):
if link set lan1 down;ip link set lan2 down
ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45
	 version 1
ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink lan3 
	supervision 45 version 1
ip link set lan4 up;ip link set lan5 up
ip link set lan3 up
ip addr add 192.168.0.11/24 dev hsr1
ip link set hsr1 up

# Configuration - DAN-H (EVB-KSZ9477):
ip link set lan1 down;ip link set lan2 down
ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45
	version 1
ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 supervision 45
	version 1
ip link set lan4 up;ip link set lan5 up
ip addr add 192.168.0.12/24 dev hsr1
ip link set hsr1 up

More info (also regarding HSR testing with QEMU) can be found from:
https://lpc.events/event/18/contributions/1969/attachments/1456/3092/lpc-2024-HSR-v1.0-e26d140f6845e94afea.pdf


As fair as I remember - the Node Table can be read from debugfs.

However, such approach has been regarded as obsolete - by the
community. 

In the future development plans there was the idea to use netlink (or
iproute separate program) to get the data now available in debugfs and
extend it to also print REDBOX node info (not only DANH).

> When the hsr_get_port_type() is called in the device driver, if the
> port can be found in the HSR port list, the HSR port type can be
> obtained. Therefore, before calling the device driver, we need to
> first add the hsr_port to the HSR port list.
> 
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> ---
>  include/linux/if_hsr.h |  8 ++++++++
>  net/hsr/hsr_device.c   | 20 ++++++++++++++++++++
>  net/hsr/hsr_slave.c    |  7 ++++---
>  3 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
> index d7941fd88032..4d6452ca2ac8 100644
> --- a/include/linux/if_hsr.h
> +++ b/include/linux/if_hsr.h
> @@ -43,6 +43,8 @@ extern bool is_hsr_master(struct net_device *dev);
>  extern int hsr_get_version(struct net_device *dev, enum hsr_version
> *ver); struct net_device *hsr_get_port_ndev(struct net_device *ndev,
>  				     enum hsr_port_type pt);
> +extern int hsr_get_port_type(struct net_device *hsr_dev, struct
> net_device *dev,
> +			     enum hsr_port_type *type);
>  #else
>  static inline bool is_hsr_master(struct net_device *dev)
>  {
> @@ -59,6 +61,12 @@ static inline struct net_device
> *hsr_get_port_ndev(struct net_device *ndev, {
>  	return ERR_PTR(-EINVAL);
>  }
> +
> +static inline int hsr_get_port_type(struct net_device *hsr_dev,
> struct net_device *dev,
> +				    enum hsr_port_type *type)
> +{
> +	return -EINVAL;
> +}
>  #endif /* CONFIG_HSR */
>  
>  #endif /*_LINUX_IF_HSR_H_*/
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index 88657255fec1..d4bea847527c 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -679,6 +679,26 @@ struct net_device *hsr_get_port_ndev(struct
> net_device *ndev, }
>  EXPORT_SYMBOL(hsr_get_port_ndev);
>  
> +/* Get hsr port type, return -EINVAL if not get.
> + */
> +int hsr_get_port_type(struct net_device *hsr_dev, struct net_device
> *dev, enum hsr_port_type *type) +{
> +	struct hsr_priv *hsr;
> +	struct hsr_port *port;
> +
> +	hsr = netdev_priv(hsr_dev);
> +
> +	hsr_for_each_port(hsr, port) {
> +		if (port->dev == dev) {
> +			*type = port->type;
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(hsr_get_port_type);
> +
>  /* Default multicast address for HSR Supervision frames */
>  static const unsigned char def_multicast_addr[ETH_ALEN] __aligned(2)
> = { 0x01, 0x15, 0x4e, 0x00, 0x01, 0x00
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index b87b6a6fe070..e11ab1ed3320 100644
> --- a/net/hsr/hsr_slave.c
> +++ b/net/hsr/hsr_slave.c
> @@ -198,14 +198,14 @@ int hsr_add_port(struct hsr_priv *hsr, struct
> net_device *dev, port->type = type;
>  	ether_addr_copy(port->original_macaddress, dev->dev_addr);
>  
> +	list_add_tail_rcu(&port->port_list, &hsr->ports);
> +
>  	if (type != HSR_PT_MASTER) {
>  		res = hsr_portdev_setup(hsr, dev, port, extack);
>  		if (res)
>  			goto fail_dev_setup;
>  	}
>  
> -	list_add_tail_rcu(&port->port_list, &hsr->ports);
> -
>  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
>  	netdev_update_features(master->dev);
>  	dev_set_mtu(master->dev, hsr_get_max_mtu(hsr));
> @@ -213,7 +213,8 @@ int hsr_add_port(struct hsr_priv *hsr, struct
> net_device *dev, return 0;
>  
>  fail_dev_setup:
> -	kfree(port);
> +	list_del_rcu(&port->port_list);
> +	kfree_rcu(port, rcu);
>  	return res;
>  }
>  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Johanna Denk,
Tabea Lutz 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: [EXT] Re: [RFC PATCH net-next] net: hsr: create an API to get hsr port type

[Xiaoliang Yang]

Hi Lukasz,

> 
> Hi Xiaoliang,
> 
> > If a switch device has HSR hardware ability and HSR configuration
> > offload to hardware. The device driver needs to get the HSR port type
> > when joining the port to HSR. Different port types require different
> > settings for the hardware, like HSR_PT_SLAVE_A, HSR_PT_SLAVE_B, and
> > HSR_PT_INTERLINK. Create the API hsr_get_port_type() and export it.
> >
> 
> Could you describe the use case in more detail - as pointed out by Vladimir?
> 
> In my use case - when I use the KSZ9477 switch I just provide correct arguments
> for the iproute2 configuration:
> 
> # Configuration - RedBox (EVB-KSZ9477):
> if link set lan1 down;ip link set lan2 down ip link add name hsr0 type hsr slave1
> lan1 slave2 lan2 supervision 45
> 	 version 1
> ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink lan3
> 	supervision 45 version 1
> ip link set lan4 up;ip link set lan5 up
> ip link set lan3 up
> ip addr add 192.168.0.11/24 dev hsr1
> ip link set hsr1 up
> 
> # Configuration - DAN-H (EVB-KSZ9477):
> ip link set lan1 down;ip link set lan2 down ip link add name hsr0 type hsr slave1
> lan1 slave2 lan2 supervision 45
> 	version 1
> ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 supervision 45
> 	version 1
> ip link set lan4 up;ip link set lan5 up
> ip addr add 192.168.0.12/24 dev hsr1
> ip link set hsr1 up
> 
> More info (also regarding HSR testing with QEMU) can be found from:
> https://lpc.events/event/18/contributions/1969/attachments/1456/3092/lpc-
> 2024-HSR-v1.0-e26d140f6845e94afea.pdf
> 
> 
> As fair as I remember - the Node Table can be read from debugfs.
> 
> However, such approach has been regarded as obsolete - by the community.
> 
> In the future development plans there was the idea to use netlink (or iproute
> separate program) to get the data now available in debugfs and extend it to also
> print REDBOX node info (not only DANH).
> 
I need to offload the NETIF_F_HW_HSR_TAG_INS and NETIF_F_HW_HSR_TAG_RM
to hardware. The hardware needs to know which ports are slave ports, which is
interlink port.

Hardware remove HSR tag on interlink port if it is egress port, keep the HSR tag 
on HSR slave ports. The frames from ring network are removed HSR tag and 
forwarded to interlink port in hardware, not received in HSR stack.

Thanks,
Xiaoliang

Re: [EXT] Re: [RFC PATCH net-next] net: hsr: create an API to get hsr port type

[Lukasz Majewski]

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

Hi Xiaoliang,

> Hi Lukasz,
> 
> > 
> > Hi Xiaoliang,
> >   
> > > If a switch device has HSR hardware ability and HSR configuration
> > > offload to hardware. The device driver needs to get the HSR port
> > > type when joining the port to HSR. Different port types require
> > > different settings for the hardware, like HSR_PT_SLAVE_A,
> > > HSR_PT_SLAVE_B, and HSR_PT_INTERLINK. Create the API
> > > hsr_get_port_type() and export it. 
> > 
> > Could you describe the use case in more detail - as pointed out by
> > Vladimir?
> > 
> > In my use case - when I use the KSZ9477 switch I just provide
> > correct arguments for the iproute2 configuration:
> > 
> > # Configuration - RedBox (EVB-KSZ9477):
> > if link set lan1 down;ip link set lan2 down ip link add name hsr0
> > type hsr slave1 lan1 slave2 lan2 supervision 45
> > 	 version 1
> > ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink
> > lan3 supervision 45 version 1
> > ip link set lan4 up;ip link set lan5 up
> > ip link set lan3 up
> > ip addr add 192.168.0.11/24 dev hsr1
> > ip link set hsr1 up
> > 
> > # Configuration - DAN-H (EVB-KSZ9477):
> > ip link set lan1 down;ip link set lan2 down ip link add name hsr0
> > type hsr slave1 lan1 slave2 lan2 supervision 45
> > 	version 1
> > ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 supervision
> > 45 version 1
> > ip link set lan4 up;ip link set lan5 up
> > ip addr add 192.168.0.12/24 dev hsr1
> > ip link set hsr1 up
> > 
> > More info (also regarding HSR testing with QEMU) can be found from:
> > https://lpc.events/event/18/contributions/1969/attachments/1456/3092/lpc-
> > 2024-HSR-v1.0-e26d140f6845e94afea.pdf
> > 
> > 
> > As fair as I remember - the Node Table can be read from debugfs.
> > 
> > However, such approach has been regarded as obsolete - by the
> > community.
> > 
> > In the future development plans there was the idea to use netlink
> > (or iproute separate program) to get the data now available in
> > debugfs and extend it to also print REDBOX node info (not only
> > DANH). 
> I need to offload the NETIF_F_HW_HSR_TAG_INS and NETIF_F_HW_HSR_TAG_RM
> to hardware.

I've recently added some "offloading" support for KSZ9477 switch IC.
You can use it as a reference.

> The hardware needs to know which ports are slave ports,
> which is interlink port.

This information you provide to the network driver when you call:

ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45
version 1

Then the lan1 is configured as slave 1 and lan2 as slave 2

For interlink (RedBOX):

ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink 
lan3 supervision 45 version 1

> 
> Hardware remove HSR tag on interlink port if it is egress port, keep
> the HSR tag on HSR slave ports. The frames from ring network are
> removed HSR tag and forwarded to interlink port in hardware, not
> received in HSR stack.

Just out of curiosity - which HW IP block has such great capability to
remove in HW HSR tag from RedBox frames?

> 
> Thanks,
> Xiaoliang




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Johanna Denk,
Tabea Lutz 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: [EXT] Re: [RFC PATCH net-next] net: hsr: create an API to get hsr port type

[Vladimir Oltean]

Hi Xiaoliang,

On Thu, Jul 24, 2025 at 01:54:51PM +0300, Xiaoliang Yang wrote:
> Hi Lukasz,
> 
> > 
> > Hi Xiaoliang,
> > 
> > > If a switch device has HSR hardware ability and HSR configuration
> > > offload to hardware. The device driver needs to get the HSR port type
> > > when joining the port to HSR. Different port types require different
> > > settings for the hardware, like HSR_PT_SLAVE_A, HSR_PT_SLAVE_B, and
> > > HSR_PT_INTERLINK. Create the API hsr_get_port_type() and export it.
> > >
> > 
> > Could you describe the use case in more detail - as pointed out by Vladimir?
> > 
> > In my use case - when I use the KSZ9477 switch I just provide correct arguments
> > for the iproute2 configuration:
> > 
> > # Configuration - RedBox (EVB-KSZ9477):
> > if link set lan1 down;ip link set lan2 down ip link add name hsr0 type hsr slave1
> > lan1 slave2 lan2 supervision 45
> > 	 version 1
> > ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink lan3
> > 	supervision 45 version 1
> > ip link set lan4 up;ip link set lan5 up
> > ip link set lan3 up
> > ip addr add 192.168.0.11/24 dev hsr1
> > ip link set hsr1 up
> > 
> > # Configuration - DAN-H (EVB-KSZ9477):
> > ip link set lan1 down;ip link set lan2 down ip link add name hsr0 type hsr slave1
> > lan1 slave2 lan2 supervision 45
> > 	version 1
> > ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 supervision 45
> > 	version 1
> > ip link set lan4 up;ip link set lan5 up
> > ip addr add 192.168.0.12/24 dev hsr1
> > ip link set hsr1 up
> > 
> > More info (also regarding HSR testing with QEMU) can be found from:
> > https://lpc.events/event/18/contributions/1969/attachments/1456/3092/lpc-
> > 2024-HSR-v1.0-e26d140f6845e94afea.pdf
> > 
> > 
> > As fair as I remember - the Node Table can be read from debugfs.
> > 
> > However, such approach has been regarded as obsolete - by the community.
> > 
> > In the future development plans there was the idea to use netlink (or iproute
> > separate program) to get the data now available in debugfs and extend it to also
> > print REDBOX node info (not only DANH).
> > 
> I need to offload the NETIF_F_HW_HSR_TAG_INS and NETIF_F_HW_HSR_TAG_RM
> to hardware. The hardware needs to know which ports are slave ports, which is
> interlink port.
> 
> Hardware remove HSR tag on interlink port if it is egress port, keep the HSR tag 
> on HSR slave ports. The frames from ring network are removed HSR tag and 
> forwarded to interlink port in hardware, not received in HSR stack.
> 
> Thanks,
> Xiaoliang

Sorry for the delay, but I remembered just now that there exists an use
case for which hsr_get_port_type() can already be added to the kernel,
without the need for submitting a new driver.

The discussion is here:
https://lore.kernel.org/netdev/20240620090210.drop6jwh7e5qw556@skbuf/
the basic idea is that the xrs700x.c driver only supports offloading
HSR_PT_SLAVE_A and HSR_PT_SLAVE_B (which were the only port types at the
time the offload for this driver was written). However, the API does not
explicitly tell it what port has what role. The driver can get confused
and think that it can support a configuration which it actually can't
(see the table in the link attached, and the xrs700x_hsr_join() function).
The only way to solve that is to introduce an API for the HSR port type
and use it in the xrs700x driver.

Would you be willing to resend this patch, making xrs700x_hsr_join() the
first user?

Actually I think it can be considered a bug fix worthwile of backporting
to stable kernels (unsupported configuration fails to be rejected), with
Fixes: 5055cccfc2d1 ("net: hsr: Provide RedBox support (HSR-SAN)")