netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <michael@ellerman.id.au>
To: <jgarzik@pobox.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linuxppc64-dev@ozlabs.org
Subject: [PATCH 9/18] iseries_veth: Use kobjects to track lifecycle of connection structs
Date: Thu,  1 Sep 2005 11:29:09 +1000 (EST)	[thread overview]
Message-ID: <20050901012909.C98036820F@ozlabs.org> (raw)
In-Reply-To: <1125538127.859382.875909607846.qpush@concordia>

The iseries_veth driver can attach to multiple vlans, which correspond to
multiple net devices. However there is only 1 connection between each LPAR,
so the connection structure may be shared by multiple net devices.

This makes module removal messy, because we can't deallocate the connections
until we know there are no net devices still using them. The solution is to
use ref counts on the connections, so we can delete them (actually stop) as
soon as the ref count hits zero.

This patch fixes (part of) a bug we were seeing with IPv6 sending probes to
a dead LPAR, which would then hang us forever due to leftover skbs.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 drivers/net/iseries_veth.c |  121 ++++++++++++++++++++++++++++++---------------
 1 files changed, 83 insertions(+), 38 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
@@ -129,6 +129,7 @@ struct veth_lpar_connection {
 	int num_events;
 	struct VethCapData local_caps;
 
+	struct kobject kobject;
 	struct timer_list ack_timer;
 
 	spinlock_t lock;
@@ -171,6 +172,11 @@ static void veth_recycle_msg(struct veth
 static void veth_flush_pending(struct veth_lpar_connection *cnx);
 static void veth_receive(struct veth_lpar_connection *, struct VethLpEvent *);
 static void veth_timed_ack(unsigned long connectionPtr);
+static void veth_release_connection(struct kobject *kobject);
+
+static struct kobj_type veth_lpar_connection_ktype = {
+	.release	= veth_release_connection
+};
 
 /*
  * Utility functions
@@ -611,7 +617,7 @@ static int veth_init_connection(u8 rlp)
 {
 	struct veth_lpar_connection *cnx;
 	struct veth_msg *msgs;
-	int i;
+	int i, rc;
 
 	if ( (rlp == this_lp)
 	     || ! HvLpConfig_doLpsCommunicateOnVirtualLan(this_lp, rlp) )
@@ -632,6 +638,14 @@ static int veth_init_connection(u8 rlp)
 
 	veth_cnx[rlp] = cnx;
 
+	/* This gets us 1 reference, which is held on behalf of the driver
+	 * infrastructure. It's released at module unload. */
+	kobject_init(&cnx->kobject);
+	cnx->kobject.ktype = &veth_lpar_connection_ktype;
+	rc = kobject_set_name(&cnx->kobject, "cnx%.2d", rlp);
+	if (rc != 0)
+		return rc;
+
 	msgs = kmalloc(VETH_NUMBUFFERS * sizeof(struct veth_msg), GFP_KERNEL);
 	if (! msgs) {
 		veth_error("Can't allocate buffers for LPAR %d.\n", rlp);
@@ -660,11 +674,9 @@ static int veth_init_connection(u8 rlp)
 	return 0;
 }
 
-static void veth_stop_connection(u8 rlp)
+static void veth_stop_connection(struct veth_lpar_connection *cnx)
 {
-	struct veth_lpar_connection *cnx = veth_cnx[rlp];
-
-	if (! cnx)
+	if (!cnx)
 		return;
 
 	spin_lock_irq(&cnx->lock);
@@ -685,11 +697,9 @@ static void veth_stop_connection(u8 rlp)
 	flush_scheduled_work();
 }
 
-static void veth_destroy_connection(u8 rlp)
+static void veth_destroy_connection(struct veth_lpar_connection *cnx)
 {
-	struct veth_lpar_connection *cnx = veth_cnx[rlp];
-
-	if (! cnx)
+	if (!cnx)
 		return;
 
 	if (cnx->num_events > 0)
@@ -704,8 +714,16 @@ static void veth_destroy_connection(u8 r
 				      NULL, NULL);
 
 	kfree(cnx->msgs);
+	veth_cnx[cnx->remote_lp] = NULL;
 	kfree(cnx);
-	veth_cnx[rlp] = NULL;
+}
+
+static void veth_release_connection(struct kobject *kobj)
+{
+	struct veth_lpar_connection *cnx;
+	cnx = container_of(kobj, struct veth_lpar_connection, kobject);
+	veth_stop_connection(cnx);
+	veth_destroy_connection(cnx);
 }
 
 /*
@@ -1349,15 +1367,31 @@ static void veth_timed_ack(unsigned long
 
 static int veth_remove(struct vio_dev *vdev)
 {
-	int i = vdev->unit_address;
+	struct veth_lpar_connection *cnx;
 	struct net_device *dev;
+	struct veth_port *port;
+	int i;
 
-	dev = veth_dev[i];
-	if (dev != NULL) {
-		veth_dev[i] = NULL;
-		unregister_netdev(dev);
-		free_netdev(dev);
+	dev = veth_dev[vdev->unit_address];
+
+	if (! dev)
+		return 0;
+
+	port = netdev_priv(dev);
+
+	for (i = 0; i < HVMAXARCHITECTEDLPS; i++) {
+		cnx = veth_cnx[i];
+
+		if (cnx && (port->lpar_map & (1 << i))) {
+			/* Drop our reference to connections on our VLAN */
+			kobject_put(&cnx->kobject);
+		}
 	}
+
+	veth_dev[vdev->unit_address] = NULL;
+	unregister_netdev(dev);
+	free_netdev(dev);
+
 	return 0;
 }
 
@@ -1365,6 +1399,7 @@ static int veth_probe(struct vio_dev *vd
 {
 	int i = vdev->unit_address;
 	struct net_device *dev;
+	struct veth_port *port;
 
 	dev = veth_probe_one(i, &vdev->dev);
 	if (dev == NULL) {
@@ -1373,11 +1408,23 @@ static int veth_probe(struct vio_dev *vd
 	}
 	veth_dev[i] = dev;
 
-	/* Start the state machine on each connection, to commence
-	 * link negotiation */
-	for (i = 0; i < HVMAXARCHITECTEDLPS; i++)
-		if (veth_cnx[i])
-			veth_kick_statemachine(veth_cnx[i]);
+	port = (struct veth_port*)netdev_priv(dev);
+
+	/* Start the state machine on each connection on this vlan. If we're
+	 * the first dev to do so this will commence link negotiation */
+	for (i = 0; i < HVMAXARCHITECTEDLPS; i++) {
+		struct veth_lpar_connection *cnx;
+
+		if (! (port->lpar_map & (1 << i)))
+			continue;
+
+		cnx = veth_cnx[i];
+		if (!cnx)
+			continue;
+
+		kobject_get(&cnx->kobject);
+		veth_kick_statemachine(cnx);
+	}
 
 	return 0;
 }
@@ -1406,29 +1453,27 @@ static struct vio_driver veth_driver = {
 void __exit veth_module_cleanup(void)
 {
 	int i;
+	struct veth_lpar_connection *cnx;
 
-	/* Stop the queues first to stop any new packets being sent. */
-	for (i = 0; i < HVMAXARCHITECTEDVIRTUALLANS; i++)
-		if (veth_dev[i])
-			netif_stop_queue(veth_dev[i]);
-
-	/* Stop the connections before we unregister the driver. This
-	 * ensures there's no skbs lying around holding the device open. */
-	for (i = 0; i < HVMAXARCHITECTEDLPS; ++i)
-		veth_stop_connection(i);
-
+	/* Disconnect our "irq" to stop events coming from the Hypervisor. */
 	HvLpEvent_unregisterHandler(HvLpEvent_Type_VirtualLan);
 
-	/* Hypervisor callbacks may have scheduled more work while we
-	 * were stoping connections. Now that we've disconnected from
-	 * the hypervisor make sure everything's finished. */
+	/* Make sure any work queued from Hypervisor callbacks is finished. */
 	flush_scheduled_work();
 
-	vio_unregister_driver(&veth_driver);
+	for (i = 0; i < HVMAXARCHITECTEDLPS; ++i) {
+		cnx = veth_cnx[i];
+
+		if (!cnx)
+			continue;
 
-	for (i = 0; i < HVMAXARCHITECTEDLPS; ++i)
-		veth_destroy_connection(i);
+		/* Drop the driver's reference to the connection */
+		kobject_put(&cnx->kobject);
+	}
 
+	/* Unregister the driver, which will close all the netdevs and stop
+	 * the connections when they're no longer referenced. */
+	vio_unregister_driver(&veth_driver);
 }
 module_exit(veth_module_cleanup);
 
@@ -1456,7 +1501,7 @@ int __init veth_module_init(void)
 
 error:
 	for (i = 0; i < HVMAXARCHITECTEDLPS; ++i) {
-		veth_destroy_connection(i);
+		veth_destroy_connection(veth_cnx[i]);
 	}
 
 	return rc;

  parent reply	other threads:[~2005-09-01  1:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-09-01  1:28 [PATCH 0/18] Updates & bug fixes for iseries_veth network driver Michael Ellerman
2005-09-01  1:28 ` [PATCH 1/18] iseries_veth: Cleanup error and debug messages Michael Ellerman
2005-09-01  1:28 ` [PATCH 2/18] iseries_veth: Remove a FIXME WRT deletion of the ack_timer Michael Ellerman
2005-09-01  1:29 ` [PATCH 3/18] iseries_veth: Try to avoid pathological reset behaviour Michael Ellerman
2005-09-01  1:29 ` [PATCH 4/18] iseries_veth: Fix broken promiscuous handling Michael Ellerman
2005-09-01  1:29 ` [PATCH 5/18] iseries_veth: Remove redundant message stack lock Michael Ellerman
2005-09-01  1:29 ` [PATCH 6/18] iseries_veth: Replace lock-protected atomic with an ordinary variable Michael Ellerman
2005-09-01  1:29 ` [PATCH 7/18] iseries_veth: Only call dma_unmap_single() if dma_map_single() succeeded Michael Ellerman
2005-09-01  1:29 ` [PATCH 8/18] iseries_veth: Make init_connection() & destroy_connection() symmetrical Michael Ellerman
2005-09-01  1:29 ` Michael Ellerman [this message]
2005-09-01  1:29 ` [PATCH 10/18] iseries_veth: Remove TX timeout code Michael Ellerman
2005-09-01  1:29 ` [PATCH 11/18] iseries_veth: Add a per-connection ack timer Michael Ellerman
2005-09-01  1:29 ` [PATCH 12/18] iseries_veth: Simplify full-queue handling Michael Ellerman
2005-09-01  1:29 ` [PATCH 13/18] iseries_veth: Fix bogus counting of TX errors Michael Ellerman
2005-09-01  1:29 ` [PATCH 14/18] iseries_veth: Add sysfs support for connection structs Michael Ellerman
2005-09-01  1:29 ` [PATCH 15/18] iseries_veth: Add sysfs support for port structs Michael Ellerman
2005-09-01  1:29 ` [PATCH 16/18] iseries_veth: Incorporate iseries_veth.h in iseries_veth.c Michael Ellerman
2005-09-01  1:29 ` [PATCH 17/18] iseries_veth: Remove studly caps from iseries_veth.c Michael Ellerman
2005-09-01  1:29 ` [PATCH 18/18] iseries_veth: Be consistent about driver name, increment version Michael Ellerman
2005-09-01  2:43 ` [PATCH 0/18] Updates & bug fixes for iseries_veth network driver Jeff Garzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20050901012909.C98036820F@ozlabs.org \
    --to=michael@ellerman.id.au \
    --cc=jgarzik@pobox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc64-dev@ozlabs.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).