From 70ebe4a47185db15f3c55be9611a1a971237870b Mon Sep 17 00:00:00 2001 From: Arvid Brodin Date: Fri, 4 Jul 2014 23:34:38 +0200 Subject: net/hsr: Better variable names and update of contact info. Signed-off-by: Arvid Brodin Signed-off-by: David S. Miller --- net/hsr/hsr_netlink.c | 54 +++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'net/hsr/hsr_netlink.c') diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index 01a5261ac7a5..bea250ec3586 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -1,4 +1,4 @@ -/* Copyright 2011-2013 Autronica Fire and Security AS +/* Copyright 2011-2014 Autronica Fire and Security AS * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the Free @@ -6,7 +6,7 @@ * any later version. * * Author(s): - * 2011-2013 Arvid Brodin, arvid.brodin@xdin.com + * 2011-2014 Arvid Brodin, arvid.brodin@alten.se * * Routines for handling Netlink messages for HSR. */ @@ -63,21 +63,21 @@ static int hsr_newlink(struct net *src_net, struct net_device *dev, static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev) { - struct hsr_priv *hsr_priv; + struct hsr_priv *hsr; - hsr_priv = netdev_priv(dev); + hsr = netdev_priv(dev); - if (hsr_priv->slave[0]) - if (nla_put_u32(skb, IFLA_HSR_SLAVE1, hsr_priv->slave[0]->ifindex)) + if (hsr->slave[0]) + if (nla_put_u32(skb, IFLA_HSR_SLAVE1, hsr->slave[0]->ifindex)) goto nla_put_failure; - if (hsr_priv->slave[1]) - if (nla_put_u32(skb, IFLA_HSR_SLAVE2, hsr_priv->slave[1]->ifindex)) + if (hsr->slave[1]) + if (nla_put_u32(skb, IFLA_HSR_SLAVE2, hsr->slave[1]->ifindex)) goto nla_put_failure; if (nla_put(skb, IFLA_HSR_SUPERVISION_ADDR, ETH_ALEN, - hsr_priv->sup_multicast_addr) || - nla_put_u16(skb, IFLA_HSR_SEQ_NR, hsr_priv->sequence_nr)) + hsr->sup_multicast_addr) || + nla_put_u16(skb, IFLA_HSR_SEQ_NR, hsr->sequence_nr)) goto nla_put_failure; return 0; @@ -128,7 +128,7 @@ static const struct genl_multicast_group hsr_mcgrps[] = { * over one of the slave interfaces. This would indicate an open network ring * (i.e. a link has failed somewhere). */ -void hsr_nl_ringerror(struct hsr_priv *hsr_priv, unsigned char addr[ETH_ALEN], +void hsr_nl_ringerror(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN], enum hsr_dev_idx dev_idx) { struct sk_buff *skb; @@ -148,8 +148,8 @@ void hsr_nl_ringerror(struct hsr_priv *hsr_priv, unsigned char addr[ETH_ALEN], if (res < 0) goto nla_put_failure; - if (hsr_priv->slave[dev_idx]) - ifindex = hsr_priv->slave[dev_idx]->ifindex; + if (hsr->slave[dev_idx]) + ifindex = hsr->slave[dev_idx]->ifindex; else ifindex = -1; res = nla_put_u32(skb, HSR_A_IFINDEX, ifindex); @@ -165,13 +165,13 @@ nla_put_failure: kfree_skb(skb); fail: - netdev_warn(hsr_priv->dev, "Could not send HSR ring error message\n"); + netdev_warn(hsr->dev, "Could not send HSR ring error message\n"); } /* This is called when we haven't heard from the node with MAC address addr for * some time (just before the node is removed from the node table/list). */ -void hsr_nl_nodedown(struct hsr_priv *hsr_priv, unsigned char addr[ETH_ALEN]) +void hsr_nl_nodedown(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN]) { struct sk_buff *skb; void *msg_head; @@ -199,7 +199,7 @@ nla_put_failure: kfree_skb(skb); fail: - netdev_warn(hsr_priv->dev, "Could not send HSR node down\n"); + netdev_warn(hsr->dev, "Could not send HSR node down\n"); } @@ -220,7 +220,7 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) /* For sending */ struct sk_buff *skb_out; void *msg_head; - struct hsr_priv *hsr_priv; + struct hsr_priv *hsr; unsigned char hsr_node_addr_b[ETH_ALEN]; int hsr_node_if1_age; u16 hsr_node_if1_seq; @@ -267,8 +267,8 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) if (res < 0) goto nla_put_failure; - hsr_priv = netdev_priv(hsr_dev); - res = hsr_get_node_data(hsr_priv, + hsr = netdev_priv(hsr_dev); + res = hsr_get_node_data(hsr, (unsigned char *) nla_data(info->attrs[HSR_A_NODE_ADDR]), hsr_node_addr_b, &addr_b_ifindex, @@ -301,9 +301,9 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) res = nla_put_u16(skb_out, HSR_A_IF1_SEQ, hsr_node_if1_seq); if (res < 0) goto nla_put_failure; - if (hsr_priv->slave[0]) + if (hsr->slave[0]) res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX, - hsr_priv->slave[0]->ifindex); + hsr->slave[0]->ifindex); if (res < 0) goto nla_put_failure; @@ -313,9 +313,9 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) res = nla_put_u16(skb_out, HSR_A_IF2_SEQ, hsr_node_if2_seq); if (res < 0) goto nla_put_failure; - if (hsr_priv->slave[1]) + if (hsr->slave[1]) res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX, - hsr_priv->slave[1]->ifindex); + hsr->slave[1]->ifindex); genlmsg_end(skb_out, msg_head); genlmsg_unicast(genl_info_net(info), skb_out, info->snd_portid); @@ -345,7 +345,7 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info) /* For sending */ struct sk_buff *skb_out; void *msg_head; - struct hsr_priv *hsr_priv; + struct hsr_priv *hsr; void *pos; unsigned char addr[ETH_ALEN]; int res; @@ -385,17 +385,17 @@ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info) if (res < 0) goto nla_put_failure; - hsr_priv = netdev_priv(hsr_dev); + hsr = netdev_priv(hsr_dev); rcu_read_lock(); - pos = hsr_get_next_node(hsr_priv, NULL, addr); + pos = hsr_get_next_node(hsr, NULL, addr); while (pos) { res = nla_put(skb_out, HSR_A_NODE_ADDR, ETH_ALEN, addr); if (res < 0) { rcu_read_unlock(); goto nla_put_failure; } - pos = hsr_get_next_node(hsr_priv, pos, addr); + pos = hsr_get_next_node(hsr, pos, addr); } rcu_read_unlock(); -- cgit v1.2.2 From 51f3c605318b056ac5deb9079bbef2a976558827 Mon Sep 17 00:00:00 2001 From: Arvid Brodin Date: Fri, 4 Jul 2014 23:37:27 +0200 Subject: net/hsr: Move slave init to hsr_slave.c. Also try to prevent some possible slave dereference race conditions. This is finalized in the next patch, which abandons the slave array in favour of a list_head list and list RCU. Signed-off-by: Arvid Brodin Signed-off-by: David S. Miller --- net/hsr/hsr_netlink.c | 53 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 15 deletions(-) (limited to 'net/hsr/hsr_netlink.c') diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index bea250ec3586..a2ce359774f3 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -64,16 +64,28 @@ static int hsr_newlink(struct net *src_net, struct net_device *dev, static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev) { struct hsr_priv *hsr; + struct net_device *slave; + int res; hsr = netdev_priv(dev); - if (hsr->slave[0]) - if (nla_put_u32(skb, IFLA_HSR_SLAVE1, hsr->slave[0]->ifindex)) - goto nla_put_failure; + res = 0; - if (hsr->slave[1]) - if (nla_put_u32(skb, IFLA_HSR_SLAVE2, hsr->slave[1]->ifindex)) - goto nla_put_failure; + rcu_read_lock(); + slave = hsr->slave[0]; + if (slave) + res = nla_put_u32(skb, IFLA_HSR_SLAVE1, slave->ifindex); + rcu_read_unlock(); + if (res) + goto nla_put_failure; + + rcu_read_lock(); + slave = hsr->slave[1]; + if (slave) + res = nla_put_u32(skb, IFLA_HSR_SLAVE2, slave->ifindex); + rcu_read_unlock(); + if (res) + goto nla_put_failure; if (nla_put(skb, IFLA_HSR_SUPERVISION_ADDR, ETH_ALEN, hsr->sup_multicast_addr) || @@ -132,6 +144,7 @@ void hsr_nl_ringerror(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN], enum hsr_dev_idx dev_idx) { struct sk_buff *skb; + struct net_device *slave; void *msg_head; int res; int ifindex; @@ -148,10 +161,14 @@ void hsr_nl_ringerror(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN], if (res < 0) goto nla_put_failure; - if (hsr->slave[dev_idx]) - ifindex = hsr->slave[dev_idx]->ifindex; + rcu_read_lock(); + slave = hsr->slave[dev_idx]; + if (slave) + ifindex = slave->ifindex; else ifindex = -1; + rcu_read_unlock(); + res = nla_put_u32(skb, HSR_A_IFINDEX, ifindex); if (res < 0) goto nla_put_failure; @@ -215,7 +232,7 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) { /* For receiving */ struct nlattr *na; - struct net_device *hsr_dev; + struct net_device *hsr_dev, *slave; /* For sending */ struct sk_buff *skb_out; @@ -301,9 +318,11 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) res = nla_put_u16(skb_out, HSR_A_IF1_SEQ, hsr_node_if1_seq); if (res < 0) goto nla_put_failure; - if (hsr->slave[0]) - res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX, - hsr->slave[0]->ifindex); + rcu_read_lock(); + slave = hsr->slave[0]; + if (slave) + res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX, slave->ifindex); + rcu_read_unlock(); if (res < 0) goto nla_put_failure; @@ -313,9 +332,13 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) res = nla_put_u16(skb_out, HSR_A_IF2_SEQ, hsr_node_if2_seq); if (res < 0) goto nla_put_failure; - if (hsr->slave[1]) - res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX, - hsr->slave[1]->ifindex); + rcu_read_lock(); + slave = hsr->slave[1]; + if (slave) + res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX, slave->ifindex); + rcu_read_unlock(); + if (res < 0) + goto nla_put_failure; genlmsg_end(skb_out, msg_head); genlmsg_unicast(genl_info_net(info), skb_out, info->snd_portid); -- cgit v1.2.2 From c5a7591172100269e426cf630da0f2dc8138a206 Mon Sep 17 00:00:00 2001 From: Arvid Brodin Date: Fri, 4 Jul 2014 23:38:05 +0200 Subject: net/hsr: Use list_head (and rcu) instead of array for slave devices. Signed-off-by: Arvid Brodin Signed-off-by: David S. Miller --- net/hsr/hsr_netlink.c | 57 ++++++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 28 deletions(-) (limited to 'net/hsr/hsr_netlink.c') diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index a2ce359774f3..67082453928c 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -64,7 +64,7 @@ static int hsr_newlink(struct net *src_net, struct net_device *dev, static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev) { struct hsr_priv *hsr; - struct net_device *slave; + struct hsr_port *port; int res; hsr = netdev_priv(dev); @@ -72,17 +72,17 @@ static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev) res = 0; rcu_read_lock(); - slave = hsr->slave[0]; - if (slave) - res = nla_put_u32(skb, IFLA_HSR_SLAVE1, slave->ifindex); + port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_A); + if (port) + res = nla_put_u32(skb, IFLA_HSR_SLAVE1, port->dev->ifindex); rcu_read_unlock(); if (res) goto nla_put_failure; rcu_read_lock(); - slave = hsr->slave[1]; - if (slave) - res = nla_put_u32(skb, IFLA_HSR_SLAVE2, slave->ifindex); + port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_B); + if (port) + res = nla_put_u32(skb, IFLA_HSR_SLAVE2, port->dev->ifindex); rcu_read_unlock(); if (res) goto nla_put_failure; @@ -141,13 +141,12 @@ static const struct genl_multicast_group hsr_mcgrps[] = { * (i.e. a link has failed somewhere). */ void hsr_nl_ringerror(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN], - enum hsr_dev_idx dev_idx) + struct hsr_port *port) { struct sk_buff *skb; - struct net_device *slave; void *msg_head; + struct hsr_port *master; int res; - int ifindex; skb = genlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC); if (!skb) @@ -161,15 +160,7 @@ void hsr_nl_ringerror(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN], if (res < 0) goto nla_put_failure; - rcu_read_lock(); - slave = hsr->slave[dev_idx]; - if (slave) - ifindex = slave->ifindex; - else - ifindex = -1; - rcu_read_unlock(); - - res = nla_put_u32(skb, HSR_A_IFINDEX, ifindex); + res = nla_put_u32(skb, HSR_A_IFINDEX, port->dev->ifindex); if (res < 0) goto nla_put_failure; @@ -182,7 +173,10 @@ nla_put_failure: kfree_skb(skb); fail: - netdev_warn(hsr->dev, "Could not send HSR ring error message\n"); + rcu_read_lock(); + master = hsr_port_get_hsr(hsr, HSR_PT_MASTER); + netdev_warn(master->dev, "Could not send HSR ring error message\n"); + rcu_read_unlock(); } /* This is called when we haven't heard from the node with MAC address addr for @@ -192,6 +186,7 @@ void hsr_nl_nodedown(struct hsr_priv *hsr, unsigned char addr[ETH_ALEN]) { struct sk_buff *skb; void *msg_head; + struct hsr_port *master; int res; skb = genlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC); @@ -216,7 +211,10 @@ nla_put_failure: kfree_skb(skb); fail: - netdev_warn(hsr->dev, "Could not send HSR node down\n"); + rcu_read_lock(); + master = hsr_port_get_hsr(hsr, HSR_PT_MASTER); + netdev_warn(master->dev, "Could not send HSR node down\n"); + rcu_read_unlock(); } @@ -232,12 +230,13 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) { /* For receiving */ struct nlattr *na; - struct net_device *hsr_dev, *slave; + struct net_device *hsr_dev; /* For sending */ struct sk_buff *skb_out; void *msg_head; struct hsr_priv *hsr; + struct hsr_port *port; unsigned char hsr_node_addr_b[ETH_ALEN]; int hsr_node_if1_age; u16 hsr_node_if1_seq; @@ -319,9 +318,10 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) if (res < 0) goto nla_put_failure; rcu_read_lock(); - slave = hsr->slave[0]; - if (slave) - res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX, slave->ifindex); + port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_A); + if (port) + res = nla_put_u32(skb_out, HSR_A_IF1_IFINDEX, + port->dev->ifindex); rcu_read_unlock(); if (res < 0) goto nla_put_failure; @@ -333,9 +333,10 @@ static int hsr_get_node_status(struct sk_buff *skb_in, struct genl_info *info) if (res < 0) goto nla_put_failure; rcu_read_lock(); - slave = hsr->slave[1]; - if (slave) - res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX, slave->ifindex); + port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_B); + if (port) + res = nla_put_u32(skb_out, HSR_A_IF2_IFINDEX, + port->dev->ifindex); rcu_read_unlock(); if (res < 0) goto nla_put_failure; -- cgit v1.2.2 From f266a683a4804dc499efc6c2206ef68efed029d0 Mon Sep 17 00:00:00 2001 From: Arvid Brodin Date: Fri, 4 Jul 2014 23:41:03 +0200 Subject: net/hsr: Better frame dispatch This patch removes the separate paths for frames coming from the outside, and frames sent from the HSR device, and instead makes all frames go through hsr_forward_skb() in hsr_forward.c. This greatly improves code readability and also opens up the possibility for future support of the HSR Interlink device that is the basis for HSR RedBoxes and HSR QuadBoxes, as well as VLAN compatibility. Other improvements: * A reduction in the number of times an skb is copied on machines without HAVE_EFFICIENT_UNALIGNED_ACCESS, which improves throughput somewhat. * Headers are now created using the standard eth_header(), and using the standard hard_header_len. * Each HSR slave now gets its own private skb, so slave-specific fields can be correctly set. Signed-off-by: Arvid Brodin Signed-off-by: David S. Miller --- net/hsr/hsr_netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/hsr/hsr_netlink.c') diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index 67082453928c..fbdf53f1d874 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -358,7 +358,7 @@ fail: return res; } -/* Get a list of MacAddressA of all nodes known to this node (other than self). +/* Get a list of MacAddressA of all nodes known to this node (including self). */ static int hsr_get_node_list(struct sk_buff *skb_in, struct genl_info *info) { -- cgit v1.2.2 From a718dcc5e56546a62d00f57cc875faac2f42c8bf Mon Sep 17 00:00:00 2001 From: Arvid Brodin Date: Fri, 4 Jul 2014 23:42:00 +0200 Subject: net/hsr: Fix NULL pointer dereference on incomplete hsr_newlink() params. If none of the slave interfaces are specified, struct nlattr *data[] may be NULL. Make sure to check for that. While I'm at it, fix the horrible error messages displayed when only one of the slave interfaces isn't specified. Signed-off-by: Arvid Brodin Signed-off-by: David S. Miller --- net/hsr/hsr_netlink.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'net/hsr/hsr_netlink.c') diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index fbdf53f1d874..a2c7e4c0ac1e 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -37,13 +37,17 @@ static int hsr_newlink(struct net *src_net, struct net_device *dev, struct net_device *link[2]; unsigned char multicast_spec; + if (!data) { + netdev_info(dev, "HSR: No slave devices specified\n"); + return -EINVAL; + } if (!data[IFLA_HSR_SLAVE1]) { - netdev_info(dev, "IFLA_HSR_SLAVE1 missing!\n"); + netdev_info(dev, "HSR: Slave1 device not specified\n"); return -EINVAL; } link[0] = __dev_get_by_index(src_net, nla_get_u32(data[IFLA_HSR_SLAVE1])); if (!data[IFLA_HSR_SLAVE2]) { - netdev_info(dev, "IFLA_HSR_SLAVE2 missing!\n"); + netdev_info(dev, "HSR: Slave2 device not specified\n"); return -EINVAL; } link[1] = __dev_get_by_index(src_net, nla_get_u32(data[IFLA_HSR_SLAVE2])); -- cgit v1.2.2