| Message ID | 20200506163033.3843-1-m-karicheri2@ti.com |
|---|---|
| Headers | show |
| Series | net: hsr: Add PRP driver | expand |
Hello netdev experts, On 5/6/20 12:30 PM, Murali Karicheri wrote: > 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. > > 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. > > 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.... Could someone please review this and provide some feedback to take this forward? Thanks and regards, > > Murali Karicheri > Texas Instruments -Cut-------------------------
Hi David, et all, On 5/13/20 8:27 AM, Murali Karicheri wrote: > Hello netdev experts, > > On 5/6/20 12:30 PM, Murali Karicheri wrote: >> 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. >> >> 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. >> >> 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.... > Could someone please review this and provide some feedback to take > this forward? > > Thanks and regards, >> >> Murali Karicheri >> Texas Instruments > > > -Cut------------------------- I plan to send a formal patch early next week as we would like to move forward with this series. So please take some high level look at this and guide me if I am on the right track or this requires rework for a formal patch.
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. 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 >
Hi Vinicius, On 5/21/20 1:31 PM, Vinicius Costa Gomes wrote: > Murali Karicheri <m-karicheri2@ti.com> writes: > ------------ Snip------------- >> - 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. > > 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)() > }; > Thanks for your response! I agree with you that the prefix renaming is ugly. However I wasn't sure if it is okay to use a hsr prefixed code to handle PRP as well as it may not be intuitive to anyone investigating the code. For the same reason, handling 802.1CB specifc functions using the hsr_ prefixed code. If that is okay, then patch 1-6 are unnecessary. We could also add some documentation at the top of the file to indicate that both hsr and prp are implemented in the code or something like that. BTW, I need to investigate more into 802.1CB and this was not known when I developed this code few years ago. Main difference between HSR and PRP is how they handle the protocol tag or rct and create or handle the protocol specific part in the frame. For that part, we should be able to define ops() like you have suggested, instead of doing if check throughout the code. Hope that is what you meant by hsr_ops() for this. Again shouldn't we use some generic name like proto_ops or red_ops instead of hsr_ops() and assign protocol specific implementaion to them? i.e hsr_ or prp_ or 802.1CB specific functions assigned to the function pointers. For now I see handle_frame(), handle_sv_frame, create_frame(), create_sv_frame() etc implemented differently (This is currently part of patch 11 & 12). So something like struct proto_ops { int (*handle_frame)(); int (*create_frame)(); int (*handle_sv_frame)(); int (*create_sv_frame)(); }; and call dev->proto_ops->handle_frame() to process a frame from the main hook. proto_ops gets initialized to of the set if implementation at device or interface creation in hsr_dev_finalize(). >> >> 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? May be this is due to the specific implementation that I chose. Currently I have separate netlink socket for HSR and PRP which may be an overkill since bith are similar protocol. Currently hsr inteface is created as ip link add name hsr0 type hsr slave1 eth0 slave2 eth1 supervision 0 So I have implemented similar command for prp ip link add name prp0 type prp slave1 eth0 slave2 eth1 supervision 0 In patch 7/13 I renamed existing HSR netlink socket attributes that defines the hsr interface with the assumption that we can obsolete the old definitions in favor of new common definitions with the HSR_PRP prefix. Then I have separate code for creating prp interface and related functions, even though they are similar. So using common definitions, I re-use the code in netlink and iproute2 (see patch 8 and 9 to re-use the code). PRP netlink socket code in patch 10 which register prp_genl_family similar to HSR. +static struct genl_family prp_genl_family __ro_after_init = { + .hdrsize = 0, + .name = "PRP", + .version = 1, + .maxattr = HSR_PRP_A_MAX, + .policy = prp_genl_policy, + .module = THIS_MODULE, + .ops = prp_ops, + .n_ops = ARRAY_SIZE(prp_ops), + .mcgrps = prp_mcgrps, + .n_mcgrps = ARRAY_SIZE(prp_mcgrps), +}; + +int __init prp_netlink_init(void) +{ + int rc; + + rc = rtnl_link_register(&prp_link_ops); + if (rc) + goto fail_rtnl_link_register; + + rc = genl_register_family(&prp_genl_family); + if (rc) + goto fail_genl_register_family; If we choose to re-use the existing HSR_ uapi defines, then should we re-use the hsr netlink socket interface for PRP as well and add additional attribute for differentiating the protocol specific part? i.e introduce protocol attribute to existing HSR uapi defines for netlink socket to handle creation of prp interface. enum { HSR_A_UNSPEC, HSR_A_NODE_ADDR, HSR_A_IFINDEX, HSR_A_IF1_AGE, HSR_A_IF2_AGE, HSR_A_NODE_ADDR_B, HSR_A_IF1_SEQ, HSR_A_IF2_SEQ, HSR_A_IF1_IFINDEX, HSR_A_IF2_IFINDEX, HSR_A_ADDR_B_IFINDEX, + HSR_A_PROTOCOL <====if missing it is HSR (backward compatibility) defines HSR or PRP or 802.1CB in future. __HSR_A_MAX, }; So if ip link command is ip link add name <if name> type <proto> slave1 eth0 slave2 eth1 supervision 0 Add HSR_A_PROTOCOL attribute with HSR/PRP specific value. This way, the iprout2 code mostly remain the same as hsr, but will change a bit to introduced this new attribute if user choose proto as 'prp' vs 'hsr' BTW, I have posted the existing iproute2 code also to the mailing list with title 'iproute2: Add PRP support'. If re-using hsr code with existing prefix is fine for PRP or any future protocol such as 801.1B, then I will drop patch 1-6 that are essentially doing some renaming and re-use existing hsr netlink code for PRP with added attribute to differentiate the protocol at the driver as described above along with proto_ops and re-spin the series. Let me know. Regards, Murali > >> >> 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 >> >
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
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@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. > 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 >
Hi Vladimir, Vladimir Oltean <olteanv@gmail.com> writes: >> >> 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? What I was trying to express is the idea of using "hsr" as the directory name/prefix for all the features that deal with frame replication for reliability, including 802.1CB. At least until we find a better name. > 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. Same understanding here. Cheers,
Hi Murali, On Tue, 26 May 2020 at 17:12, Murali Karicheri <m-karicheri2@ti.com> wrote: > > Hi Vladimir, > > 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 IEEE 802.1CB redundancy tag sits between Source MAC address and Ethertype or any VLAN tag, is 6 bytes in length, of which: - first 2 bytes are the 0xf1c1 EtherType - next 2 bytes are reserved - last 2 bytes are the sequence number There is also a pre-standard version of the IEEE 802.1CB redundancy tag, which is only 4 bytes in length. I assume vendors of pre-standard equipment will want to have support for this 4-byte tag as well, as well as a mechanism of converting between HSR/PRP/pre-standard 802.1CB tag on one set of ports, and 802.1CB on another set of ports. > > -- > Murali Karicheri > Texas Instruments Thanks, -Vladimir
Murali Karicheri <m-karicheri2@ti.com> writes: > Hi Vinicius, > > On 5/21/20 1:31 PM, Vinicius Costa Gomes wrote: >> Murali Karicheri <m-karicheri2@ti.com> writes: >> > ------------ Snip------------- > >>> - 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. >> >> 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)() >> }; >> > > Thanks for your response! > > I agree with you that the prefix renaming is ugly. However I wasn't > sure if it is okay to use a hsr prefixed code to handle PRP as > well as it may not be intuitive to anyone investigating the code. For > the same reason, handling 802.1CB specifc functions using the hsr_ > prefixed code. If that is okay, then patch 1-6 are unnecessary. We could > also add some documentation at the top of the file to indicate that > both hsr and prp are implemented in the code or something like that. > BTW, I need to investigate more into 802.1CB and this was not known > when I developed this code few years ago. I think for now it's better to make it clear how similar PRP and HSR are. As for the renaming, I am afraid that this boat has sailed, as the netlink API already uses HSR_ and it's better to reuse that than create a new family for, at least conceptually, the same thing (PRP and 802.1CB). And this is important bit, the userspace API. And even for 802.1CB using name "High-availability Seamless Redudancy" is as good as any, if very pompous. > > Main difference between HSR and PRP is how they handle the protocol tag > or rct and create or handle the protocol specific part in the frame. > For that part, we should be able to define ops() like you have > suggested, instead of doing if check throughout the code. Hope that > is what you meant by hsr_ops() for this. Again shouldn't we use some > generic name like proto_ops or red_ops instead of hsr_ops() and assign > protocol specific implementaion to them? i.e hsr_ or prp_ > or 802.1CB specific functions assigned to the function pointers. For > now I see handle_frame(), handle_sv_frame, create_frame(), > create_sv_frame() etc implemented differently (This is currently part of > patch 11 & 12). So something like > > struct proto_ops { > int (*handle_frame)(); > int (*create_frame)(); > int (*handle_sv_frame)(); > int (*create_sv_frame)(); > }; That's it. That was the idea I was trying to communicate :-) > > and call dev->proto_ops->handle_frame() to process a frame from the > main hook. proto_ops gets initialized to of the set if implementation > at device or interface creation in hsr_dev_finalize(). > >>> >>> 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? > > May be this is due to the specific implementation that I chose. > Currently I have separate netlink socket for HSR and PRP which may > be an overkill since bith are similar protocol. > > Currently hsr inteface is created as > > ip link add name hsr0 type hsr slave1 eth0 slave2 eth1 supervision 0 > > So I have implemented similar command for prp > > ip link add name prp0 type prp slave1 eth0 slave2 eth1 supervision 0 > > In patch 7/13 I renamed existing HSR netlink socket attributes that > defines the hsr interface with the assumption that we can obsolete > the old definitions in favor of new common definitions with the > HSR_PRP prefix. Then I have separate code for creating prp > interface and related functions, even though they are similar. > So using common definitions, I re-use the code in netlink and > iproute2 (see patch 8 and 9 to re-use the code). PRP netlink > socket code in patch 10 which register prp_genl_family similar > to HSR. Deprecating an userspace API is hard and takes a long time. So let's avoid that if it makes sense. > > +static struct genl_family prp_genl_family __ro_after_init = { > + .hdrsize = 0, > + .name = "PRP", > + .version = 1, > + .maxattr = HSR_PRP_A_MAX, > + .policy = prp_genl_policy, > + .module = THIS_MODULE, > + .ops = prp_ops, > + .n_ops = ARRAY_SIZE(prp_ops), > + .mcgrps = prp_mcgrps, > + .n_mcgrps = ARRAY_SIZE(prp_mcgrps), > +}; > + > +int __init prp_netlink_init(void) > +{ > + int rc; > + > + rc = rtnl_link_register(&prp_link_ops); > + if (rc) > + goto fail_rtnl_link_register; > + > + rc = genl_register_family(&prp_genl_family); > + if (rc) > + goto fail_genl_register_family; > > > If we choose to re-use the existing HSR_ uapi defines, then should we > re-use the hsr netlink socket interface for PRP as well and > add additional attribute for differentiating the protocol specific > part? Yes, that seems the way to go. > > i.e introduce protocol attribute to existing HSR uapi defines for > netlink socket to handle creation of prp interface. > > enum { > HSR_A_UNSPEC, > HSR_A_NODE_ADDR, > HSR_A_IFINDEX, > HSR_A_IF1_AGE, > HSR_A_IF2_AGE, > HSR_A_NODE_ADDR_B, > HSR_A_IF1_SEQ, > HSR_A_IF2_SEQ, > HSR_A_IF1_IFINDEX, > HSR_A_IF2_IFINDEX, > HSR_A_ADDR_B_IFINDEX, > + HSR_A_PROTOCOL <====if missing it is HSR (backward > compatibility) > defines HSR or PRP or 802.1CB in future. > __HSR_A_MAX, > }; > > So if ip link command is > > ip link add name <if name> type <proto> slave1 eth0 slave2 eth1 > supervision 0 > > Add HSR_A_PROTOCOL attribute with HSR/PRP specific value. > > This way, the iprout2 code mostly remain the same as hsr, but will > change a bit to introduced this new attribute if user choose proto as > 'prp' vs 'hsr' Sounds good, I think. > > BTW, I have posted the existing iproute2 code also to the mailing list > with title 'iproute2: Add PRP support'. > > If re-using hsr code with existing prefix is fine for PRP or any future > protocol such as 801.1B, then I will drop patch 1-6 that are essentially > doing some renaming and re-use existing hsr netlink code for PRP with > added attribute to differentiate the protocol at the driver as described > above along with proto_ops and re-spin the series. If I forget that HSR is also the name of a protocol, what the acronym means makes sense for 802.1CB, so it's not too bad, I think. > > Let me know. > > Regards, > > Murali Cheers,
Hi Vladimir On 5/26/20 2:25 PM, Vladimir Oltean wrote: > Hi Murali, > > On Tue, 26 May 2020 at 17:12, Murali Karicheri <m-karicheri2@ti.com> wrote: >> >> Hi Vladimir, >> > >> 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 > > IEEE 802.1CB redundancy tag sits between Source MAC address and > Ethertype or any VLAN tag, is 6 bytes in length, of which: > - first 2 bytes are the 0xf1c1 EtherType > - next 2 bytes are reserved > - last 2 bytes are the sequence number > There is also a pre-standard version of the IEEE 802.1CB redundancy > tag, which is only 4 bytes in length. I assume vendors of pre-standard > equipment will want to have support for this 4-byte tag as well, as > well as a mechanism of converting between HSR/PRP/pre-standard 802.1CB > tag on one set of ports, and 802.1CB on another set of ports. > Thanks for sharing the details. I also took a quick glance at the 802.1CB spec today. It answered also my above question 1) In addition to FRER tag, it also includes HSR tag and PRP trailer that can be provisioned through management objects. 2) Now I think I get what Vinicius refers to the interoperability. there can be HSR tag received on ingress port and PRP on the egress port of a relay function. Essentially tag usage is configurable on a stream basis. Since both HSR and PRP functions for frame formatting and decoding would be re-usable. In addition driver could be enhanced for FRER functions. Regards, Murali >> >> -- >> Murali Karicheri >> Texas Instruments > > Thanks, > -Vladimir >
Hi Vinicius, On 5/26/20 2:56 PM, Vinicius Costa Gomes wrote: > Murali Karicheri <m-karicheri2@ti.com> writes: > >> Hi Vinicius, >> >> On 5/21/20 1:31 PM, Vinicius Costa Gomes wrote: >>> Murali Karicheri <m-karicheri2@ti.com> writes: >>> >> ------------ Snip------------- >>> 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)() >>> }; >>> >> >> Thanks for your response! >> >> I agree with you that the prefix renaming is ugly. However I wasn't >> sure if it is okay to use a hsr prefixed code to handle PRP as >> well as it may not be intuitive to anyone investigating the code. For >> the same reason, handling 802.1CB specifc functions using the hsr_ >> prefixed code. If that is okay, then patch 1-6 are unnecessary. We could >> also add some documentation at the top of the file to indicate that >> both hsr and prp are implemented in the code or something like that. >> BTW, I need to investigate more into 802.1CB and this was not known >> when I developed this code few years ago. > > I think for now it's better to make it clear how similar PRP and HSR > are. > > As for the renaming, I am afraid that this boat has sailed, as the > netlink API already uses HSR_ and it's better to reuse that than create > a new family for, at least conceptually, the same thing (PRP and > 802.1CB). And this is important bit, the userspace API. > > And even for 802.1CB using name "High-availability Seamless Redudancy" > is as good as any, if very pompous. > I have reviewed the 802.1CB at a high level. The idea of 802.1CB is also high availability and redundancy similar to HSR and PRP but at stream level. So now I feel more comfortable to re-use the hsr prefix until we find a better name. I can document this in all file headers to make this explicit when I spin the formal patch for this. I will wait for a couple of weeks before start the work on a formal patch series so that others have a chance to respond as well. >> >> Main difference between HSR and PRP is how they handle the protocol tag >> or rct and create or handle the protocol specific part in the frame. >> For that part, we should be able to define ops() like you have >> suggested, instead of doing if check throughout the code. Hope that >> is what you meant by hsr_ops() for this. Again shouldn't we use some >> generic name like proto_ops or red_ops instead of hsr_ops() and assign >> protocol specific implementaion to them? i.e hsr_ or prp_ >> or 802.1CB specific functions assigned to the function pointers. For >> now I see handle_frame(), handle_sv_frame, create_frame(), >> create_sv_frame() etc implemented differently (This is currently part of >> patch 11 & 12). So something like >> >> struct proto_ops { >> int (*handle_frame)(); >> int (*create_frame)(); >> int (*handle_sv_frame)(); >> int (*create_sv_frame)(); >> }; > > That's it. That was the idea I was trying to communicate :-) > Ok >> >> and call dev->proto_ops->handle_frame() to process a frame from the >> main hook. proto_ops gets initialized to of the set if implementation >> at device or interface creation in hsr_dev_finalize(). >> >>>> >>>> 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? >> >> May be this is due to the specific implementation that I chose. >> Currently I have separate netlink socket for HSR and PRP which may >> be an overkill since bith are similar protocol. >> >> Currently hsr inteface is created as >> >> ip link add name hsr0 type hsr slave1 eth0 slave2 eth1 supervision 0 >> >> So I have implemented similar command for prp >> >> ip link add name prp0 type prp slave1 eth0 slave2 eth1 supervision 0 >> >> In patch 7/13 I renamed existing HSR netlink socket attributes that >> defines the hsr interface with the assumption that we can obsolete >> the old definitions in favor of new common definitions with the >> HSR_PRP prefix. Then I have separate code for creating prp >> interface and related functions, even though they are similar. >> So using common definitions, I re-use the code in netlink and >> iproute2 (see patch 8 and 9 to re-use the code). PRP netlink >> socket code in patch 10 which register prp_genl_family similar >> to HSR. > > Deprecating an userspace API is hard and takes a long time. So let's > avoid that if it makes sense. > Ok, make sense. >> >> +static struct genl_family prp_genl_family __ro_after_init = { >> + .hdrsize = 0, >> + .name = "PRP", >> + .version = 1, >> + .maxattr = HSR_PRP_A_MAX, >> + .policy = prp_genl_policy, >> + .module = THIS_MODULE, >> + .ops = prp_ops, >> + .n_ops = ARRAY_SIZE(prp_ops), >> + .mcgrps = prp_mcgrps, >> + .n_mcgrps = ARRAY_SIZE(prp_mcgrps), >> +}; >> + >> +int __init prp_netlink_init(void) >> +{ >> + int rc; >> + >> + rc = rtnl_link_register(&prp_link_ops); >> + if (rc) >> + goto fail_rtnl_link_register; >> + >> + rc = genl_register_family(&prp_genl_family); >> + if (rc) >> + goto fail_genl_register_family; >> >> >> If we choose to re-use the existing HSR_ uapi defines, then should we >> re-use the hsr netlink socket interface for PRP as well and >> add additional attribute for differentiating the protocol specific >> part? > > Yes, that seems the way to go. > Ok. >> >> i.e introduce protocol attribute to existing HSR uapi defines for >> netlink socket to handle creation of prp interface. >> >> enum { >> HSR_A_UNSPEC, >> HSR_A_NODE_ADDR, >> HSR_A_IFINDEX, >> HSR_A_IF1_AGE, >> HSR_A_IF2_AGE, >> HSR_A_NODE_ADDR_B, >> HSR_A_IF1_SEQ, >> HSR_A_IF2_SEQ, >> HSR_A_IF1_IFINDEX, >> HSR_A_IF2_IFINDEX, >> HSR_A_ADDR_B_IFINDEX, >> + HSR_A_PROTOCOL <====if missing it is HSR (backward >> compatibility) >> defines HSR or PRP or 802.1CB in future. >> __HSR_A_MAX, >> }; >> >> So if ip link command is >> >> ip link add name <if name> type <proto> slave1 eth0 slave2 eth1 >> supervision 0 >> >> Add HSR_A_PROTOCOL attribute with HSR/PRP specific value. >> >> This way, the iprout2 code mostly remain the same as hsr, but will >> change a bit to introduced this new attribute if user choose proto as >> 'prp' vs 'hsr' > > Sounds good, I think. Ok. If we want to add 802.1CB later, specific value used can be extended to use 802.1CB. > >> >> BTW, I have posted the existing iproute2 code also to the mailing list >> with title 'iproute2: Add PRP support'. >> >> If re-using hsr code with existing prefix is fine for PRP or any future >> protocol such as 801.1B, then I will drop patch 1-6 that are essentially >> doing some renaming and re-use existing hsr netlink code for PRP with >> added attribute to differentiate the protocol at the driver as described >> above along with proto_ops and re-spin the series. > > If I forget that HSR is also the name of a protocol, what the acronym > means makes sense for 802.1CB, so it's not too bad, I think. > Agree. >> >> Let me know. >> >> Regards, >> >> Murali > > > Cheers, >
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. 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. 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