Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 7 Feb 2010 18:01:19 +0000 (UTC)
From:      Luigi Rizzo <luigi@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   svn commit: r203619 - in user/luigi/ipfw3-head: sbin/ipfw sys/netinet sys/netinet/ipfw
Message-ID:  <201002071801.o17I1J4S092978@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: luigi
Date: Sun Feb  7 18:01:19 2010
New Revision: 203619
URL: http://svn.freebsd.org/changeset/base/203619

Log:
  Several bugfixes from Riccardo Panicucci, including:
  - fix handling of queue hash tables when we have both flowset and
    scheduler mask;
  - add a flag in kernel/userland messages to tell whether we issue
    a "pipe config" or "sched config", and preserve mask configuration
    for the latter.
  - compute the space correctly when copying information to userland.
    Include a detailed explanation on how the computation is made.
  On passing, also move the initializationn of dn_cfg to ip_dn_init().

Modified:
  user/luigi/ipfw3-head/sbin/ipfw/altq.c
  user/luigi/ipfw3-head/sbin/ipfw/dummynet.c
  user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dn_io.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c

Modified: user/luigi/ipfw3-head/sbin/ipfw/altq.c
==============================================================================
--- user/luigi/ipfw3-head/sbin/ipfw/altq.c	Sun Feb  7 18:00:13 2010	(r203618)
+++ user/luigi/ipfw3-head/sbin/ipfw/altq.c	Sun Feb  7 18:01:19 2010	(r203619)
@@ -39,6 +39,7 @@
 
 #include <net/if.h>		/* IFNAMSIZ */
 #include <net/pfvar.h>
+#include <netinet/in.h>	/* in_addr */
 #include <netinet/ip_fw.h>
 
 /*

Modified: user/luigi/ipfw3-head/sbin/ipfw/dummynet.c
==============================================================================
--- user/luigi/ipfw3-head/sbin/ipfw/dummynet.c	Sun Feb  7 18:00:13 2010	(r203618)
+++ user/luigi/ipfw3-head/sbin/ipfw/dummynet.c	Sun Feb  7 18:01:19 2010	(r203619)
@@ -780,7 +780,7 @@ ipfw_config_pipe(int ac, char **av)
 	struct dn_profile *pf = NULL;
 	struct ipfw_flow_id *mask = NULL;
 	int lmax;
-	uint32_t _foo = 0, *flags = &_foo;
+	uint32_t _foo = 0, *flags = &_foo , *buckets = &_foo;
 
 	/*
 	 * allocate space for 1 header,
@@ -823,6 +823,8 @@ ipfw_config_pipe(int ac, char **av)
 		sch->oid.subtype = 0;	/* defaults to WF2Q+ */
 		mask = &sch->sched_mask;
 		flags = &sch->flags;
+		buckets = &sch->buckets;
+		*flags |= DN_PIPE_CMD;
 
 		p->link_nr = i;
 
@@ -836,6 +838,7 @@ ipfw_config_pipe(int ac, char **av)
 		fs->fs_nr = i;
 		mask = &fs->flow_mask;
 		flags = &fs->flags;
+		buckets = &fs->buckets;
 		break;
 
 	case 3: /* "sched N config ..." */
@@ -844,6 +847,7 @@ ipfw_config_pipe(int ac, char **av)
 		sch->sched_nr = i;
 		mask = &sch->sched_mask;
 		flags = &sch->flags;
+		buckets = &sch->buckets;
 		/* fs is used only with !MULTIQUEUE schedulers */
 		fs->fs_nr = i + DN_MAX_ID;
 		fs->sched_nr = i;
@@ -899,7 +903,7 @@ ipfw_config_pipe(int ac, char **av)
 		case TOK_BUCKETS:
 			NEED(fs, "buckets is only for pipes or flowsets");
 			NEED1("buckets needs argument\n");
-			fs->buckets = strtoul(av[0], NULL, 0);
+			*buckets = strtoul(av[0], NULL, 0);
 			ac--; av++;
 			break;
 

Modified: user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h	Sun Feb  7 18:00:13 2010	(r203618)
+++ user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h	Sun Feb  7 18:01:19 2010	(r203619)
@@ -90,10 +90,12 @@ enum { /* subtype for schedulers, flowse
 enum {	/* user flags */
 	DN_HAVE_MASK	= 0x0001,	/* fs or sched has a mask */
 	DN_NOERROR	= 0x0002,	/* do not report errors */
+	DN_QHT_HASH	= 0x0004,	/* qht is a hash table */
 	DN_QSIZE_BYTES	= 0x0008,	/* queue size is in bytes */
 	DN_HAS_PROFILE	= 0x0010,	/* a link has a profile */
 	DN_IS_RED	= 0x0020,
 	DN_IS_GENTLE_RED= 0x0040,
+	DN_PIPE_CMD	= 0x1000,	/* pipe config... */
 };
 
 /*
@@ -122,7 +124,7 @@ struct dn_link {
 struct dn_fs {
 	struct dn_id oid;
 	uint32_t fs_nr;	/* the flowset number */
-	int flags;	/* userland flags */
+	uint32_t flags;	/* userland flags */
 	int qsize;	/* queue size in slots or bytes */
 	int32_t plr;	/* PLR, pkt loss rate (2^31-1 means 100%) */
 	uint32_t buckets;	/* buckets used for the queue hash table */

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dn_io.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dn_io.c	Sun Feb  7 18:00:13 2010	(r203618)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dn_io.c	Sun Feb  7 18:01:19 2010	(r203619)
@@ -68,16 +68,7 @@ __FBSDID("$FreeBSD$");
  */
 static uint64_t curr_time = 0; /* current simulation time */
 
-struct dn_parms dn_cfg = {
-	.slot_limit = 100, /* Foot shooting limit for queues. */
-	.byte_limit = 1024 * 1024,
-
-	.red_lookup_depth = 256,	/* RED - default lookup table depth */
-	.red_avg_pkt_size = 512,      /* RED - default medium packet size */
-	.red_max_pkt_size = 1500,     /* RED - default max packet size */
-	.max_hash_size = 1024,		/* max in the hash tables */
-	.hash_size = 64,		/* default hash size */
-};
+struct dn_parms dn_cfg;
 
 static long tick_last;		/* Last tick duration (usec). */
 static long tick_delta;		/* Last vs standard tick diff (usec). */

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c	Sun Feb  7 18:00:13 2010	(r203618)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c	Sun Feb  7 18:01:19 2010	(r203619)
@@ -76,6 +76,16 @@ static struct callout dn_timeout;
 static struct task	dn_task;
 static struct taskqueue	*dn_tq = NULL;
 
+/*
+ * XXX max_qlen is used as a temporary measure to store the
+ * max size of 'struct dn_queue' plus scheduler-specific extensions.
+ * This is used to determine how much space is needed on a
+ * getsockopt() to copy queues up.
+ * Eventually this h should go away as we only want to copy the
+ * basic dn_queue.
+ */
+static int max_qlen = 0;
+
 static void
 dummynet(void * __unused unused)
 {
@@ -295,7 +305,7 @@ q_new(uintptr_t key, int flags, void *ar
 	}
 
 	set_oid(&q->ni.oid, DN_QUEUE, size);
-	if (fs->fs.flags & DN_HAVE_MASK)
+	if (fs->fs.flags & DN_QHT_HASH)
 		q->ni.fid = *(struct ipfw_flow_id *)key;
 	q->fs = fs;
 	q->_si = template->_si;
@@ -779,6 +789,11 @@ copy_obj(char **start, char *end, void *
 	}
 	ND("type %d %s %d len %d", o->type, msg, i, o->len);
 	bcopy(_o, *start, o->len);
+	if (o->type == DN_LINK) {
+		/* Adjust burst parameter for link */
+		struct dn_link *l = (struct dn_link *)*start;
+		l->burst =  div64(l->burst, 8 * hz);
+	}
 	*start += o->len;
 	return 0;
 }
@@ -1152,6 +1167,11 @@ config_sched(struct dn_sch *_nsch, struc
 	struct schk_new_arg a; /* argument for schk_new */
 	int i;
 	struct dn_link p;	/* copy of oldlink */
+	/* Used to preserv mask parameter */
+	struct ipfw_flow_id new_mask;
+	int new_buckets = 0;
+	int new_flags = 0;
+	int pipe_cmd;
 
 	a.sch = _nsch;
 	if (a.sch->oid.len != sizeof(*a.sch)) {
@@ -1167,6 +1187,15 @@ config_sched(struct dn_sch *_nsch, struc
 			1, dn_cfg.max_hash_size, "sched buckets");
 	/* XXX other sanity checks */
 	bzero(&p, sizeof(p));
+
+	pipe_cmd = a.sch->flags & DN_PIPE_CMD;
+	a.sch->flags &= ~DN_PIPE_CMD; //XXX do it even if is not set?
+	if (pipe_cmd) {
+		/* Copy mask parameter */
+		new_mask = a.sch->sched_mask;
+		new_buckets = a.sch->buckets;
+		new_flags = a.sch->flags;
+	}
 	DN_BH_WLOCK();
 again: /* run twice, for wfq and fifo */
 	/*
@@ -1183,7 +1212,21 @@ again: /* run twice, for wfq and fifo */
 		s = dn_ht_find(dn_cfg.schedhash, i, 0, &a);
 		if (s != NULL) {
 			a.fp = s->fp;
+			/* Scheduler exists, skip to FIFO scheduler 
+			 * if command was pipe config...
+			 */
+			if (pipe_cmd)
+				goto next;
 		} else {
+			/* New scheduler, create a wf2q+ with no mask
+			 * if command was pipe config...
+			 */
+			if (pipe_cmd) {
+				/* clear mask parameter */
+				bzero(&a.sch->sched_mask, sizeof(new_mask));
+				a.sch->buckets = 0;
+				a.sch->flags &= ~DN_HAVE_MASK;
+			}
 			a.sch->oid.subtype = DN_SCHED_WF2QP;
 			goto again;
 		}
@@ -1258,8 +1301,22 @@ again: /* run twice, for wfq and fifo */
 	if (s->fp->config)
 		s->fp->config(s);
 	update_fs(s);
+next:
 	if (i < DN_MAX_ID) { /* now configure the FIFO instance */
 		i += DN_MAX_ID;
+		if (pipe_cmd) {
+			/* Restore mask parameter for FIFO */
+			a.sch->sched_mask = new_mask;
+			a.sch->buckets = new_buckets;
+			a.sch->flags = new_flags;
+		} else {
+			/* sched config shouldn't modify the FIFO scheduler */
+			if (dn_ht_find(dn_cfg.schedhash, i, 0, &a) != NULL) {
+				/* FIFO already exist, don't touch it */
+				DN_BH_WUNLOCK();
+				return 0;
+			}
+		}
 		a.sch->sched_nr = i;
 		a.sch->oid.subtype = DN_SCHED_FIFO;
 		bzero(a.sch->name, sizeof(a.sch->name));
@@ -1453,14 +1510,45 @@ compute_space(struct dn_id *cmd, int *to
 {
 	int x = 0, need = 0;
 
+	/* NOTE about compute space:
+	 * NP 	= dn_cfg.schk_count
+	 * NSI 	= dn_cfg.si_count
+	 * NF 	= dn_cfg.fsk_count
+	 * NQ 	= dn_cfg.queue_count
+	 * - ipfw pipe show
+	 *   (NP/2)*(dn_link + dn_sch + dn_id + dn_fs) only half scheduler
+	 *                             link, scheduler template, flowset
+	 *                             integrated in scheduler and header
+	 *                             for flowset list
+	 *   (NSI)*(dn_flow + dn_queue) all scheduler instance + one
+	 *                              queue per instance
+	 * - ipfw sched show
+	 *   (NP/2)*(dn_link + dn_sch + dn_id + dn_fs) only half scheduler
+	 *                             link, scheduler template, flowset
+	 *                             integrated in scheduler and header
+	 *                             for flowset list
+	 *   (NSI * dn_flow) all scheduler instances
+	 *   (NF * sizeof(uint_32)) space for flowset list linked to scheduler
+	 *   (NQ * dn_queue) all queue [XXXfor now not listed]
+	 * - ipfw queue show
+	 *   (NF * dn_fs) all flowset
+	 *   (NQ * dn_queue) all queues
+	 * I use 'max_qlen' instead of sizeof(dn_queue) because
+	 *   a queue can be of variable size, so use the max queue size.
+	 */
 	switch (cmd->subtype) {
 	default:
 		return -1;
 	case DN_LINK:	/* pipe show */
-		x = DN_C_LINK | DN_C_FS | DN_SCH | DN_C_FLOW;
+		x = DN_C_LINK /*| DN_C_FS*/ | DN_C_SCH | DN_C_FLOW;
+		need += dn_cfg.schk_count * sizeof(struct dn_fs) / 2;
+		need += dn_cfg.si_count * max_qlen;
+		need += dn_cfg.fsk_count * sizeof(uint32_t);
 		break;
 	case DN_SCH:	/* sched show */
-		x = DN_C_SCH | DN_C_LINK | DN_C_FLOW;
+		need += dn_cfg.schk_count * sizeof(struct dn_fs) / 2;
+		need += dn_cfg.fsk_count * sizeof(uint32_t);
+		x = DN_C_SCH | DN_C_LINK | DN_C_FLOW /*|| DN_C_QUEUE*/;
 		break;
 	case DN_FS:	/* queue show */
 		x = DN_C_FS | DN_C_QUEUE;
@@ -1468,20 +1556,20 @@ compute_space(struct dn_id *cmd, int *to
 	}
 	*to_copy = x;
 	if (x & DN_C_SCH) {
-		need += dn_cfg.schk_count * sizeof(struct dn_sch);
-		/* also, each fs might be attached to a sched */
-		need += dn_cfg.fsk_count *
-			(sizeof(struct dn_id) + 4);
+		need += dn_cfg.schk_count * sizeof(struct dn_sch) / 2;
+		/* NOT also, each fs might be attached to a sched */
+		need += dn_cfg.schk_count * sizeof(struct dn_id) / 2;
 	}
 	if (x & DN_C_FS)
 		need += dn_cfg.fsk_count * sizeof(struct dn_fs);
-	if (x & DN_C_LINK)
-		need += dn_cfg.schk_count * sizeof(struct dn_link);
+	if (x & DN_C_LINK) {
+		need += dn_cfg.schk_count * sizeof(struct dn_link) / 2;
+	}
 	/* XXX queue space might be variable */
 	if (x & DN_C_QUEUE)
-		need += dn_cfg.queue_count * sizeof(struct dn_queue);
+		need += dn_cfg.queue_count * max_qlen;
 	if (x & DN_C_FLOW)
-		need += dn_cfg.si_count * sizeof(struct dn_flow);
+		need += dn_cfg.si_count * (sizeof(struct dn_flow));
 	return need;
 }
 
@@ -1532,7 +1620,7 @@ dummynet_get(struct sockopt *sopt)
 		"%d:%d si %d, %d:%d queues %d",
 		dn_cfg.schk_count, sizeof(struct dn_sch), DN_SCH,
 		dn_cfg.schk_count, sizeof(struct dn_link), DN_LINK,
-		dn_cfg.fsk_count, sizeof(struct new_fs), DN_FS,
+		dn_cfg.fsk_count, sizeof(struct dn_fs), DN_FS,
 		dn_cfg.si_count, sizeof(struct dn_flow), DN_SCH_I,
 		dn_cfg.queue_count, sizeof(struct dn_queue), DN_QUEUE);
 	sopt->sopt_valsize = sopt_valsize;
@@ -1622,6 +1710,20 @@ ip_dn_init(void)
 	if (bootverbose)
 		printf("DUMMYNET with IPv6 initialized (100131)\n");
 
+	/* init defaults here, MSVC does not accept initializers */
+	/* queue limits */
+	dn_cfg.slot_limit = 100; /* Foot shooting limit for queues. */
+	dn_cfg.byte_limit = 1024 * 1024;
+
+	/* RED parameters */
+	dn_cfg.red_lookup_depth = 256;	/* default lookup table depth */
+	dn_cfg.red_avg_pkt_size = 512;	/* default medium packet size */
+	dn_cfg.red_max_pkt_size = 1500;	/* default max packet size */
+
+	/* hash tables */
+	dn_cfg.max_hash_size = 1024;	/* max in the hash tables */
+	dn_cfg.hash_size = 64;		/* default hash size */
+
 	/* create hash tables for schedulers and flowsets.
 	 * In both we search by key and by pointer.
 	 */
@@ -1702,11 +1804,17 @@ static int
 load_dn_sched(struct dn_alg *d)
 {
 	struct dn_alg *s;
+	int q_len = 0;
 
 	if (d == NULL)
 		return 1; /* error */
 	ip_dn_init();	/* just in case, we need the lock */
 
+	/* check the max queue lenght */
+	q_len = sizeof(struct dn_queue) + d->q_datalen;
+	if (max_qlen <= q_len) {
+		max_qlen = q_len;
+	}
 	/* Check that mandatory funcs exists */
 	if (d->enqueue == NULL || d->dequeue == NULL) {
 		D("missing enqueue or dequeue for %s", d->name);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201002071801.o17I1J4S092978>