[NET] Add proc file to display the state of all qdiscs.

[NET] Add proc file to display the state of all qdiscs.

[Christoph Lameter]

[NET] Add proc file to display the state of all qdiscs

TC is a complicated tool and it currently does not allow the display of all
qdisc states. It does not support multiple tx queues and also not
localhost, nor does it display the current operating state of the queues.

This functionality could be added to tc / netlink but the tool is already
complex to handle. The simple proc file here allows easy scanning by
scripts and other tools. However, tc still needs to be updated to allow
the modifications of multiqueue TX settings. tc's main focus is the
configuration of qdiscs. The qdisc_stats file just shows the current
state.

This patch adds

	/proc/net/qdisc_stats

which displays the current state of all qdiscs on the system.

F.e.

$ cat /proc/net/qdisc_stats
Queue    Device  State   Bytes  Packets Qlen Blog   Drops Requeue Overlimit
TX0/root     lo   -          0        0    0    0       0       0       0
 RX/root     lo   -          0        0    0    0       0       0       0
TX0/root   eth0   -       5518       60    0    0       0       0       0
TX1/root   eth0   -       2549       37    0    0       0       0       0
TX2/root   eth0   -      63625      272    0    0       0       0       0
TX3/root   eth0   -       1580       21    0    0       0       0       0
TX4/root   eth0   R   88979440   260183    0 3532   43176    2111       0
TX5/root   eth0   -       4698       56    0    0       0       0       0
TX6/root   eth0   - 3598883129 10523140    0    0       0       0       0
TX7/root   eth0   -       1750       21    0    0       0       0       0


Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 net/sched/sch_api.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

Index: linux-2.6/net/sched/sch_api.c
===================================================================
--- linux-2.6.orig/net/sched/sch_api.c	2009-09-01 12:27:24.000000000 -0500
+++ linux-2.6/net/sched/sch_api.c	2009-09-01 14:39:27.000000000 -0500
@@ -1699,6 +1699,135 @@ static const struct file_operations psch
 	.llseek = seq_lseek,
 	.release = single_release,
 };
+
+static void dump_qdisc(struct seq_file *seq, struct net_device *dev,
+				struct Qdisc *q, char *inout, char *text)
+{
+	char state[5];
+	char *p = state;
+
+	if (test_bit(__QDISC_STATE_RUNNING, &q->state))
+		*p++ = 'R';
+	if (test_bit(__QDISC_STATE_SCHED, &q->state))
+		*p++ = 'S';
+	if (test_bit(__QDISC_STATE_DEACTIVATED, &q->state))
+		*p++ = 'D';
+	if (q->state == 0)
+		*p++ = '-';
+	*p = 0;
+
+	seq_printf(seq, "%3s/%2s %6s %3s %10lld %8d %4d %4d %7d %7d %7d\n",
+		inout, text, dev->name, state,
+		q->bstats.bytes, q->bstats.packets,
+		q->qstats.qlen, q->qstats.backlog, q->qstats.drops,
+		q->qstats.requeues, q->qstats.overlimits);
+}
+
+static void dump_qdisc_root(struct seq_file *seq, struct net_device *dev,
+					 struct Qdisc *root, char *inout)
+{
+	struct Qdisc *q;
+	int n = 0;
+
+	if (!root)
+		return;
+
+	dump_qdisc(seq, dev, root, inout, "root");
+
+	list_for_each_entry(q, &root->list, list) {
+		char buffer[10];
+
+		sprintf(buffer,"Q%d", ++n);
+		dump_qdisc(seq, dev, q, inout, buffer);
+	}
+}
+
+
+static void qdisc_seq_out(struct seq_file *seq, struct net_device *dev)
+{
+	struct netdev_queue *dev_queue;
+	int i;
+
+	for (i = 0; i < dev->real_num_tx_queues; i++) {
+		char buffer[10];
+
+		dev_queue = netdev_get_tx_queue(dev, i);
+		sprintf(buffer, "TX%d", i);
+		dump_qdisc_root(seq, dev, dev_queue->qdisc_sleeping, buffer);
+	}
+	dev_queue = &dev->rx_queue;
+	dump_qdisc_root(seq, dev, dev_queue->qdisc_sleeping, "RX");
+}
+
+static int qdisc_seq_show(struct seq_file *seq, void *v)
+{
+	if (v == SEQ_START_TOKEN) {
+		seq_printf(seq, "Queue    Device  State   Bytes  Packets "
+			"Qlen Blog   Drops Requeue Overlimit\n");
+	} else
+		qdisc_seq_out(seq, v);
+
+	return 0;
+}
+
+static void *qdisc_seq_start(struct seq_file *seq, loff_t *pos)
+	__acquires(dev_base_lock)
+{
+	struct net *net = seq_file_net(seq);
+	loff_t off;
+	struct net_device *dev;
+
+	read_lock(&dev_base_lock);
+
+	if (!*pos)
+		return SEQ_START_TOKEN;
+
+	off = 1;
+
+	for_each_netdev(net, dev)
+		if (off++ == *pos)
+			return dev;
+
+	return NULL;
+}
+
+static void *qdisc_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+	struct net *net = seq_file_net(seq);
+
+	++*pos;
+	return v == SEQ_START_TOKEN ?
+		first_net_device(net) : next_net_device((struct net_device *)v);
+}
+
+static void qdisc_seq_stop(struct seq_file *seq, void *v)
+	__releases(dev_base_lock)
+{
+	read_unlock(&dev_base_lock);
+}
+
+
+static const struct seq_operations qdisc_seq_ops = {
+	.start	= qdisc_seq_start,
+	.next	= qdisc_seq_next,
+	.stop	= qdisc_seq_stop,
+	.show	= qdisc_seq_show,
+};
+
+static int qdisc_open(struct inode *inode, struct file *file)
+{
+	return seq_open_net(inode, file, &qdisc_seq_ops,
+			sizeof(struct seq_net_private));
+}
+
+
+static const struct file_operations qdisc_fops = {
+	.owner = THIS_MODULE,
+	.open = qdisc_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release,
+};
 #endif

 static int __init pktsched_init(void)
@@ -1706,6 +1835,7 @@ static int __init pktsched_init(void)
 	register_qdisc(&pfifo_qdisc_ops);
 	register_qdisc(&bfifo_qdisc_ops);
 	proc_net_fops_create(&init_net, "psched", 0, &psched_fops);
+	proc_net_fops_create(&init_net, "qdisc_stats", 0, &qdisc_fops);

 	rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL);
 	rtnl_register(PF_UNSPEC, RTM_DELQDISC, tc_get_qdisc, NULL);

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

[Resent with fixed netdev@ address]

On 02-09-2009 01:52, Christoph Lameter wrote:
> [NET] Add proc file to display the state of all qdiscs
> 
> TC is a complicated tool and it currently does not allow the display of all
> qdisc states. It does not support multiple tx queues and also not
> localhost, nor does it display the current operating state of the queues.

I think, tc should've no problem with displaying summary stats of
multiqueue qdiscs or even all of them separately, as mentioned by
Patrick. And, maybe I still miss something, but there should be
nothing special with tc vs. localhost either.

> 
> This functionality could be added to tc / netlink but the tool is already
> complex to handle. The simple proc file here allows easy scanning by
> scripts and other tools. However, tc still needs to be updated to allow
> the modifications of multiqueue TX settings. tc's main focus is the
> configuration of qdiscs. The qdisc_stats file just shows the current
> state.
> 
> This patch adds
> 
> 	/proc/net/qdisc_stats
> 
> which displays the current state of all qdiscs on the system.
> 
> F.e.
> 
> $ cat /proc/net/qdisc_stats
> Queue    Device  State   Bytes  Packets Qlen Blog   Drops Requeue Overlimit
> TX0/root     lo   -          0        0    0    0       0       0       0
>  RX/root     lo   -          0        0    0    0       0       0       0
> TX0/root   eth0   -       5518       60    0    0       0       0       0
> TX1/root   eth0   -       2549       37    0    0       0       0       0
> TX2/root   eth0   -      63625      272    0    0       0       0       0
> TX3/root   eth0   -       1580       21    0    0       0       0       0
> TX4/root   eth0   R   88979440   260183    0 3532   43176    2111       0
> TX5/root   eth0   -       4698       56    0    0       0       0       0
> TX6/root   eth0   - 3598883129 10523140    0    0       0       0       0
> TX7/root   eth0   -       1750       21    0    0       0       0       0

As I wrote earlier, it's a nice work, but since it makes a new API I
guess we shouldn't hurry with merging this patch, and publish it as an
RFC first.

Then my humble suggestions would be to reserve more space for most of
the columns to make it readable not only for scripts when more TX#,
bytes, packets etc. Users of non-default qdiscs would also miss things
like: q->ops->id, q->handle, and q->parent at least. Plus, as I
mentioned earlier, q->qstats.qlen update with q->q.qlen (or using it
directly) is needed.

Jarek P.

> 
> Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
> 
> ---
>  net/sched/sch_api.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 130 insertions(+)
> 
> Index: linux-2.6/net/sched/sch_api.c
> ===================================================================
> --- linux-2.6.orig/net/sched/sch_api.c	2009-09-01 12:27:24.000000000 -0500
> +++ linux-2.6/net/sched/sch_api.c	2009-09-01 14:39:27.000000000 -0500
> @@ -1699,6 +1699,135 @@ static const struct file_operations psch
>  	.llseek = seq_lseek,
>  	.release = single_release,
>  };
> +
> +static void dump_qdisc(struct seq_file *seq, struct net_device *dev,
> +				struct Qdisc *q, char *inout, char *text)
> +{
> +	char state[5];
> +	char *p = state;
> +
> +	if (test_bit(__QDISC_STATE_RUNNING, &q->state))
> +		*p++ = 'R';
> +	if (test_bit(__QDISC_STATE_SCHED, &q->state))
> +		*p++ = 'S';
> +	if (test_bit(__QDISC_STATE_DEACTIVATED, &q->state))
> +		*p++ = 'D';
> +	if (q->state == 0)
> +		*p++ = '-';
> +	*p = 0;
> +
> +	seq_printf(seq, "%3s/%2s %6s %3s %10lld %8d %4d %4d %7d %7d %7d\n",
> +		inout, text, dev->name, state,
> +		q->bstats.bytes, q->bstats.packets,
> +		q->qstats.qlen, q->qstats.backlog, q->qstats.drops,
> +		q->qstats.requeues, q->qstats.overlimits);
> +}
> +
> +static void dump_qdisc_root(struct seq_file *seq, struct net_device *dev,
> +					 struct Qdisc *root, char *inout)
> +{
> +	struct Qdisc *q;
> +	int n = 0;
> +
> +	if (!root)
> +		return;
> +
> +	dump_qdisc(seq, dev, root, inout, "root");
> +
> +	list_for_each_entry(q, &root->list, list) {
> +		char buffer[10];
> +
> +		sprintf(buffer,"Q%d", ++n);
> +		dump_qdisc(seq, dev, q, inout, buffer);
> +	}
> +}
> +
> +
> +static void qdisc_seq_out(struct seq_file *seq, struct net_device *dev)
> +{
> +	struct netdev_queue *dev_queue;
> +	int i;
> +
> +	for (i = 0; i < dev->real_num_tx_queues; i++) {
> +		char buffer[10];
> +
> +		dev_queue = netdev_get_tx_queue(dev, i);
> +		sprintf(buffer, "TX%d", i);
> +		dump_qdisc_root(seq, dev, dev_queue->qdisc_sleeping, buffer);
> +	}
> +	dev_queue = &dev->rx_queue;
> +	dump_qdisc_root(seq, dev, dev_queue->qdisc_sleeping, "RX");
> +}
> +
> +static int qdisc_seq_show(struct seq_file *seq, void *v)
> +{
> +	if (v == SEQ_START_TOKEN) {
> +		seq_printf(seq, "Queue    Device  State   Bytes  Packets "
> +			"Qlen Blog   Drops Requeue Overlimit\n");
> +	} else
> +		qdisc_seq_out(seq, v);
> +
> +	return 0;
> +}
> +
> +static void *qdisc_seq_start(struct seq_file *seq, loff_t *pos)
> +	__acquires(dev_base_lock)
> +{
> +	struct net *net = seq_file_net(seq);
> +	loff_t off;
> +	struct net_device *dev;
> +
> +	read_lock(&dev_base_lock);
> +
> +	if (!*pos)
> +		return SEQ_START_TOKEN;
> +
> +	off = 1;
> +
> +	for_each_netdev(net, dev)
> +		if (off++ == *pos)
> +			return dev;
> +
> +	return NULL;
> +}
> +
> +static void *qdisc_seq_next(struct seq_file *seq, void *v, loff_t *pos)
> +{
> +	struct net *net = seq_file_net(seq);
> +
> +	++*pos;
> +	return v == SEQ_START_TOKEN ?
> +		first_net_device(net) : next_net_device((struct net_device *)v);
> +}
> +
> +static void qdisc_seq_stop(struct seq_file *seq, void *v)
> +	__releases(dev_base_lock)
> +{
> +	read_unlock(&dev_base_lock);
> +}
> +
> +
> +static const struct seq_operations qdisc_seq_ops = {
> +	.start	= qdisc_seq_start,
> +	.next	= qdisc_seq_next,
> +	.stop	= qdisc_seq_stop,
> +	.show	= qdisc_seq_show,
> +};
> +
> +static int qdisc_open(struct inode *inode, struct file *file)
> +{
> +	return seq_open_net(inode, file, &qdisc_seq_ops,
> +			sizeof(struct seq_net_private));
> +}
> +
> +
> +static const struct file_operations qdisc_fops = {
> +	.owner = THIS_MODULE,
> +	.open = qdisc_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = seq_release,
> +};
>  #endif
> 
>  static int __init pktsched_init(void)
> @@ -1706,6 +1835,7 @@ static int __init pktsched_init(void)
>  	register_qdisc(&pfifo_qdisc_ops);
>  	register_qdisc(&bfifo_qdisc_ops);
>  	proc_net_fops_create(&init_net, "psched", 0, &psched_fops);
> +	proc_net_fops_create(&init_net, "qdisc_stats", 0, &qdisc_fops);
> 
>  	rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL);
>  	rtnl_register(PF_UNSPEC, RTM_DELQDISC, tc_get_qdisc, NULL);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [NET] Add proc file to display the state of all qdiscs.

[Eric Dumazet]

Jarek Poplawski a écrit :
> [Resent with fixed netdev@ address]
> 
> On 02-09-2009 01:52, Christoph Lameter wrote:
>> [NET] Add proc file to display the state of all qdiscs
>>
>> TC is a complicated tool and it currently does not allow the display of all
>> qdisc states. It does not support multiple tx queues and also not
>> localhost, nor does it display the current operating state of the queues.
> 
> I think, tc should've no problem with displaying summary stats of
> multiqueue qdiscs or even all of them separately, as mentioned by
> Patrick. And, maybe I still miss something, but there should be
> nothing special with tc vs. localhost either.
>

I made a patch, but for a 8 queue device (bnx2), here is the "tc -s -d qdisc" result :

$ tc -s -d qdisc show
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 51814 bytes 459 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0


Same name "eth0" is displayed, that might confuse parsers...

What naming convention should we choose for multiqueue devices ?

Re: [NET] Add proc file to display the state of all qdiscs.

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Sep 2009 10:28:55 +0200

> What naming convention should we choose for multiqueue devices ?

We could give an index field to multiple root qdiscs assigned
to a device.

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

On Wed, Sep 02, 2009 at 10:28:55AM +0200, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > [Resent with fixed netdev@ address]
> > 
> > On 02-09-2009 01:52, Christoph Lameter wrote:
> >> [NET] Add proc file to display the state of all qdiscs
> >>
> >> TC is a complicated tool and it currently does not allow the display of all
> >> qdisc states. It does not support multiple tx queues and also not
> >> localhost, nor does it display the current operating state of the queues.
> > 
> > I think, tc should've no problem with displaying summary stats of
> > multiqueue qdiscs or even all of them separately, as mentioned by
> > Patrick. And, maybe I still miss something, but there should be
> > nothing special with tc vs. localhost either.
> >
> 
> I made a patch, but for a 8 queue device (bnx2), here is the "tc -s -d qdisc" result :
> 
> $ tc -s -d qdisc show
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 51814 bytes 459 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> 
> 
> Same name "eth0" is displayed, that might confuse parsers...
> 
> What naming convention should we choose for multiqueue devices ?
> 

Hmm... anything could break here something for somebody, so there is
still a (Patrick's) question if not sum it all? Otherwise, I wonder
about using the qdisc handle (tcm_handle>>16): there would be at
least one "pfifo_fast 0:" looking like proper root for somebody...

Jarek P.

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

On Wed, Sep 02, 2009 at 09:18:54AM +0000, Jarek Poplawski wrote:
> On Wed, Sep 02, 2009 at 10:28:55AM +0200, Eric Dumazet wrote:
...
> > qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> >  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> >  rate 0bit 0pps backlog 0b 0p requeues 0
> > 
> > 
> > Same name "eth0" is displayed, that might confuse parsers...
> > 
> > What naming convention should we choose for multiqueue devices ?
> > 
> 
> Hmm... anything could break here something for somebody, so there is
> still a (Patrick's) question if not sum it all? Otherwise, I wonder
> about using the qdisc handle (tcm_handle>>16): there would be at
> least one "pfifo_fast 0:" looking like proper root for somebody...

I meant "proper" for pfifo_fast. On the other hand, I wonder why these
multiqueue qdisc handles can't be really given such unique per dev
(instead of per queue) handles?

Jarek P.

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

On Wed, Sep 02, 2009 at 09:33:29AM +0000, Jarek Poplawski wrote:
> On Wed, Sep 02, 2009 at 09:18:54AM +0000, Jarek Poplawski wrote:
> > On Wed, Sep 02, 2009 at 10:28:55AM +0200, Eric Dumazet wrote:
> ...
> > > qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> > >  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> > >  rate 0bit 0pps backlog 0b 0p requeues 0
> > > 
> > > 
> > > Same name "eth0" is displayed, that might confuse parsers...
> > > 
> > > What naming convention should we choose for multiqueue devices ?
> > > 
> > 
> > Hmm... anything could break here something for somebody, so there is
> > still a (Patrick's) question if not sum it all? Otherwise, I wonder
> > about using the qdisc handle (tcm_handle>>16): there would be at
> > least one "pfifo_fast 0:" looking like proper root for somebody...
> 
> I meant "proper" for pfifo_fast. On the other hand, I wonder why these
> multiqueue qdisc handles can't be really given such unique per dev

should be:
multiqueue qdiscs can't be really given such unique per dev

> (instead of per queue) handles?
> 
> Jarek P.

[PATCH net-next-2.6] tc: report informations for multiqueue devices

[Eric Dumazet]

David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 02 Sep 2009 10:28:55 +0200
> 
>> What naming convention should we choose for multiqueue devices ?
> 
> We could give an index field to multiple root qdiscs assigned
> to a device.

Here is a patch then :)

Only point is that I am iterating from 0 to dev->real_num_tx_queues
instead of dev->num_tx_queues. I hope it's fine, because there are
allocated qdisc, but not really used.

Next patches to allow selective qdisc change/fetch (providing a TCA_QINDEX
selector value to kernel)

Thanks


[PATCH net-next-2.6] tc: report informations for multiqueue devices

qdisc and classes are not yet displayed by "tc -s -d {qdisc|class} show"
for multiqueue devices.

We use a new TCA_QINDEX attribute, to report queue index to user space.
iproute2 tc should be changed to eventually display this queue index as in :

$ tc -s -d qdisc
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 52498 bytes 465 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 qindex 1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0



Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/rtnetlink.h |    1
 net/sched/sch_api.c       |  118 ++++++++++++++++++++----------------
 2 files changed, 67 insertions(+), 52 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index ba3254e..b80e0f6 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -490,6 +490,7 @@ enum
 	TCA_FCNT,
 	TCA_STATS2,
 	TCA_STAB,
+	TCA_QINDEX,
 	__TCA_MAX
 };
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 24d17ce..74cde83 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -35,9 +35,9 @@
 #include <net/pkt_sched.h>
 
 static int qdisc_notify(struct sk_buff *oskb, struct nlmsghdr *n, u32 clid,
-			struct Qdisc *old, struct Qdisc *new);
+			struct Qdisc *old, struct Qdisc *new, int qnum);
 static int tclass_notify(struct sk_buff *oskb, struct nlmsghdr *n,
-			 struct Qdisc *q, unsigned long cl, int event);
+			 struct Qdisc *q, unsigned long cl, int event, int qnum);
 
 /*
 
@@ -671,10 +671,10 @@ void qdisc_tree_decrease_qlen(struct Qdisc *sch, unsigned int n)
 EXPORT_SYMBOL(qdisc_tree_decrease_qlen);
 
 static void notify_and_destroy(struct sk_buff *skb, struct nlmsghdr *n, u32 clid,
-			       struct Qdisc *old, struct Qdisc *new)
+			       struct Qdisc *old, struct Qdisc *new, int qnum)
 {
 	if (new || old)
-		qdisc_notify(skb, n, clid, old, new);
+		qdisc_notify(skb, n, clid, old, new, qnum);
 
 	if (old)
 		qdisc_destroy(old);
@@ -720,7 +720,7 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 			if (new && i > 0)
 				atomic_inc(&new->refcnt);
 
-			notify_and_destroy(skb, n, classid, old, new);
+			notify_and_destroy(skb, n, classid, old, new, i);
 		}
 
 		if (dev->flags & IFF_UP)
@@ -738,7 +738,7 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 			}
 		}
 		if (!err)
-			notify_and_destroy(skb, n, classid, old, new);
+			notify_and_destroy(skb, n, classid, old, new, 0);
 	}
 	return err;
 }
@@ -999,7 +999,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 		if ((err = qdisc_graft(dev, p, skb, n, clid, NULL, q)) != 0)
 			return err;
 	} else {
-		qdisc_notify(skb, n, clid, NULL, q);
+		qdisc_notify(skb, n, clid, NULL, q, 0);
 	}
 	return 0;
 }
@@ -1116,7 +1116,7 @@ replay:
 		return -EINVAL;
 	err = qdisc_change(q, tca);
 	if (err == 0)
-		qdisc_notify(skb, n, clid, NULL, q);
+		qdisc_notify(skb, n, clid, NULL, q, 0);
 	return err;
 
 create_n_graft:
@@ -1148,7 +1148,7 @@ graft:
 }
 
 static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
-			 u32 pid, u32 seq, u16 flags, int event)
+			 u32 pid, u32 seq, u16 flags, int event, int qnum)
 {
 	struct tcmsg *tcm;
 	struct nlmsghdr  *nlh;
@@ -1187,6 +1187,9 @@ static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
 	if (gnet_stats_finish_copy(&d) < 0)
 		goto nla_put_failure;
 
+	if (qnum)
+		NLA_PUT_U32(skb, TCA_QINDEX, qnum);
+
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 	return skb->len;
 
@@ -1197,7 +1200,8 @@ nla_put_failure:
 }
 
 static int qdisc_notify(struct sk_buff *oskb, struct nlmsghdr *n,
-			u32 clid, struct Qdisc *old, struct Qdisc *new)
+			u32 clid, struct Qdisc *old, struct Qdisc *new,
+			int qnum)
 {
 	struct sk_buff *skb;
 	u32 pid = oskb ? NETLINK_CB(oskb).pid : 0;
@@ -1207,11 +1211,13 @@ static int qdisc_notify(struct sk_buff *oskb, struct nlmsghdr *n,
 		return -ENOBUFS;
 
 	if (old && old->handle) {
-		if (tc_fill_qdisc(skb, old, clid, pid, n->nlmsg_seq, 0, RTM_DELQDISC) < 0)
+		if (tc_fill_qdisc(skb, old, clid, pid, n->nlmsg_seq, 0,
+				  RTM_DELQDISC, qnum) < 0)
 			goto err_out;
 	}
 	if (new) {
-		if (tc_fill_qdisc(skb, new, clid, pid, n->nlmsg_seq, old ? NLM_F_REPLACE : 0, RTM_NEWQDISC) < 0)
+		if (tc_fill_qdisc(skb, new, clid, pid, n->nlmsg_seq,
+				  old ? NLM_F_REPLACE : 0, RTM_NEWQDISC, qnum) < 0)
 			goto err_out;
 	}
 
@@ -1230,7 +1236,7 @@ static bool tc_qdisc_dump_ignore(struct Qdisc *q)
 
 static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
 			      struct netlink_callback *cb,
-			      int *q_idx_p, int s_q_idx)
+			      int *q_idx_p, int s_q_idx, int qnum)
 {
 	int ret = 0, q_idx = *q_idx_p;
 	struct Qdisc *q;
@@ -1239,23 +1245,18 @@ static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
 		return 0;
 
 	q = root;
-	if (q_idx < s_q_idx) {
-		q_idx++;
-	} else {
-		if (!tc_qdisc_dump_ignore(q) &&
-		    tc_fill_qdisc(skb, q, q->parent, NETLINK_CB(cb->skb).pid,
-				  cb->nlh->nlmsg_seq, NLM_F_MULTI, RTM_NEWQDISC) <= 0)
-			goto done;
-		q_idx++;
-	}
+	if (q_idx >= s_q_idx &&
+	    !tc_qdisc_dump_ignore(q) &&
+	    tc_fill_qdisc(skb, q, q->parent, NETLINK_CB(cb->skb).pid,
+			  cb->nlh->nlmsg_seq, NLM_F_MULTI, RTM_NEWQDISC, qnum) <= 0)
+		goto done;
+	q_idx++;
+
 	list_for_each_entry(q, &root->list, list) {
-		if (q_idx < s_q_idx) {
-			q_idx++;
-			continue;
-		}
-		if (!tc_qdisc_dump_ignore(q) && 
+		if (q_idx >= s_q_idx &&
+		    !tc_qdisc_dump_ignore(q) && 
 		    tc_fill_qdisc(skb, q, q->parent, NETLINK_CB(cb->skb).pid,
-				  cb->nlh->nlmsg_seq, NLM_F_MULTI, RTM_NEWQDISC) <= 0)
+				  cb->nlh->nlmsg_seq, NLM_F_MULTI, RTM_NEWQDISC, qnum) <= 0)
 			goto done;
 		q_idx++;
 	}
@@ -1284,6 +1285,7 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 	idx = 0;
 	for_each_netdev(&init_net, dev) {
 		struct netdev_queue *dev_queue;
+		int ntx;
 
 		if (idx < s_idx)
 			goto cont;
@@ -1291,12 +1293,15 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 			s_q_idx = 0;
 		q_idx = 0;
 
-		dev_queue = netdev_get_tx_queue(dev, 0);
-		if (tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb, &q_idx, s_q_idx) < 0)
-			goto done;
-
+		for (ntx = 0 ; ntx < dev->real_num_tx_queues; ntx++) {
+			dev_queue = netdev_get_tx_queue(dev, ntx);
+			if (tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb,
+					       cb, &q_idx, s_q_idx, ntx) < 0)
+				goto done;
+		}
 		dev_queue = &dev->rx_queue;
-		if (tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb, &q_idx, s_q_idx) < 0)
+		if (tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb,
+				       &q_idx, s_q_idx, 0) < 0)
 			goto done;
 
 cont:
@@ -1419,10 +1424,10 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 		case RTM_DELTCLASS:
 			err = cops->delete(q, cl);
 			if (err == 0)
-				tclass_notify(skb, n, q, cl, RTM_DELTCLASS);
+				tclass_notify(skb, n, q, cl, RTM_DELTCLASS, 0);
 			goto out;
 		case RTM_GETTCLASS:
-			err = tclass_notify(skb, n, q, cl, RTM_NEWTCLASS);
+			err = tclass_notify(skb, n, q, cl, RTM_NEWTCLASS, 0);
 			goto out;
 		default:
 			err = -EINVAL;
@@ -1433,7 +1438,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 	new_cl = cl;
 	err = cops->change(q, clid, pid, tca, &new_cl);
 	if (err == 0)
-		tclass_notify(skb, n, q, new_cl, RTM_NEWTCLASS);
+		tclass_notify(skb, n, q, new_cl, RTM_NEWTCLASS, 0);
 
 out:
 	if (cl)
@@ -1445,7 +1450,7 @@ out:
 
 static int tc_fill_tclass(struct sk_buff *skb, struct Qdisc *q,
 			  unsigned long cl,
-			  u32 pid, u32 seq, u16 flags, int event)
+			  u32 pid, u32 seq, u16 flags, int event, int qnum)
 {
 	struct tcmsg *tcm;
 	struct nlmsghdr  *nlh;
@@ -1474,6 +1479,9 @@ static int tc_fill_tclass(struct sk_buff *skb, struct Qdisc *q,
 	if (gnet_stats_finish_copy(&d) < 0)
 		goto nla_put_failure;
 
+	if (qnum)
+		NLA_PUT_U32(skb, TCA_QINDEX, qnum);
+
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 	return skb->len;
 
@@ -1484,7 +1492,8 @@ nla_put_failure:
 }
 
 static int tclass_notify(struct sk_buff *oskb, struct nlmsghdr *n,
-			  struct Qdisc *q, unsigned long cl, int event)
+			 struct Qdisc *q, unsigned long cl, int event,
+			 int qnum)
 {
 	struct sk_buff *skb;
 	u32 pid = oskb ? NETLINK_CB(oskb).pid : 0;
@@ -1493,7 +1502,7 @@ static int tclass_notify(struct sk_buff *oskb, struct nlmsghdr *n,
 	if (!skb)
 		return -ENOBUFS;
 
-	if (tc_fill_tclass(skb, q, cl, pid, n->nlmsg_seq, 0, event) < 0) {
+	if (tc_fill_tclass(skb, q, cl, pid, n->nlmsg_seq, 0, event, qnum) < 0) {
 		kfree_skb(skb);
 		return -EINVAL;
 	}
@@ -1503,9 +1512,10 @@ static int tclass_notify(struct sk_buff *oskb, struct nlmsghdr *n,
 
 struct qdisc_dump_args
 {
-	struct qdisc_walker w;
-	struct sk_buff *skb;
+	struct qdisc_walker	w;
+	struct sk_buff		*skb;
 	struct netlink_callback *cb;
+	int			qnum;
 };
 
 static int qdisc_class_dump(struct Qdisc *q, unsigned long cl, struct qdisc_walker *arg)
@@ -1513,12 +1523,13 @@ static int qdisc_class_dump(struct Qdisc *q, unsigned long cl, struct qdisc_walk
 	struct qdisc_dump_args *a = (struct qdisc_dump_args *)arg;
 
 	return tc_fill_tclass(a->skb, q, cl, NETLINK_CB(a->cb->skb).pid,
-			      a->cb->nlh->nlmsg_seq, NLM_F_MULTI, RTM_NEWTCLASS);
+			      a->cb->nlh->nlmsg_seq, NLM_F_MULTI, RTM_NEWTCLASS,
+			      a->qnum);
 }
 
 static int tc_dump_tclass_qdisc(struct Qdisc *q, struct sk_buff *skb,
 				struct tcmsg *tcm, struct netlink_callback *cb,
-				int *t_p, int s_t)
+				int *t_p, int s_t, int qnum)
 {
 	struct qdisc_dump_args arg;
 
@@ -1537,6 +1548,7 @@ static int tc_dump_tclass_qdisc(struct Qdisc *q, struct sk_buff *skb,
 	arg.w.stop  = 0;
 	arg.w.skip = cb->args[1];
 	arg.w.count = 0;
+	arg.qnum = qnum;
 	q->ops->cl_ops->walk(q, &arg.w);
 	cb->args[1] = arg.w.count;
 	if (arg.w.stop)
@@ -1547,18 +1559,18 @@ static int tc_dump_tclass_qdisc(struct Qdisc *q, struct sk_buff *skb,
 
 static int tc_dump_tclass_root(struct Qdisc *root, struct sk_buff *skb,
 			       struct tcmsg *tcm, struct netlink_callback *cb,
-			       int *t_p, int s_t)
+			       int *t_p, int s_t, int qnum)
 {
 	struct Qdisc *q;
 
 	if (!root)
 		return 0;
 
-	if (tc_dump_tclass_qdisc(root, skb, tcm, cb, t_p, s_t) < 0)
+	if (tc_dump_tclass_qdisc(root, skb, tcm, cb, t_p, s_t, qnum) < 0)
 		return -1;
 
 	list_for_each_entry(q, &root->list, list) {
-		if (tc_dump_tclass_qdisc(q, skb, tcm, cb, t_p, s_t) < 0)
+		if (tc_dump_tclass_qdisc(q, skb, tcm, cb, t_p, s_t, qnum) < 0)
 			return -1;
 	}
 
@@ -1571,7 +1583,7 @@ static int tc_dump_tclass(struct sk_buff *skb, struct netlink_callback *cb)
 	struct net *net = sock_net(skb->sk);
 	struct netdev_queue *dev_queue;
 	struct net_device *dev;
-	int t, s_t;
+	int t, s_t, ntx;
 
 	if (net != &init_net)
 		return 0;
@@ -1584,12 +1596,14 @@ static int tc_dump_tclass(struct sk_buff *skb, struct netlink_callback *cb)
 	s_t = cb->args[0];
 	t = 0;
 
-	dev_queue = netdev_get_tx_queue(dev, 0);
-	if (tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb, &t, s_t) < 0)
-		goto done;
-
+	for (ntx = 0 ; ntx < dev->real_num_tx_queues; ntx++) {
+		dev_queue = netdev_get_tx_queue(dev, ntx);
+		if (tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm,
+					cb, &t, s_t, ntx) < 0)
+			goto done;
+	}
 	dev_queue = &dev->rx_queue;
-	if (tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb, &t, s_t) < 0)
+	if (tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb, &t, s_t, 0) < 0)
 		goto done;
 
 done:

Re: [NET] Add proc file to display the state of all qdiscs.

[Patrick McHardy]

Jarek Poplawski wrote:
> On Wed, Sep 02, 2009 at 09:33:29AM +0000, Jarek Poplawski wrote:
>> On Wed, Sep 02, 2009 at 09:18:54AM +0000, Jarek Poplawski wrote:
>>> On Wed, Sep 02, 2009 at 10:28:55AM +0200, Eric Dumazet wrote:
>> ...
>>>> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>>>>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>>>>  rate 0bit 0pps backlog 0b 0p requeues 0
>>>>
>>>>
>>>> Same name "eth0" is displayed, that might confuse parsers...
>>>>
>>>> What naming convention should we choose for multiqueue devices ?
>>>>
>>> Hmm... anything could break here something for somebody, so there is
>>> still a (Patrick's) question if not sum it all? Otherwise, I wonder
>>> about using the qdisc handle (tcm_handle>>16): there would be at
>>> least one "pfifo_fast 0:" looking like proper root for somebody...
>> I meant "proper" for pfifo_fast. On the other hand, I wonder why these
>> multiqueue qdisc handles can't be really given such unique per dev
> 
> should be:
> multiqueue qdiscs can't be really given such unique per dev
> 
>> (instead of per queue) handles?

In my opinion the best way would be to sum up the stats and display
them for the "main" qdisc to avoid any compatibility problems and
additionally dump each queue for informational purposes.

This raises a few more questions however. First of all, there's no
"main" qdisc, so if we just use the first one for the summed up stats,
the question is whether the parameters of all root qdiscs are the same.
Currently they should be since pfifo_fast doesn't support changing
parameters, but this might change in the future, in which case we
might be displaying "incorrect" parameters.

The next question would be whether and how to support changing
parameters of individual multiq qdiscs. Similar to dumps, when
changing qdisc parameters we always pick queue number 0. It we want
to support changing parameters of different queues, we could
replicate changes to all queues, which would avoid the problem
stated above, or we could add an optional identifier for the
queue number, or we could use a combination of both which only
replicates settings when no queue number is specified.

In the second and third case, one possibility to get around the
incorrect parameters for the "main" qdisc would be to display
a virtual (non-existant) qdisc as the root, which only contains
the stats. I don't think the default choice of root qdisc type
should be counted as belonging to the API and userspace should
be prepared to deal with this.

And finally, the TC API used to be symetrical in the sense that
you could replay a dump to the kernel to get the same configuration.
Dumping the individual queues would break that property unless
we also support creating and configuring them in a symetrical
fashion.

So here's something of a possible solution, assuming that at some
point we do want to support changing parameters for individual root
qdiscs and taking the above into account:

- when creating a qdisc on a multiqueue-device through the TC
  API, we keep the current behaviour and share it between all
  queues. Configuration changes don't need to be replicated since
  the qdisc is shared. Dumps only contain a single instance of
  the qdisc. This is exactly what is done today.

- for non-shared root qdiscs on a multiqueue device like the
  pfifo_fast qdiscs created by default, we dump a virtual root
  qdisc (multiqueue) which only contains the number of bands
  and the statistics and dump the real root qdiscs as children
  of that qdisc.

- to create or configure a qdisc for a specific queue number, we
  require the user to create the virtual root qdisc (which only
  sets a flag or something like that) and then address each queue
  number. When the virtual qdisc has been created, we don't
  replicate any changes. This restores symetry with dumps and
  allows to address individual queues.

This is just a first collection of thoughts and probably can be
improved. Comments welcome :)

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Eric Dumazet]

Eric Dumazet a écrit :
> David Miller a écrit :
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Wed, 02 Sep 2009 10:28:55 +0200
>>
>>> What naming convention should we choose for multiqueue devices ?
>> We could give an index field to multiple root qdiscs assigned
>> to a device.
> 
> Here is a patch then :)
> 
> Only point is that I am iterating from 0 to dev->real_num_tx_queues
> instead of dev->num_tx_queues. I hope it's fine, because there are
> allocated qdisc, but not really used.
> 
> Next patches to allow selective qdisc change/fetch (providing a TCA_QINDEX
> selector value to kernel)
> 
> Thanks
> 
> 
> [PATCH net-next-2.6] tc: report informations for multiqueue devices
> 
> qdisc and classes are not yet displayed by "tc -s -d {qdisc|class} show"
> for multiqueue devices.
> 
> We use a new TCA_QINDEX attribute, to report queue index to user space.
> iproute2 tc should be changed to eventually display this queue index as in :
> 
> $ tc -s -d qdisc
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 52498 bytes 465 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 qindex 1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> 


Here is the iproute2 patch as well, to display queue indexes

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index ba3254e..b80e0f6 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -490,6 +490,7 @@ enum
 	TCA_FCNT,
 	TCA_STATS2,
 	TCA_STAB,
+	TCA_QINDEX,
 	__TCA_MAX
 };
 
diff --git a/tc/tc_class.c b/tc/tc_class.c
index 9d4eea5..1bc4bc6 100644
--- a/tc/tc_class.c
+++ b/tc/tc_class.c
@@ -196,6 +196,13 @@ int print_class(const struct sockaddr_nl *who,
 	if (filter_ifindex == 0)
 		fprintf(fp, "dev %s ", ll_index_to_name(t->tcm_ifindex));
 
+	if (tb[TCA_QINDEX]) {
+		int qindex = 0;
+
+		memcpy(&qindex, RTA_DATA(tb[TCA_QINDEX]), MIN(RTA_PAYLOAD(tb[TCA_QINDEX]), sizeof(int)));
+		fprintf(fp, "qindex %d ", qindex);
+	}
+
 	if (t->tcm_parent == TC_H_ROOT)
 		fprintf(fp, "root ");
 	else {
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index c7f2988..3ec2cf4 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -232,6 +232,14 @@ int print_qdisc(const struct sockaddr_nl *who,
 	fprintf(fp, "qdisc %s %x: ", (char*)RTA_DATA(tb[TCA_KIND]), t->tcm_handle>>16);
 	if (filter_ifindex == 0)
 		fprintf(fp, "dev %s ", ll_index_to_name(t->tcm_ifindex));
+
+	if (tb[TCA_QINDEX]) {
+		int qindex = 0;
+
+		memcpy(&qindex, RTA_DATA(tb[TCA_QINDEX]), MIN(RTA_PAYLOAD(tb[TCA_QINDEX]), sizeof(int)));
+		fprintf(fp, "qindex %d ", qindex);
+	}
+
 	if (t->tcm_parent == TC_H_ROOT)
 		fprintf(fp, "root ");
 	else if (t->tcm_parent) {

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Patrick McHardy]

Eric Dumazet wrote:
> [PATCH net-next-2.6] tc: report informations for multiqueue devices
> 
> qdisc and classes are not yet displayed by "tc -s -d {qdisc|class} show"
> for multiqueue devices.
> 
> We use a new TCA_QINDEX attribute, to report queue index to user space.
> iproute2 tc should be changed to eventually display this queue index as in :
> 
> $ tc -s -d qdisc
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 52498 bytes 465 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth0 qindex 1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0

This might confuse existing userspace since the handle is not unique
anymore. libnl f.i. will treat all but the first root qdisc as an
update and use it to update the state of the first one. There's also
no combined view for applications unaware of multiqueue.

Please have a look at the mail I just wrote for some possible ways
around this.

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Eric Dumazet]

Patrick McHardy a écrit :
> Eric Dumazet wrote:
>> [PATCH net-next-2.6] tc: report informations for multiqueue devices
>>
>> qdisc and classes are not yet displayed by "tc -s -d {qdisc|class} show"
>> for multiqueue devices.
>>
>> We use a new TCA_QINDEX attribute, to report queue index to user space.
>> iproute2 tc should be changed to eventually display this queue index as in :
>>
>> $ tc -s -d qdisc
>> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>>  Sent 52498 bytes 465 pkt (dropped 0, overlimits 0 requeues 0)
>>  rate 0bit 0pps backlog 0b 0p requeues 0
>> qdisc pfifo_fast 0: dev eth0 qindex 1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>>  rate 0bit 0pps backlog 0b 0p requeues 0
> 
> This might confuse existing userspace since the handle is not unique
> anymore. libnl f.i. will treat all but the first root qdisc as an
> update and use it to update the state of the first one. There's also
> no combined view for applications unaware of multiqueue.
> 
> Please have a look at the mail I just wrote for some possible ways
> around this.

Hum, how can we combine infos on qdisc/class if in the future we allow each queue index
to have its own qdisc/classes ?

htb on queue index 0
cbq on queue index 1

Combining info would lock us and not allow for special configurations.
Say 
   macvlan device 0 mapped to queue index 0
   macvlan device 1 mapped to queue index 1...

For old apps, just give informations for queue 0 as we do now, and
allow kernel to give more informations only if new application provided a TCA_INDEX attribute
in its request ?

(-1 : all queue indexes,  >=0 for a given queue index)

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Patrick McHardy]

Eric Dumazet wrote:
> Patrick McHardy a écrit :
>> Eric Dumazet wrote:
>>> [PATCH net-next-2.6] tc: report informations for multiqueue devices
>>>
>>> qdisc and classes are not yet displayed by "tc -s -d {qdisc|class} show"
>>> for multiqueue devices.
>>>
>>> We use a new TCA_QINDEX attribute, to report queue index to user space.
>>> iproute2 tc should be changed to eventually display this queue index as in :
>>>
>>> $ tc -s -d qdisc
>>> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>>>  Sent 52498 bytes 465 pkt (dropped 0, overlimits 0 requeues 0)
>>>  rate 0bit 0pps backlog 0b 0p requeues 0
>>> qdisc pfifo_fast 0: dev eth0 qindex 1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>>>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>>>  rate 0bit 0pps backlog 0b 0p requeues 0
>> This might confuse existing userspace since the handle is not unique
>> anymore. libnl f.i. will treat all but the first root qdisc as an
>> update and use it to update the state of the first one. There's also
>> no combined view for applications unaware of multiqueue.
>>
>> Please have a look at the mail I just wrote for some possible ways
>> around this.
> 
> Hum, how can we combine infos on qdisc/class if in the future we allow each queue index
> to have its own qdisc/classes ?
> 
> htb on queue index 0
> cbq on queue index 1

My suggestion was to only dump the statistics in the combined
view and use a virtual qdisc, something like:

qdisc multiqueue 0: dev eth0 root queues 8
  Sent ...
  rate ...

and show each real qdisc as child of this qdisc:

qdisc pfifo_fast <unique handle> dev eth0 parent 0: bands 3 ...
qdisc pfifo_fast <unique handle> dev eth0 parent 0: bands 3 ...

Configuration would be symetrical to this:

tc qdisc add dev eth0 handle 0: root multiqueue
tc qdisc add dev eth0 handle x: parent 0: pfifo_fast
...

without the virtual multiqueue qdisc, the root qdisc would simply
be shared among all queues as today.

> Combining info would lock us and not allow for special configurations.
> Say 
>    macvlan device 0 mapped to queue index 0
>    macvlan device 1 mapped to queue index 1...

Why not?

> For old apps, just give informations for queue 0 as we do now, and
> allow kernel to give more informations only if new application provided a TCA_INDEX attribute
> in its request ?
> 
> (-1 : all queue indexes,  >=0 for a given queue index)

If we don't combine the information, existing multiqueue unaware
applications will get incorrect information. There's also the
problem of non-unique handles, I think we should encode the queue
index in the handle instead of using a new attribute. This needs
a bit more thought though to avoid clashes with user-defined handles.

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Stephen Hemminger]

On Wed, 02 Sep 2009 14:48:27 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Eric Dumazet a écrit :
> > David Miller a écrit :
> >> From: Eric Dumazet <eric.dumazet@gmail.com>
> >> Date: Wed, 02 Sep 2009 10:28:55 +0200
> >>
> >>> What naming convention should we choose for multiqueue devices ?
> >> We could give an index field to multiple root qdiscs assigned
> >> to a device.
> > 
> > Here is a patch then :)
> > 
> > Only point is that I am iterating from 0 to dev->real_num_tx_queues
> > instead of dev->num_tx_queues. I hope it's fine, because there are
> > allocated qdisc, but not really used.
> > 
> > Next patches to allow selective qdisc change/fetch (providing a TCA_QINDEX
> > selector value to kernel)
> > 
> > Thanks
> > 
> > 
> > [PATCH net-next-2.6] tc: report informations for multiqueue devices
> > 
> > qdisc and classes are not yet displayed by "tc -s -d {qdisc|class} show"
> > for multiqueue devices.
> > 
> > We use a new TCA_QINDEX attribute, to report queue index to user space.
> > iproute2 tc should be changed to eventually display this queue index as in :
> > 
> > $ tc -s -d qdisc
> > qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> >  Sent 52498 bytes 465 pkt (dropped 0, overlimits 0 requeues 0)
> >  rate 0bit 0pps backlog 0b 0p requeues 0
> > qdisc pfifo_fast 0: dev eth0 qindex 1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> >  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> >  rate 0bit 0pps backlog 0b 0p requeues 0
> > 
> 
> 
> Here is the iproute2 patch as well, to display queue indexes
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> 
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index ba3254e..b80e0f6 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -490,6 +490,7 @@ enum
>  	TCA_FCNT,
>  	TCA_STATS2,
>  	TCA_STAB,
> +	TCA_QINDEX,
>  	__TCA_MAX
>  };
>  
> diff --git a/tc/tc_class.c b/tc/tc_class.c
> index 9d4eea5..1bc4bc6 100644
> --- a/tc/tc_class.c
> +++ b/tc/tc_class.c
> @@ -196,6 +196,13 @@ int print_class(const struct sockaddr_nl *who,
>  	if (filter_ifindex == 0)
>  		fprintf(fp, "dev %s ", ll_index_to_name(t->tcm_ifindex));
>  
> +	if (tb[TCA_QINDEX]) {
> +		int qindex = 0;
> +
> +		memcpy(&qindex, RTA_DATA(tb[TCA_QINDEX]), MIN(RTA_PAYLOAD(tb[TCA_QINDEX]), sizeof(int)));
> +		fprintf(fp, "qindex %d ", qindex);
> +	}
> +
>  	if (t->tcm_parent == TC_H_ROOT)
>  		fprintf(fp, "root ");
>  	else {

Even if the kernel keeps the value in something else for the API
QINDEX needs to be a fixed size unsigned type like u32.

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Patrick McHardy]

Stephen Hemminger wrote:
> Even if the kernel keeps the value in something else for the API
> QINDEX needs to be a fixed size unsigned type like u32.

Just to avoid duplicate work - I'm currently trying to put a patch
together that presents multiqueue qdiscs to userspace as regular
child qdiscs of a dummy qdisc, which also contains the aggregated
statistics. So far it actually looks pretty sane :)

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Eric Dumazet]

Patrick McHardy a écrit :
> Stephen Hemminger wrote:
>> Even if the kernel keeps the value in something else for the API
>> QINDEX needs to be a fixed size unsigned type like u32.
> 
> Just to avoid duplicate work - I'm currently trying to put a patch
> together that presents multiqueue qdiscs to userspace as regular
> child qdiscs of a dummy qdisc, which also contains the aggregated
> statistics. So far it actually looks pretty sane :)

Excellent !

I am cooking a patch to make vlan devices multiqueue aware
(ie having same tx_queue numbers than their real_dev)

[PATCH net-next-2.6] vlan: multiqueue vlan devices

[Eric Dumazet]

vlan devices are currently not multi-queue capable.

We can do that with a new rtnl_link_ops method,
get_tx_queues(), called from rtnl_create_link()

This new method gets num_tx_queues/real_num_tx_queues
from real device.

register_vlan_device() is also handled.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/rtnetlink.h  |    2 ++
 net/8021q/vlan.c         |    5 +++--
 net/8021q/vlan_netlink.c |   20 ++++++++++++++++++++
 net/core/rtnetlink.c     |   10 +++++++++-
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 3c1895e..0525a1d 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -70,6 +70,8 @@ struct rtnl_link_ops {
 	size_t			(*get_xstats_size)(const struct net_device *dev);
 	int			(*fill_xstats)(struct sk_buff *skb,
 					       const struct net_device *dev);
+	int			(*get_tx_queues)(struct net*, struct nlattr *tb[],
+						 int *tx_queues, int *real_tx_queues);
 };
 
 extern int	__rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index e814794..8836575 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -330,12 +330,13 @@ static int register_vlan_device(struct net_device *real_dev, u16 vlan_id)
 		snprintf(name, IFNAMSIZ, "vlan%.4i", vlan_id);
 	}
 
-	new_dev = alloc_netdev(sizeof(struct vlan_dev_info), name,
-			       vlan_setup);
+	new_dev = alloc_netdev_mq(sizeof(struct vlan_dev_info), name,
+				  vlan_setup, real_dev->num_tx_queues);
 
 	if (new_dev == NULL)
 		return -ENOBUFS;
 
+	new_dev->real_num_tx_queues = real_dev->real_num_tx_queues;
 	dev_net_set(new_dev, net);
 	/* need 4 bytes for extra VLAN header info,
 	 * hope the underlying device can handle it.
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index e9c91dc..8ce4122 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -100,6 +100,25 @@ static int vlan_changelink(struct net_device *dev,
 	return 0;
 }
 
+static int vlan_get_tx_queues(struct net *net, 
+			      struct nlattr *tb[],
+			      int *num_tx_queues,
+			      int *real_num_tx_queues)
+{
+	struct net_device *real_dev;
+
+	if (!tb[IFLA_LINK])
+		return -EINVAL;
+
+	real_dev = __dev_get_by_index(net, nla_get_u32(tb[IFLA_LINK]));
+	if (!real_dev)
+		return -ENODEV;
+
+	*num_tx_queues      = real_dev->num_tx_queues;
+	*real_num_tx_queues = real_dev->real_num_tx_queues;
+	return 0;
+}
+
 static int vlan_newlink(struct net_device *dev,
 			struct nlattr *tb[], struct nlattr *data[])
 {
@@ -216,6 +235,7 @@ struct rtnl_link_ops vlan_link_ops __read_mostly = {
 	.maxtype	= IFLA_VLAN_MAX,
 	.policy		= vlan_policy,
 	.priv_size	= sizeof(struct vlan_dev_info),
+	.get_tx_queues  = vlan_get_tx_queues,
 	.setup		= vlan_setup,
 	.validate	= vlan_validate,
 	.newlink	= vlan_newlink,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b44775f..5c1fe53 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -974,12 +974,20 @@ struct net_device *rtnl_create_link(struct net *net, char *ifname,
 {
 	int err;
 	struct net_device *dev;
+	int num_queues = 1;
+	int real_num_queues = 1;
 
+	if (ops->get_tx_queues) {
+		err = ops->get_tx_queues(net, tb, &num_queues, &real_num_queues);
+		if (err)
+			goto err;
+	}
 	err = -ENOMEM;
-	dev = alloc_netdev(ops->priv_size, ifname, ops->setup);
+	dev = alloc_netdev_mq(ops->priv_size, ifname, ops->setup, num_queues);
 	if (!dev)
 		goto err;
 
+	dev->real_num_tx_queues = real_num_queues;
 	if (strchr(dev->name, '%')) {
 		err = dev_alloc_name(dev, dev->name);
 		if (err < 0)

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Eric Dumazet]

Patrick McHardy a écrit :
> Stephen Hemminger wrote:
>> Even if the kernel keeps the value in something else for the API
>> QINDEX needs to be a fixed size unsigned type like u32.
> 
> Just to avoid duplicate work - I'm currently trying to put a patch
> together that presents multiqueue qdiscs to userspace as regular
> child qdiscs of a dummy qdisc, which also contains the aggregated
> statistics. So far it actually looks pretty sane :)

There is one thing that bothers me with "tc -s -d qdisc", maybe
its the right time to speak

qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 54823 bytes 490 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 468 bytes 6 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0


The rate estimation is given by kernel even if no rate estimation is performed.

User space doesnt have an indication saying this 0 numbers are real or fake
(rate 0bit 0pps).

Could we set a bit at Qdisc level when a gen_new_estimator() is really
done one a Qdisc, so that we do not call gnet_stats_copy_rate_est()
from tc_fill_qdisc() if this bit is not set ?

->

qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 54823 bytes 490 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 468 bytes 6 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[Patrick McHardy]

Eric Dumazet wrote:
> vlan devices are currently not multi-queue capable.
> 
> We can do that with a new rtnl_link_ops method,
> get_tx_queues(), called from rtnl_create_link()
> 
> This new method gets num_tx_queues/real_num_tx_queues
> from real device.
> 
> register_vlan_device() is also handled.

Looks great. This will also make it easier to test multiqueue
scheduling without a multiqueue-capable device :)

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Patrick McHardy]

Eric Dumazet wrote:
> Patrick McHardy a écrit :
>> Just to avoid duplicate work - I'm currently trying to put a patch
>> together that presents multiqueue qdiscs to userspace as regular
>> child qdiscs of a dummy qdisc, which also contains the aggregated
>> statistics. So far it actually looks pretty sane :)
> 
> There is one thing that bothers me with "tc -s -d qdisc", maybe
> its the right time to speak
> 
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 54823 bytes 490 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev eth1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 468 bytes 6 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> 
> 
> The rate estimation is given by kernel even if no rate estimation is performed.
> 
> User space doesnt have an indication saying this 0 numbers are real or fake
> (rate 0bit 0pps).
> 
> Could we set a bit at Qdisc level when a gen_new_estimator() is really
> done one a Qdisc, so that we do not call gnet_stats_copy_rate_est()
> from tc_fill_qdisc() if this bit is not set ?

That should be possible, I'll look into it once I've finished my patch.

Re: [NET] Add proc file to display the state of all qdiscs.

[Christoph Lameter]

On Wed, 2 Sep 2009, Jarek Poplawski wrote:

> I think, tc should've no problem with displaying summary stats of
> multiqueue qdiscs or even all of them separately, as mentioned by
> Patrick. And, maybe I still miss something, but there should be
> nothing special with tc vs. localhost either.

Ok. Can you come up with a patch? net/sched/sch_api.c can likely be
patches with the loop logic that Eric suggested earlier.

Re: [NET] Add proc file to display the state of all qdiscs.

[Christoph Lameter]

On Wed, 2 Sep 2009, Eric Dumazet wrote:

> Same name "eth0" is displayed, that might confuse parsers...
>
> What naming convention should we choose for multiqueue devices ?

eth0/tx<number> ?

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[Brian Haley]

Eric Dumazet wrote:
> diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
> index 3c1895e..0525a1d 100644
> --- a/include/net/rtnetlink.h
> +++ b/include/net/rtnetlink.h
> @@ -70,6 +70,8 @@ struct rtnl_link_ops {
>  	size_t			(*get_xstats_size)(const struct net_device *dev);
>  	int			(*fill_xstats)(struct sk_buff *skb,
>  					       const struct net_device *dev);
> +	int			(*get_tx_queues)(struct net*, struct nlattr *tb[],
> +						 int *tx_queues, int *real_tx_queues);

	int			(*get_tx_queues)(struct net *net, struct nlattr *tb[],

Total nitpick, but tbird highlighted it because of the missing "net".

-Brian

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[Eric Dumazet]

Brian Haley a écrit :
> Eric Dumazet wrote:
>> diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
>> index 3c1895e..0525a1d 100644
>> --- a/include/net/rtnetlink.h
>> +++ b/include/net/rtnetlink.h
>> @@ -70,6 +70,8 @@ struct rtnl_link_ops {
>>  	size_t			(*get_xstats_size)(const struct net_device *dev);
>>  	int			(*fill_xstats)(struct sk_buff *skb,
>>  					       const struct net_device *dev);
>> +	int			(*get_tx_queues)(struct net*, struct nlattr *tb[],
>> +						 int *tx_queues, int *real_tx_queues);
> 
> 	int			(*get_tx_queues)(struct net *net, struct nlattr *tb[],
> 
> Total nitpick, but tbird highlighted it because of the missing "net".
> 

Thats right, and I believe some gcc versions could complain

Thanks Brian

[PATCH net-next-2.6] vlan: multiqueue vlan device

vlan devices are currently not multi-queue capable.

We can do that with a new rtnl_link_ops method,
get_tx_queues(), called from rtnl_create_link()

This new method gets num_tx_queues/real_num_tx_queues
from real device.

register_vlan_device() is also handled.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/rtnetlink.h  |    2 ++
 net/8021q/vlan.c         |    5 +++--
 net/8021q/vlan_netlink.c |   20 ++++++++++++++++++++
 net/core/rtnetlink.c     |   10 +++++++++-
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 3c1895e..3f4ab54 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -70,6 +70,8 @@ struct rtnl_link_ops {
 	size_t			(*get_xstats_size)(const struct net_device *dev);
 	int			(*fill_xstats)(struct sk_buff *skb,
 					       const struct net_device *dev);
+	int			(*get_tx_queues)(struct net *net, struct nlattr *tb[],
+						 int *tx_queues, int *real_tx_queues);
 };
 
 extern int	__rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index e814794..8836575 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -330,12 +330,13 @@ static int register_vlan_device(struct net_device *real_dev, u16 vlan_id)
 		snprintf(name, IFNAMSIZ, "vlan%.4i", vlan_id);
 	}
 
-	new_dev = alloc_netdev(sizeof(struct vlan_dev_info), name,
-			       vlan_setup);
+	new_dev = alloc_netdev_mq(sizeof(struct vlan_dev_info), name,
+				  vlan_setup, real_dev->num_tx_queues);
 
 	if (new_dev == NULL)
 		return -ENOBUFS;
 
+	new_dev->real_num_tx_queues = real_dev->real_num_tx_queues;
 	dev_net_set(new_dev, net);
 	/* need 4 bytes for extra VLAN header info,
 	 * hope the underlying device can handle it.
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index e9c91dc..8ce4122 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -100,6 +100,25 @@ static int vlan_changelink(struct net_device *dev,
 	return 0;
 }
 
+static int vlan_get_tx_queues(struct net *net, 
+			      struct nlattr *tb[],
+			      int *num_tx_queues,
+			      int *real_num_tx_queues)
+{
+	struct net_device *real_dev;
+
+	if (!tb[IFLA_LINK])
+		return -EINVAL;
+
+	real_dev = __dev_get_by_index(net, nla_get_u32(tb[IFLA_LINK]));
+	if (!real_dev)
+		return -ENODEV;
+
+	*num_tx_queues      = real_dev->num_tx_queues;
+	*real_num_tx_queues = real_dev->real_num_tx_queues;
+	return 0;
+}
+
 static int vlan_newlink(struct net_device *dev,
 			struct nlattr *tb[], struct nlattr *data[])
 {
@@ -216,6 +235,7 @@ struct rtnl_link_ops vlan_link_ops __read_mostly = {
 	.maxtype	= IFLA_VLAN_MAX,
 	.policy		= vlan_policy,
 	.priv_size	= sizeof(struct vlan_dev_info),
+	.get_tx_queues  = vlan_get_tx_queues,
 	.setup		= vlan_setup,
 	.validate	= vlan_validate,
 	.newlink	= vlan_newlink,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b44775f..5c1fe53 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -974,12 +974,20 @@ struct net_device *rtnl_create_link(struct net *net, char *ifname,
 {
 	int err;
 	struct net_device *dev;
+	int num_queues = 1;
+	int real_num_queues = 1;
 
+	if (ops->get_tx_queues) {
+		err = ops->get_tx_queues(net, tb, &num_queues, &real_num_queues);
+		if (err)
+			goto err;
+	}
 	err = -ENOMEM;
-	dev = alloc_netdev(ops->priv_size, ifname, ops->setup);
+	dev = alloc_netdev_mq(ops->priv_size, ifname, ops->setup, num_queues);
 	if (!dev)
 		goto err;
 
+	dev->real_num_tx_queues = real_num_queues;
 	if (strchr(dev->name, '%')) {
 		err = dev_alloc_name(dev, dev->name);
 		if (err < 0)

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[Stephen Hemminger]

On Wed, 02 Sep 2009 20:55:24 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Brian Haley a écrit :
> > Eric Dumazet wrote:
> >> diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
> >> index 3c1895e..0525a1d 100644
> >> --- a/include/net/rtnetlink.h
> >> +++ b/include/net/rtnetlink.h
> >> @@ -70,6 +70,8 @@ struct rtnl_link_ops {
> >>  	size_t			(*get_xstats_size)(const struct net_device *dev);
> >>  	int			(*fill_xstats)(struct sk_buff *skb,
> >>  					       const struct net_device *dev);
> >> +	int			(*get_tx_queues)(struct net*, struct nlattr *tb[],
> >> +						 int *tx_queues, int *real_tx_queues);
> > 
> > 	int			(*get_tx_queues)(struct net *net, struct nlattr *tb[],
> > 
> > Total nitpick, but tbird highlighted it because of the missing "net".
> > 
> 
> Thats right, and I believe some gcc versions could complain
> 
> Thanks Brian
> 
> [PATCH net-next-2.6] vlan: multiqueue vlan device
> 
> vlan devices are currently not multi-queue capable.
> 
> We can do that with a new rtnl_link_ops method,
> get_tx_queues(), called from rtnl_create_link()
> 
> This new method gets num_tx_queues/real_num_tx_queues
> from real device.
> 
> register_vlan_device() is also handled.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  include/net/rtnetlink.h  |    2 ++
>  net/8021q/vlan.c         |    5 +++--
>  net/8021q/vlan_netlink.c |   20 ++++++++++++++++++++
>  net/core/rtnetlink.c     |   10 +++++++++-
>  4 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
> index 3c1895e..3f4ab54 100644
> --- a/include/net/rtnetlink.h
> +++ b/include/net/rtnetlink.h
> @@ -70,6 +70,8 @@ struct rtnl_link_ops {
>  	size_t			(*get_xstats_size)(const struct net_device *dev);
>  	int			(*fill_xstats)(struct sk_buff *skb,
>  					       const struct net_device *dev);
> +	int			(*get_tx_queues)(struct net *net, struct nlattr *tb[],
> +						 int *tx_queues, int *real_tx_queues);
>  };
>  
>  extern int	__rtnl_link_register(struct rtnl_link_ops *ops);
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index e814794..8836575 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -330,12 +330,13 @@ static int register_vlan_device(struct net_device *real_dev, u16 vlan_id)
>  		snprintf(name, IFNAMSIZ, "vlan%.4i", vlan_id);
>  	}
>  
> -	new_dev = alloc_netdev(sizeof(struct vlan_dev_info), name,
> -			       vlan_setup);
> +	new_dev = alloc_netdev_mq(sizeof(struct vlan_dev_info), name,
> +				  vlan_setup, real_dev->num_tx_queues);
>  
>  	if (new_dev == NULL)
>  		return -ENOBUFS;
>  
> +	new_dev->real_num_tx_queues = real_dev->real_num_tx_queues;
>  	dev_net_set(new_dev, net);
>  	/* need 4 bytes for extra VLAN header info,
>  	 * hope the underlying device can handle it.
> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
> index e9c91dc..8ce4122 100644
> --- a/net/8021q/vlan_netlink.c
> +++ b/net/8021q/vlan_netlink.c
> @@ -100,6 +100,25 @@ static int vlan_changelink(struct net_device *dev,
>  	return 0;
>  }
>  
> +static int vlan_get_tx_queues(struct net *net, 
> +			      struct nlattr *tb[],
> +			      int *num_tx_queues,
> +			      int *real_num_tx_queues)


Nitpick again.  use unsigned unless you have some reason
to allow -1.

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[Eric Dumazet]

Stephen Hemminger a écrit :
> On Wed, 02 Sep 2009 20:55:24 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> +static int vlan_get_tx_queues(struct net *net, 
>> +			      struct nlattr *tb[],
>> +			      int *num_tx_queues,
>> +			      int *real_num_tx_queues)
> 
> 
> Nitpick again.  use unsigned unless you have some reason
> to allow -1.

No problem :)

Thanks Stephen

[PATCH net-next-2.6] vlan: multiqueue vlan device

vlan devices are currently not multi-queue capable.

We can do that with a new rtnl_link_ops method,
get_tx_queues(), called from rtnl_create_link()

This new method gets num_tx_queues/real_num_tx_queues
from real device.

register_vlan_device() is also handled.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/rtnetlink.h  |    3 +++
 net/8021q/vlan.c         |    5 +++--
 net/8021q/vlan_netlink.c |   20 ++++++++++++++++++++
 net/core/rtnetlink.c     |   10 +++++++++-
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 3c1895e..b630196 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -70,6 +70,9 @@ struct rtnl_link_ops {
 	size_t			(*get_xstats_size)(const struct net_device *dev);
 	int			(*fill_xstats)(struct sk_buff *skb,
 					       const struct net_device *dev);
+	int			(*get_tx_queues)(struct net *net, struct nlattr *tb[],
+						 unsigned int *tx_queues,
+						 unsigned int *real_tx_queues);
 };
 
 extern int	__rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index e814794..8836575 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -330,12 +330,13 @@ static int register_vlan_device(struct net_device *real_dev, u16 vlan_id)
 		snprintf(name, IFNAMSIZ, "vlan%.4i", vlan_id);
 	}
 
-	new_dev = alloc_netdev(sizeof(struct vlan_dev_info), name,
-			       vlan_setup);
+	new_dev = alloc_netdev_mq(sizeof(struct vlan_dev_info), name,
+				  vlan_setup, real_dev->num_tx_queues);
 
 	if (new_dev == NULL)
 		return -ENOBUFS;
 
+	new_dev->real_num_tx_queues = real_dev->real_num_tx_queues;
 	dev_net_set(new_dev, net);
 	/* need 4 bytes for extra VLAN header info,
 	 * hope the underlying device can handle it.
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index e9c91dc..cd5c2ae 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -100,6 +100,25 @@ static int vlan_changelink(struct net_device *dev,
 	return 0;
 }
 
+static int vlan_get_tx_queues(struct net *net, 
+			      struct nlattr *tb[],
+			      unsigned int *num_tx_queues,
+			      unsigned int *real_num_tx_queues)
+{
+	struct net_device *real_dev;
+
+	if (!tb[IFLA_LINK])
+		return -EINVAL;
+
+	real_dev = __dev_get_by_index(net, nla_get_u32(tb[IFLA_LINK]));
+	if (!real_dev)
+		return -ENODEV;
+
+	*num_tx_queues      = real_dev->num_tx_queues;
+	*real_num_tx_queues = real_dev->real_num_tx_queues;
+	return 0;
+}
+
 static int vlan_newlink(struct net_device *dev,
 			struct nlattr *tb[], struct nlattr *data[])
 {
@@ -216,6 +235,7 @@ struct rtnl_link_ops vlan_link_ops __read_mostly = {
 	.maxtype	= IFLA_VLAN_MAX,
 	.policy		= vlan_policy,
 	.priv_size	= sizeof(struct vlan_dev_info),
+	.get_tx_queues  = vlan_get_tx_queues,
 	.setup		= vlan_setup,
 	.validate	= vlan_validate,
 	.newlink	= vlan_newlink,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b44775f..bbcba2a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -974,12 +974,20 @@ struct net_device *rtnl_create_link(struct net *net, char *ifname,
 {
 	int err;
 	struct net_device *dev;
+	unsigned int num_queues = 1;
+	unsigned int real_num_queues = 1;
 
+	if (ops->get_tx_queues) {
+		err = ops->get_tx_queues(net, tb, &num_queues, &real_num_queues);
+		if (err)
+			goto err;
+	}
 	err = -ENOMEM;
-	dev = alloc_netdev(ops->priv_size, ifname, ops->setup);
+	dev = alloc_netdev_mq(ops->priv_size, ifname, ops->setup, num_queues);
 	if (!dev)
 		goto err;
 
+	dev->real_num_tx_queues = real_num_queues;
 	if (strchr(dev->name, '%')) {
 		err = dev_alloc_name(dev, dev->name);
 		if (err < 0)

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

On Wed, Sep 02, 2009 at 01:12:55PM -0500, Christoph Lameter wrote:
> 
> On Wed, 2 Sep 2009, Jarek Poplawski wrote:
> 
> > I think, tc should've no problem with displaying summary stats of
> > multiqueue qdiscs or even all of them separately, as mentioned by
> > Patrick. And, maybe I still miss something, but there should be
> > nothing special with tc vs. localhost either.
> 
> Ok. Can you come up with a patch? net/sched/sch_api.c can likely be
> patches with the loop logic that Eric suggested earlier.

Yes, it's nice Eric and Patrick found time to fix it most properly.
 
Jarek P.

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

Christoph Lameter wrote, On 09/02/2009 01:52 AM:

> [NET] Add proc file to display the state of all qdiscs
> 
> TC is a complicated tool and it currently does not allow the display of all
> qdisc states. It does not support multiple tx queues and also not
> localhost, nor does it display the current operating state of the queues.
> 
> This functionality could be added to tc / netlink but the tool is already
> complex to handle. The simple proc file here allows easy scanning by
> scripts and other tools. However, tc still needs to be updated to allow
> the modifications of multiqueue TX settings. tc's main focus is the
> configuration of qdiscs. The qdisc_stats file just shows the current
> state.
> 
> This patch adds
> 
> 	/proc/net/qdisc_stats
> 
> which displays the current state of all qdiscs on the system.
> 
> F.e.
> 
> $ cat /proc/net/qdisc_stats
> Queue    Device  State   Bytes  Packets Qlen Blog   Drops Requeue Overlimit
> TX0/root     lo   -          0        0    0    0       0       0       0
>  RX/root     lo   -          0        0    0    0       0       0       0
> TX0/root   eth0   -       5518       60    0    0       0       0       0
> TX1/root   eth0   -       2549       37    0    0       0       0       0
> TX2/root   eth0   -      63625      272    0    0       0       0       0
> TX3/root   eth0   -       1580       21    0    0       0       0       0
> TX4/root   eth0   R   88979440   260183    0 3532   43176    2111       0
> TX5/root   eth0   -       4698       56    0    0       0       0       0
> TX6/root   eth0   - 3598883129 10523140    0    0       0       0       0
> TX7/root   eth0   -       1750       21    0    0       0       0       0
> 
> 
> Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
> 
> ---
>  net/sched/sch_api.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 130 insertions(+)
> 
> Index: linux-2.6/net/sched/sch_api.c
> ===================================================================
> --- linux-2.6.orig/net/sched/sch_api.c	2009-09-01 12:27:24.000000000 -0500
> +++ linux-2.6/net/sched/sch_api.c	2009-09-01 14:39:27.000000000 -0500
...
> +static void qdisc_seq_out(struct seq_file *seq, struct net_device *dev)
> +{
> +	struct netdev_queue *dev_queue;
> +	int i;
> +
> +	for (i = 0; i < dev->real_num_tx_queues; i++) {
> +		char buffer[10];
> +
> +		dev_queue = netdev_get_tx_queue(dev, i);
> +		sprintf(buffer, "TX%d", i);
> +		dump_qdisc_root(seq, dev, dev_queue->qdisc_sleeping, buffer);
> +	}
...

Btw, Patrick's comments reminded me this is probably not what you want
in case of non-default qdiscs: the root qdisc like prio will be
repeated for each tx queue with the same stats. I guess you need to do
here an additional query e.g. by comparing dev_queue->qdisc_sleeping
with that of i = 0.

Jarek P.

Re: [PATCH net-next-2.6] tc: report informations for multiqueue devices

[Jarek Poplawski]

On Wed, Sep 02, 2009 at 04:07:03PM +0200, Patrick McHardy wrote:
> Eric Dumazet wrote:
> > Patrick McHardy a écrit :
> >> Eric Dumazet wrote:
> >>> [PATCH net-next-2.6] tc: report informations for multiqueue devices
> >>>
> >>> qdisc and classes are not yet displayed by "tc -s -d {qdisc|class} show"
> >>> for multiqueue devices.
> >>>
> >>> We use a new TCA_QINDEX attribute, to report queue index to user space.
> >>> iproute2 tc should be changed to eventually display this queue index as in :
> >>>
> >>> $ tc -s -d qdisc
> >>> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> >>>  Sent 52498 bytes 465 pkt (dropped 0, overlimits 0 requeues 0)
> >>>  rate 0bit 0pps backlog 0b 0p requeues 0
> >>> qdisc pfifo_fast 0: dev eth0 qindex 1 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> >>>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> >>>  rate 0bit 0pps backlog 0b 0p requeues 0
> >> This might confuse existing userspace since the handle is not unique
> >> anymore. libnl f.i. will treat all but the first root qdisc as an
> >> update and use it to update the state of the first one. There's also
> >> no combined view for applications unaware of multiqueue.
> >>
> >> Please have a look at the mail I just wrote for some possible ways
> >> around this.
> > 
> > Hum, how can we combine infos on qdisc/class if in the future we allow each queue index
> > to have its own qdisc/classes ?
> > 
> > htb on queue index 0
> > cbq on queue index 1
> 
> My suggestion was to only dump the statistics in the combined
> view and use a virtual qdisc, something like:
> 
> qdisc multiqueue 0: dev eth0 root queues 8
>   Sent ...
>   rate ...
> 
> and show each real qdisc as child of this qdisc:
> 
> qdisc pfifo_fast <unique handle> dev eth0 parent 0: bands 3 ...
> qdisc pfifo_fast <unique handle> dev eth0 parent 0: bands 3 ...
> 
> Configuration would be symetrical to this:
> 
> tc qdisc add dev eth0 handle 0: root multiqueue
> tc qdisc add dev eth0 handle x: parent 0: pfifo_fast
> ...

Actually, I wonder why it can't be a real "virtual" qdisc with
classes, similar to... multiq, doing such mappings inside, according
to the current api:

tc qdisc add dev eth0 handle 1: root multiqueue
tc qdisc add dev eth0 handle x: parent 1:1 pfifo_fast

Jarek P.

PS: but I'd prefer (more) different name, even mq, mtq or something.

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Sep 2009 21:12:03 +0200

> [PATCH net-next-2.6] vlan: multiqueue vlan device
> 
> vlan devices are currently not multi-queue capable.
> 
> We can do that with a new rtnl_link_ops method,
> get_tx_queues(), called from rtnl_create_link()
> 
> This new method gets num_tx_queues/real_num_tx_queues
> from real device.
> 
> register_vlan_device() is also handled.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, but now I need you to do an audit :-)

I believe that drivers will change the number of TX queues
in use at times where we'll need to trigger an event or
something so that vlan's can learn about the value changing.

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[Eric Dumazet]

David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 02 Sep 2009 21:12:03 +0200
> 
>> [PATCH net-next-2.6] vlan: multiqueue vlan device
>>
>> vlan devices are currently not multi-queue capable.
>>
>> We can do that with a new rtnl_link_ops method,
>> get_tx_queues(), called from rtnl_create_link()
>>
>> This new method gets num_tx_queues/real_num_tx_queues
>> from real device.
>>
>> register_vlan_device() is also handled.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Applied, but now I need you to do an audit :-)
> 
> I believe that drivers will change the number of TX queues
> in use at times where we'll need to trigger an event or
> something so that vlan's can learn about the value changing.

Yes, I know :)

This is why I kept both num_tx_queues and real_num_tx_queues values,
and not the later :)

Thanks

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[Eric Dumazet]

David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 02 Sep 2009 21:12:03 +0200
> 
>> [PATCH net-next-2.6] vlan: multiqueue vlan device
>>
>> vlan devices are currently not multi-queue capable.
>>
>> We can do that with a new rtnl_link_ops method,
>> get_tx_queues(), called from rtnl_create_link()
>>
>> This new method gets num_tx_queues/real_num_tx_queues
>> from real device.
>>
>> register_vlan_device() is also handled.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Applied, but now I need you to do an audit :-)
> 
> I believe that drivers will change the number of TX queues
> in use at times where we'll need to trigger an event or
> something so that vlan's can learn about the value changing.

Here is a followup to enable 'true' multiqueue vlan xmit.

I missed this in my first patch.

[PATCH net-next-2.6] vlan: enable multiqueue xmits

vlan_dev_hard_start_xmit() & vlan_dev_hwaccel_hard_start_xmit()
select txqueue number 0, instead of using index provided by
skb_get_queue_mapping().

This is not correct after commit 2e59af3dcbdf11635c03f
[vlan: multiqueue vlan device] because
txq->tx_packets  & txq->tx_bytes changes are performed on
a single location, and not the right locking.

Fix is to take the appropriate struct netdev_queue pointer

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 53f84c7..3938c3e 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -291,7 +291,8 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 					    struct net_device *dev)
 {
-	struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
+	int i = skb_get_queue_mapping(skb);
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
 	struct vlan_ethhdr *veth = (struct vlan_ethhdr *)(skb->data);
 
 	/* Handle non-VLAN frames if they are sent to us, for example by DHCP.
@@ -329,7 +330,8 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 static netdev_tx_t vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
 						    struct net_device *dev)
 {
-	struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
+	int i = skb_get_queue_mapping(skb);
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
 	u16 vlan_tci;
 
 	vlan_tci = vlan_dev_info(dev)->vlan_id;

Re: [PATCH net-next-2.6] vlan: multiqueue vlan devices

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 03 Sep 2009 11:17:13 +0200

> [PATCH net-next-2.6] vlan: enable multiqueue xmits
> 
> vlan_dev_hard_start_xmit() & vlan_dev_hwaccel_hard_start_xmit()
> select txqueue number 0, instead of using index provided by
> skb_get_queue_mapping().
> 
> This is not correct after commit 2e59af3dcbdf11635c03f
> [vlan: multiqueue vlan device] because
> txq->tx_packets  & txq->tx_bytes changes are performed on
> a single location, and not the right locking.
> 
> Fix is to take the appropriate struct netdev_queue pointer
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Details details :-)

Looks good, applied!

Re: [NET] Add proc file to display the state of all qdiscs.

[Jesper Dangaard Brouer]

On Wed, 2 Sep 2009, Christoph Lameter wrote:
> On Wed, 2 Sep 2009, Eric Dumazet wrote:
>
>> Same name "eth0" is displayed, that might confuse parsers...
>>
>> What naming convention should we choose for multiqueue devices ?
>
> eth0/tx<number> ?

Remember that we already have a naming convention in /proc/interrupts

  eth0-tx-<number>

Lets not introduce too many new once ;-)

Cheers,
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

Re: [NET] Add proc file to display the state of all qdiscs.

[Patrick McHardy]

Jesper Dangaard Brouer wrote:
> 
> On Wed, 2 Sep 2009, Christoph Lameter wrote:
>> On Wed, 2 Sep 2009, Eric Dumazet wrote:
>>
>>> Same name "eth0" is displayed, that might confuse parsers...
>>>
>>> What naming convention should we choose for multiqueue devices ?
>>
>> eth0/tx<number> ?
> 
> Remember that we already have a naming convention in /proc/interrupts
> 
>  eth0-tx-<number>
> 
> Lets not introduce too many new once ;-)

The approach I'm currently working on will present multiqueue root
qdiscs as children of a dummy classful qdisc. This avoids handle
clashes and the need for new identifiers and allows to address each
qdisc seperately, similar to how it works with other classful qdiscs:

qdisc mq 1: root refcnt 2
 Sent 126 bytes 3 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0

class mq 1:1 root leaf 8001:
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq 1:2 root leaf 8002:
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq 1:3 root leaf 8003:
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq 1:4 root leaf 8004:
 Sent 126 bytes 3 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

Its a bit tricky though so I'm likely going to need a few more hours.

Re: [NET] Add proc file to display the state of all qdiscs.

[Eric Dumazet]

Patrick McHardy a écrit :
> Jesper Dangaard Brouer wrote:
>> On Wed, 2 Sep 2009, Christoph Lameter wrote:
>>> On Wed, 2 Sep 2009, Eric Dumazet wrote:
>>>
>>>> Same name "eth0" is displayed, that might confuse parsers...
>>>>
>>>> What naming convention should we choose for multiqueue devices ?
>>> eth0/tx<number> ?
>> Remember that we already have a naming convention in /proc/interrupts
>>
>>  eth0-tx-<number>
>>
>> Lets not introduce too many new once ;-)
> 
> The approach I'm currently working on will present multiqueue root
> qdiscs as children of a dummy classful qdisc. This avoids handle
> clashes and the need for new identifiers and allows to address each
> qdisc seperately, similar to how it works with other classful qdiscs:
> 
> qdisc mq 1: root refcnt 2
>  Sent 126 bytes 3 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> 
> class mq 1:1 root leaf 8001:
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> class mq 1:2 root leaf 8002:
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> class mq 1:3 root leaf 8003:
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> class mq 1:4 root leaf 8004:
>  Sent 126 bytes 3 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> 
> Its a bit tricky though so I'm likely going to need a few more hours.

Seems pretty cool, thanks Patrick.

Re: [NET] Add proc file to display the state of all qdiscs.

[Jesper Dangaard Brouer]

On Thu, 3 Sep 2009, Patrick McHardy wrote:

> Jesper Dangaard Brouer wrote:
>>
>> On Wed, 2 Sep 2009, Christoph Lameter wrote:
>>> On Wed, 2 Sep 2009, Eric Dumazet wrote:
>>>
>>>> Same name "eth0" is displayed, that might confuse parsers...
>>>>
>>>> What naming convention should we choose for multiqueue devices ?
>>>
>>> eth0/tx<number> ?
>>
>> Remember that we already have a naming convention in /proc/interrupts
>>
>>  eth0-tx-<number>
>>
>> Lets not introduce too many new once ;-)
>
> The approach I'm currently working on will present multiqueue root
> qdiscs as children of a dummy classful qdisc. This avoids handle
> clashes and the need for new identifiers and allows to address each
> qdisc seperately, similar to how it works with other classful qdiscs:

I like your approach. Its well suited for the qdiscs :-)

I especially like the possibility to access each qdisc seperately.  Does 
it then support having seperate qdisc per TX queue?  (I'm toying with the 
idea of transmitting our multicast traffic into/via a seperate TX hardware 
queue, and making a special qdisc for IPTV MPEG2-TS shaping)

Cheers,
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

Re: [NET] Add proc file to display the state of all qdiscs.

[Patrick McHardy]

Jesper Dangaard Brouer wrote:
> 
> On Thu, 3 Sep 2009, Patrick McHardy wrote:
> 
>> The approach I'm currently working on will present multiqueue root
>> qdiscs as children of a dummy classful qdisc. This avoids handle
>> clashes and the need for new identifiers and allows to address each
>> qdisc seperately, similar to how it works with other classful qdiscs:
> 
> I like your approach. Its well suited for the qdiscs :-)
> 
> I especially like the possibility to access each qdisc seperately.  Does
> it then support having seperate qdisc per TX queue?  (I'm toying with
> the idea of transmitting our multicast traffic into/via a seperate TX
> hardware queue, and making a special qdisc for IPTV MPEG2-TS shaping)

Yes, you can attach qdiscs to the classes representing the queues.
At least it should work :) It would probably also be possible to
use TC classifiers for queue selection.

Re: [NET] Add proc file to display the state of all qdiscs.

[Christoph Lameter]

On Wed, 2 Sep 2009, Jarek Poplawski wrote:

> Btw, Patrick's comments reminded me this is probably not what you want
> in case of non-default qdiscs: the root qdisc like prio will be
> repeated for each tx queue with the same stats. I guess you need to do
> here an additional query e.g. by comparing dev_queue->qdisc_sleeping
> with that of i = 0.

Hmmm.. Maybe I better leave it to the experts then.

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

On Thu, Sep 03, 2009 at 01:03:37PM -0500, Christoph Lameter wrote:
> On Wed, 2 Sep 2009, Jarek Poplawski wrote:
> 
> > Btw, Patrick's comments reminded me this is probably not what you want
> > in case of non-default qdiscs: the root qdisc like prio will be
> > repeated for each tx queue with the same stats. I guess you need to do
> > here an additional query e.g. by comparing dev_queue->qdisc_sleeping
> > with that of i = 0.
> 
> Hmmm.. Maybe I better leave it to the experts then.
> 

This is certainly not what I wanted... The experts missed this problem
long enough. It might only need to wait a bit for Patrick's changes.

Jarek P.

Re: [NET] Add proc file to display the state of all qdiscs.

[Eric Dumazet]

Jarek Poplawski a écrit :
> On Thu, Sep 03, 2009 at 01:03:37PM -0500, Christoph Lameter wrote:
>> On Wed, 2 Sep 2009, Jarek Poplawski wrote:
>>
>>> Btw, Patrick's comments reminded me this is probably not what you want
>>> in case of non-default qdiscs: the root qdisc like prio will be
>>> repeated for each tx queue with the same stats. I guess you need to do
>>> here an additional query e.g. by comparing dev_queue->qdisc_sleeping
>>> with that of i = 0.
>> Hmmm.. Maybe I better leave it to the experts then.
>>
> 
> This is certainly not what I wanted... The experts missed this problem
> long enough. It might only need to wait a bit for Patrick's changes.

The experts ? Anyway, you can say "the expert", there is only one :)

Re: [NET] Add proc file to display the state of all qdiscs.

[Jarek Poplawski]

On Thu, Sep 03, 2009 at 09:38:54PM +0200, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > On Thu, Sep 03, 2009 at 01:03:37PM -0500, Christoph Lameter wrote:
> >> On Wed, 2 Sep 2009, Jarek Poplawski wrote:
> >>
> >>> Btw, Patrick's comments reminded me this is probably not what you want
> >>> in case of non-default qdiscs: the root qdisc like prio will be
> >>> repeated for each tx queue with the same stats. I guess you need to do
> >>> here an additional query e.g. by comparing dev_queue->qdisc_sleeping
> >>> with that of i = 0.
> >> Hmmm.. Maybe I better leave it to the experts then.
> >>
> > 
> > This is certainly not what I wanted... The experts missed this problem
> > long enough. It might only need to wait a bit for Patrick's changes.
> 
> The experts ? Anyway, you can say "the expert", there is only one :)

Right! I admire Davem too... ;-)

Jarek P.

Re: [NET] Add proc file to display the state of all qdiscs.

[David Miller]

From: Jesper Dangaard Brouer <hawk@diku.dk>
Date: Thu, 3 Sep 2009 16:19:44 +0200 (CEST)

> 
> On Wed, 2 Sep 2009, Christoph Lameter wrote:
>> On Wed, 2 Sep 2009, Eric Dumazet wrote:
>>
>>> Same name "eth0" is displayed, that might confuse parsers...
>>>
>>> What naming convention should we choose for multiqueue devices ?
>>
>> eth0/tx<number> ?
> 
> Remember that we already have a naming convention in /proc/interrupts
> 
>  eth0-tx-<number>
> 
> Lets not introduce too many new once ;-)

Yes, please :-)

Re: [NET] Add proc file to display the state of all qdiscs.

[David Miller]

From: Jesper Dangaard Brouer <hawk@diku.dk>
Date: Thu, 3 Sep 2009 19:30:00 +0200 (CEST)

> I especially like the possibility to access each qdisc seperately.
> Does it then support having seperate qdisc per TX queue?  (I'm toying
> with the idea of transmitting our multicast traffic into/via a
> seperate TX hardware queue, and making a special qdisc for IPTV
> MPEG2-TS shaping)

Indeed it sounds interesting.

But I want to warn everyone against adding support for anything
that adds a way to do operations "on every device qdisc" at the
kernel level.

I tried to do that and the error unwinding complexity is unacceptable.

We can make the tools support things like this.  Keep the kernel
simple and allow it to only operate on one queue at a time for
a given individual config request.

Re: [NET] Add proc file to display the state of all qdiscs.

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 03 Sep 2009 19:56:33 +0200

> It would probably also be possible to use TC classifiers for queue
> selection.

You mean just like Intel's multiqueue scheduler and multiqueue
classifiers which we already have in the tree :-)

Re: [NET] Add proc file to display the state of all qdiscs.

[Patrick McHardy]

David Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Thu, 03 Sep 2009 19:56:33 +0200
>
>   
>> It would probably also be possible to use TC classifiers for queue
>> selection.
>>     
>
> You mean just like Intel's multiqueue scheduler and multiqueue
> classifiers which we already have in the tree :-

No, actually integrated in the current infrastructure :) The patches
add a virtual root qdisc, so we still have a single root from userspace's
POV. This qdisc is only used for sch_api purposes and doesn't necessarily
match the real root qdiscs. When a "mq" qdisc is attached (done by
default for multiqueue devices), the queues are exposed as child
classes of this qdisc and can be individually grafted with different
qdiscs.

Since the virtual root is global per device, what we could do is allow
to attach classifiers and execute them in dev_pick_tx() to select the
queue. But currently this would require global locking, so this would
have to be fixed previously. Probably not an easy thing to do.

Re: [NET] Add proc file to display the state of all qdiscs.

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

Patrick McHardy wrote:
> David Miller wrote:
>   
>> You mean just like Intel's multiqueue scheduler and multiqueue
>> classifiers which we already have in the tree :-
>>     
>
> No, actually integrated in the current infrastructure :) The patches
> add a virtual root qdisc, so we still have a single root from userspace's
> POV. This qdisc is only used for sch_api purposes and doesn't necessarily
> match the real root qdiscs. When a "mq" qdisc is attached (done by
> default for multiqueue devices), the queues are exposed as child
> classes of this qdisc and can be individually grafted with different
> qdiscs.
>   

Since its getting late and I won't finish this tonight, here are the
patches I'm currently working on in case someone already wants to have
a look. Its working as intended and should be testable. Patch 1 is good
shape IMO, patch 3 still needs some work. Userspace needs no changes.



[-- Attachment #2: 01.diff --]
[-- Type: text/x-diff, Size: 9550 bytes --]

commit 6ab8d4a8e3df7976fc2db1e1181216a213df89d5
Author: Patrick McHardy <kaber@trash.net>
Date:   Fri Sep 4 03:07:39 2009 +0200

    net_sched: reintroduce dev->qdisc for use by sch_api
    
    Currently the multiqueue integration with the qdisc API suffers from
    a few problems:
    
    - with multiple queues, all root qdiscs use the same handle. This means
      they can't be exposed to userspace in a backwards compatible fashion.
    
    - all API operations always refer to queue number 0. Newly created
      qdiscs are automatically shared between all queues, its not possible
      to address individual queues or restore multiqueue behaviour once a
      shared qdisc has been attached.
    
    - Dumps only contain the root qdisc of queue 0, in case of non-shared
      qdiscs this means the statistics are incomplete.
    
    This patch reintroduces dev->qdisc, which points to the (single) root qdisc
    from userspace's point of view. Currently it either points to the first
    (non-shared) default qdisc, or a qdisc shared between all queues. The
    following patches will introduce a classful dummy qdisc, which will be used
    as root qdisc and contain the per-queue qdiscs as children.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 121cbad..a44118b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -832,6 +832,9 @@ struct net_device
 	/* Number of TX queues currently active in device  */
 	unsigned int		real_num_tx_queues;
 
+	/* root qdisc from userspace point of view */
+	struct Qdisc		*qdisc;
+
 	unsigned long		tx_queue_len;	/* Max frames per queue allowed */
 	spinlock_t		tx_global_lock;
 /*
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5c1fe53..1aebdc1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -606,7 +606,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
 			    unsigned int flags)
 {
-	struct netdev_queue *txq;
 	struct ifinfomsg *ifm;
 	struct nlmsghdr *nlh;
 	const struct net_device_stats *stats;
@@ -637,9 +636,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	if (dev->master)
 		NLA_PUT_U32(skb, IFLA_MASTER, dev->master->ifindex);
 
-	txq = netdev_get_tx_queue(dev, 0);
-	if (txq->qdisc_sleeping)
-		NLA_PUT_STRING(skb, IFLA_QDISC, txq->qdisc_sleeping->ops->id);
+	if (dev->qdisc)
+		NLA_PUT_STRING(skb, IFLA_QDISC, dev->qdisc->ops->id);
 
 	if (dev->ifalias)
 		NLA_PUT_STRING(skb, IFLA_IFALIAS, dev->ifalias);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 09cdcdf..e857441 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -168,8 +168,7 @@ replay:
 
 	/* Find qdisc */
 	if (!parent) {
-		struct netdev_queue *dev_queue = netdev_get_tx_queue(dev, 0);
-		q = dev_queue->qdisc_sleeping;
+		q = dev->qdisc;
 		parent = q->handle;
 	} else {
 		q = qdisc_lookup(dev, TC_H_MAJ(t->tcm_parent));
@@ -405,7 +404,6 @@ static int tcf_node_dump(struct tcf_proto *tp, unsigned long n,
 static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct net *net = sock_net(skb->sk);
-	struct netdev_queue *dev_queue;
 	int t;
 	int s_t;
 	struct net_device *dev;
@@ -424,9 +422,8 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 	if ((dev = dev_get_by_index(&init_net, tcm->tcm_ifindex)) == NULL)
 		return skb->len;
 
-	dev_queue = netdev_get_tx_queue(dev, 0);
 	if (!tcm->tcm_parent)
-		q = dev_queue->qdisc_sleeping;
+		q = dev->qdisc;
 	else
 		q = qdisc_lookup(dev, TC_H_MAJ(tcm->tcm_parent));
 	if (!q)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 24d17ce..285ca25 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -207,7 +207,7 @@ static struct Qdisc *qdisc_match_from_root(struct Qdisc *root, u32 handle)
 static void qdisc_list_add(struct Qdisc *q)
 {
 	if ((q->parent != TC_H_ROOT) && !(q->flags & TCQ_F_INGRESS))
-		list_add_tail(&q->list, &qdisc_root_sleeping(q)->list);
+		list_add_tail(&q->list, &qdisc_dev(q)->qdisc->list);
 }
 
 void qdisc_list_del(struct Qdisc *q)
@@ -219,17 +219,11 @@ EXPORT_SYMBOL(qdisc_list_del);
 
 struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle)
 {
-	unsigned int i;
 	struct Qdisc *q;
 
-	for (i = 0; i < dev->num_tx_queues; i++) {
-		struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
-		struct Qdisc *txq_root = txq->qdisc_sleeping;
-
-		q = qdisc_match_from_root(txq_root, handle);
-		if (q)
-			goto out;
-	}
+	q = qdisc_match_from_root(dev->qdisc, handle);
+	if (q)
+		goto out;
 
 	q = qdisc_match_from_root(dev->rx_queue.qdisc_sleeping, handle);
 out:
@@ -720,9 +714,14 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 			if (new && i > 0)
 				atomic_inc(&new->refcnt);
 
-			notify_and_destroy(skb, n, classid, old, new);
+			qdisc_destroy(old);
 		}
 
+		notify_and_destroy(skb, n, classid, dev->qdisc, new);
+		if (new)
+			atomic_inc(&new->refcnt);
+		dev->qdisc = new ? : &noop_qdisc;
+
 		if (dev->flags & IFF_UP)
 			dev_activate(dev);
 	} else {
@@ -974,9 +973,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 				q = dev->rx_queue.qdisc_sleeping;
 			}
 		} else {
-			struct netdev_queue *dev_queue;
-			dev_queue = netdev_get_tx_queue(dev, 0);
-			q = dev_queue->qdisc_sleeping;
+			q = dev->qdisc;
 		}
 		if (!q)
 			return -ENOENT;
@@ -1044,9 +1041,7 @@ replay:
 				q = dev->rx_queue.qdisc_sleeping;
 			}
 		} else {
-			struct netdev_queue *dev_queue;
-			dev_queue = netdev_get_tx_queue(dev, 0);
-			q = dev_queue->qdisc_sleeping;
+			q = dev->qdisc;
 		}
 
 		/* It may be default qdisc, ignore it */
@@ -1291,8 +1286,7 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 			s_q_idx = 0;
 		q_idx = 0;
 
-		dev_queue = netdev_get_tx_queue(dev, 0);
-		if (tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb, &q_idx, s_q_idx) < 0)
+		if (tc_dump_qdisc_root(dev->qdisc, skb, cb, &q_idx, s_q_idx) < 0)
 			goto done;
 
 		dev_queue = &dev->rx_queue;
@@ -1323,7 +1317,6 @@ done:
 static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 {
 	struct net *net = sock_net(skb->sk);
-	struct netdev_queue *dev_queue;
 	struct tcmsg *tcm = NLMSG_DATA(n);
 	struct nlattr *tca[TCA_MAX + 1];
 	struct net_device *dev;
@@ -1361,7 +1354,6 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 
 	/* Step 1. Determine qdisc handle X:0 */
 
-	dev_queue = netdev_get_tx_queue(dev, 0);
 	if (pid != TC_H_ROOT) {
 		u32 qid1 = TC_H_MAJ(pid);
 
@@ -1372,7 +1364,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 		} else if (qid1) {
 			qid = qid1;
 		} else if (qid == 0)
-			qid = dev_queue->qdisc_sleeping->handle;
+			qid = dev->qdisc->handle;
 
 		/* Now qid is genuine qdisc handle consistent
 		   both with parent and child.
@@ -1383,7 +1375,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 			pid = TC_H_MAKE(qid, pid);
 	} else {
 		if (qid == 0)
-			qid = dev_queue->qdisc_sleeping->handle;
+			qid = dev->qdisc->handle;
 	}
 
 	/* OK. Locate qdisc */
@@ -1584,8 +1576,7 @@ static int tc_dump_tclass(struct sk_buff *skb, struct netlink_callback *cb)
 	s_t = cb->args[0];
 	t = 0;
 
-	dev_queue = netdev_get_tx_queue(dev, 0);
-	if (tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb, &t, s_t) < 0)
+	if (tc_dump_tclass_root(dev->qdisc, skb, tcm, cb, &t, s_t) < 0)
 		goto done;
 
 	dev_queue = &dev->rx_queue;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 6128e6f..a91f079 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -623,19 +623,6 @@ void qdisc_destroy(struct Qdisc *qdisc)
 }
 EXPORT_SYMBOL(qdisc_destroy);
 
-static bool dev_all_qdisc_sleeping_noop(struct net_device *dev)
-{
-	unsigned int i;
-
-	for (i = 0; i < dev->num_tx_queues; i++) {
-		struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
-
-		if (txq->qdisc_sleeping != &noop_qdisc)
-			return false;
-	}
-	return true;
-}
-
 static void attach_one_default_qdisc(struct net_device *dev,
 				     struct netdev_queue *dev_queue,
 				     void *_unused)
@@ -677,6 +664,7 @@ static void transition_one_qdisc(struct net_device *dev,
 
 void dev_activate(struct net_device *dev)
 {
+	struct netdev_queue *txq;
 	int need_watchdog;
 
 	/* No queueing discipline is attached to device;
@@ -685,9 +673,14 @@ void dev_activate(struct net_device *dev)
 	   virtual interfaces
 	 */
 
-	if (dev_all_qdisc_sleeping_noop(dev))
+	if (dev->qdisc == &noop_qdisc) {
 		netdev_for_each_tx_queue(dev, attach_one_default_qdisc, NULL);
 
+		txq = netdev_get_tx_queue(dev, 0);
+		dev->qdisc = txq->qdisc_sleeping;
+		atomic_inc(&dev->qdisc->refcnt);
+	}
+
 	if (!netif_carrier_ok(dev))
 		/* Delay activation until next carrier-on event */
 		return;
@@ -777,6 +770,7 @@ static void dev_init_scheduler_queue(struct net_device *dev,
 
 void dev_init_scheduler(struct net_device *dev)
 {
+	dev->qdisc = &noop_qdisc;
 	netdev_for_each_tx_queue(dev, dev_init_scheduler_queue, &noop_qdisc);
 	dev_init_scheduler_queue(dev, &dev->rx_queue, &noop_qdisc);
 
@@ -802,5 +796,8 @@ void dev_shutdown(struct net_device *dev)
 {
 	netdev_for_each_tx_queue(dev, shutdown_scheduler_queue, &noop_qdisc);
 	shutdown_scheduler_queue(dev, &dev->rx_queue, &noop_qdisc);
+	qdisc_destroy(dev->qdisc);
+	dev->qdisc = &noop_qdisc;
+
 	WARN_ON(timer_pending(&dev->watchdog_timer));
 }

[-- Attachment #3: 02.diff --]
[-- Type: text/x-diff, Size: 2943 bytes --]

commit 9b1d1d62936b6ec6e51956d77b83743bea67e1ac
Author: Patrick McHardy <kaber@trash.net>
Date:   Fri Sep 4 03:13:54 2009 +0200

    net_sched: move dev_graft_qdisc() to sch_generic.c
    
    It will be used in a following patch by the multiqueue qdisc.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 84b3fc2..a1a3f3f 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -302,6 +302,8 @@ extern void dev_init_scheduler(struct net_device *dev);
 extern void dev_shutdown(struct net_device *dev);
 extern void dev_activate(struct net_device *dev);
 extern void dev_deactivate(struct net_device *dev);
+extern struct Qdisc *dev_graft_qdisc(struct netdev_queue *dev_queue,
+				     struct Qdisc *qdisc);
 extern void qdisc_reset(struct Qdisc *qdisc);
 extern void qdisc_destroy(struct Qdisc *qdisc);
 extern void qdisc_tree_decrease_qlen(struct Qdisc *qdisc, unsigned int n);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 285ca25..c0d2704 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -610,32 +610,6 @@ static u32 qdisc_alloc_handle(struct net_device *dev)
 	return i>0 ? autohandle : 0;
 }
 
-/* Attach toplevel qdisc to device queue. */
-
-static struct Qdisc *dev_graft_qdisc(struct netdev_queue *dev_queue,
-				     struct Qdisc *qdisc)
-{
-	struct Qdisc *oqdisc = dev_queue->qdisc_sleeping;
-	spinlock_t *root_lock;
-
-	root_lock = qdisc_lock(oqdisc);
-	spin_lock_bh(root_lock);
-
-	/* Prune old scheduler */
-	if (oqdisc && atomic_read(&oqdisc->refcnt) <= 1)
-		qdisc_reset(oqdisc);
-
-	/* ... and graft new one */
-	if (qdisc == NULL)
-		qdisc = &noop_qdisc;
-	dev_queue->qdisc_sleeping = qdisc;
-	rcu_assign_pointer(dev_queue->qdisc, &noop_qdisc);
-
-	spin_unlock_bh(root_lock);
-
-	return oqdisc;
-}
-
 void qdisc_tree_decrease_qlen(struct Qdisc *sch, unsigned int n)
 {
 	const struct Qdisc_class_ops *cops;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index a91f079..e7c47ce 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -623,6 +623,31 @@ void qdisc_destroy(struct Qdisc *qdisc)
 }
 EXPORT_SYMBOL(qdisc_destroy);
 
+/* Attach toplevel qdisc to device queue. */
+struct Qdisc *dev_graft_qdisc(struct netdev_queue *dev_queue,
+			      struct Qdisc *qdisc)
+{
+	struct Qdisc *oqdisc = dev_queue->qdisc_sleeping;
+	spinlock_t *root_lock;
+
+	root_lock = qdisc_lock(oqdisc);
+	spin_lock_bh(root_lock);
+
+	/* Prune old scheduler */
+	if (oqdisc && atomic_read(&oqdisc->refcnt) <= 1)
+		qdisc_reset(oqdisc);
+
+	/* ... and graft new one */
+	if (qdisc == NULL)
+		qdisc = &noop_qdisc;
+	dev_queue->qdisc_sleeping = qdisc;
+	rcu_assign_pointer(dev_queue->qdisc, &noop_qdisc);
+
+	spin_unlock_bh(root_lock);
+
+	return oqdisc;
+}
+
 static void attach_one_default_qdisc(struct net_device *dev,
 				     struct netdev_queue *dev_queue,
 				     void *_unused)

[-- Attachment #4: 03.diff --]
[-- Type: text/x-diff, Size: 10612 bytes --]

commit 7f8e95c412644cc23173c9c5673df794fb9b65f7
Author: Patrick McHardy <kaber@trash.net>
Date:   Fri Sep 4 03:24:28 2009 +0200

    net_sched: add classful multiqueue dummy scheduler
    
    This patch adds a classful dummy scheduler which can be used as root qdisc
    for multiqueue devices and exposes each device queue as a child class.
    
    This allows to address queues individually and graft them similar to regular
    classes. Additionally it presents an accumulated view of the statistics of
    all real root qdiscs in the dummy root.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index a1a3f3f..b76e4fa 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -122,6 +122,7 @@ struct Qdisc_ops
 	void			(*reset)(struct Qdisc *);
 	void			(*destroy)(struct Qdisc *);
 	int			(*change)(struct Qdisc *, struct nlattr *arg);
+	void			(*attach)(struct Qdisc *);
 
 	int			(*dump)(struct Qdisc *, struct sk_buff *);
 	int			(*dump_stats)(struct Qdisc *, struct gnet_dump *);
@@ -255,6 +256,8 @@ static inline void sch_tree_unlock(struct Qdisc *q)
 
 extern struct Qdisc noop_qdisc;
 extern struct Qdisc_ops noop_qdisc_ops;
+extern struct Qdisc_ops pfifo_fast_ops;
+extern struct Qdisc_ops mq_qdisc_ops;
 
 struct Qdisc_class_common
 {
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 54d950c..f14e71b 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the Linux Traffic Control Unit.
 #
 
-obj-y	:= sch_generic.o
+obj-y	:= sch_generic.o sch_mq.o
 
 obj-$(CONFIG_NET_SCHED)		+= sch_api.o sch_blackhole.o
 obj-$(CONFIG_NET_CLS)		+= cls_api.o
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index c0d2704..dc41cb1 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -678,6 +678,11 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 		if (dev->flags & IFF_UP)
 			dev_deactivate(dev);
 
+		if (new && new->ops->attach) {
+			new->ops->attach(new);
+			num_q = 0;
+		}
+
 		for (i = 0; i < num_q; i++) {
 			struct netdev_queue *dev_queue = &dev->rx_queue;
 
@@ -692,7 +697,7 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 		}
 
 		notify_and_destroy(skb, n, classid, dev->qdisc, new);
-		if (new)
+		if (new && !new->ops->attach)
 			atomic_inc(&new->refcnt);
 		dev->qdisc = new ? : &noop_qdisc;
 
@@ -1670,6 +1675,7 @@ static int __init pktsched_init(void)
 {
 	register_qdisc(&pfifo_qdisc_ops);
 	register_qdisc(&bfifo_qdisc_ops);
+	register_qdisc(&mq_qdisc_ops);
 	proc_net_fops_create(&init_net, "psched", 0, &psched_fops);
 
 	rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL);
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index e7c47ce..4ae6aa5 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -514,7 +514,7 @@ static int pfifo_fast_init(struct Qdisc *qdisc, struct nlattr *opt)
 	return 0;
 }
 
-static struct Qdisc_ops pfifo_fast_ops __read_mostly = {
+struct Qdisc_ops pfifo_fast_ops __read_mostly = {
 	.id		=	"pfifo_fast",
 	.priv_size	=	sizeof(struct pfifo_fast_priv),
 	.enqueue	=	pfifo_fast_enqueue,
@@ -670,6 +670,26 @@ static void attach_one_default_qdisc(struct net_device *dev,
 	dev_queue->qdisc_sleeping = qdisc;
 }
 
+static void attach_default_qdiscs(struct net_device *dev)
+{
+	struct netdev_queue *txq;
+	struct Qdisc *qdisc;
+
+	txq = netdev_get_tx_queue(dev, 0);
+
+	if (!netif_is_multiqueue(dev) || dev->tx_queue_len == 0) {
+		netdev_for_each_tx_queue(dev, attach_one_default_qdisc, NULL);
+		dev->qdisc = txq->qdisc_sleeping;
+		atomic_inc(&dev->qdisc->refcnt);
+	} else {
+		qdisc = qdisc_create_dflt(dev, txq, &mq_qdisc_ops, TC_H_ROOT);
+		if (qdisc) {
+			qdisc->ops->attach(qdisc);
+			dev->qdisc = qdisc;
+		}
+	}
+}
+
 static void transition_one_qdisc(struct net_device *dev,
 				 struct netdev_queue *dev_queue,
 				 void *_need_watchdog)
@@ -689,7 +709,6 @@ static void transition_one_qdisc(struct net_device *dev,
 
 void dev_activate(struct net_device *dev)
 {
-	struct netdev_queue *txq;
 	int need_watchdog;
 
 	/* No queueing discipline is attached to device;
@@ -698,13 +717,8 @@ void dev_activate(struct net_device *dev)
 	   virtual interfaces
 	 */
 
-	if (dev->qdisc == &noop_qdisc) {
-		netdev_for_each_tx_queue(dev, attach_one_default_qdisc, NULL);
-
-		txq = netdev_get_tx_queue(dev, 0);
-		dev->qdisc = txq->qdisc_sleeping;
-		atomic_inc(&dev->qdisc->refcnt);
-	}
+	if (dev->qdisc == &noop_qdisc)
+		attach_default_qdiscs(dev);
 
 	if (!netif_carrier_ok(dev))
 		/* Delay activation until next carrier-on event */
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
new file mode 100644
index 0000000..fadaa6d
--- /dev/null
+++ b/net/sched/sch_mq.c
@@ -0,0 +1,244 @@
+/*
+ * net/sched/sch_mq.c		Classful multiqueue dummy scheduler
+ *
+ * Copyright (c) 2009 Patrick McHardy <kaber@trash.net>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+
+struct mq_sched {
+	struct Qdisc		**qdiscs;
+};
+
+static int mq_init(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mq_sched *priv = qdisc_priv(sch);
+	struct netdev_queue *dev_queue;
+	struct Qdisc *qdisc;
+	unsigned int ntx;
+
+	if (sch->parent != TC_H_ROOT)
+		return -EOPNOTSUPP;
+
+	if (!netif_is_multiqueue(dev))
+		return -EOPNOTSUPP;
+
+	/* pre-allocate qdiscs, attachment can't fail */
+	priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
+			       GFP_KERNEL);
+	if (priv->qdiscs == NULL)
+		return -ENOMEM;
+
+	for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
+		dev_queue = netdev_get_tx_queue(dev, ntx);
+		qdisc = qdisc_create_dflt(dev, dev_queue, &pfifo_fast_ops,
+					  TC_H_MAKE(TC_H_MAJ(sch->handle),
+						    TC_H_MIN(ntx + 1)));
+		if (qdisc == NULL)
+			goto err;
+		qdisc->flags |= TCQ_F_CAN_BYPASS;
+		priv->qdiscs[ntx] = qdisc;
+	}
+
+	return 0;
+
+err:
+	while (ntx > 0)
+		qdisc_destroy(priv->qdiscs[--ntx]);
+	kfree(priv->qdiscs);
+	return -ENOMEM;
+}
+
+static void mq_attach(struct Qdisc *sch)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mq_sched *priv = qdisc_priv(sch);
+	struct Qdisc *qdisc;
+	unsigned int ntx;
+
+	for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
+		qdisc = priv->qdiscs[ntx];
+		qdisc = dev_graft_qdisc(qdisc->dev_queue, qdisc);
+		if (qdisc)
+			qdisc_destroy(qdisc);
+	}
+	kfree(priv->qdiscs);
+}
+
+static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct Qdisc *qdisc;
+	unsigned int ntx;
+
+	sch->q.qlen = 0;
+	memset(&sch->bstats, 0, sizeof(sch->bstats));
+	memset(&sch->qstats, 0, sizeof(sch->qstats));
+
+	for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
+		qdisc = netdev_get_tx_queue(dev, ntx)->qdisc;
+		sch->q.qlen		+= qdisc->q.qlen;
+		sch->bstats.bytes	+= qdisc->bstats.bytes;
+		sch->bstats.packets	+= qdisc->bstats.packets;
+		sch->qstats.qlen	+= qdisc->qstats.qlen;
+		sch->qstats.backlog	+= qdisc->qstats.backlog;
+		sch->qstats.drops	+= qdisc->qstats.drops;
+		sch->qstats.requeues	+= qdisc->qstats.requeues;
+		sch->qstats.overlimits	+= qdisc->qstats.overlimits;
+	}
+	return 0;
+}
+
+static struct netdev_queue *mq_queue_get(struct Qdisc *sch, unsigned long arg)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	unsigned long ntx = arg - 1;
+
+	if (ntx >= dev->num_tx_queues)
+		return NULL;
+	return netdev_get_tx_queue(dev, ntx);
+}
+
+static int mq_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
+		    struct Qdisc **old)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct netdev_queue *dev_queue;
+
+	dev_queue = mq_queue_get(sch, arg);
+	if (dev_queue == NULL)
+		return -ENOENT;
+
+	if (dev->flags & IFF_UP)
+		dev_deactivate(dev);
+
+	*old = dev_graft_qdisc(dev_queue, new);
+
+	if (dev->flags & IFF_UP)
+		dev_activate(dev);
+	return 0;
+}
+
+static struct Qdisc *mq_leaf(struct Qdisc *sch, unsigned long arg)
+{
+	struct netdev_queue *dev_queue;
+
+	dev_queue = mq_queue_get(sch, arg);
+	if (dev_queue == NULL)
+		return NULL;
+
+	return dev_queue->qdisc;
+}
+
+static unsigned long mq_get(struct Qdisc *sch, u32 classid)
+{
+	unsigned int ntx = TC_H_MIN(classid);
+
+	if (!mq_queue_get(sch, ntx))
+		return 0;
+	return ntx;
+}
+
+static void mq_put(struct Qdisc *sch, unsigned long cl)
+{
+	return;
+}
+
+static int mq_change(struct Qdisc *sch, u32 handle, u32 parent,
+		     struct nlattr **tca, unsigned long *arg)
+{
+	if (!mq_queue_get(sch, *arg))
+		return -ENOENT;
+	return 0;
+}
+
+static int mq_delete(struct Qdisc *sch, unsigned long cl)
+{
+	return -EOPNOTSUPP;
+}
+
+static int mq_dump_class(struct Qdisc *sch, unsigned long cl,
+			 struct sk_buff *skb, struct tcmsg *tcm)
+{
+	struct netdev_queue *dev_queue;
+
+	dev_queue = mq_queue_get(sch, cl);
+	if (dev_queue == NULL)
+		return -ENOENT;
+
+	tcm->tcm_parent = TC_H_ROOT;
+	tcm->tcm_handle |= TC_H_MIN(cl);
+	if (dev_queue->qdisc)
+		tcm->tcm_info = dev_queue->qdisc->handle;
+	return 0;
+}
+
+static int mq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
+			       struct gnet_dump *d)
+{
+	struct netdev_queue *dev_queue;
+
+	dev_queue = mq_queue_get(sch, cl);
+	sch = dev_queue->qdisc_sleeping;
+
+	if (gnet_stats_copy_basic(d, &sch->bstats) < 0 ||
+	    gnet_stats_copy_queue(d, &sch->qstats) < 0)
+		return -1;
+	return 0;
+}
+
+static void mq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	unsigned int ntx;
+
+	if (arg->stop)
+		return;
+
+	arg->count = arg->skip;
+	for (ntx = arg->skip; ntx < dev->num_tx_queues; ntx++) {
+		if (arg->fn(sch, ntx + 1, arg) < 0) {
+			arg->stop = 1;
+			break;
+		}
+		arg->count++;
+	}
+}
+
+static const struct Qdisc_class_ops mq_class_ops = {
+	.graft		= mq_graft,
+	.leaf		= mq_leaf,
+	.get		= mq_get,
+	.put		= mq_put,
+	.change		= mq_change,
+	.delete		= mq_delete,
+	.walk		= mq_walk,
+	.tcf_chain	= NULL,
+	.bind_tcf	= NULL,
+	.unbind_tcf	= NULL,
+	.dump		= mq_dump_class,
+	.dump_stats	= mq_dump_class_stats,
+};
+
+struct Qdisc_ops mq_qdisc_ops __read_mostly = {
+	.cl_ops		= &mq_class_ops,
+	.id		= "mq",
+	.priv_size	= sizeof(struct mq_sched),
+	.init		= mq_init,
+	//.reset	= mq_reset,
+	//.destroy	= mq_destroy,
+	.attach		= mq_attach,
+	.dump		= mq_dump,
+	.owner		= THIS_MODULE,
+};