lkml.org 
[lkml]   [2020]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [net-next RFC PATCH 00/13] net: hsr: Add PRP driver
    Hi Vinicius,

    On Thu, 21 May 2020 at 20:33, Vinicius Costa Gomes
    <vinicius.gomes@intel.com> wrote:
    >
    > Murali Karicheri <m-karicheri2@ti.com> writes:
    >
    > > This RFC series add support for Parallel Redundancy Protocol (PRP)
    > > as defined in IEC-62439-3 in the kernel networking subsystem. PRP
    > > Uses a Redundancy Control Trailer (RCT) the format of which is
    > > similar to HSR Tag. This is used for implementing redundancy.
    > > RCT consists of 6 bytes similar to HSR tag and contain following
    > > fields:-
    > >
    > > - 16-bit sequence number (SeqNr);
    > > - 4-bit LAN identifier (LanId);
    > > - 12 bit frame size (LSDUsize);
    > > - 16-bit suffix (PRPsuffix).
    > >
    > > The PRPsuffix identifies PRP frames and distinguishes PRP frames
    > > from other protocols that also append a trailer to their useful
    > > data. The LSDUsize field allows the receiver to distinguish PRP
    > > frames from random, nonredundant frames as an additional check.
    > > LSDUsize is the size of the Ethernet payload inclusive of the
    > > RCT. Sequence number along with LanId is used for duplicate
    > > detection and discard.
    > >
    > > PRP node is also known as Dual Attached Node (DAN-P) since it
    > > is typically attached to two different LAN for redundancy.
    > > DAN-P duplicates each of L2 frames and send it over the two
    > > Ethernet links. Each outgoing frame is appended with RCT.
    > > Unlike HSR, these are added to the end of L2 frame and may be
    > > treated as padding by bridges and therefore would be work with
    > > traditional bridges or switches, where as HSR wouldn't as Tag
    > > is prefixed to the Ethenet frame. At the remote end, these are
    > > received and the duplicate frame is discarded before the stripped
    > > frame is send up the networking stack. Like HSR, PRP also sends
    > > periodic Supervision frames to the network. These frames are
    > > received and MAC address from the SV frames are populated in a
    > > database called Node Table. The above functions are grouped into
    > > a block called Link Redundancy Entity (LRE) in the IEC spec.
    > >
    > > As there are many similarities between HSR and PRP protocols,
    > > this patch re-use the code from HSR driver to implement PRP
    > > driver. As many part of the code can be re-used, this patch
    > > introduces a new common API definitions for both protocols and
    > > propose to obsolete the existing HSR defines in
    > > include/uapi/linux/if_link.h. New definitions are prefixed
    > > with a HSR_PRP prefix. Similarly include/uapi/linux/hsr_netlink.h
    > > is proposed to be replaced with include/uapi/linux/hsr_prp_netlink.h
    > > which also uses the HSR_PRP prefix. The netlink socket interface
    > > code is migrated (as well as the iproute2 being sent as a follow up
    > > patch) to use the new API definitions. To re-use the code,
    > > following are done as a preparatory patch before adding the PRP
    > > functionality:-
    > >
    > > - prefix all common code with hsr_prp
    > > - net/hsr -> renamed to net/hsr-prp
    > > - All common struct types, constants, functions renamed with
    > > hsr{HSR}_prp{PRP} prefix.
    >
    > I don't really like these prefixes, I am thinking of when support for
    > IEEE 802.1CB is added, do we rename this to "hsr_prp_frer"?
    >
    > And it gets even more complicated, and using 802.1CB you can configure
    > the tagging method and the stream identification function so a system
    > can interoperate in a HSR or PRP network.
    >

    Is it a given that 802.1CB in Linux should be implemented using an hsr
    upper device?
    802.1CB is _much_ more flexible than both HSR and PRP. You can have
    more than 2 ports, you can have per-stream rules (each stream has its
    own sequence number), and those rules can identify the source, the
    destination, or both the source and the destination.

    > So, I see this as different methods of achieving the same result, which
    > makes me think that the different "methods/types" (HSR and PRP in your
    > case) should be basically different implementations of a "struct
    > hsr_ops" interface. With this hsr_ops something like this:
    >
    > struct hsr_ops {
    > int (*handle_frame)()
    > int (*add_port)()
    > int (*remove_port)()
    > int (*setup)()
    > void (*teardown)()
    > };
    >
    > >
    > > Please review this and provide me feedback so that I can work to
    > > incorporate them and send a formal patch series for this. As this
    > > series impacts user space, I am not sure if this is the right
    > > approach to introduce a new definitions and obsolete the old
    > > API definitions for HSR. The current approach is choosen
    > > to avoid redundant code in iproute2 and in the netlink driver
    > > code (hsr_netlink.c). Other approach we discussed internally was
    > > to Keep the HSR prefix in the user space and kernel code, but
    > > live with the redundant code in the iproute2 and hsr netlink
    > > code. Would like to hear from you what is the best way to add
    > > this feature to networking core. If there is any other
    > > alternative approach possible, I would like to hear about the
    > > same.
    >
    > Why redudant code is needed in the netlink parts and in iproute2 when
    > keeping the hsr prefix?
    >
    > >
    > > The patch was tested using two TI AM57x IDK boards which are
    > > connected back to back over two CPSW ports.
    > >
    > > Script used for creating the hsr/prp interface is given below
    > > and uses the ip link command. Also provided logs from the tests
    > > I have executed for your reference.
    > >
    > > iproute2 related patches will follow soon....
    > >
    > > Murali Karicheri
    > > Texas Instruments
    > >
    > > ============ setup.sh =================================================
    > > #!/bin/sh
    > > if [ $# -lt 4 ]
    > > then
    > > echo "setup-cpsw.sh <hsr/prp> <MAC-Address of slave-A>"
    > > echo " <ip address for hsr/prp interface>"
    > > echo " <if_name of hsr/prp interface>"
    > > exit
    > > fi
    > >
    > > if [ "$1" != "hsr" ] && [ "$1" != "prp" ]
    > > then
    > > echo "use hsr or prp as first argument"
    > > exit
    > > fi
    > >
    > > if_a=eth2
    > > if_b=eth3
    > > if_name=$4
    > >
    > > ifconfig $if_a down
    > > ifconfig $if_b down
    > > ifconfig $if_a hw ether $2
    > > ifconfig $if_b hw ether $2
    > > ifconfig $if_a up
    > > ifconfig $if_b up
    > >
    > > echo "Setting up $if_name with MAC address $2 for slaves and IP address $3"
    > > echo " using $if_a and $if_b"
    > >
    > > if [ "$1" = "hsr" ]; then
    > > options="version 1"
    > > else
    > > options=""
    > > fi
    > >
    > > ip link add name $if_name type $1 slave1 $if_a slave2 $if_b supervision 0 $options
    > > ifconfig $if_name $3 up
    > > ==================================================================================
    > > PRP Logs:
    > >
    > > DUT-1 : https://pastebin.ubuntu.com/p/hhsRjTQpcr/
    > > DUT-2 : https://pastebin.ubuntu.com/p/snPFKhnpk4/
    > >
    > > HSR Logs:
    > >
    > > DUT-1 : https://pastebin.ubuntu.com/p/FZPNc6Nwdm/
    > > DUT-2 : https://pastebin.ubuntu.com/p/CtV4ZVS3Yd/
    > >
    > > Murali Karicheri (13):
    > > net: hsr: Re-use Kconfig option to support PRP
    > > net: hsr: rename hsr directory to hsr-prp to introduce PRP
    > > net: hsr: rename files to introduce PRP support
    > > net: hsr: rename hsr variable inside struct hsr_port to priv
    > > net: hsr: rename hsr_port_get_hsr() to hsr_prp_get_port()
    > > net: hsr: some renaming to introduce PRP driver support
    > > net: hsr: introduce common uapi include/definitions for HSR and PRP
    > > net: hsr: migrate HSR netlink socket code to use new common API
    > > net: hsr: move re-usable code for PRP to hsr_prp_netlink.c
    > > net: hsr: add netlink socket interface for PRP
    > > net: prp: add supervision frame generation and handling support
    > > net: prp: add packet handling support
    > > net: prp: enhance debugfs to display PRP specific info in node table
    > >
    > > MAINTAINERS | 2 +-
    > > include/uapi/linux/hsr_netlink.h | 3 +
    > > include/uapi/linux/hsr_prp_netlink.h | 50 ++
    > > include/uapi/linux/if_link.h | 19 +
    > > net/Kconfig | 2 +-
    > > net/Makefile | 2 +-
    > > net/hsr-prp/Kconfig | 37 ++
    > > net/hsr-prp/Makefile | 11 +
    > > net/hsr-prp/hsr_netlink.c | 202 +++++++
    > > net/{hsr => hsr-prp}/hsr_netlink.h | 15 +-
    > > .../hsr_prp_debugfs.c} | 82 +--
    > > net/hsr-prp/hsr_prp_device.c | 562 ++++++++++++++++++
    > > net/hsr-prp/hsr_prp_device.h | 23 +
    > > net/hsr-prp/hsr_prp_forward.c | 558 +++++++++++++++++
    > > .../hsr_prp_forward.h} | 10 +-
    > > .../hsr_prp_framereg.c} | 323 +++++-----
    > > net/hsr-prp/hsr_prp_framereg.h | 68 +++
    > > net/hsr-prp/hsr_prp_main.c | 194 ++++++
    > > net/hsr-prp/hsr_prp_main.h | 289 +++++++++
    > > net/hsr-prp/hsr_prp_netlink.c | 365 ++++++++++++
    > > net/hsr-prp/hsr_prp_netlink.h | 28 +
    > > net/hsr-prp/hsr_prp_slave.c | 222 +++++++
    > > net/hsr-prp/hsr_prp_slave.h | 37 ++
    > > net/hsr-prp/prp_netlink.c | 141 +++++
    > > net/hsr-prp/prp_netlink.h | 27 +
    > > net/hsr/Kconfig | 29 -
    > > net/hsr/Makefile | 10 -
    > > net/hsr/hsr_device.c | 509 ----------------
    > > net/hsr/hsr_device.h | 22 -
    > > net/hsr/hsr_forward.c | 379 ------------
    > > net/hsr/hsr_framereg.h | 62 --
    > > net/hsr/hsr_main.c | 154 -----
    > > net/hsr/hsr_main.h | 188 ------
    > > net/hsr/hsr_netlink.c | 514 ----------------
    > > net/hsr/hsr_slave.c | 198 ------
    > > net/hsr/hsr_slave.h | 33 -
    > > 36 files changed, 3084 insertions(+), 2286 deletions(-)
    > > create mode 100644 include/uapi/linux/hsr_prp_netlink.h
    > > create mode 100644 net/hsr-prp/Kconfig
    > > create mode 100644 net/hsr-prp/Makefile
    > > create mode 100644 net/hsr-prp/hsr_netlink.c
    > > rename net/{hsr => hsr-prp}/hsr_netlink.h (58%)
    > > rename net/{hsr/hsr_debugfs.c => hsr-prp/hsr_prp_debugfs.c} (52%)
    > > create mode 100644 net/hsr-prp/hsr_prp_device.c
    > > create mode 100644 net/hsr-prp/hsr_prp_device.h
    > > create mode 100644 net/hsr-prp/hsr_prp_forward.c
    > > rename net/{hsr/hsr_forward.h => hsr-prp/hsr_prp_forward.h} (50%)
    > > rename net/{hsr/hsr_framereg.c => hsr-prp/hsr_prp_framereg.c} (56%)
    > > create mode 100644 net/hsr-prp/hsr_prp_framereg.h
    > > create mode 100644 net/hsr-prp/hsr_prp_main.c
    > > create mode 100644 net/hsr-prp/hsr_prp_main.h
    > > create mode 100644 net/hsr-prp/hsr_prp_netlink.c
    > > create mode 100644 net/hsr-prp/hsr_prp_netlink.h
    > > create mode 100644 net/hsr-prp/hsr_prp_slave.c
    > > create mode 100644 net/hsr-prp/hsr_prp_slave.h
    > > create mode 100644 net/hsr-prp/prp_netlink.c
    > > create mode 100644 net/hsr-prp/prp_netlink.h
    > > delete mode 100644 net/hsr/Kconfig
    > > delete mode 100644 net/hsr/Makefile
    > > delete mode 100644 net/hsr/hsr_device.c
    > > delete mode 100644 net/hsr/hsr_device.h
    > > delete mode 100644 net/hsr/hsr_forward.c
    > > delete mode 100644 net/hsr/hsr_framereg.h
    > > delete mode 100644 net/hsr/hsr_main.c
    > > delete mode 100644 net/hsr/hsr_main.h
    > > delete mode 100644 net/hsr/hsr_netlink.c
    > > delete mode 100644 net/hsr/hsr_slave.c
    > > delete mode 100644 net/hsr/hsr_slave.h
    > >
    > > --
    > > 2.17.1
    > >
    >
    > --
    > Vinicius

    Thanks,
    -Vladimir

    \
     
     \ /
      Last update: 2020-05-25 23:39    [W:3.123 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site