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]
Message-ID: <CAFSKS=OO8oi=757b9DqOMpS4X6jqf5rg+X=GO8G-hOQ+S7LaKQ@mail.gmail.com>
Date:   Mon, 8 Feb 2021 15:09:34 -0600
From:   George McCollister <george.mccollister@...il.com>
To:     Tobias Waldekranz <tobias@...dekranz.com>
Cc:     Jakub Kicinski <kuba@...nel.org>, 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
Subject: Re: [PATCH net-next v2 0/4] add HSR offloading support for DSA switches

On Mon, Feb 8, 2021 at 2:16 PM Tobias Waldekranz <tobias@...dekranz.com> wrote:
>
> 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

There is also the HSR-HSR use case and HSR-PRP use case.

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

Correct. net/hsr doesn't support this use case right now. It will
stomp the outgoing source MAC with that of the interface for instance.
It also doesn't implement a ProxyNodeTable (though that actually
wouldn't matter if you were offloading to the xrs700x I think). Try
commenting out the ether_addr_copy() line in hsr_xmit and see if it
makes your use case work.

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

Depends what you mean by configured as an interlink. I believe bit 9
of HSR_CFG in the switch is only supposed to be used for the HSR-HSR
and HSR-PRP use case, not HSR-SAN.

>
>       br0
>      /   \
>    hsr0   \
>    /  \    \
> swp1 swp2 swp3
>
> Or are there some additional semantics involved in forwarding between
> the redundant ports and the interlink?

That sounds right.

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

Okay. I'll look into this. Though a lot of the restrictions have to do
with using the third gigabit port for an HSR/PRP interlink (not
HSR-SAN) which I'm not currently supporting anyway.

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

This was changed between 62439-3:2010 and 62439-3:2012.
"Prefixing the supervision frames on HSR by an HSR tag to simplify the hardware
implementation and introduce a unique EtherType for HSR to simplify processing."
The Linux HSR driver calls the former HSR v0 and the later HSR v1. I'm
not sure what their intention was with this feature. The inbound
policies are pretty flexible so maybe they didn't have anything so
specific in mind.
I don't think the xrs7000 series could offload HSR v0 anyway because
the tag ether type is different.

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

Describe what you mean a bit more. Do you mean join and leave should
each only be called once with both hsr ports being passed in?

>
> Thanks for working on this!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ