Re: [net-next RFC PATCH 00/13] net: hsr: Add PRP driver
From: Murali Karicheri
Date: Tue May 26 2020 - 10:12:37 EST
Hi Vladimir,
On 5/25/20 5:37 PM, Vladimir Oltean wrote:
Hi Vinicius,
On Thu, 21 May 2020 at 20:33, Vinicius Costa Gomes
<vinicius.gomes@xxxxxxxxx> wrote:
Murali Karicheri <m-karicheri2@xxxxxx> 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.
I haven't looked the spec for 802.1CB. If they re-use HSR/PRP Tag in the
L2 protocol it make sense to enhance the driver. Else I don't see any
re-use possibility. Do you know the above?
Thanks
Murali
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
--
Murali Karicheri
Texas Instruments