netdev - Re: [PATCH net-next v2 0/4] add HSR offloading support for DSA switches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 08 Feb 2021 21:16:35 +0100
From:   Tobias Waldekranz <tobias@...dekranz.com>
To:     George McCollister <george.mccollister@...il.com>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Jonathan Corbet <corbet@....net>, netdev@...r.kernel.org,
        George McCollister <george.mccollister@...il.com>
Subject: Re: [PATCH net-next v2 0/4] add HSR offloading support for DSA switches

On Thu, Feb 04, 2021 at 15:59, George McCollister <george.mccollister@...il.com> wrote:
> Add support for offloading HSR/PRP (IEC 62439-3) tag insertion, tag
> removal, forwarding and duplication on DSA switches.
> This series adds offloading to the xrs700x DSA driver.
>
> Changes since RFC:
>  * Split hsr and dsa patches. (Florian Fainelli)
>
> Changes since v1:
>  * Fixed some typos/wording. (Vladimir Oltean)
>  * eliminate IFF_HSR and use is_hsr_master instead. (Vladimir Oltean)
>  * Make hsr_handle_sup_frame handle skb_std as well (required when offloading)
>  * Don't add hsr tag for HSR v0 supervisory frames.
>  * Fixed tag insertion offloading for PRP.
>
> George McCollister (4):
>   net: hsr: generate supervision frame without HSR/PRP tag
>   net: hsr: add offloading support
>   net: dsa: add support for offloading HSR
>   net: dsa: xrs700x: add HSR offloading support
>
>  Documentation/networking/netdev-features.rst |  21 ++++++
>  drivers/net/dsa/xrs700x/xrs700x.c            | 106 +++++++++++++++++++++++++++
>  drivers/net/dsa/xrs700x/xrs700x_reg.h        |   5 ++
>  include/linux/if_hsr.h                       |  27 +++++++
>  include/linux/netdev_features.h              |   9 +++
>  include/net/dsa.h                            |  13 ++++
>  net/dsa/dsa_priv.h                           |  11 +++
>  net/dsa/port.c                               |  34 +++++++++
>  net/dsa/slave.c                              |  14 ++++
>  net/dsa/switch.c                             |  24 ++++++
>  net/dsa/tag_xrs700x.c                        |   7 +-
>  net/ethtool/common.c                         |   4 +
>  net/hsr/hsr_device.c                         |  46 ++----------
>  net/hsr/hsr_device.h                         |   1 -
>  net/hsr/hsr_forward.c                        |  33 ++++++++-
>  net/hsr/hsr_forward.h                        |   1 +
>  net/hsr/hsr_framereg.c                       |   2 +
>  net/hsr/hsr_main.c                           |  11 +++
>  net/hsr/hsr_main.h                           |   8 +-
>  net/hsr/hsr_slave.c                          |  10 ++-
>  20 files changed, 331 insertions(+), 56 deletions(-)
>  create mode 100644 include/linux/if_hsr.h
>
> -- 
> 2.11.0

Hi George,

I will hopefully have some more time to look into this during the coming
weeks. What follows are some random thoughts so far, I hope you can
accept the windy road :)

Broadly speaking, I gather there are two common topologies that will be
used with the XRS chip: "End-device" and "RedBox".

End-device:    RedBox:
 .-----.       .-----.
 | CPU |       | CPU |
 '--+--'       '--+--'
    |             |
.---0---.     .---0---.
|  XRS  |     |  XRS  3--- Non-redundant network
'-1---2-'     '-1---2-'
  |   |         |   |
 HSR Ring      HSR Ring

>From the looks of it, this series only deals with the end-device
use-case. Is that right?

I will be targeting a RedBox setup, and I believe that means that the
remaining port has to be configured as an "interlink". (HSR/PRP is still
pretty new to me). Is that equivalent to a Linux config like this:

      br0
     /   \
   hsr0   \
   /  \    \
swp1 swp2 swp3

Or are there some additional semantics involved in forwarding between
the redundant ports and the interlink?

The chip is very rigid in the sense that most roles are statically
allocated to specific ports. I think we need to add checks for this.

Looking at the packets being generated on the redundant ports, both
regular traffic and supervision frames seem to be HSR-tagged. Are
supervision frames not supposed to be sent with an outer ethertype of
0x88fb? The manual talks about the possibility of setting up a policy
entry to bypass HSR-tagging (section 6.1.5), is this what that is for?

In the DSA layer (dsa_slave_changeupper), could we merge the two HSR
join/leave calls somehow? My guess is all drivers are going to end up
having to do the same dance of deferring configuration until both ports
are known.

Thanks for working on this!