Skip to contentSkip to conversation

aoe: improved network congestion handling from v60 to v64+

PATCH In review Ed Cashin 2 participants 10 messages Nov 9, 2012

Subsystems (2)
Block layer ATA over Ethernet (aoe) driver
lore
Share & tools
lore

Ed Cashin

author
#0 lore raw

cover aoe: improved network congestion handling from v60 to v64+

This patch series is based on linux-next/akpm from 8 Nov, commit db3846344ad57222.

Ed L. Cashin (8):
aoe: avoid running request handler on plugged queue
aoe: provide ATA identify device content to user on request
aoe: improve network congestion handling
aoe: err device: include MAC addresses for unexpected responses
aoe: manipulate aoedev network stats under lock
aoe: use high-resolution RTTs with fallback to low-res
aoe: commands in retransmit queue use new destination on failure
aoe: update driver-internal version to 64+

 drivers/block/aoe/aoe.h    |   24 +++-
 drivers/block/aoe/aoeblk.c |   30 ++++
 drivers/block/aoe/aoecmd.c |  317 +++++++++++++++++++++++++++++++-------------
 drivers/block/aoe/aoedev.c |   38 ++++--
 4 files changed, 296 insertions(+), 113 deletions(-)
author
#1 lore raw

PATCH 1/8 aoe: avoid running request handler on plugged queue

Calling the request handler directly on a plugged queue defeats the performance improvements provided by the plugging mechanism. Use the __blk_run_queue function instead of calling the request handler directly, so that we don't interfere with the block layer's ability to plug the queue.

Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoecmd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/block/aoe/aoecmd.c +1 -1
index c491fba..3ce01f6 100644
@@ -925,7 +925,7 @@ aoe_end_request(struct aoedev *d, struct request *rq, int fastfail)
 
 	/* cf. http://lkml.org/lkml/2006/10/31/28 */
 	if (!fastfail)
-		q->request_fn(q);
+		__blk_run_queue(q);
 }
 
 static void
Signature · 1 line
1.7.1
author
#3 lore raw

PATCH 2/8 aoe: provide ATA identify device content to user on request

In reply to Ed Cashin

This patch makes the aoe driver follow expected behavior when the user uses ioctl to get the ATA device identify information, allowing access to model, serial number, etc.

Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoe.h    |    1 +
 drivers/block/aoe/aoeblk.c |   30 ++++++++++++++++++++++++++++++
 drivers/block/aoe/aoecmd.c |   16 ++++++++++++++++
 3 files changed, 47 insertions(+), 0 deletions(-)
drivers/block/aoe/aoe.h +1 -0
index 536942b..f6e0c03 100644
@@ -169,6 +169,7 @@ struct aoedev {
 	struct aoetgt *htgt;	/* target needing rexmit assistance */
 	ulong ntargets;
 	ulong kicked;
+	char ident[512];
 };
 
 /* kthread tracking */
drivers/block/aoe/aoeblk.c +30 -0
index 56736cd..7ba0fcf 100644
@@ -17,6 +17,7 @@
 #include <linux/mutex.h>
 #include <linux/export.h>
 #include <linux/moduleparam.h>
+#include <scsi/sg.h>
 #include "aoe.h"
 
 static DEFINE_MUTEX(aoeblk_mutex);

@@ -212,9 +213,38 @@ aoeblk_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 	return 0;
 }
 
+static int
+aoeblk_ioctl(struct block_device *bdev, fmode_t mode, uint cmd, ulong arg)
+{
+	struct aoedev *d;
+
+	if (!arg)
+		return -EINVAL;
+
+	d = bdev->bd_disk->private_data;
+	if ((d->flags & DEVFL_UP) == 0) {
+		pr_err("aoe: disk not up\n");
+		return -ENODEV;
+	}
+
+	if (cmd == HDIO_GET_IDENTITY) {
+		if (!copy_to_user((void __user *) arg, &d->ident,
+			sizeof(d->ident)))
+			return 0;
+		return -EFAULT;
+	}
+
+	/* udev calls scsi_id, which uses SG_IO, resulting in noise */
+	if (cmd != SG_IO)
+		pr_info("aoe: unknown ioctl 0x%x\n", cmd);
+
+	return -ENOTTY;
+}
+
 static const struct block_device_operations aoe_bdops = {
 	.open = aoeblk_open,
 	.release = aoeblk_release,
+	.ioctl = aoeblk_ioctl,
 	.getgeo = aoeblk_getgeo,
 	.owner = THIS_MODULE,
 };
drivers/block/aoe/aoecmd.c +16 -0
index 3ce01f6..c4ff70b 100644
@@ -799,6 +799,17 @@ aoecmd_sleepwork(struct work_struct *work)
 }
 
 static void
+ata_ident_fixstring(u16 *id, int ns)
+{
+	u16 s;
+
+	while (ns-- > 0) {
+		s = *id;
+		*id++ = s >> 8 | s << 8;
+	}
+}
+
+static void
 ataid_complete(struct aoedev *d, struct aoetgt *t, unsigned char *id)
 {
 	u64 ssize;

@@ -833,6 +844,11 @@ ataid_complete(struct aoedev *d, struct aoetgt *t, unsigned char *id)
 		d->geo.sectors = get_unaligned_le16(&id[56 << 1]);
 	}
 
+	ata_ident_fixstring((u16 *) &id[10<<1], 10);	/* serial */
+	ata_ident_fixstring((u16 *) &id[23<<1], 4);	/* firmware */
+	ata_ident_fixstring((u16 *) &id[27<<1], 20);	/* model */
+	memcpy(d->ident, id, sizeof(d->ident));
+
 	if (d->ssize != ssize)
 		printk(KERN_INFO
 			"aoe: %pm e%ld.%d v%04x has %llu sectors\n",
Signature · 1 line
1.7.1
author
#4 lore raw

PATCH 3/8 aoe: improve network congestion handling

In reply to Ed Cashin

The aoe driver already had some congestion handling, but it was limited in its ability to cope with the kind of congestion that can arise on more complex networks such as those involving paths through multiple ethernet switches.

Some of the lessons from TCP's history of development can be applied to improving the congestion control and avoidance on AoE storage networks. These changes use familar concepts from Van Jacobson's "Congestion Avoidance and Control" paper from '88, without adding significant overhead.

This patch depends on an upcoming patch that covers the failover case when AoE commands being retransmitted are transferred from one retransmit queue to another. Another upcoming patch increases the timing accuracy.

Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoe.h    |   16 +++--
 drivers/block/aoe/aoecmd.c |  173 +++++++++++++++++++++++++++-----------------
 drivers/block/aoe/aoedev.c |    6 +-
 3 files changed, 121 insertions(+), 74 deletions(-)
drivers/block/aoe/aoe.h +11 -5
index f6e0c03..9e884ac 100644
@@ -86,8 +86,11 @@ enum {
 	NFACTIVE = 61,
 
 	TIMERTICK = HZ / 10,
-	MINTIMER = HZ >> 2,
+	RTTSCALE = 8,
+	RTTDSCALE = 3,
 	MAXTIMER = HZ << 1,
+	RTTAVG_INIT = HZ / 4 << RTTSCALE,
+	RTTDEV_INIT = RTTAVG_INIT / 4,
 };
 
 struct buf {

@@ -127,10 +130,11 @@ struct aoetgt {
 	struct list_head ffree;			/* list of free frames */
 	struct aoeif ifs[NAOEIFS];
 	struct aoeif *ifp;	/* current aoeif in use */
-	ushort nout;
+	ushort nout;		/* value of nout when skb was sent */
 	ushort maxout;		/* current value for max outstanding */
+	ushort next_cwnd;	/* incr maxout after decrementing to zero */
+	ushort ssthresh;	/* slow start threshold */
 	ulong falloc;		/* number of allocated frames */
-	ulong lastwadj;		/* last window adjustment */
 	int minbcnt;
 	int wpkts, rpkts;
 };

@@ -142,8 +146,8 @@ struct aoedev {
 	u16 aoeminor;
 	u16 flags;
 	u16 nopen;		/* (bd_openers isn't available without sleeping) */
-	u16 rttavg;		/* round trip average of requests/responses */
-	u16 mintimer;
+	u16 rttavg;		/* scaled AoE round trip time average */
+	u16 rttdev;		/* scaled round trip time mean deviation */
 	u16 fw_ver;		/* version of blade's firmware */
 	u16 lasttag;		/* last tag sent */
 	u16 useme;

@@ -164,6 +168,7 @@ struct aoedev {
 	} ip;
 	ulong maxbcnt;
 	struct list_head factive[NFACTIVE];	/* hash of active frames */
+	struct list_head rexmitq; /* deferred retransmissions */
 	struct aoetgt *targets[NTARGETS];
 	struct aoetgt **tgt;	/* target in use when working */
 	struct aoetgt *htgt;	/* target needing rexmit assistance */

@@ -196,6 +201,7 @@ void aoecmd_cfg(ushort aoemajor, unsigned char aoeminor);
 struct sk_buff *aoecmd_ata_rsp(struct sk_buff *);
 void aoecmd_cfg_rsp(struct sk_buff *);
 void aoecmd_sleepwork(struct work_struct *);
+void aoecmd_wreset(struct aoetgt *t);
 void aoecmd_cleanslate(struct aoedev *);
 void aoecmd_exit(void);
 int aoecmd_init(void);
drivers/block/aoe/aoecmd.c +106 -67

287 lines not on this page · raw message

drivers/block/aoe/aoedev.c +4 -2
index 63b2660..3c3aef2 100644
@@ -198,7 +198,7 @@ aoedev_downdev(struct aoedev *d)
 	tt = d->targets;
 	te = tt + NTARGETS;
 	for (; tt < te && (t = *tt); tt++) {
-		t->maxout = t->nframes;
+		aoecmd_wreset(t);
 		t->nout = 0;
 	}
 

@@ -391,10 +391,12 @@ aoedev_by_aoeaddr(ulong maj, int min, int do_alloc)
 	d->ref = 1;
 	for (i = 0; i < NFACTIVE; i++)
 		INIT_LIST_HEAD(&d->factive[i]);
+	INIT_LIST_HEAD(&d->rexmitq);
 	d->sysminor = sysminor;
 	d->aoemajor = maj;
 	d->aoeminor = min;
-	d->mintimer = MINTIMER;
+	d->rttavg = RTTAVG_INIT;
+	d->rttdev = RTTDEV_INIT;
 	d->next = devlist;
 	devlist = d;
  out:
Signature · 1 line
1.7.1
author
#5 lore raw

PATCH 4/8 aoe: err device: include MAC addresses for unexpected responses

In reply to Ed Cashin

The /dev/etherd/err character device provides low-level information about normal but sometimes interesting AoE command retransmits and "unexpected responses", i.e., responses for packets that have already been retransmitted.

This change adds MAC addresses to the messages about unexpected responses, so that when they occur, it's more easy to determine the network paths to which they belong.

Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoecmd.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/block/aoe/aoecmd.c +4 -2
index f849fa2..6ea27fd 100644
@@ -1204,12 +1204,14 @@ aoecmd_ata_rsp(struct sk_buff *skb)
 			spin_unlock_irqrestore(&d->lock, flags);
 			aoedev_put(d);
 			snprintf(ebuf, sizeof(ebuf),
-				 "%15s e%d.%d    tag=%08x@%08lx\n",
+				 "%15s e%d.%d    tag=%08x@%08lx s=%pm d=%pm\n",
 				 "unexpected rsp",
 				 get_unaligned_be16(&h->major),
 				 h->minor,
 				 get_unaligned_be32(&h->tag),
-				 jiffies);
+				 jiffies,
+				 h->src,
+				 h->dst);
 			aoechr_error(ebuf);
 			return skb;
 		}
Signature · 1 line
1.7.1
author
#6 lore raw

PATCH 5/8 aoe: manipulate aoedev network stats under lock

In reply to Ed Cashin

With this bugfix in place the calculation of the criterion for "lateness" is performed under lock. Without the lock, there is a chance that one of the non-atomic operations performed on the round trip time statistics could be incomplete, such that an incorrect lateness criterion would be calculated.

Without this change, the effect of the bug would be rare unecessary but benign retransmissions.

Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoecmd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/block/aoe/aoecmd.c +2 -2
index 6ea27fd..9aefbe3 100644
@@ -603,14 +603,14 @@ rexmit_timer(ulong vp)
 
 	d = (struct aoedev *) vp;
 
+	spin_lock_irqsave(&d->lock, flags);
+
 	/* timeout based on observed timings and variations */
 	timeout = 2 * d->rttavg >> RTTSCALE;
 	timeout += 8 * d->rttdev >> RTTDSCALE;
 	if (timeout == 0)
 		timeout = 1;
 
-	spin_lock_irqsave(&d->lock, flags);
-
 	if (d->flags & DEVFL_TKILL) {
 		spin_unlock_irqrestore(&d->lock, flags);
 		return;
Signature · 1 line
1.7.1
author
#7 lore raw

PATCH 6/8 aoe: use high-resolution RTTs with fallback to low-res

In reply to Ed Cashin

These changes improve the accuracy of the decision about whether it's time to retransmit an AoE command by using the microsecond-resolution gettimeofday instead of jiffies.

Because the system time can jump suddenly, the decision reverts to using jiffies if the high-resolution time difference is relatively large. Otherwise the AoE targets could be considered failed inappropriately.

Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoe.h    |    9 ++++---
 drivers/block/aoe/aoecmd.c |   57 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 55 insertions(+), 11 deletions(-)
drivers/block/aoe/aoe.h +5 -4
index 9e884ac..9fb68fc 100644
@@ -88,8 +88,7 @@ enum {
 	TIMERTICK = HZ / 10,
 	RTTSCALE = 8,
 	RTTDSCALE = 3,
-	MAXTIMER = HZ << 1,
-	RTTAVG_INIT = HZ / 4 << RTTSCALE,
+	RTTAVG_INIT = USEC_PER_SEC / 4 << RTTSCALE,
 	RTTDEV_INIT = RTTAVG_INIT / 4,
 };
 

@@ -106,6 +105,8 @@ struct buf {
 struct frame {
 	struct list_head head;
 	u32 tag;
+	struct timeval sent;	/* high-res time packet was sent */
+	u32 sent_jiffs;		/* low-res jiffies-based sent time */
 	ulong waited;
 	struct aoetgt *t;		/* parent target I belong to */
 	sector_t lba;

@@ -143,11 +144,11 @@ struct aoedev {
 	struct aoedev *next;
 	ulong sysminor;
 	ulong aoemajor;
+	u32 rttavg;		/* scaled AoE round trip time average */
+	u32 rttdev;		/* scaled round trip time mean deviation */
 	u16 aoeminor;
 	u16 flags;
 	u16 nopen;		/* (bd_openers isn't available without sleeping) */
-	u16 rttavg;		/* scaled AoE round trip time average */
-	u16 rttdev;		/* scaled round trip time mean deviation */
 	u16 fw_ver;		/* version of blade's firmware */
 	u16 lasttag;		/* last tag sent */
 	u16 useme;
drivers/block/aoe/aoecmd.c +50 -7

115 lines not on this page · raw message

Signature · 1 line
1.7.1
author
#8 lore raw

PATCH 7/8 aoe: commands in retransmit queue use new destination on failure

In reply to Ed Cashin

When one remote MAC address isn't working as a destination for AoE commands, the frames used to track information associated with the AoE commands are moved to a new aoetgt (defined by the tuple of {AoE major, AoE minor, target MAC address}).

This patch makes sure that the frames on the queue for retransmits that need to be done are updated to use the new destination, so that retransmits will be sent through a working network path.

Without this change, packets on the retransmit queue will be needlessly retransmitted to the unresponsive destination MAC, possibly causing premature target failure before there's time for the retransmit timer to run again, decide to retransmit again, and finally update the destination to a working MAC address on the AoE target.

Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoe.h    |    1 +
 drivers/block/aoe/aoecmd.c |   75 +++++++++++++++++++++++++++++++-------------
 drivers/block/aoe/aoedev.c |   32 ++++++++++++------
 3 files changed, 75 insertions(+), 33 deletions(-)
drivers/block/aoe/aoe.h +1 -0
index 9fb68fc..c253cca 100644
@@ -108,6 +108,7 @@ struct frame {
 	struct timeval sent;	/* high-res time packet was sent */
 	u32 sent_jiffs;		/* low-res jiffies-based sent time */
 	ulong waited;
+	ulong waited_total;
 	struct aoetgt *t;		/* parent target I belong to */
 	sector_t lba;
 	struct sk_buff *skb;		/* command skb freed on module exit */
drivers/block/aoe/aoecmd.c +53 -22

136 lines not on this page · raw message

drivers/block/aoe/aoedev.c +21 -11
index 3c3aef2..91f7c99 100644
@@ -170,30 +170,40 @@ aoe_failip(struct aoedev *d)
 		aoe_end_request(d, rq, 0);
 }
 
+static void
+downdev_frame(struct list_head *pos)
+{
+	struct frame *f;
+
+	f = list_entry(pos, struct frame, head);
+	list_del(pos);
+	if (f->buf) {
+		f->buf->nframesout--;
+		aoe_failbuf(f->t->d, f->buf);
+	}
+	aoe_freetframe(f);
+}
+
 void
 aoedev_downdev(struct aoedev *d)
 {
 	struct aoetgt *t, **tt, **te;
-	struct frame *f;
 	struct list_head *head, *pos, *nx;
 	struct request *rq;
 	int i;
 
 	d->flags &= ~DEVFL_UP;
 
-	/* clean out active buffers */
+	/* clean out active and to-be-retransmitted buffers */
 	for (i = 0; i < NFACTIVE; i++) {
 		head = &d->factive[i];
-		list_for_each_safe(pos, nx, head) {
-			f = list_entry(pos, struct frame, head);
-			list_del(pos);
-			if (f->buf) {
-				f->buf->nframesout--;
-				aoe_failbuf(d, f->buf);
-			}
-			aoe_freetframe(f);
-		}
+		list_for_each_safe(pos, nx, head)
+			downdev_frame(pos);
 	}
+	head = &d->rexmitq;
+	list_for_each_safe(pos, nx, head)
+		downdev_frame(pos);
+
 	/* reset window dressings */
 	tt = d->targets;
 	te = tt + NTARGETS;
Signature · 1 line
1.7.1
author
#9 lore raw

PATCH 8/8 aoe: update driver-internal version to 64+

In reply to Ed Cashin
Signed-off-by Ed Cashin <[email protected]>
---
 drivers/block/aoe/aoe.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/block/aoe/aoe.h +2 -1
index c253cca..9655ce3 100644
@@ -1,5 +1,6 @@
+
 /* Copyright (c) 2012 Coraid, Inc.  See COPYING for GPL terms. */
-#define VERSION "60"
+#define VERSION "64+"
 #define AOE_MAJOR 152
 #define DEVICE_NAME "aoe"
Signature · 1 line
1.7.1
maintainer
#2 lore raw

Re: [PATCH 1/8] aoe: avoid running request handler on plugged queue

In reply to Ed Cashin
Quoted 5 lines · Ed Cashin

Calling the request handler directly on a plugged queue defeats the performance improvements provided by the plugging mechanism. Use the __blk_run_queue function instead of calling the request handler directly, so that we don't interfere with the block layer's ability to plug the queue.

Thanks Ed, applied!

Signature · 1 line
Jens Axboe