ipoe: allow session interfaces to be removed with ip link del

Sessions left in the kernel by a dead daemon could only be got rid of by
restarting accel-pppd, which flushes all of them and so drops everyone
still online, or by unloading the module. Neither is of much use when a
single stale interface is holding an address and every subscriber that
is later handed it is refused with EEXIST.

Register rtnl_link_ops, so that RTM_DELLINK reaches the driver:

	ip link show type ipoe		enumerate them
	ip -d link show ipoe42		reports kind "ipoe"
	ip link del ipoe42		remove a single one

newlink returns EOPNOTSUPP rather than being left out. A session carries
private state that IPOE_CMD_CREATE sets up, and a kernel that falls back
to register_netdevice() when newlink is NULL would hand out a device
with zeroed private state and no percpu counters, which oopses on the
first packet or on ip -s link show.

dellink runs under rtnl and outside genl_mutex, and is therefore the
first writer in this driver that does not serialise against the genl
handlers: it can unlink a session that IPOE_CMD_DELETE is about to
unlink as well, since the device stays registered until both are done.
Add ipoe_session.dying, set under ipoe_wlock by whichever path starts
the teardown, and let the other one bail out.

That leaves rtnl nested inside ipoe_wlock, which is safe because no path
does it the other way round: ipoe_create() releases rtnl before taking
ipoe_wlock, ipoe_nl_cmd_delete() releases ipoe_wlock before calling
unregister_netdev(), and the interface commands use rtnl alone. dellink
does have to sleep in synchronize_rcu() and while draining the session
refcount with rtnl held, which the genl path avoids by deferring both
until after it has dropped everything.

A session left behind by a dead daemon is indistinguishable from one
that is still in use - same peer address, same lists, same flags - so
the removal cannot be refused on state alone. Warn instead, and only
when somebody is still subscribed to the packet multicast group, so that
clearing stale interfaces on a box where accel-pppd is not running stays
quiet.

1 files changed, 112 insertions, 0 deletions

diff --git a/drivers/ipoe/ipoe.c b/drivers/ipoe/ipoe.c
index 8f1c852b..a9b11fa2 100644
--- a/drivers/ipoe/ipoe.c
+++ b/drivers/ipoe/ipoe.c
@@ -104,6 +104,10 @@ struct ipoe_session {
 
 	atomic_t refs;
 
+	/* set under ipoe_wlock by whoever starts tearing the session down,
+	 * so that the genl and the rtnetlink path do not do it twice */
+	unsigned int dying:1;
+
 	struct ipoe_stats __percpu *rx_stats;
 	struct ipoe_stats __percpu *tx_stats;
 };
@@ -1116,6 +1120,83 @@ static const struct header_ops ipoe_hard_header_ops = {
 	.cache_update	= eth_header_cache_update,
 };
 
+/* Is anybody subscribed to our packet group, i.e. is a control daemon
+ * running at all? */
+static int ipoe_ctrl_attached(void)
+{
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0) && RHEL_MAJOR < 7
+	return netlink_has_listeners(init_net.genl_sock, ipoe_nl_mcg.id);
+#else
+	return genl_has_listeners(&ipoe_nl_family, &init_net, 0);
+#endif
+}
+
+/* Sessions are created through IPOE_CMD_CREATE, which also sets up the
+ * private state.  A device made by rtnetlink would have none of it, so
+ * refuse 'ip link add ... type ipoe' explicitly. */
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6,13,0)
+static int ipoe_newlink(struct net_device *dev,
+			struct rtnl_newlink_params *params,
+			struct netlink_ext_ack *extack)
+#elif LINUX_VERSION_CODE >= KERNEL_VERSION(4,13,0)
+static int ipoe_newlink(struct net *src_net, struct net_device *dev,
+			struct nlattr *tb[], struct nlattr *data[],
+			struct netlink_ext_ack *extack)
+#else
+static int ipoe_newlink(struct net *src_net, struct net_device *dev,
+			struct nlattr *tb[], struct nlattr *data[])
+#endif
+{
+	return -EOPNOTSUPP;
+}
+
+/* Called by rtnetlink with rtnl held, so the teardown ipoe_nl_cmd_delete()
+ * does after dropping ipoe_wlock has to happen here as well. */
+static void ipoe_dellink(struct net_device *dev, struct list_head *head)
+{
+	struct ipoe_session *ses = netdev_priv(dev);
+
+	down(&ipoe_wlock);
+
+	if (ses->dying) {
+		/* the genl path is already removing it, it will unregister
+		 * the device itself once it gets rtnl */
+		up(&ipoe_wlock);
+		return;
+	}
+
+	ses->dying = 1;
+
+	if (ses->peer_addr)
+		list_del_rcu(&ses->entry);
+	list_del(&ses->entry2);
+	if (ses->u.hwaddr_u)
+		list_del_rcu(&ses->entry3);
+
+	up(&ipoe_wlock);
+
+	/* A session left behind by a dead daemon looks exactly like one that
+	 * is still in use, so removing it can not be refused on state alone.
+	 * Complain only if somebody is still subscribed to our multicast
+	 * group, which means a daemon is around to be surprised by it. */
+	if (ses->peer_addr && ipoe_ctrl_attached())
+		pr_warn("ipoe: %s: removed through rtnetlink while bound to %pI4"
+			" and a control daemon is attached\n",
+			dev->name, &ses->peer_addr);
+
+	synchronize_rcu();
+
+	while (atomic_read(&ses->refs))
+		schedule_timeout_uninterruptible(1);
+
+	if (ses->link_dev) {
+		dev_put(ses->link_dev);
+		ses->link_dev = NULL;
+	}
+
+	unregister_netdevice_queue(dev, head);
+}
+
 static void ipoe_netdev_setup(struct net_device *dev)
 {
 	dev->netdev_ops = &ipoe_netdev_ops;
@@ -1145,6 +1226,14 @@ static void ipoe_netdev_setup(struct net_device *dev)
 	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
 }
 
+static struct rtnl_link_ops ipoe_link_ops __read_mostly = {
+	.kind		= "ipoe",
+	.priv_size	= sizeof(struct ipoe_session),
+	.setup		= ipoe_netdev_setup,
+	.newlink	= ipoe_newlink,
+	.dellink	= ipoe_dellink,
+};
+
 static int ipoe_create(__be32 peer_addr, __be32 addr, __be32 gw, int ifindex, const __u8 *hwaddr)
 {
 	struct ipoe_session *ses;
@@ -1220,6 +1309,7 @@ static int ipoe_create(__be32 peer_addr, __be32 addr, __be32 gw, int ifindex, co
 	}*/
 
 	dev->tx_queue_len = 100;
+	dev->rtnl_link_ops = &ipoe_link_ops;
 
 	rtnl_lock();
 	r = register_netdevice(dev);
@@ -1393,6 +1483,14 @@ static int ipoe_nl_cmd_delete(struct sk_buff *skb, struct genl_info *info)
 
 	//pr_info("ipoe: delete %08x\n", ses->peer_addr);
 
+	if (ses->dying) {
+		/* already on its way out through ipoe_dellink() or a flush */
+		ret = 0;
+		goto out_unlock;
+	}
+
+	ses->dying = 1;
+
 	if (ses->peer_addr)
 		list_del_rcu(&ses->entry);
 	list_del(&ses->entry2);
@@ -1431,6 +1529,8 @@ static int ipoe_nl_cmd_flush(struct sk_buff *skb, struct genl_info *info)
 	list_splice_init(&ipoe_list2, &list);
 
 	list_for_each_entry(ses, &list, entry2) {
+		ses->dying = 1;
+
 		if (ses->peer_addr)
 			list_del_rcu(&ses->entry);
 		if (ses->u.hwaddr_u)
@@ -2020,6 +2120,12 @@ static int __init ipoe_init(void)
 	skb_queue_head_init(&ipoe_queue);
 	INIT_WORK(&ipoe_queue_work, ipoe_process_queue);
 
+	err = rtnl_link_register(&ipoe_link_ops);
+	if (err < 0) {
+		printk(KERN_INFO "ipoe: can't register link operations\n");
+		return err;
+	}
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0) && RHEL_MAJOR < 7
 	err = genl_register_family_with_ops(&ipoe_nl_family, ipoe_nl_ops, ARRAY_SIZE(ipoe_nl_ops));
 #elif LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
@@ -2029,6 +2135,7 @@ static int __init ipoe_init(void)
 #endif
 	if (err < 0) {
 		printk(KERN_INFO "ipoe: can't register netlink interface\n");
+		rtnl_link_unregister(&ipoe_link_ops);
 		return err;
 	}
 
@@ -2037,6 +2144,7 @@ static int __init ipoe_init(void)
 	if (err < 0) {
 		printk(KERN_INFO "ipoe: can't register netlink multicast group\n");
 		genl_unregister_family(&ipoe_nl_family);
+		rtnl_link_unregister(&ipoe_link_ops);
 		return err;
 	}
 #endif
@@ -2056,6 +2164,10 @@ static void __exit ipoe_fini(void)
 #endif
 	genl_unregister_family(&ipoe_nl_family);
 
+	/* takes down whatever sessions are left through ipoe_dellink() and
+	 * keeps rtnetlink from starting another one behind our back */
+	rtnl_link_unregister(&ipoe_link_ops);
+
 	down(&ipoe_wlock);
 	up(&ipoe_wlock);