net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)

High-availability Seamless Redundancy ("HSR") provides instant failover redundancy for Ethernet networks. It requires a special network topology where all nodes are connected in a ring (each node having two physical network interfaces). It is suited for applications that demand high availability and very short reaction time.

 net/hsr/hsr_device.h          |  30 +++
 net/hsr/hsr_framereg.h        |  54 ++++
 net/hsr/hsr_main.h            | 167 ++++++++++++
 net/hsr/hsr_netlink.h         |  74 ++++++

Quoted 5 lines · Arvid Brodin

High-availability Seamless Redundancy ("HSR") provides instant failover redundancy for Ethernet networks. It requires a special network topology where all nodes are connected in a ring (each node having two physical network interfaces). It is suited for applications that demand high availability and very short reaction time.

trivia:

You should probably use checkpatch.pl --strict for files in net. It will suggest aligning arguments in the more common net style.

Quoted 4 lines

 net/hsr/hsr_device.h          |  30 +++
 net/hsr/hsr_framereg.h        |  54 ++++
 net/hsr/hsr_main.h            | 167 ++++++++++++
 net/hsr/hsr_netlink.h         |  74 ++++++

Maybe some of these .h files should go into include/net/...

A future version of checkpatch may read all the include files and exempt any CamelCase defines/typedefs/functions from CamelCase warnings.

net/hsr/hsr_device.c line 220

Quoted 2 lines

net/hsr/hsr_device.c new file +2 -0

+	if ((skb->protocol != htons(ETH_P_PRP)) ||
+			(hsr_ethhdr->ethhdr.h_proto != htons(ETH_P_PRP))) {

Please align indents after the appropriate open parenthesis.

	if (foo ||
	    bar) {

net/hsr/hsr_device.c line 455

Quoted 10 lines

net/hsr/hsr_device.c new file +10 -0

+bool is_hsr_master(struct net_device *dev)
+{
+	if (!dev) {
+		WARN_ON_ONCE(1);
+		return 0;
+	}
+	if (!dev->netdev_ops) {
+		WARN_ON_ONCE(1);
+		return 0;
+	}

probably better to combine and give a textual reason

net/hsr/hsr_main.h

Quoted 1 line

net/hsr/hsr_main.h +0 -0

[]

net/hsr/hsr_main.h line 26

Quoted 3 lines

net/hsr/hsr_main.h +3 -0

+#define HSR_LIFE_CHECK_INTERVAL		 2000 /* ms */
+#define HSR_NODE_FORGET_TIME		60000 /* ms */
+#define HSR_ANNOUNCE_INTERVAL		  100 /* ms */

Odd alignment

net/hsr/hsr_netlink.c

Quoted 1 line

net/hsr/hsr_netlink.c +0 -0

[]

net/hsr/hsr_netlink.c line 81

Quoted 6 lines

net/hsr/hsr_netlink.c +6 -0

+static const struct nla_policy hsr_genl_policy[HSR_A_MAX + 1] = {
+	[HSR_A_NODE_ADDR] = { .type = NLA_BINARY, .len = ETH_ALEN },
+	[HSR_A_NODE_ADDR_B] = { .type = NLA_BINARY, .len = ETH_ALEN },
+	[HSR_A_IFINDEX] = { .type = NLA_U32 },
+	[HSR_A_IF1_AGE] = { .type = NLA_U32 }, /* Actually signed 32-bit */
+	[HSR_A_IF2_AGE] = { .type = NLA_U32 }, /* Actually signed 32-bit */

Why not use NLA_S32?

Quoted 7 lines · Joe Perches

Quoted 5 lines · Arvid Brodin

High-availability Seamless Redundancy ("HSR") provides instant failover redundancy for Ethernet networks. It requires a special network topology where all nodes are connected in a ring (each node having two physical network interfaces). It is suited for applications that demand high availability and very short reaction time.

Quoted 4 lines

trivia:

You should probably use checkpatch.pl --strict for files in net. It will suggest aligning arguments in the more common net style.

Does this mean I should also remove spaces after casts (IMO this would reduce readability somewhat)?

I cannot judge if these files should go into include/net/ or not. Where can I get a final say on this?

Some of the definitions in hsr_netlink.h are needed by userspace tools that want to listen for ring errors etc from the HSR driver, so it would be a good thing if this file could be part of the kernel headers install. How can I accomplish this?

Quoted 4 lines

Quoted 4 lines

net/hsr/hsr_device.c new file +4 -0

+bool is_hsr_master(struct net_device *dev)
+{
+	if (!dev) {
+		WARN_ON_ONCE(1);

Quoted 8 lines

net/hsr/hsr_device.c line 455

Quoted 6 lines

net/hsr/hsr_device.c new file +6 -0

+		return 0;
+	}
+	if (!dev->netdev_ops) {
+		WARN_ON_ONCE(1);
+		return 0;
+	}

probably better to combine and give a textual reason

Or perhaps better to remove them altogether? I guess you could call them debug statements...

Quoted 7 lines

net/hsr/hsr_main.h

Quoted 1 line

net/hsr/hsr_main.h +0 -0

[]

net/hsr/hsr_main.h line 26

Quoted 3 lines

net/hsr/hsr_main.h +3 -0

+#define HSR_LIFE_CHECK_INTERVAL		 2000 /* ms */
+#define HSR_NODE_FORGET_TIME		60000 /* ms */
+#define HSR_ANNOUNCE_INTERVAL		  100 /* ms */

Odd alignment

Only because of the plus chars added by diff. :)

Quoted 10 lines

net/hsr/hsr_netlink.c

Quoted 1 line

net/hsr/hsr_netlink.c +0 -0

[]

net/hsr/hsr_netlink.c line 81

Quoted 6 lines

net/hsr/hsr_netlink.c +6 -0

+static const struct nla_policy hsr_genl_policy[HSR_A_MAX + 1] = {
+	[HSR_A_NODE_ADDR] = { .type = NLA_BINARY, .len = ETH_ALEN },
+	[HSR_A_NODE_ADDR_B] = { .type = NLA_BINARY, .len = ETH_ALEN },
+	[HSR_A_IFINDEX] = { .type = NLA_U32 },
+	[HSR_A_IF1_AGE] = { .type = NLA_U32 }, /* Actually signed 32-bit */
+	[HSR_A_IF2_AGE] = { .type = NLA_U32 }, /* Actually signed 32-bit */

Why not use NLA_S32?

We need the code to work on older kernels as well, where NLA_S32 does not exist.

Actually,
these values never become negative with the current code. During development we returned a negative value to mean "out of range" but we have switched to INT_MAX instead. So perhaps it's best just to remove these comments?