Commit 68e2c376 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'hsr-several-code-cleanup-for-hsr-module' 



Taehee Yoo says:

====================
hsr: several code cleanup for hsr module

This patchset is to clean up hsr module code.

1. The first patch is to use debugfs_remove_recursive().
If it uses debugfs_remove_recursive() instead of debugfs_remove(),
hsr_priv() doesn't need to have "node_tbl_file" pointer variable.

2. The second patch is to use extack error message.
If HSR uses the extack instead of netdev_info(), users can get
error messages immediately without any checking the kernel message.

3. The third patch is to use netdev_err() instead of WARN_ONCE().
When a packet is being sent, hsr_addr_subst_dest() is called and
it tries to find the node with the ethernet destination address.
If it couldn't find a node, it warns with WARN_ONCE().
But, using WARN_ONCE() is a little bit overdoing.
So, in this patch, netdev_err() is used instead.

4. The fourth patch is to remove unnecessary rcu_read_{lock/unlock}().
There are some rcu_read_{lock/unlock}() in hsr module and some of
them are unnecessary. In this patch,
these unnecessary rcu_read_{lock/unlock}() will be removed.

5. The fifth patch is to use upper/lower device infrastructure.
netdev_upper_dev_link() is useful to manage lower/upper interfaces.
And this function internally validates looping, maximum depth.
If hsr module uses upper/lower device infrastructure,
it can prevent these above problems.
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 892e0915 e0a4b997

net/hsr/hsr_debugfs.c

+1 −4
Original line number Diff line number Diff line
@@ -113,7 +113,6 @@ void hsr_debugfs_init(struct hsr_priv *priv, struct net_device *hsr_dev) 
		priv->node_tbl_root = NULL;

		return;

	}

	priv->node_tbl_file = de;

}



/* hsr_debugfs_term - Tear down debugfs intrastructure
@@ -125,9 +124,7 @@ void hsr_debugfs_init(struct hsr_priv *priv, struct net_device *hsr_dev) 
void

hsr_debugfs_term(struct hsr_priv *priv)

{

	debugfs_remove(priv->node_tbl_file);

	priv->node_tbl_file = NULL;

	debugfs_remove(priv->node_tbl_root);

	debugfs_remove_recursive(priv->node_tbl_root);

	priv->node_tbl_root = NULL;

}

net/hsr/hsr_device.c

+32 −32
Original line number Diff line number Diff line
@@ -57,24 +57,19 @@ static void hsr_set_operstate(struct hsr_port *master, bool has_carrier) 
static bool hsr_check_carrier(struct hsr_port *master)

{

	struct hsr_port *port;

	bool has_carrier;



	has_carrier = false;

	ASSERT_RTNL();



	rcu_read_lock();

	hsr_for_each_port(master->hsr, port)

	hsr_for_each_port(master->hsr, port) {

		if (port->type != HSR_PT_MASTER && is_slave_up(port->dev)) {

			has_carrier = true;

			break;

			netif_carrier_on(master->dev);

			return true;

		}

	}

	rcu_read_unlock();



	if (has_carrier)

		netif_carrier_on(master->dev);

	else

	netif_carrier_off(master->dev);



	return has_carrier;

	return false;

}



static void hsr_check_announce(struct net_device *hsr_dev,
@@ -118,11 +113,9 @@ int hsr_get_max_mtu(struct hsr_priv *hsr) 
	struct hsr_port *port;



	mtu_max = ETH_DATA_LEN;

	rcu_read_lock();

	hsr_for_each_port(hsr, port)

		if (port->type != HSR_PT_MASTER)

			mtu_max = min(port->dev->mtu, mtu_max);

	rcu_read_unlock();



	if (mtu_max < HSR_HLEN)

		return 0;
@@ -157,7 +150,6 @@ static int hsr_dev_open(struct net_device *dev) 
	hsr = netdev_priv(dev);

	designation = '\0';



	rcu_read_lock();

	hsr_for_each_port(hsr, port) {

		if (port->type == HSR_PT_MASTER)

			continue;
@@ -175,7 +167,6 @@ static int hsr_dev_open(struct net_device *dev) 
			netdev_warn(dev, "Slave %c (%s) is not up; please bring it up to get a fully working HSR network\n",

				    designation, port->dev->name);

	}

	rcu_read_unlock();



	if (designation == '\0')

		netdev_warn(dev, "No slave devices configured\n");
@@ -350,22 +341,33 @@ static void hsr_announce(struct timer_list *t) 
	rcu_read_unlock();

}



static void hsr_del_ports(struct hsr_priv *hsr)

{

	struct hsr_port *port;



	port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_A);

	if (port)

		hsr_del_port(port);



	port = hsr_port_get_hsr(hsr, HSR_PT_SLAVE_B);

	if (port)

		hsr_del_port(port);



	port = hsr_port_get_hsr(hsr, HSR_PT_MASTER);

	if (port)

		hsr_del_port(port);

}



/* This has to be called after all the readers are gone.

 * Otherwise we would have to check the return value of

 * hsr_port_get_hsr().

 */

static void hsr_dev_destroy(struct net_device *hsr_dev)

{

	struct hsr_priv *hsr;

	struct hsr_port *port;

	struct hsr_port *tmp;



	hsr = netdev_priv(hsr_dev);

	struct hsr_priv *hsr = netdev_priv(hsr_dev);



	hsr_debugfs_term(hsr);



	list_for_each_entry_safe(port, tmp, &hsr->ports, port_list)

		hsr_del_port(port);

	hsr_del_ports(hsr);



	del_timer_sync(&hsr->prune_timer);

	del_timer_sync(&hsr->announce_timer);
@@ -431,11 +433,10 @@ static const unsigned char def_multicast_addr[ETH_ALEN] __aligned(2) = { 
};



int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],

		     unsigned char multicast_spec, u8 protocol_version)

		     unsigned char multicast_spec, u8 protocol_version,

		     struct netlink_ext_ack *extack)

{

	struct hsr_priv *hsr;

	struct hsr_port *port;

	struct hsr_port *tmp;

	int res;



	hsr = netdev_priv(hsr_dev);
@@ -478,7 +479,7 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2], 
	/* Make sure the 1st call to netif_carrier_on() gets through */

	netif_carrier_off(hsr_dev);



	res = hsr_add_port(hsr, hsr_dev, HSR_PT_MASTER);

	res = hsr_add_port(hsr, hsr_dev, HSR_PT_MASTER, extack);

	if (res)

		goto err_add_master;
@@ -486,11 +487,11 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2], 
	if (res)

		goto err_unregister;



	res = hsr_add_port(hsr, slave[0], HSR_PT_SLAVE_A);

	res = hsr_add_port(hsr, slave[0], HSR_PT_SLAVE_A, extack);

	if (res)

		goto err_add_slaves;



	res = hsr_add_port(hsr, slave[1], HSR_PT_SLAVE_B);

	res = hsr_add_port(hsr, slave[1], HSR_PT_SLAVE_B, extack);

	if (res)

		goto err_add_slaves;
@@ -502,8 +503,7 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2], 
err_add_slaves:

	unregister_netdevice(hsr_dev);

err_unregister:

	list_for_each_entry_safe(port, tmp, &hsr->ports, port_list)

		hsr_del_port(port);

	hsr_del_ports(hsr);

err_add_master:

	hsr_del_self_node(hsr);

net/hsr/hsr_device.h

+2 −1
Original line number Diff line number Diff line
@@ -13,7 +13,8 @@ 


void hsr_dev_setup(struct net_device *dev);

int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],

		     unsigned char multicast_spec, u8 protocol_version);

		     unsigned char multicast_spec, u8 protocol_version,

		     struct netlink_ext_ack *extack);

void hsr_check_carrier_and_operstate(struct hsr_priv *hsr);

bool is_hsr_master(struct net_device *dev);

int hsr_get_max_mtu(struct hsr_priv *hsr);

net/hsr/hsr_framereg.c

+2 −1
Original line number Diff line number Diff line
@@ -318,7 +318,8 @@ void hsr_addr_subst_dest(struct hsr_node *node_src, struct sk_buff *skb, 
	node_dst = find_node_by_addr_A(&port->hsr->node_db,

				       eth_hdr(skb)->h_dest);

	if (!node_dst) {

		WARN_ONCE(1, "%s: Unknown node\n", __func__);

		if (net_ratelimit())

			netdev_err(skb->dev, "%s: Unknown node\n", __func__);

		return;

	}

	if (port->type != node_dst->addr_B_port)

net/hsr/hsr_main.c

+2 −1
Original line number Diff line number Diff line
@@ -85,6 +85,7 @@ static int hsr_netdev_notify(struct notifier_block *nb, unsigned long event, 
		master->dev->mtu = mtu_max;

		break;

	case NETDEV_UNREGISTER:

		if (!is_hsr_master(dev))

			hsr_del_port(port);

		break;

	case NETDEV_PRE_TYPE_CHANGE:

CRA Git | Maintained and supported by SUSTech CRA and CCSE