| Message ID | 20230922133108.2090612-1-lukma@denx.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: dsa: hsr: Enable HSR HW offloading for KSZ9477 | expand |
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!
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
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
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
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
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?
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!
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
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
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
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
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
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
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.