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 11/18] iseries_veth: Add a per-connection ack timer
Date: Thu,  1 Sep 2005 11:29:17 +1000 (EST)	[thread overview]
Message-ID: <20050901012917.18BA6681F5@ozlabs.org> (raw)
In-Reply-To: <1125538127.859382.875909607846.qpush@concordia>

Currently the iseries_veth driver contravenes the specification in
Documentation/networking/driver.txt, in that if packets are not acked by
the other LPAR they will sit around forever.

This patch adds a per-connection timer which fires if we've had no acks for
five seconds. This is superior to the generic TX timer because it catches
the case of a small number of packets being sent and never acked.

This fixes a bug we were seeing on real systems, where some IPv6 neighbour
discovery packets would not be acked and then prevent the module from being
removed, due to skbs lying around.

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

 drivers/net/iseries_veth.c |   75 +++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 69 insertions(+), 6 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
@@ -132,6 +132,11 @@ struct veth_lpar_connection {
 	struct kobject kobject;
 	struct timer_list ack_timer;
 
+	struct timer_list reset_timer;
+	unsigned int reset_timeout;
+	unsigned long last_contact;
+	int outstanding_tx;
+
 	spinlock_t lock;
 	unsigned long state;
 	HvLpInstanceId src_inst;
@@ -171,8 +176,9 @@ static int veth_start_xmit(struct sk_buf
 static void veth_recycle_msg(struct veth_lpar_connection *, struct veth_msg *);
 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 void veth_timed_ack(unsigned long ptr);
+static void veth_timed_reset(unsigned long ptr);
 
 static struct kobj_type veth_lpar_connection_ktype = {
 	.release	= veth_release_connection
@@ -360,7 +366,7 @@ static void veth_handle_int(struct VethL
 	HvLpIndex rlp = event->base_event.xSourceLp;
 	struct veth_lpar_connection *cnx = veth_cnx[rlp];
 	unsigned long flags;
-	int i;
+	int i, acked = 0;
 
 	BUG_ON(! cnx);
 
@@ -374,13 +380,22 @@ static void veth_handle_int(struct VethL
 		break;
 	case VethEventTypeFramesAck:
 		spin_lock_irqsave(&cnx->lock, flags);
+
 		for (i = 0; i < VETH_MAX_ACKS_PER_MSG; ++i) {
 			u16 msgnum = event->u.frames_ack_data.token[i];
 
-			if (msgnum < VETH_NUMBUFFERS)
+			if (msgnum < VETH_NUMBUFFERS) {
 				veth_recycle_msg(cnx, cnx->msgs + msgnum);
+				cnx->outstanding_tx--;
+				acked++;
+			}
 		}
+
+		if (acked > 0)
+			cnx->last_contact = jiffies;
+
 		spin_unlock_irqrestore(&cnx->lock, flags);
+
 		veth_flush_pending(cnx);
 		break;
 	case VethEventTypeFrames:
@@ -454,8 +469,6 @@ static void veth_statemachine(void *p)
 
  restart:
 	if (cnx->state & VETH_STATE_RESET) {
-		int i;
-
 		if (cnx->state & VETH_STATE_OPEN)
 			HvCallEvent_closeLpEventPath(cnx->remote_lp,
 						     HvLpEvent_Type_VirtualLan);
@@ -474,15 +487,20 @@ static void veth_statemachine(void *p)
 				| VETH_STATE_SENTCAPACK | VETH_STATE_READY);
 
 		/* Clean up any leftover messages */
-		if (cnx->msgs)
+		if (cnx->msgs) {
+			int i;
 			for (i = 0; i < VETH_NUMBUFFERS; ++i)
 				veth_recycle_msg(cnx, cnx->msgs + i);
+		}
+		cnx->outstanding_tx = 0;
 
 		/* Drop the lock so we can do stuff that might sleep or
 		 * take other locks. */
 		spin_unlock_irq(&cnx->lock);
 
 		del_timer_sync(&cnx->ack_timer);
+		del_timer_sync(&cnx->reset_timer);
+
 		veth_flush_pending(cnx);
 
 		spin_lock_irq(&cnx->lock);
@@ -631,9 +649,16 @@ static int veth_init_connection(u8 rlp)
 	cnx->remote_lp = rlp;
 	spin_lock_init(&cnx->lock);
 	INIT_WORK(&cnx->statemachine_wq, veth_statemachine, cnx);
+
 	init_timer(&cnx->ack_timer);
 	cnx->ack_timer.function = veth_timed_ack;
 	cnx->ack_timer.data = (unsigned long) cnx;
+
+	init_timer(&cnx->reset_timer);
+	cnx->reset_timer.function = veth_timed_reset;
+	cnx->reset_timer.data = (unsigned long) cnx;
+	cnx->reset_timeout = 5 * HZ * (VETH_ACKTIMEOUT / 1000000);
+
 	memset(&cnx->pending_acks, 0xff, sizeof (cnx->pending_acks));
 
 	veth_cnx[rlp] = cnx;
@@ -948,6 +973,13 @@ static int veth_transmit_to_one(struct s
 	if (rc != HvLpEvent_Rc_Good)
 		goto recycle_and_drop;
 
+	/* If the timer's not already running, start it now. */
+	if (0 == cnx->outstanding_tx)
+		mod_timer(&cnx->reset_timer, jiffies + cnx->reset_timeout);
+
+	cnx->last_contact = jiffies;
+	cnx->outstanding_tx++;
+
 	spin_unlock_irqrestore(&cnx->lock, flags);
 	return 0;
 
@@ -1093,6 +1125,37 @@ static void veth_flush_pending(struct ve
 	}
 }
 
+static void veth_timed_reset(unsigned long ptr)
+{
+	struct veth_lpar_connection *cnx = (struct veth_lpar_connection *)ptr;
+	unsigned long trigger_time, flags;
+
+	/* FIXME is it possible this fires after veth_stop_connection()?
+	 * That would reschedule the statemachine for 5 seconds and probably
+	 * execute it after the module's been unloaded. Hmm. */
+
+	spin_lock_irqsave(&cnx->lock, flags);
+
+	if (cnx->outstanding_tx > 0) {
+		trigger_time = cnx->last_contact + cnx->reset_timeout;
+
+		if (trigger_time < jiffies) {
+			cnx->state |= VETH_STATE_RESET;
+			veth_kick_statemachine(cnx);
+			veth_error("%d packets not acked by LPAR %d within %d "
+					"seconds, resetting.\n",
+					cnx->outstanding_tx, cnx->remote_lp,
+					cnx->reset_timeout / HZ);
+		} else {
+			/* Reschedule the timer */
+			trigger_time = jiffies + cnx->reset_timeout;
+			mod_timer(&cnx->reset_timer, trigger_time);
+		}
+	}
+
+	spin_unlock_irqrestore(&cnx->lock, flags);
+}
+
 /*
  * Rx path
  */

  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 ` [PATCH 9/18] iseries_veth: Use kobjects to track lifecycle of connection structs Michael Ellerman
2005-09-01  1:29 ` [PATCH 10/18] iseries_veth: Remove TX timeout code Michael Ellerman
2005-09-01  1:29 ` Michael Ellerman [this message]
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=20050901012917.18BA6681F5@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).