drivers/net/dsa/microchip/ksz_common.c | 11 +++++++++++ include/net/dsa.h | 12 ++++++++++++ 2 files changed, 23 insertions(+)
From: Frieder Schrempf <frieder.schrempf@kontron.de>
The KSZ9477 supports NETIF_F_HW_HSR_FWD to forward packets between
HSR ports. This is set up when creating the HSR interface via
ksz9477_hsr_join() and ksz9477_cfg_port_member().
At the same time ksz_update_port_member() is called on every
state change of a port and reconfiguring the forwarding to the
default state which means packets get only forwarded to the CPU
port.
If the ports are brought up before setting up the HSR interface
and then the port state is not changed afterwards, everything works
as intended:
ip link set lan1 up
ip link set lan2 up
ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
ip addr add dev hsr 10.0.0.10/24
ip link set hsr up
If the port state is changed after creating the HSR interface, this results
in a non-working HSR setup:
ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
ip addr add dev hsr 10.0.0.10/24
ip link set lan1 up
ip link set lan2 up
ip link set hsr up
In this state, packets will not get forwarded between the HSR ports and
communication between HSR nodes that are not direct neighbours in the
topology fails.
To avoid this, we prevent all forwarding reconfiguration requests for ports
that are part of a HSR setup with NETIF_F_HW_HSR_FWD enabled.
Fixes: 2d61298fdd7b ("net: dsa: microchip: Enable HSR offloading for KSZ9477")
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
I'm posting this as RFC as my knowledge of the driver and the stack in
general is very limited. Please review thoroughly and provide feedback.
Thanks!
---
---
drivers/net/dsa/microchip/ksz_common.c | 11 +++++++++++
include/net/dsa.h | 12 ++++++++++++
2 files changed, 23 insertions(+)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 7c142c17b3f69..56370ecdfe4ee 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2286,6 +2286,17 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
return;
dp = dsa_to_port(ds, port);
+
+ /*
+ * HSR ports might use forwarding configured during setup. Prevent any
+ * modifications as long as the port is part of a HSR setup with
+ * NETIF_F_HW_HSR_FWD enabled.
+ */
+ if (dev->hsr_dev && dp->user &&
+ (dp->user->features & NETIF_F_HW_HSR_FWD) &&
+ dsa_is_hsr_port(ds, dev->hsr_dev, port))
+ return;
+
cpu_port = BIT(dsa_upstream_port(ds, port));
for (i = 0; i < ds->num_ports; i++) {
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 55e2d97f247eb..846a2cc2f2fc3 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -565,6 +565,18 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_USER;
}
+static inline bool dsa_is_hsr_port(struct dsa_switch *ds, struct net_device *hsr, int p)
+{
+ struct dsa_port *hsr_dp;
+
+ dsa_hsr_foreach_port(hsr_dp, ds, hsr) {
+ if (hsr_dp->index == p)
+ return true;
+ }
+
+ return false;
+}
+
#define dsa_tree_for_each_user_port(_dp, _dst) \
list_for_each_entry((_dp), &(_dst)->ports, list) \
if (dsa_port_is_user((_dp)))
--
2.50.1On Wed, Aug 13, 2025 at 05:26:12PM +0200, Frieder Schrempf wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > The KSZ9477 supports NETIF_F_HW_HSR_FWD to forward packets between > HSR ports. This is set up when creating the HSR interface via > ksz9477_hsr_join() and ksz9477_cfg_port_member(). > > At the same time ksz_update_port_member() is called on every > state change of a port and reconfiguring the forwarding to the > default state which means packets get only forwarded to the CPU > port. > > If the ports are brought up before setting up the HSR interface > and then the port state is not changed afterwards, everything works > as intended: > > ip link set lan1 up > ip link set lan2 up > ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision 45 version 1 > ip addr add dev hsr 10.0.0.10/24 > ip link set hsr up > > If the port state is changed after creating the HSR interface, this results > in a non-working HSR setup: > > ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision 45 version 1 > ip addr add dev hsr 10.0.0.10/24 > ip link set lan1 up > ip link set lan2 up > ip link set hsr up So, restating what i said in a different thread, what happens if only software was used? No hardware offload. Andrew
Am 15.08.25 um 00:59 schrieb Andrew Lunn: > On Wed, Aug 13, 2025 at 05:26:12PM +0200, Frieder Schrempf wrote: >> From: Frieder Schrempf <frieder.schrempf@kontron.de> >> >> The KSZ9477 supports NETIF_F_HW_HSR_FWD to forward packets between >> HSR ports. This is set up when creating the HSR interface via >> ksz9477_hsr_join() and ksz9477_cfg_port_member(). >> >> At the same time ksz_update_port_member() is called on every >> state change of a port and reconfiguring the forwarding to the >> default state which means packets get only forwarded to the CPU >> port. >> >> If the ports are brought up before setting up the HSR interface >> and then the port state is not changed afterwards, everything works >> as intended: >> >> ip link set lan1 up >> ip link set lan2 up >> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision 45 version 1 >> ip addr add dev hsr 10.0.0.10/24 >> ip link set hsr up >> >> If the port state is changed after creating the HSR interface, this results >> in a non-working HSR setup: >> >> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision 45 version 1 >> ip addr add dev hsr 10.0.0.10/24 >> ip link set lan1 up >> ip link set lan2 up >> ip link set hsr up > > So, restating what i said in a different thread, what happens if only > software was used? No hardware offload. Sorry, I don't understand what you are aiming at. Yes, this issue is related to hardware offloading. As far as I know there is no option (for the user) to force HSR into SW-only mode. The KSZ9477 driver uses hardware offloading up to the capabilities of the HW by default. Yes, if I disable the offloading by modifying the driver code as already described in the other thread, the issue can be fixed at the cost of loosing the HW acceleration. In this case the driver consistently configures the HSR ports to forward any packets to the CPU which then forwards them as needed. With the driver code as-is, there are two conflicting values used for the register that configures the forwarding. One is set during the HSR setup and makes sure that HSR ports forward packets among each other (and not only to the CPU), the other is set while changing the link state of the HSR ports and causes the forwarding to only happen between each port and the CPU, therefore effectively disabling the HW offloading while the driver still assumes it is enabled. This is obviously a problem that should be fixed in the driver as changing the link state of the ports *after* setup of the HSR is a completely valid operation that shouldn't break things like it currently does.
Hi Frieder,
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> The KSZ9477 supports NETIF_F_HW_HSR_FWD to forward packets between
> HSR ports. This is set up when creating the HSR interface via
> ksz9477_hsr_join() and ksz9477_cfg_port_member().
>
> At the same time ksz_update_port_member() is called on every
> state change of a port and reconfiguring the forwarding to the
> default state which means packets get only forwarded to the CPU
> port.
>
> If the ports are brought up before setting up the HSR interface
> and then the port state is not changed afterwards, everything works
> as intended:
>
> ip link set lan1 up
> ip link set lan2 up
> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision
> 45 version 1 ip addr add dev hsr 10.0.0.10/24
> ip link set hsr up
>
> If the port state is changed after creating the HSR interface, this
> results in a non-working HSR setup:
>
> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision
> 45 version 1 ip addr add dev hsr 10.0.0.10/24
> ip link set lan1 up
> ip link set lan2 up
> ip link set hsr up
>
> In this state, packets will not get forwarded between the HSR ports
> and communication between HSR nodes that are not direct neighbours in
> the topology fails.
>
> To avoid this, we prevent all forwarding reconfiguration requests for
> ports that are part of a HSR setup with NETIF_F_HW_HSR_FWD enabled.
>
> Fixes: 2d61298fdd7b ("net: dsa: microchip: Enable HSR offloading for
> KSZ9477") Signed-off-by: Frieder Schrempf
> <frieder.schrempf@kontron.de> ---
> I'm posting this as RFC as my knowledge of the driver and the stack in
> general is very limited. Please review thoroughly and provide
> feedback. Thanks!
I don't have the HW at hand at the moment (temporary).
Could you check if this patch works when you create two hsr interfaces
- i.e. hsr1 would use HW offloading from KSZ9744 and hsr2 is just the
one supporting HSR in software.
> ---
> ---
> drivers/net/dsa/microchip/ksz_common.c | 11 +++++++++++
> include/net/dsa.h | 12 ++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c
> b/drivers/net/dsa/microchip/ksz_common.c index
> 7c142c17b3f69..56370ecdfe4ee 100644 ---
> a/drivers/net/dsa/microchip/ksz_common.c +++
> b/drivers/net/dsa/microchip/ksz_common.c @@ -2286,6 +2286,17 @@
> static void ksz_update_port_member(struct ksz_device *dev, int port)
> return;
> dp = dsa_to_port(ds, port);
> +
> + /*
> + * HSR ports might use forwarding configured during setup.
> Prevent any
> + * modifications as long as the port is part of a HSR setup
> with
> + * NETIF_F_HW_HSR_FWD enabled.
> + */
> + if (dev->hsr_dev && dp->user &&
> + (dp->user->features & NETIF_F_HW_HSR_FWD) &&
> + dsa_is_hsr_port(ds, dev->hsr_dev, port))
> + return;
> +
> cpu_port = BIT(dsa_upstream_port(ds, port));
>
> for (i = 0; i < ds->num_ports; i++) {
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 55e2d97f247eb..846a2cc2f2fc3 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -565,6 +565,18 @@ static inline bool dsa_is_user_port(struct
> dsa_switch *ds, int p) return dsa_to_port(ds, p)->type ==
> DSA_PORT_TYPE_USER; }
>
> +static inline bool dsa_is_hsr_port(struct dsa_switch *ds, struct
> net_device *hsr, int p) +{
> + struct dsa_port *hsr_dp;
> +
> + dsa_hsr_foreach_port(hsr_dp, ds, hsr) {
> + if (hsr_dp->index == p)
> + return true;
> + }
> +
> + return false;
> +}
> +
I thought that we already had such function implemented. Apparently I
must have been wrong.
> #define dsa_tree_for_each_user_port(_dp, _dst) \
> list_for_each_entry((_dp), &(_dst)->ports, list) \
> if (dsa_port_is_user((_dp)))
--
Best regards,
Lukasz Majewski
--
Nabla Software Engineering GmbH
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschftsfhrer : Stefano Babic
Am 13.08.25 um 17:45 schrieb Łukasz Majewski:
> [Sie erhalten nicht h?ufig E-Mails von lukma@nabladev.com. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi Frieder,
>
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> The KSZ9477 supports NETIF_F_HW_HSR_FWD to forward packets between
>> HSR ports. This is set up when creating the HSR interface via
>> ksz9477_hsr_join() and ksz9477_cfg_port_member().
>>
>> At the same time ksz_update_port_member() is called on every
>> state change of a port and reconfiguring the forwarding to the
>> default state which means packets get only forwarded to the CPU
>> port.
>>
>> If the ports are brought up before setting up the HSR interface
>> and then the port state is not changed afterwards, everything works
>> as intended:
>>
>> ip link set lan1 up
>> ip link set lan2 up
>> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision
>> 45 version 1 ip addr add dev hsr 10.0.0.10/24
>> ip link set hsr up
>>
>> If the port state is changed after creating the HSR interface, this
>> results in a non-working HSR setup:
>>
>> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision
>> 45 version 1 ip addr add dev hsr 10.0.0.10/24
>> ip link set lan1 up
>> ip link set lan2 up
>> ip link set hsr up
>>
>> In this state, packets will not get forwarded between the HSR ports
>> and communication between HSR nodes that are not direct neighbours in
>> the topology fails.
>>
>> To avoid this, we prevent all forwarding reconfiguration requests for
>> ports that are part of a HSR setup with NETIF_F_HW_HSR_FWD enabled.
>>
>> Fixes: 2d61298fdd7b ("net: dsa: microchip: Enable HSR offloading for
>> KSZ9477") Signed-off-by: Frieder Schrempf
>> <frieder.schrempf@kontron.de> ---
>> I'm posting this as RFC as my knowledge of the driver and the stack in
>> general is very limited. Please review thoroughly and provide
>> feedback. Thanks!
>
> I don't have the HW at hand at the moment (temporary).
>
> Could you check if this patch works when you create two hsr interfaces
> - i.e. hsr1 would use HW offloading from KSZ9744 and hsr2 is just the
> one supporting HSR in software.
My hardware only has three user ports. So that might get a bit difficult
to test. I will try to configure one unconnected port to set up two HSR
links, but I won't be able to fully test this due to the lack of the
fourth physical link.
On Wed, 13 Aug 2025 17:57:02 +0200
Frieder Schrempf <frieder.schrempf@kontron.de> wrote:
> Am 13.08.25 um 17:45 schrieb Łukasz Majewski:
> > [Sie erhalten nicht h?ufig E-Mails von lukma@nabladev.com. Weitere
> > Informationen, warum dies wichtig ist, finden Sie unter
> > https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Hi Frieder,
> >
> >> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >>
> >> The KSZ9477 supports NETIF_F_HW_HSR_FWD to forward packets between
> >> HSR ports. This is set up when creating the HSR interface via
> >> ksz9477_hsr_join() and ksz9477_cfg_port_member().
> >>
> >> At the same time ksz_update_port_member() is called on every
> >> state change of a port and reconfiguring the forwarding to the
> >> default state which means packets get only forwarded to the CPU
> >> port.
> >>
> >> If the ports are brought up before setting up the HSR interface
> >> and then the port state is not changed afterwards, everything works
> >> as intended:
> >>
> >> ip link set lan1 up
> >> ip link set lan2 up
> >> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision
> >> 45 version 1 ip addr add dev hsr 10.0.0.10/24
> >> ip link set hsr up
> >>
> >> If the port state is changed after creating the HSR interface, this
> >> results in a non-working HSR setup:
> >>
> >> ip link add name hsr type hsr slave1 lan1 slave2 lan2 supervision
> >> 45 version 1 ip addr add dev hsr 10.0.0.10/24
> >> ip link set lan1 up
> >> ip link set lan2 up
> >> ip link set hsr up
> >>
> >> In this state, packets will not get forwarded between the HSR ports
> >> and communication between HSR nodes that are not direct neighbours
> >> in the topology fails.
> >>
> >> To avoid this, we prevent all forwarding reconfiguration requests
> >> for ports that are part of a HSR setup with NETIF_F_HW_HSR_FWD
> >> enabled.
> >>
> >> Fixes: 2d61298fdd7b ("net: dsa: microchip: Enable HSR offloading
> >> for KSZ9477") Signed-off-by: Frieder Schrempf
> >> <frieder.schrempf@kontron.de> ---
> >> I'm posting this as RFC as my knowledge of the driver and the
> >> stack in general is very limited. Please review thoroughly and
> >> provide feedback. Thanks!
> >
> > I don't have the HW at hand at the moment (temporary).
> >
> > Could you check if this patch works when you create two hsr
> > interfaces
> > - i.e. hsr1 would use HW offloading from KSZ9744 and hsr2 is just
> > the one supporting HSR in software.
>
> My hardware only has three user ports. So that might get a bit
> difficult to test. I will try to configure one unconnected port to
> set up two HSR links, but I won't be able to fully test this due to
> the lack of the fourth physical link.
>
Ok, I see...
I was using the Atmel's/Microchip Devel board with 5 ports...
--
Best regards,
Lukasz Majewski
--
Nabla Software Engineering GmbH
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschftsfhrer : Stefano Babic
© 2016 - 2025 Red Hat, Inc.