Netdev List
 help / color / mirror / Atom feed
* Re: [PATCHv3 net-next] add DOVE extensions for VXLAN
From: David Miller @ 2012-11-20 18:41 UTC (permalink / raw)
  To: dlstevens; +Cc: shemminger, netdev
In-Reply-To: <201211201251.qAKCoEN8003256@lab1.dls>

From: David L Stevens <dlstevens@us.ibm.com>
Date: Tue, 20 Nov 2012 07:50:14 -0500

> 
> 	This patch provides extensions to VXLAN for supporting Distributed
> Overlay Virtual Ethernet (DOVE) networks. The patch includes:
> 
> 	+ a dove flag per VXLAN device to enable DOVE extensions
> 	+ ARP reduction, whereby a bridge-connected VXLAN tunnel endpoint
> 		answers ARP requests from the local bridge on behalf of
> 		remote DOVE clients
> 	+ route short-circuiting (aka L3 switching). Known destination IP
> 		addresses use the corresponding destination MAC address for
> 		switching rather than going to a (possibly remote) router first.
> 	+ netlink notification messages for forwarding table and L3 switching
> 		misses
> 
> Changes since v2
> 	- combined bools into "u32 flags"
> 	- replaced loop with !is_zero_ether_addr()
> 
> Signed-off-by: David L Stevens <dlstevens@us.ibm.com>

Applied, thanks David.

^ permalink raw reply

* Re: [PATCHSET REPOST v2 cgroup/for-3.8] netcls/prio_cgroup: update hierarchy support
From: David Miller @ 2012-11-20 18:37 UTC (permalink / raw)
  To: tj-DgEjT+Ai2ygdnm+yROfE0A
  Cc: nhorman-2XuSBdqkA4R54TAoqtyWWQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	daniel.wagner-98C5kh4wR6ohFhg+JK9F0w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, tgraf-G/eBtMaohhA,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353400211-5182-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Tue, 20 Nov 2012 00:30:04 -0800

>  0001-netcls_cgroup-move-config-inheritance-to-css_online-.patch
>  0002-netprio_cgroup-simplify-write_priomap.patch
>  0003-netprio_cgroup-shorten-variable-names-in-extend_netd.patch
>  0004-netprio_cgroup-reimplement-priomap-expansion.patch
>  0005-netprio_cgroup-use-cgroup-id-instead-of-cgroup_netpr.patch
>  0006-netprio_cgroup-implement-netprio-_set-_prio-helpers.patch
>  0007-netprio_cgroup-allow-nesting-and-inherit-config-on-c.patch

For series:

Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

^ permalink raw reply

* Re: /proc/net/softnet_stat & NAPI
From: Eric Dumazet @ 2012-11-20 18:25 UTC (permalink / raw)
  To: Jon Schipp; +Cc: netdev
In-Reply-To: <CAB15j_Dv8hQCDzAYxkTGs5yjz5uhETuhgRxfAvbVbt5mx-P33A@mail.gmail.com>

On Tue, 2012-11-20 at 13:06 -0500, Jon Schipp wrote:
> In relation to packet drops, assuming a new kernel, NAPI driver, and
> no use of receive packet steering,
> does /proc/net/softnet_stats provide any useful packet drop information?
> 

no

> I know that on earlier 2.4 kernels one could grab the drops in the
> backlog queue from 2nd column in /proc/net/softnet_stats.
> In a modern systems that use NAPI, does softnet_stats serve a similar
> purpose e.g. display drops in a NAPI poll queue?

Second column is number of dropped packets because
one (percpu) queue reached netdev_max_backlog

But its only in the case packet must be queued in the first place.

With a NAPI driver and no RPS/RFS, packets wont be dropped here, as NAPI
netif_receive_skb() directly calls the network stack.

If frames are dropped, they should be dropped by the NIC itself.

Even with 2.4 kernels or non NAPI driver, you could have drops at NIC
level.

^ permalink raw reply

* Re: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: David Miller @ 2012-11-20 18:15 UTC (permalink / raw)
  To: shemminger; +Cc: paolo.valente, jhs, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <20121120100958.010e7605@nehalam.linuxnetplumber.net>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 20 Nov 2012 10:09:58 -0800

> On Tue, 20 Nov 2012 13:02:02 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: Stephen Hemminger <shemminger@vyatta.com>
>> Date: Tue, 20 Nov 2012 09:53:04 -0800
>> 
>> > There are actually lots of bogus warnings than seem to only occur
>> > because gcc 4.4 does a bad job of checking. Later versions are fixed
>> > and don't generate warnings.
>> > 
>> > My preference is to not add the unnecessary initialization because
>> > if you get in the habit of doing it. The whole purpose of the uninitialized
>> > check is lost. 
>> 
>> Try again, this was with gcc-4.7.2-2 on Fedora.
>> 
>> There are too many preconditions, across multiple basic block, which
>> together ensure the skb is in fact initialized at the point in
>> question and the compiler simply isn't sophisticated enough to see
>> that.
> 
> Weird, it compiles  clean on x86-84 on Debian.
> gcc-4.7.real (Debian 4.7.2-4) 4.7.2

Fedora backports a lot more stuff into gcc than Debian does.

^ permalink raw reply

* Re: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: Stephen Hemminger @ 2012-11-20 18:09 UTC (permalink / raw)
  To: David Miller; +Cc: paolo.valente, jhs, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <20121120.130202.1918742054229219388.davem@davemloft.net>

On Tue, 20 Nov 2012 13:02:02 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 20 Nov 2012 09:53:04 -0800
> 
> > There are actually lots of bogus warnings than seem to only occur
> > because gcc 4.4 does a bad job of checking. Later versions are fixed
> > and don't generate warnings.
> > 
> > My preference is to not add the unnecessary initialization because
> > if you get in the habit of doing it. The whole purpose of the uninitialized
> > check is lost. 
> 
> Try again, this was with gcc-4.7.2-2 on Fedora.
> 
> There are too many preconditions, across multiple basic block, which
> together ensure the skb is in fact initialized at the point in
> question and the compiler simply isn't sophisticated enough to see
> that.

Weird, it compiles  clean on x86-84 on Debian.
gcc-4.7.real (Debian 4.7.2-4) 4.7.2

^ permalink raw reply

* /proc/net/softnet_stat & NAPI
From: Jon Schipp @ 2012-11-20 18:06 UTC (permalink / raw)
  To: netdev

In relation to packet drops, assuming a new kernel, NAPI driver, and
no use of receive packet steering,
does /proc/net/softnet_stats provide any useful packet drop information?

I know that on earlier 2.4 kernels one could grab the drops in the
backlog queue from 2nd column in /proc/net/softnet_stats.
In a modern systems that use NAPI, does softnet_stats serve a similar
purpose e.g. display drops in a NAPI poll queue?

Thanks
Jon

^ permalink raw reply

* Re: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: David Miller @ 2012-11-20 18:02 UTC (permalink / raw)
  To: shemminger; +Cc: paolo.valente, jhs, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <20121120095304.420ded7c@nehalam.linuxnetplumber.net>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 20 Nov 2012 09:53:04 -0800

> There are actually lots of bogus warnings than seem to only occur
> because gcc 4.4 does a bad job of checking. Later versions are fixed
> and don't generate warnings.
> 
> My preference is to not add the unnecessary initialization because
> if you get in the habit of doing it. The whole purpose of the uninitialized
> check is lost. 

Try again, this was with gcc-4.7.2-2 on Fedora.

There are too many preconditions, across multiple basic block, which
together ensure the skb is in fact initialized at the point in
question and the compiler simply isn't sophisticated enough to see
that.

^ permalink raw reply

* Re: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: David Miller @ 2012-11-20 18:00 UTC (permalink / raw)
  To: paolo.valente; +Cc: jhs, shemminger, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <50ABC19E.9030209@unimore.it>

From: Paolo Valente <paolo.valente@unimore.it>
Date: Tue, 20 Nov 2012 18:45:02 +0100

> Unfortunately I could not reproduce the warning (with
> gcc-4.7 -Wmaybe-uninitialized). I am however about to send a new
> version with skb initialized to NULL. I hope that this fix properly
> addresses this issue.

It triggers readily with Fedora's gcc-4.7.2-2 with "allmodconfig" on
x86-64.

^ permalink raw reply

* [PATCH] sctp: send abort chunk when max_retrans exceeded
From: Neil Horman @ 2012-11-20 17:59 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, Vlad Yasevich, David S. Miller, linux-sctp

In the event that an association exceeds its max_retrans attempts, we should
send an ABORT chunk indicating that we are closing the assocation as a result.
Because of the nature of the error, its unlikely to be received, but its a nice
clean way to close the association if it does make it through, and it will give
anyone watching via tcpdump a clue as to what happened.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: linux-sctp@vger.kernel.org
---
 include/net/sctp/sm.h    |  2 ++
 net/sctp/sm_make_chunk.c | 26 +++++++++++++++++++++-----
 net/sctp/sm_sideeffect.c |  9 ++++++++-
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index b5887e1..2a82d13 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -234,6 +234,8 @@ struct sctp_chunk *sctp_make_abort_violation(const struct sctp_association *,
 struct sctp_chunk *sctp_make_violation_paramlen(const struct sctp_association *,
 				   const struct sctp_chunk *,
 				   struct sctp_paramhdr *);
+struct sctp_chunk *sctp_make_violation_max_retrans(const struct sctp_association *,
+						   const struct sctp_chunk *);
 struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *,
 				  const struct sctp_transport *);
 struct sctp_chunk *sctp_make_heartbeat_ack(const struct sctp_association *,
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index fbe1636..d6a8c80 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1074,17 +1074,33 @@ struct sctp_chunk *sctp_make_violation_paramlen(
 {
 	struct sctp_chunk *retval;
 	static const char error[] = "The following parameter had invalid length:";
-	size_t payload_len = sizeof(error) + sizeof(sctp_errhdr_t) +
-				sizeof(sctp_paramhdr_t);
+	size_t payload_len = sizeof(error) + sizeof(sctp_errhdr_t);
 
 	retval = sctp_make_abort(asoc, chunk, payload_len);
 	if (!retval)
 		goto nodata;
 
-	sctp_init_cause(retval, SCTP_ERROR_PROTO_VIOLATION,
-			sizeof(error) + sizeof(sctp_paramhdr_t));
+	sctp_init_cause(retval, SCTP_ERROR_PROTO_VIOLATION, sizeof(error));
+	sctp_addto_chunk(retval, sizeof(error), error);
+
+nodata:
+	return retval;
+}
+
+struct sctp_chunk *sctp_make_violation_max_retrans(
+	const struct sctp_association *asoc,
+	const struct sctp_chunk *chunk)
+{
+	struct sctp_chunk *retval;
+	static const char error[] = "Association exceeded its max_retans count";
+	size_t payload_len = sizeof(error) + sizeof(sctp_errhdr_t);
+
+	retval = sctp_make_abort(asoc, chunk, payload_len);
+	if (!retval)
+		goto nodata;
+
+	sctp_init_cause(retval, SCTP_ERROR_PROTO_VIOLATION, sizeof(error));
 	sctp_addto_chunk(retval, sizeof(error), error);
-	sctp_addto_param(retval, sizeof(sctp_paramhdr_t), param);
 
 nodata:
 	return retval;
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 6eecf7e..c076956 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -577,7 +577,7 @@ static void sctp_cmd_assoc_failed(sctp_cmd_seq_t *commands,
 				  unsigned int error)
 {
 	struct sctp_ulpevent *event;
-
+	struct sctp_chunk *abort;
 	/* Cancel any partial delivery in progress. */
 	sctp_ulpq_abort_pd(&asoc->ulpq, GFP_ATOMIC);
 
@@ -593,6 +593,13 @@ static void sctp_cmd_assoc_failed(sctp_cmd_seq_t *commands,
 		sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
 				SCTP_ULPEVENT(event));
 
+	if (asoc->overall_error_count >= asoc->max_retrans) {
+		abort = sctp_make_violation_max_retrans(asoc, chunk);
+		if (abort)
+			sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
+					SCTP_CHUNK(abort));
+	}
+
 	sctp_add_cmd_sf(commands, SCTP_CMD_NEW_STATE,
 			SCTP_STATE(SCTP_STATE_CLOSED));
 
-- 
1.7.11.7

^ permalink raw reply related

* Re: [PATCH] ixgbe: fix broken PPTP handling
From: Alan Cox @ 2012-11-20 17:56 UTC (permalink / raw)
  To: Keller, Jacob E; +Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <02874ECE860811409154E81DA85FBB583AF32E22@ORSMSX105.amr.corp.intel.com>

> Specifically, commit 3645adbb "ixgbe: fix uninitialized event.type in ixgbe_ptp_check_pps_event"
> 
> :)
> 
> Sorry I didn't reply directly to your original email with the concern, or CC you. I will do so next time.

No problem - I missed the fact it was fixed with a different
non-overlapped patch.

Alan

------------------------------------------------------------------------------
Monitor your physical, virtual and cloud infrastructure from a single
web console. Get in-depth insight into apps, servers, databases, vmware,
SAP, cloud infrastructure, etc. Download 30-day Free Trial.
Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: Stephen Hemminger @ 2012-11-20 17:53 UTC (permalink / raw)
  To: Paolo Valente; +Cc: David Miller, jhs, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <50ABC19E.9030209@unimore.it>

On Tue, 20 Nov 2012 18:45:02 +0100
Paolo Valente <paolo.valente@unimore.it> wrote:

> Il 20/11/2012 00:48, David Miller ha scritto:
> > From: Paolo Valente <paolo.valente@unimore.it>
> > Date: Mon, 12 Nov 2012 17:48:33 +0100
> >
> >> [This patch received positive feedback from Stephen Hemminger ("put in
> >> net-next"), but no further feedback or decision. So I am (re)sending
> >> an updated version of it. The only differences with respect to the
> >> previous version are the support for TSO/GSO (taken from QFQ), and a
> >> hopefully improved description.]
> >
> > Can you rearrange the logic so that the compiler doesn't emit this
> > warning?
> >
> > In file included from net/sched/sch_qfq.c:18:0:
> > net/sched/sch_qfq.c: In function ‘qfq_dequeue’:
> > include/net/sch_generic.h:480:15: warning: ‘skb’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > net/sched/sch_qfq.c:1007:18: note: ‘skb’ was declared here
> >
> > You and I both know that SKB will be initialized at this point, but
> > the compiler can't see it clearly enough.
> >
> Unfortunately I could not reproduce the warning (with
> gcc-4.7 -Wmaybe-uninitialized). I am however about to send a new version 
> with skb initialized to NULL. I hope that this fix properly addresses 
> this issue.

There are actually lots of bogus warnings than seem to only occur
because gcc 4.4 does a bad job of checking. Later versions are fixed
and don't generate warnings.

My preference is to not add the unnecessary initialization because
if you get in the habit of doing it. The whole purpose of the uninitialized
check is lost. 

^ permalink raw reply

* [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: Paolo Valente @ 2012-11-20 17:45 UTC (permalink / raw)
  To: davem, jhs, shemminger
  Cc: linux-kernel, netdev, rizzo, fchecconi, paolo.valente

This patch turns QFQ into QFQ+, a variant of QFQ that provides the
following two benefits: 1) QFQ+ is faster than QFQ, 2) differently
from QFQ, QFQ+ correctly schedules also non-leaves classes in a
hierarchical setting.  The way how QFQ+ achieves these goals is
discussed briefly below. I also report some performance
measurements. A detailed description of QFQ+ and of these results can
be found in [1].

QFQ+ achieves a higher speed than QFQ by grouping classes into
aggregates, and uses the original QFQ scheduling algorithm to schedule
aggregates instead of single classes. An aggregate is made of at most
M classes, all with the same weight and maximum packet size.  M is
equal to the minimum between tx_queue_len+1 and 8 (value chosen to get
a good trade-off between execution time and service guarantees). QFQ+
associates each aggregate with a budget equal to the maximum packet
size for the classes in the aggregate, multiplied by the number of
classes of the aggregate. Once selected an aggregate for service, QFQ+
dequeues only the packets of its classes, until the aggregate finishes
its budget. Finally, within an aggregate, classes are scheduled with
DRR. In my measurements, described below, the execution time of QFQ+
without TSO/GSO and with M=8 was from 16% to 31% lower than that of
QFQ, and close to that of DRR.

QFQ+ does not use packet lengths for computing aggregate timestamps,
but budgets. Hence it does not need to modify any timestamp if the
head packet of a class changes. As a consequence, differently from
QFQ, which uses head-packet lengths to compute class timestamps, QFQ+
does not need further modifications to correctly schedule also
non-leaf classes and classes with non-FIFO qdiscs.

As for service guarantees, thanks to the way how M is computed, the
service of QFQ+ is close to the one of QFQ. For example, as proved in
[1], under QFQ+ every packet of a given class is guaranteed the same
worst-case completion time as under QFQ, plus an additional delay
equal to the transmission time, at the rate reserved to the class, of
three maximum-size packet. See [1, Section 7.1] for a numerical
comparison among the packet delays guaranteed by QFQ+, QFQ and DRR.

I measured the execution time of QFQ+, DRR and QFQ using the testing
environment [2]. In particular, for each scheduler I measured the
average total execution time of a packet enqueue plus a packet
dequeue, without TSO/GSO. What would happen with TSO/GSO is discussed
at the end of this description. For practical reasons, in the testing
environment each enqueue&dequeue is also charged for the cost of
generating and discarding an empty, fixed-size packet (using a free
list). The following table reports the results with an i7-2760QM,
against four different class sets. Time is measured in nanoseconds,
while each set or subset of classes is denoted as
<num_classes>-w<weight>, where <num_classes> and <weight> are,
respectively, the number of classes and the weight of every class in
the set/subset (for example, 250-w1 stands for 250 classes with weight
1). For QFQ+, the table shows the results for the two extremes for M:
1 and 8 (see [1, Section 7.2] for results with other values of M and
for more information).

 -----------------------------------------------
| Set of  |      QFQ+ (M)     |   DRR      QFQ  |
| classes |    1          8   |                 |
|-----------------------------------------------|     
| 1k-w1   |   89         63   |    56       81  |
|-----------------------------------------------|
| 500-w1, |                   |                 |
| 250-w2, |  102         71   |    87      103  |
| 250-w4  |                   |                 |
|-----------------------------------------------|
| 32k-w1  |  267        225   |   173      257  |
|-----------------------------------------------|
| 16k-w1, |                   |                 |
| 8k-w2,  |  253        187   |   252      257  |
| 8k-w4   |                   |                 |
 -----------------------------------------------

About DRR, it achieves its best performance when all the classes have
the same weight. This is fortunate, because in such scenarios it is
actually pointless to use a fair-queueing scheduler, as the latter
would provide the same quality of service as DRR. In contrast, when
classes have differentiated weights and the better service properties
of QFQ+ make a difference, QFQ+ has better performance than DRR. It
happens mainly because QFQ+ dequeues packets in an order that causes
about 8% less cache misses than DRR. As for the number of
instructions, QFQ+ executes instead about 7% more instructions than
DRR, whereas QFQ executes from 25% to 34% more instructions than DRR.

With TSO/GSO, the number of instructions per packet enqueue/dequeue
executed by QFQ and QFQ+ is the same as without TSO/GSO. In contrast,
for each dequeue, the number of iterations executed by the main loop
of DRR is 64K/quantum times as high as without TSO/GSO.

Paolo

[1] P. Valente, "Reducing the Execution Time of Fair-Queueing Schedulers"
http://algo.ing.unimo.it/people/paolo/agg-sched/agg-sched.pdf

[2] http://algo.ing.unimo.it/people/paolo/agg-sched/test-env.tgz

Signed-off-by: Paolo Valente <paolo.valente@unimore.it>
---
 net/sched/sch_qfq.c |  834 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 571 insertions(+), 263 deletions(-)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 9687fa1..fe1823a 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -1,7 +1,8 @@
 /*
- * net/sched/sch_qfq.c         Quick Fair Queueing Scheduler.
+ * net/sched/sch_qfq.c         Quick Fair Queueing Plus Scheduler.
  *
  * Copyright (c) 2009 Fabio Checconi, Luigi Rizzo, and Paolo Valente.
+ * Copyright (c) 2012 Paolo Valente.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -19,12 +20,18 @@
 #include <net/pkt_cls.h>
 
 
-/*  Quick Fair Queueing
-    ===================
+/*  Quick Fair Queueing Plus
+    ========================
 
     Sources:
 
-    Fabio Checconi, Luigi Rizzo, and Paolo Valente: "QFQ: Efficient
+    [1] Paolo Valente,
+    "Reducing the Execution Time of Fair-Queueing Schedulers."
+    http://algo.ing.unimo.it/people/paolo/agg-sched/agg-sched.pdf
+
+    Sources for QFQ:
+
+    [2] Fabio Checconi, Luigi Rizzo, and Paolo Valente: "QFQ: Efficient
     Packet Scheduling with Tight Bandwidth Distribution Guarantees."
 
     See also:
@@ -33,6 +40,20 @@
 
 /*
 
+  QFQ+ divides classes into aggregates of at most MAX_AGG_CLASSES
+  classes. Each aggregate is timestamped with a virtual start time S
+  and a virtual finish time F, and scheduled according to its
+  timestamps. S and F are computed as a function of a system virtual
+  time function V. The classes within each aggregate are instead
+  scheduled with DRR.
+
+  To speed up operations, QFQ+ divides also aggregates into a limited
+  number of groups. Which group a class belongs to depends on the
+  ratio between the maximum packet length for the class and the weight
+  of the class. Groups have their own S and F. In the end, QFQ+
+  schedules groups, then aggregates within groups, then classes within
+  aggregates. See [1] and [2] for a full description.
+
   Virtual time computations.
 
   S, F and V are all computed in fixed point arithmetic with
@@ -76,27 +97,28 @@
 #define QFQ_MAX_SLOTS	32
 
 /*
- * Shifts used for class<->group mapping.  We allow class weights that are
- * in the range [1, 2^MAX_WSHIFT], and we try to map each class i to the
+ * Shifts used for aggregate<->group mapping.  We allow class weights that are
+ * in the range [1, 2^MAX_WSHIFT], and we try to map each aggregate i to the
  * group with the smallest index that can support the L_i / r_i configured
- * for the class.
+ * for the classes in the aggregate.
  *
  * grp->index is the index of the group; and grp->slot_shift
  * is the shift for the corresponding (scaled) sigma_i.
  */
 #define QFQ_MAX_INDEX		24
-#define QFQ_MAX_WSHIFT		12
+#define QFQ_MAX_WSHIFT		10
 
-#define	QFQ_MAX_WEIGHT		(1<<QFQ_MAX_WSHIFT)
-#define QFQ_MAX_WSUM		(16*QFQ_MAX_WEIGHT)
+#define	QFQ_MAX_WEIGHT		(1<<QFQ_MAX_WSHIFT) /* see qfq_slot_insert */
+#define QFQ_MAX_WSUM		(64*QFQ_MAX_WEIGHT)
 
 #define FRAC_BITS		30	/* fixed point arithmetic */
 #define ONE_FP			(1UL << FRAC_BITS)
 #define IWSUM			(ONE_FP/QFQ_MAX_WSUM)
 
 #define QFQ_MTU_SHIFT		16	/* to support TSO/GSO */
-#define QFQ_MIN_SLOT_SHIFT	(FRAC_BITS + QFQ_MTU_SHIFT - QFQ_MAX_INDEX)
-#define QFQ_MIN_LMAX		256	/* min possible lmax for a class */
+#define QFQ_MIN_LMAX		512	/* see qfq_slot_insert */
+
+#define QFQ_MAX_AGG_CLASSES	8 /* max num classes per aggregate allowed */
 
 /*
  * Possible group states.  These values are used as indexes for the bitmaps
@@ -106,6 +128,8 @@ enum qfq_state { ER, IR, EB, IB, QFQ_MAX_STATE };
 
 struct qfq_group;
 
+struct qfq_aggregate;
+
 struct qfq_class {
 	struct Qdisc_class_common common;
 
@@ -116,7 +140,15 @@ struct qfq_class {
 	struct gnet_stats_queue qstats;
 	struct gnet_stats_rate_est rate_est;
 	struct Qdisc *qdisc;
+	struct list_head alist;		/* Link for active-classes list. */
+	struct qfq_aggregate *agg;	/* Parent aggregate. */
+	int deficit;			/* DRR deficit counter. */
+};
 
+/*
+
+ */
+struct qfq_aggregate {
 	struct hlist_node next;	/* Link for the slot list. */
 	u64 S, F;		/* flow timestamps (exact) */
 
@@ -127,8 +159,18 @@ struct qfq_class {
 	struct qfq_group *grp;
 
 	/* these are copied from the flowset. */
-	u32	inv_w;		/* ONE_FP/weight */
-	u32	lmax;		/* Max packet size for this flow. */
+	u32	class_weight; /* Weight of each class in this aggregate. */
+	/* Max pkt size for the classes in this aggregate, DRR quantum. */
+	int	lmax;
+
+	u32	inv_w;	    /* ONE_FP/(sum of weights of classes in aggr.). */
+	u32	budgetmax;  /* Max budget for this aggregate. */
+	u32	initial_budget, budget;     /* Initial and current budget. */
+
+	int		  num_classes;	/* Number of classes in this aggr. */
+	struct list_head  active;	/* DRR queue of active classes. */
+
+	struct hlist_node nonfull_next;	/* See nonfull_aggs in qfq_sched. */
 };
 
 struct qfq_group {
@@ -138,7 +180,7 @@ struct qfq_group {
 	unsigned int front;		/* Index of the front slot. */
 	unsigned long full_slots;	/* non-empty slots */
 
-	/* Array of RR lists of active classes. */
+	/* Array of RR lists of active aggregates. */
 	struct hlist_head slots[QFQ_MAX_SLOTS];
 };
 
@@ -146,13 +188,28 @@ struct qfq_sched {
 	struct tcf_proto *filter_list;
 	struct Qdisc_class_hash clhash;
 
-	u64		V;		/* Precise virtual time. */
-	u32		wsum;		/* weight sum */
+	u64			oldV, V;	/* Precise virtual times. */
+	struct qfq_aggregate	*in_serv_agg;   /* Aggregate being served. */
+	u32			num_active_agg; /* Num. of active aggregates */
+	u32			wsum;		/* weight sum */
 
 	unsigned long bitmaps[QFQ_MAX_STATE];	    /* Group bitmaps. */
 	struct qfq_group groups[QFQ_MAX_INDEX + 1]; /* The groups. */
+	u32 min_slot_shift;	/* Index of the group-0 bit in the bitmaps. */
+
+	u32 max_agg_classes;		/* Max number of classes per aggr. */
+	struct hlist_head nonfull_aggs; /* Aggs with room for more classes. */
 };
 
+/*
+ * Possible reasons why the timestamps of an aggregate are updated
+ * enqueue: the aggregate switches from idle to active and must scheduled
+ *	    for service
+ * requeue: the aggregate finishes its budget, so it stops being served and
+ *	    must be rescheduled for service
+ */
+enum update_reason {enqueue, requeue};
+
 static struct qfq_class *qfq_find_class(struct Qdisc *sch, u32 classid)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
@@ -182,18 +239,18 @@ static const struct nla_policy qfq_policy[TCA_QFQ_MAX + 1] = {
  * index = log_2(maxlen/weight) but we need to apply the scaling.
  * This is used only once at flow creation.
  */
-static int qfq_calc_index(u32 inv_w, unsigned int maxlen)
+static int qfq_calc_index(u32 inv_w, unsigned int maxlen, u32 min_slot_shift)
 {
 	u64 slot_size = (u64)maxlen * inv_w;
 	unsigned long size_map;
 	int index = 0;
 
-	size_map = slot_size >> QFQ_MIN_SLOT_SHIFT;
+	size_map = slot_size >> min_slot_shift;
 	if (!size_map)
 		goto out;
 
 	index = __fls(size_map) + 1;	/* basically a log_2 */
-	index -= !(slot_size - (1ULL << (index + QFQ_MIN_SLOT_SHIFT - 1)));
+	index -= !(slot_size - (1ULL << (index + min_slot_shift - 1)));
 
 	if (index < 0)
 		index = 0;
@@ -204,66 +261,150 @@ out:
 	return index;
 }
 
-/* Length of the next packet (0 if the queue is empty). */
-static unsigned int qdisc_peek_len(struct Qdisc *sch)
+static void qfq_deactivate_agg(struct qfq_sched *, struct qfq_aggregate *);
+static void qfq_activate_agg(struct qfq_sched *, struct qfq_aggregate *,
+			     enum update_reason);
+
+static void qfq_init_agg(struct qfq_sched *q, struct qfq_aggregate *agg,
+			 u32 lmax, u32 weight)
 {
-	struct sk_buff *skb;
+	INIT_LIST_HEAD(&agg->active);
+	hlist_add_head(&agg->nonfull_next, &q->nonfull_aggs);
 
-	skb = sch->ops->peek(sch);
-	return skb ? qdisc_pkt_len(skb) : 0;
+	agg->lmax = lmax;
+	agg->class_weight = weight;
 }
 
-static void qfq_deactivate_class(struct qfq_sched *, struct qfq_class *);
-static void qfq_activate_class(struct qfq_sched *q, struct qfq_class *cl,
-			       unsigned int len);
+static struct qfq_aggregate *qfq_find_agg(struct qfq_sched *q,
+					  u32 lmax, u32 weight)
+{
+	struct qfq_aggregate *agg;
+	struct hlist_node *n;
 
-static void qfq_update_class_params(struct qfq_sched *q, struct qfq_class *cl,
-				    u32 lmax, u32 inv_w, int delta_w)
+	hlist_for_each_entry(agg, n, &q->nonfull_aggs, nonfull_next)
+		if (agg->lmax == lmax && agg->class_weight == weight)
+			return agg;
+
+	return NULL;
+}
+
+
+/* Update aggregate as a function of the new number of classes. */
+static void qfq_update_agg(struct qfq_sched *q, struct qfq_aggregate *agg,
+			   int new_num_classes)
 {
-	int i;
+	u32 new_agg_weight;
+
+	if (new_num_classes == q->max_agg_classes)
+		hlist_del_init(&agg->nonfull_next);
 
-	/* update qfq-specific data */
-	cl->lmax = lmax;
-	cl->inv_w = inv_w;
-	i = qfq_calc_index(cl->inv_w, cl->lmax);
+	if (agg->num_classes > new_num_classes &&
+	    new_num_classes == q->max_agg_classes - 1) /* agg no more full */
+		hlist_add_head(&agg->nonfull_next, &q->nonfull_aggs);
 
-	cl->grp = &q->groups[i];
+	agg->budgetmax = new_num_classes * agg->lmax;
+	new_agg_weight = agg->class_weight * new_num_classes;
+	agg->inv_w = ONE_FP/new_agg_weight;
 
-	q->wsum += delta_w;
+	if (agg->grp == NULL) {
+		int i = qfq_calc_index(agg->inv_w, agg->budgetmax,
+				       q->min_slot_shift);
+		agg->grp = &q->groups[i];
+	}
+
+	q->wsum +=
+		(int) agg->class_weight * (new_num_classes - agg->num_classes);
+
+	agg->num_classes = new_num_classes;
 }
 
-static void qfq_update_reactivate_class(struct qfq_sched *q,
-					struct qfq_class *cl,
-					u32 inv_w, u32 lmax, int delta_w)
+/* Add class to aggregate. */
+static void qfq_add_to_agg(struct qfq_sched *q,
+			   struct qfq_aggregate *agg,
+			   struct qfq_class *cl)
 {
-	bool need_reactivation = false;
-	int i = qfq_calc_index(inv_w, lmax);
+	cl->agg = agg;
+
+	qfq_update_agg(q, agg, agg->num_classes+1);
+	if (cl->qdisc->q.qlen > 0) { /* adding an active class */
+		list_add_tail(&cl->alist, &agg->active);
+		if (list_first_entry(&agg->active, struct qfq_class, alist) ==
+		    cl && q->in_serv_agg != agg) /* agg was inactive */
+			qfq_activate_agg(q, agg, enqueue); /* schedule agg */
+	}
+}
 
-	if (&q->groups[i] != cl->grp && cl->qdisc->q.qlen > 0) {
-		/*
-		 * shift cl->F back, to not charge the
-		 * class for the not-yet-served head
-		 * packet
-		 */
-		cl->F = cl->S;
-		/* remove class from its slot in the old group */
-		qfq_deactivate_class(q, cl);
-		need_reactivation = true;
+static struct qfq_aggregate *qfq_choose_next_agg(struct qfq_sched *);
+
+static void qfq_destroy_agg(struct qfq_sched *q, struct qfq_aggregate *agg)
+{
+	if (!hlist_unhashed(&agg->nonfull_next))
+		hlist_del_init(&agg->nonfull_next);
+	if (q->in_serv_agg == agg)
+		q->in_serv_agg = qfq_choose_next_agg(q);
+	kfree(agg);
+}
+
+/* Deschedule class from within its parent aggregate. */
+static void qfq_deactivate_class(struct qfq_sched *q, struct qfq_class *cl)
+{
+	struct qfq_aggregate *agg = cl->agg;
+
+
+	list_del(&cl->alist); /* remove from RR queue of the aggregate */
+	if (list_empty(&agg->active)) /* agg is now inactive */
+		qfq_deactivate_agg(q, agg);
+}
+
+/* Remove class from its parent aggregate. */
+static void qfq_rm_from_agg(struct qfq_sched *q, struct qfq_class *cl)
+{
+	struct qfq_aggregate *agg = cl->agg;
+
+	cl->agg = NULL;
+	if (agg->num_classes == 1) { /* agg being emptied, destroy it */
+		qfq_destroy_agg(q, agg);
+		return;
 	}
+	qfq_update_agg(q, agg, agg->num_classes-1);
+}
 
-	qfq_update_class_params(q, cl, lmax, inv_w, delta_w);
+/* Deschedule class and remove it from its parent aggregate. */
+static void qfq_deact_rm_from_agg(struct qfq_sched *q, struct qfq_class *cl)
+{
+	if (cl->qdisc->q.qlen > 0) /* class is active */
+		qfq_deactivate_class(q, cl);
 
-	if (need_reactivation) /* activate in new group */
-		qfq_activate_class(q, cl, qdisc_peek_len(cl->qdisc));
+	qfq_rm_from_agg(q, cl);
 }
 
+/* Move class to a new aggregate, matching the new class weight and/or lmax */
+static int qfq_change_agg(struct Qdisc *sch, struct qfq_class *cl, u32 weight,
+			   u32 lmax)
+{
+	struct qfq_sched *q = qdisc_priv(sch);
+	struct qfq_aggregate *new_agg = qfq_find_agg(q, lmax, weight);
+
+	if (new_agg == NULL) { /* create new aggregate */
+		new_agg = kzalloc(sizeof(*new_agg), GFP_ATOMIC);
+		if (new_agg == NULL)
+			return -ENOBUFS;
+		qfq_init_agg(q, new_agg, lmax, weight);
+	}
+	qfq_deact_rm_from_agg(q, cl);
+	qfq_add_to_agg(q, new_agg, cl);
+
+	return 0;
+}
 
 static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 			    struct nlattr **tca, unsigned long *arg)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
 	struct qfq_class *cl = (struct qfq_class *)*arg;
+	bool existing = false;
 	struct nlattr *tb[TCA_QFQ_MAX + 1];
+	struct qfq_aggregate *new_agg = NULL;
 	u32 weight, lmax, inv_w;
 	int err;
 	int delta_w;
@@ -286,15 +427,6 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	} else
 		weight = 1;
 
-	inv_w = ONE_FP / weight;
-	weight = ONE_FP / inv_w;
-	delta_w = weight - (cl ? ONE_FP / cl->inv_w : 0);
-	if (q->wsum + delta_w > QFQ_MAX_WSUM) {
-		pr_notice("qfq: total weight out of range (%u + %u)\n",
-			  delta_w, q->wsum);
-		return -EINVAL;
-	}
-
 	if (tb[TCA_QFQ_LMAX]) {
 		lmax = nla_get_u32(tb[TCA_QFQ_LMAX]);
 		if (lmax < QFQ_MIN_LMAX || lmax > (1UL << QFQ_MTU_SHIFT)) {
@@ -304,7 +436,23 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	} else
 		lmax = psched_mtu(qdisc_dev(sch));
 
-	if (cl != NULL) {
+	inv_w = ONE_FP / weight;
+	weight = ONE_FP / inv_w;
+
+	if (cl != NULL &&
+	    lmax == cl->agg->lmax &&
+	    weight == cl->agg->class_weight)
+		return 0; /* nothing to change */
+
+	delta_w = weight - (cl ? cl->agg->class_weight : 0);
+
+	if (q->wsum + delta_w > QFQ_MAX_WSUM) {
+		pr_notice("qfq: total weight out of range (%d + %u)\n",
+			  delta_w, q->wsum);
+		return -EINVAL;
+	}
+
+	if (cl != NULL) { /* modify existing class */
 		if (tca[TCA_RATE]) {
 			err = gen_replace_estimator(&cl->bstats, &cl->rate_est,
 						    qdisc_root_sleeping_lock(sch),
@@ -312,25 +460,18 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 			if (err)
 				return err;
 		}
-
-		if (lmax == cl->lmax && inv_w == cl->inv_w)
-			return 0; /* nothing to update */
-
-		sch_tree_lock(sch);
-		qfq_update_reactivate_class(q, cl, inv_w, lmax, delta_w);
-		sch_tree_unlock(sch);
-
-		return 0;
+		existing = true;
+		goto set_change_agg;
 	}
 
+	/* create and init new class */
 	cl = kzalloc(sizeof(struct qfq_class), GFP_KERNEL);
 	if (cl == NULL)
 		return -ENOBUFS;
 
 	cl->refcnt = 1;
 	cl->common.classid = classid;
-
-	qfq_update_class_params(q, cl, lmax, inv_w, delta_w);
+	cl->deficit = lmax;
 
 	cl->qdisc = qdisc_create_dflt(sch->dev_queue,
 				      &pfifo_qdisc_ops, classid);
@@ -341,11 +482,8 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 		err = gen_new_estimator(&cl->bstats, &cl->rate_est,
 					qdisc_root_sleeping_lock(sch),
 					tca[TCA_RATE]);
-		if (err) {
-			qdisc_destroy(cl->qdisc);
-			kfree(cl);
-			return err;
-		}
+		if (err)
+			goto destroy_class;
 	}
 
 	sch_tree_lock(sch);
@@ -354,19 +492,39 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 
 	qdisc_class_hash_grow(sch, &q->clhash);
 
+set_change_agg:
+	sch_tree_lock(sch);
+	new_agg = qfq_find_agg(q, lmax, weight);
+	if (new_agg == NULL) { /* create new aggregate */
+		sch_tree_unlock(sch);
+		new_agg = kzalloc(sizeof(*new_agg), GFP_KERNEL);
+		if (new_agg == NULL) {
+			err = -ENOBUFS;
+			gen_kill_estimator(&cl->bstats, &cl->rate_est);
+			goto destroy_class;
+		}
+		sch_tree_lock(sch);
+		qfq_init_agg(q, new_agg, lmax, weight);
+	}
+	if (existing)
+		qfq_deact_rm_from_agg(q, cl);
+	qfq_add_to_agg(q, new_agg, cl);
+	sch_tree_unlock(sch);
+
 	*arg = (unsigned long)cl;
 	return 0;
+
+destroy_class:
+	qdisc_destroy(cl->qdisc);
+	kfree(cl);
+	return err;
 }
 
 static void qfq_destroy_class(struct Qdisc *sch, struct qfq_class *cl)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
 
-	if (cl->inv_w) {
-		q->wsum -= ONE_FP / cl->inv_w;
-		cl->inv_w = 0;
-	}
-
+	qfq_rm_from_agg(q, cl);
 	gen_kill_estimator(&cl->bstats, &cl->rate_est);
 	qdisc_destroy(cl->qdisc);
 	kfree(cl);
@@ -481,8 +639,8 @@ static int qfq_dump_class(struct Qdisc *sch, unsigned long arg,
 	nest = nla_nest_start(skb, TCA_OPTIONS);
 	if (nest == NULL)
 		goto nla_put_failure;
-	if (nla_put_u32(skb, TCA_QFQ_WEIGHT, ONE_FP/cl->inv_w) ||
-	    nla_put_u32(skb, TCA_QFQ_LMAX, cl->lmax))
+	if (nla_put_u32(skb, TCA_QFQ_WEIGHT, cl->agg->class_weight) ||
+	    nla_put_u32(skb, TCA_QFQ_LMAX, cl->agg->lmax))
 		goto nla_put_failure;
 	return nla_nest_end(skb, nest);
 
@@ -500,8 +658,8 @@ static int qfq_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 	memset(&xstats, 0, sizeof(xstats));
 	cl->qdisc->qstats.qlen = cl->qdisc->q.qlen;
 
-	xstats.weight = ONE_FP/cl->inv_w;
-	xstats.lmax = cl->lmax;
+	xstats.weight = cl->agg->class_weight;
+	xstats.lmax = cl->agg->lmax;
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
 	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
@@ -652,16 +810,16 @@ static void qfq_unblock_groups(struct qfq_sched *q, int index, u64 old_F)
  * perhaps
  *
 	old_V ^= q->V;
-	old_V >>= QFQ_MIN_SLOT_SHIFT;
+	old_V >>= q->min_slot_shift;
 	if (old_V) {
 		...
 	}
  *
  */
-static void qfq_make_eligible(struct qfq_sched *q, u64 old_V)
+static void qfq_make_eligible(struct qfq_sched *q)
 {
-	unsigned long vslot = q->V >> QFQ_MIN_SLOT_SHIFT;
-	unsigned long old_vslot = old_V >> QFQ_MIN_SLOT_SHIFT;
+	unsigned long vslot = q->V >> q->min_slot_shift;
+	unsigned long old_vslot = q->oldV >> q->min_slot_shift;
 
 	if (vslot != old_vslot) {
 		unsigned long mask = (1UL << fls(vslot ^ old_vslot)) - 1;
@@ -672,34 +830,38 @@ static void qfq_make_eligible(struct qfq_sched *q, u64 old_V)
 
 
 /*
- * If the weight and lmax (max_pkt_size) of the classes do not change,
- * then QFQ guarantees that the slot index is never higher than
- * 2 + ((1<<QFQ_MTU_SHIFT)/QFQ_MIN_LMAX) * (QFQ_MAX_WEIGHT/QFQ_MAX_WSUM).
+ * The index of the slot in which the aggregate is to be inserted must
+ * not be higher than QFQ_MAX_SLOTS-2. There is a '-2' and not a '-1'
+ * because the start time of the group may be moved backward by one
+ * slot after the aggregate has been inserted, and this would cause
+ * non-empty slots to be right-shifted by one position.
  *
- * With the current values of the above constants, the index is
- * then guaranteed to never be higher than 2 + 256 * (1 / 16) = 18.
+ * If the weight and lmax (max_pkt_size) of the classes do not change,
+ * then QFQ+ does meet the above contraint according to the current
+ * values of its parameters. In fact, if the weight and lmax of the
+ * classes do not change, then, from the theory, QFQ+ guarantees that
+ * the slot index is never higher than
+ * 2 + QFQ_MAX_AGG_CLASSES * ((1<<QFQ_MTU_SHIFT)/QFQ_MIN_LMAX) *
+ * (QFQ_MAX_WEIGHT/QFQ_MAX_WSUM) = 2 + 8 * 128 * (1 / 64) = 18
  *
  * When the weight of a class is increased or the lmax of the class is
- * decreased, a new class with smaller slot size may happen to be
- * activated. The activation of this class should be properly delayed
- * to when the service of the class has finished in the ideal system
- * tracked by QFQ. If the activation of the class is not delayed to
- * this reference time instant, then this class may be unjustly served
- * before other classes waiting for service. This may cause
- * (unfrequently) the above bound to the slot index to be violated for
- * some of these unlucky classes.
+ * decreased, a new aggregate with smaller slot size than the original
+ * parent aggregate of the class may happen to be activated. The
+ * activation of this aggregate should be properly delayed to when the
+ * service of the class has finished in the ideal system tracked by
+ * QFQ+. If the activation of the aggregate is not delayed to this
+ * reference time instant, then this aggregate may be unjustly served
+ * before other aggregates waiting for service. This may cause the
+ * above bound to the slot index to be violated for some of these
+ * unlucky aggregates.
  *
- * Instead of delaying the activation of the new class, which is quite
- * complex, the following inaccurate but simple solution is used: if
- * the slot index is higher than QFQ_MAX_SLOTS-2, then the timestamps
- * of the class are shifted backward so as to let the slot index
- * become equal to QFQ_MAX_SLOTS-2. This threshold is used because, if
- * the slot index is above it, then the data structure implementing
- * the bucket list either gets immediately corrupted or may get
- * corrupted on a possible next packet arrival that causes the start
- * time of the group to be shifted backward.
+ * Instead of delaying the activation of the new aggregate, which is
+ * quite complex, the following inaccurate but simple solution is used:
+ * if the slot index is higher than QFQ_MAX_SLOTS-2, then the
+ * timestamps of the aggregate are shifted backward so as to let the
+ * slot index become equal to QFQ_MAX_SLOTS-2.
  */
-static void qfq_slot_insert(struct qfq_group *grp, struct qfq_class *cl,
+static void qfq_slot_insert(struct qfq_group *grp, struct qfq_aggregate *agg,
 			    u64 roundedS)
 {
 	u64 slot = (roundedS - grp->S) >> grp->slot_shift;
@@ -708,22 +870,22 @@ static void qfq_slot_insert(struct qfq_group *grp, struct qfq_class *cl,
 	if (unlikely(slot > QFQ_MAX_SLOTS - 2)) {
 		u64 deltaS = roundedS - grp->S -
 			((u64)(QFQ_MAX_SLOTS - 2)<<grp->slot_shift);
-		cl->S -= deltaS;
-		cl->F -= deltaS;
+		agg->S -= deltaS;
+		agg->F -= deltaS;
 		slot = QFQ_MAX_SLOTS - 2;
 	}
 
 	i = (grp->front + slot) % QFQ_MAX_SLOTS;
 
-	hlist_add_head(&cl->next, &grp->slots[i]);
+	hlist_add_head(&agg->next, &grp->slots[i]);
 	__set_bit(slot, &grp->full_slots);
 }
 
 /* Maybe introduce hlist_first_entry?? */
-static struct qfq_class *qfq_slot_head(struct qfq_group *grp)
+static struct qfq_aggregate *qfq_slot_head(struct qfq_group *grp)
 {
 	return hlist_entry(grp->slots[grp->front].first,
-			   struct qfq_class, next);
+			   struct qfq_aggregate, next);
 }
 
 /*
@@ -731,20 +893,20 @@ static struct qfq_class *qfq_slot_head(struct qfq_group *grp)
  */
 static void qfq_front_slot_remove(struct qfq_group *grp)
 {
-	struct qfq_class *cl = qfq_slot_head(grp);
+	struct qfq_aggregate *agg = qfq_slot_head(grp);
 
-	BUG_ON(!cl);
-	hlist_del(&cl->next);
+	BUG_ON(!agg);
+	hlist_del(&agg->next);
 	if (hlist_empty(&grp->slots[grp->front]))
 		__clear_bit(0, &grp->full_slots);
 }
 
 /*
- * Returns the first full queue in a group. As a side effect,
- * adjust the bucket list so the first non-empty bucket is at
- * position 0 in full_slots.
+ * Returns the first aggregate in the first non-empty bucket of the
+ * group. As a side effect, adjusts the bucket list so the first
+ * non-empty bucket is at position 0 in full_slots.
  */
-static struct qfq_class *qfq_slot_scan(struct qfq_group *grp)
+static struct qfq_aggregate *qfq_slot_scan(struct qfq_group *grp)
 {
 	unsigned int i;
 
@@ -780,7 +942,7 @@ static void qfq_slot_rotate(struct qfq_group *grp, u64 roundedS)
 	grp->front = (grp->front - i) % QFQ_MAX_SLOTS;
 }
 
-static void qfq_update_eligible(struct qfq_sched *q, u64 old_V)
+static void qfq_update_eligible(struct qfq_sched *q)
 {
 	struct qfq_group *grp;
 	unsigned long ineligible;
@@ -792,137 +954,229 @@ static void qfq_update_eligible(struct qfq_sched *q, u64 old_V)
 			if (qfq_gt(grp->S, q->V))
 				q->V = grp->S;
 		}
-		qfq_make_eligible(q, old_V);
+		qfq_make_eligible(q);
 	}
 }
 
-/*
- * Updates the class, returns true if also the group needs to be updated.
- */
-static bool qfq_update_class(struct qfq_group *grp, struct qfq_class *cl)
+/* Dequeue head packet of the head class in the DRR queue of the aggregate. */
+static void agg_dequeue(struct qfq_aggregate *agg,
+			struct qfq_class *cl, unsigned int len)
 {
-	unsigned int len = qdisc_peek_len(cl->qdisc);
+	qdisc_dequeue_peeked(cl->qdisc);
 
-	cl->S = cl->F;
-	if (!len)
-		qfq_front_slot_remove(grp);	/* queue is empty */
-	else {
-		u64 roundedS;
-
-		cl->F = cl->S + (u64)len * cl->inv_w;
-		roundedS = qfq_round_down(cl->S, grp->slot_shift);
-		if (roundedS == grp->S)
-			return false;
+	cl->deficit -= (int) len;
 
-		qfq_front_slot_remove(grp);
-		qfq_slot_insert(grp, cl, roundedS);
+	if (cl->qdisc->q.qlen == 0) /* no more packets, remove from list */
+		list_del(&cl->alist);
+	else if (cl->deficit < qdisc_pkt_len(cl->qdisc->ops->peek(cl->qdisc))) {
+		cl->deficit += agg->lmax;
+		list_move_tail(&cl->alist, &agg->active);
 	}
+}
+
+static inline struct sk_buff *qfq_peek_skb(struct qfq_aggregate *agg,
+					   struct qfq_class **cl,
+					   unsigned int *len)
+{
+	struct sk_buff *skb;
 
-	return true;
+	*cl = list_first_entry(&agg->active, struct qfq_class, alist);
+	skb = (*cl)->qdisc->ops->peek((*cl)->qdisc);
+	if (skb == NULL)
+		WARN_ONCE(1, "qfq_dequeue: non-workconserving leaf\n");
+	else
+		*len = qdisc_pkt_len(skb);
+
+	return skb;
+}
+
+/* Update F according to the actual service received by the aggregate. */
+static inline void charge_actual_service(struct qfq_aggregate *agg)
+{
+	/* compute the service received by the aggregate */
+	u32 service_received = agg->initial_budget - agg->budget;
+
+	agg->F = agg->S + (u64)service_received * agg->inv_w;
 }
 
 static struct sk_buff *qfq_dequeue(struct Qdisc *sch)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
-	struct qfq_group *grp;
+	struct qfq_aggregate *in_serv_agg = q->in_serv_agg;
 	struct qfq_class *cl;
-	struct sk_buff *skb;
+	struct sk_buff *skb = NULL;
 	unsigned int len;
-	u64 old_V;
 
-	if (!q->bitmaps[ER])
+	if (in_serv_agg == NULL)
 		return NULL;
 
-	grp = qfq_ffs(q, q->bitmaps[ER]);
+	if (!list_empty(&in_serv_agg->active)) {
+		skb = qfq_peek_skb(in_serv_agg, &cl, &len);
+		if (!skb)
+			return NULL;
+	} else
+		len = 0; /* no more active classes in the in-service agg */
 
-	cl = qfq_slot_head(grp);
-	skb = qdisc_dequeue_peeked(cl->qdisc);
-	if (!skb) {
-		WARN_ONCE(1, "qfq_dequeue: non-workconserving leaf\n");
-		return NULL;
+	/*
+	 * If there are no active classes in the in-service aggregate,
+	 * or if the aggregate has not enough budget to serve its next
+	 * class, then choose the next aggregate to serve.
+	 */
+	if (len == 0 || in_serv_agg->budget < len) {
+		charge_actual_service(in_serv_agg);
+
+		/* recharge the budget of the aggregate */
+		in_serv_agg->initial_budget = in_serv_agg->budget =
+			in_serv_agg->budgetmax;
+
+		if (!list_empty(&in_serv_agg->active))
+			/*
+			 * Still active: reschedule for
+			 * service. Possible optimization: if no other
+			 * aggregate is active, then there is no point
+			 * in rescheduling this aggregate, and we can
+			 * just keep it as the in-service one. This
+			 * should be however a corner case, and to
+			 * handle it, we would need to maintain an
+			 * extra num_active_aggs field.
+			*/
+			qfq_activate_agg(q, in_serv_agg, requeue);
+		else if (sch->q.qlen == 0) { /* no aggregate to serve */
+			q->in_serv_agg = NULL;
+			return NULL;
+		}
+
+		/*
+		 * If we get here, there are other aggregates queued:
+		 * choose the new aggregate to serve.
+		 */
+		in_serv_agg = q->in_serv_agg = qfq_choose_next_agg(q);
+		skb = qfq_peek_skb(in_serv_agg, &cl, &len);
+		if (!skb)
+			return NULL;
 	}
 
 	sch->q.qlen--;
 	qdisc_bstats_update(sch, skb);
 
-	old_V = q->V;
-	len = qdisc_pkt_len(skb);
+	agg_dequeue(in_serv_agg, cl, len);
+	in_serv_agg->budget -= len;
 	q->V += (u64)len * IWSUM;
 	pr_debug("qfq dequeue: len %u F %lld now %lld\n",
-		 len, (unsigned long long) cl->F, (unsigned long long) q->V);
+		 len, (unsigned long long) in_serv_agg->F,
+		 (unsigned long long) q->V);
 
-	if (qfq_update_class(grp, cl)) {
-		u64 old_F = grp->F;
+	return skb;
+}
 
-		cl = qfq_slot_scan(grp);
-		if (!cl)
-			__clear_bit(grp->index, &q->bitmaps[ER]);
-		else {
-			u64 roundedS = qfq_round_down(cl->S, grp->slot_shift);
-			unsigned int s;
+static struct qfq_aggregate *qfq_choose_next_agg(struct qfq_sched *q)
+{
+	struct qfq_group *grp;
+	struct qfq_aggregate *agg, *new_front_agg;
+	u64 old_F;
 
-			if (grp->S == roundedS)
-				goto skip_unblock;
-			grp->S = roundedS;
-			grp->F = roundedS + (2ULL << grp->slot_shift);
-			__clear_bit(grp->index, &q->bitmaps[ER]);
-			s = qfq_calc_state(q, grp);
-			__set_bit(grp->index, &q->bitmaps[s]);
-		}
+	qfq_update_eligible(q);
+	q->oldV = q->V;
+
+	if (!q->bitmaps[ER])
+		return NULL;
+
+	grp = qfq_ffs(q, q->bitmaps[ER]);
+	old_F = grp->F;
 
-		qfq_unblock_groups(q, grp->index, old_F);
+	agg = qfq_slot_head(grp);
+
+	/* agg starts to be served, remove it from schedule */
+	qfq_front_slot_remove(grp);
+
+	new_front_agg = qfq_slot_scan(grp);
+
+	if (new_front_agg == NULL) /* group is now inactive, remove from ER */
+		__clear_bit(grp->index, &q->bitmaps[ER]);
+	else {
+		u64 roundedS = qfq_round_down(new_front_agg->S,
+					      grp->slot_shift);
+		unsigned int s;
+
+		if (grp->S == roundedS)
+			return agg;
+		grp->S = roundedS;
+		grp->F = roundedS + (2ULL << grp->slot_shift);
+		__clear_bit(grp->index, &q->bitmaps[ER]);
+		s = qfq_calc_state(q, grp);
+		__set_bit(grp->index, &q->bitmaps[s]);
 	}
 
-skip_unblock:
-	qfq_update_eligible(q, old_V);
+	qfq_unblock_groups(q, grp->index, old_F);
 
-	return skb;
+	return agg;
 }
 
 /*
- * Assign a reasonable start time for a new flow k in group i.
+ * Assign a reasonable start time for a new aggregate in group i.
  * Admissible values for \hat(F) are multiples of \sigma_i
  * no greater than V+\sigma_i . Larger values mean that
  * we had a wraparound so we consider the timestamp to be stale.
  *
  * If F is not stale and F >= V then we set S = F.
  * Otherwise we should assign S = V, but this may violate
- * the ordering in ER. So, if we have groups in ER, set S to
- * the F_j of the first group j which would be blocking us.
+ * the ordering in EB (see [2]). So, if we have groups in ER,
+ * set S to the F_j of the first group j which would be blocking us.
  * We are guaranteed not to move S backward because
  * otherwise our group i would still be blocked.
  */
-static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl)
+static void qfq_update_start(struct qfq_sched *q, struct qfq_aggregate *agg)
 {
 	unsigned long mask;
 	u64 limit, roundedF;
-	int slot_shift = cl->grp->slot_shift;
+	int slot_shift = agg->grp->slot_shift;
 
-	roundedF = qfq_round_down(cl->F, slot_shift);
+	roundedF = qfq_round_down(agg->F, slot_shift);
 	limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift);
 
-	if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) {
+	if (!qfq_gt(agg->F, q->V) || qfq_gt(roundedF, limit)) {
 		/* timestamp was stale */
-		mask = mask_from(q->bitmaps[ER], cl->grp->index);
+		mask = mask_from(q->bitmaps[ER], agg->grp->index);
 		if (mask) {
 			struct qfq_group *next = qfq_ffs(q, mask);
 			if (qfq_gt(roundedF, next->F)) {
 				if (qfq_gt(limit, next->F))
-					cl->S = next->F;
+					agg->S = next->F;
 				else /* preserve timestamp correctness */
-					cl->S = limit;
+					agg->S = limit;
 				return;
 			}
 		}
-		cl->S = q->V;
+		agg->S = q->V;
 	} else  /* timestamp is not stale */
-		cl->S = cl->F;
+		agg->S = agg->F;
+}
+
+/*
+ * Update the timestamps of agg before scheduling/rescheduling it for
+ * service.  In particular, assign to agg->F its maximum possible
+ * value, i.e., the virtual finish time with which the aggregate
+ * should be labeled if it used all its budget once in service.
+ */
+static inline void
+qfq_update_agg_ts(struct qfq_sched *q,
+		    struct qfq_aggregate *agg, enum update_reason reason)
+{
+	if (reason != requeue)
+		qfq_update_start(q, agg);
+	else /* just charge agg for the service received */
+		agg->S = agg->F;
+
+	agg->F = agg->S + (u64)agg->budgetmax * agg->inv_w;
 }
 
+static void qfq_schedule_agg(struct qfq_sched *, struct qfq_aggregate *);
+
 static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
 	struct qfq_class *cl;
+	struct qfq_aggregate *agg;
 	int err = 0;
 
 	cl = qfq_classify(skb, sch, &err);
@@ -934,11 +1188,13 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	}
 	pr_debug("qfq_enqueue: cl = %x\n", cl->common.classid);
 
-	if (unlikely(cl->lmax < qdisc_pkt_len(skb))) {
+	if (unlikely(cl->agg->lmax < qdisc_pkt_len(skb))) {
 		pr_debug("qfq: increasing maxpkt from %u to %u for class %u",
-			  cl->lmax, qdisc_pkt_len(skb), cl->common.classid);
-		qfq_update_reactivate_class(q, cl, cl->inv_w,
-					    qdisc_pkt_len(skb), 0);
+			 cl->agg->lmax, qdisc_pkt_len(skb), cl->common.classid);
+		err = qfq_change_agg(sch, cl, cl->agg->class_weight,
+				     qdisc_pkt_len(skb));
+		if (err)
+			return err;
 	}
 
 	err = qdisc_enqueue(skb, cl->qdisc);
@@ -954,35 +1210,50 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	bstats_update(&cl->bstats, skb);
 	++sch->q.qlen;
 
-	/* If the new skb is not the head of queue, then done here. */
-	if (cl->qdisc->q.qlen != 1)
+	agg = cl->agg;
+	/* if the queue was not empty, then done here */
+	if (cl->qdisc->q.qlen != 1) {
+		if (unlikely(skb == cl->qdisc->ops->peek(cl->qdisc)) &&
+		    list_first_entry(&agg->active, struct qfq_class, alist)
+		    == cl && cl->deficit < qdisc_pkt_len(skb))
+			list_move_tail(&cl->alist, &agg->active);
+
 		return err;
+	}
+
+	/* schedule class for service within the aggregate */
+	cl->deficit = agg->lmax;
+	list_add_tail(&cl->alist, &agg->active);
+
+	if (list_first_entry(&agg->active, struct qfq_class, alist) != cl)
+		return err; /* aggregate was not empty, nothing else to do */
+
+	/* recharge budget */
+	agg->initial_budget = agg->budget = agg->budgetmax;
 
-	/* If reach this point, queue q was idle */
-	qfq_activate_class(q, cl, qdisc_pkt_len(skb));
+	qfq_update_agg_ts(q, agg, enqueue);
+	if (q->in_serv_agg == NULL)
+		q->in_serv_agg = agg;
+	else if (agg != q->in_serv_agg)
+		qfq_schedule_agg(q, agg);
 
 	return err;
 }
 
 /*
- * Handle class switch from idle to backlogged.
+ * Schedule aggregate according to its timestamps.
  */
-static void qfq_activate_class(struct qfq_sched *q, struct qfq_class *cl,
-			       unsigned int pkt_len)
+static void qfq_schedule_agg(struct qfq_sched *q, struct qfq_aggregate *agg)
 {
-	struct qfq_group *grp = cl->grp;
+	struct qfq_group *grp = agg->grp;
 	u64 roundedS;
 	int s;
 
-	qfq_update_start(q, cl);
-
-	/* compute new finish time and rounded start. */
-	cl->F = cl->S + (u64)pkt_len * cl->inv_w;
-	roundedS = qfq_round_down(cl->S, grp->slot_shift);
+	roundedS = qfq_round_down(agg->S, grp->slot_shift);
 
 	/*
-	 * insert cl in the correct bucket.
-	 * If cl->S >= grp->S we don't need to adjust the
+	 * Insert agg in the correct bucket.
+	 * If agg->S >= grp->S we don't need to adjust the
 	 * bucket list and simply go to the insertion phase.
 	 * Otherwise grp->S is decreasing, we must make room
 	 * in the bucket list, and also recompute the group state.
@@ -990,10 +1261,10 @@ static void qfq_activate_class(struct qfq_sched *q, struct qfq_class *cl,
 	 * was in ER make sure to adjust V.
 	 */
 	if (grp->full_slots) {
-		if (!qfq_gt(grp->S, cl->S))
+		if (!qfq_gt(grp->S, agg->S))
 			goto skip_update;
 
-		/* create a slot for this cl->S */
+		/* create a slot for this agg->S */
 		qfq_slot_rotate(grp, roundedS);
 		/* group was surely ineligible, remove */
 		__clear_bit(grp->index, &q->bitmaps[IR]);
@@ -1008,46 +1279,61 @@ static void qfq_activate_class(struct qfq_sched *q, struct qfq_class *cl,
 
 	pr_debug("qfq enqueue: new state %d %#lx S %lld F %lld V %lld\n",
 		 s, q->bitmaps[s],
-		 (unsigned long long) cl->S,
-		 (unsigned long long) cl->F,
+		 (unsigned long long) agg->S,
+		 (unsigned long long) agg->F,
 		 (unsigned long long) q->V);
 
 skip_update:
-	qfq_slot_insert(grp, cl, roundedS);
+	qfq_slot_insert(grp, agg, roundedS);
 }
 
 
+/* Update agg ts and schedule agg for service */
+static void qfq_activate_agg(struct qfq_sched *q, struct qfq_aggregate *agg,
+			     enum update_reason reason)
+{
+	qfq_update_agg_ts(q, agg, reason);
+	qfq_schedule_agg(q, agg);
+}
+
 static void qfq_slot_remove(struct qfq_sched *q, struct qfq_group *grp,
-			    struct qfq_class *cl)
+			    struct qfq_aggregate *agg)
 {
 	unsigned int i, offset;
 	u64 roundedS;
 
-	roundedS = qfq_round_down(cl->S, grp->slot_shift);
+	roundedS = qfq_round_down(agg->S, grp->slot_shift);
 	offset = (roundedS - grp->S) >> grp->slot_shift;
+
 	i = (grp->front + offset) % QFQ_MAX_SLOTS;
 
-	hlist_del(&cl->next);
+	hlist_del(&agg->next);
 	if (hlist_empty(&grp->slots[i]))
 		__clear_bit(offset, &grp->full_slots);
 }
 
 /*
- * called to forcibly destroy a queue.
- * If the queue is not in the front bucket, or if it has
- * other queues in the front bucket, we can simply remove
- * the queue with no other side effects.
+ * Called to forcibly deschedule an aggregate.  If the aggregate is
+ * not in the front bucket, or if the latter has other aggregates in
+ * the front bucket, we can simply remove the aggregate with no other
+ * side effects.
  * Otherwise we must propagate the event up.
  */
-static void qfq_deactivate_class(struct qfq_sched *q, struct qfq_class *cl)
+static void qfq_deactivate_agg(struct qfq_sched *q, struct qfq_aggregate *agg)
 {
-	struct qfq_group *grp = cl->grp;
+	struct qfq_group *grp = agg->grp;
 	unsigned long mask;
 	u64 roundedS;
 	int s;
 
-	cl->F = cl->S;
-	qfq_slot_remove(q, grp, cl);
+	if (agg == q->in_serv_agg) {
+		charge_actual_service(agg);
+		q->in_serv_agg = qfq_choose_next_agg(q);
+		return;
+	}
+
+	agg->F = agg->S;
+	qfq_slot_remove(q, grp, agg);
 
 	if (!grp->full_slots) {
 		__clear_bit(grp->index, &q->bitmaps[IR]);
@@ -1066,8 +1352,8 @@ static void qfq_deactivate_class(struct qfq_sched *q, struct qfq_class *cl)
 		}
 		__clear_bit(grp->index, &q->bitmaps[ER]);
 	} else if (hlist_empty(&grp->slots[grp->front])) {
-		cl = qfq_slot_scan(grp);
-		roundedS = qfq_round_down(cl->S, grp->slot_shift);
+		agg = qfq_slot_scan(grp);
+		roundedS = qfq_round_down(agg->S, grp->slot_shift);
 		if (grp->S != roundedS) {
 			__clear_bit(grp->index, &q->bitmaps[ER]);
 			__clear_bit(grp->index, &q->bitmaps[IR]);
@@ -1080,7 +1366,7 @@ static void qfq_deactivate_class(struct qfq_sched *q, struct qfq_class *cl)
 		}
 	}
 
-	qfq_update_eligible(q, q->V);
+	qfq_update_eligible(q);
 }
 
 static void qfq_qlen_notify(struct Qdisc *sch, unsigned long arg)
@@ -1092,6 +1378,32 @@ static void qfq_qlen_notify(struct Qdisc *sch, unsigned long arg)
 		qfq_deactivate_class(q, cl);
 }
 
+static unsigned int qfq_drop_from_slot(struct qfq_sched *q,
+				       struct hlist_head *slot)
+{
+	struct qfq_aggregate *agg;
+	struct hlist_node *n;
+	struct qfq_class *cl;
+	unsigned int len;
+
+	hlist_for_each_entry(agg, n, slot, next) {
+		list_for_each_entry(cl, &agg->active, alist) {
+
+			if (!cl->qdisc->ops->drop)
+				continue;
+
+			len = cl->qdisc->ops->drop(cl->qdisc);
+			if (len > 0) {
+				if (cl->qdisc->q.qlen == 0)
+					qfq_deactivate_class(q, cl);
+
+				return len;
+			}
+		}
+	}
+	return 0;
+}
+
 static unsigned int qfq_drop(struct Qdisc *sch)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
@@ -1101,24 +1413,13 @@ static unsigned int qfq_drop(struct Qdisc *sch)
 	for (i = 0; i <= QFQ_MAX_INDEX; i++) {
 		grp = &q->groups[i];
 		for (j = 0; j < QFQ_MAX_SLOTS; j++) {
-			struct qfq_class *cl;
-			struct hlist_node *n;
-
-			hlist_for_each_entry(cl, n, &grp->slots[j], next) {
-
-				if (!cl->qdisc->ops->drop)
-					continue;
-
-				len = cl->qdisc->ops->drop(cl->qdisc);
-				if (len > 0) {
-					sch->q.qlen--;
-					if (!cl->qdisc->q.qlen)
-						qfq_deactivate_class(q, cl);
-
-					return len;
-				}
+			len = qfq_drop_from_slot(q, &grp->slots[j]);
+			if (len > 0) {
+				sch->q.qlen--;
+				return len;
 			}
 		}
+
 	}
 
 	return 0;
@@ -1129,44 +1430,51 @@ static int qfq_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
 	struct qfq_sched *q = qdisc_priv(sch);
 	struct qfq_group *grp;
 	int i, j, err;
+	u32 max_cl_shift, maxbudg_shift, max_classes;
 
 	err = qdisc_class_hash_init(&q->clhash);
 	if (err < 0)
 		return err;
 
+	if (qdisc_dev(sch)->tx_queue_len + 1 > QFQ_MAX_AGG_CLASSES)
+		max_classes = QFQ_MAX_AGG_CLASSES;
+	else
+		max_classes = qdisc_dev(sch)->tx_queue_len + 1;
+	/* max_cl_shift = floor(log_2(max_classes)) */
+	max_cl_shift = __fls(max_classes);
+	q->max_agg_classes = 1<<max_cl_shift;
+
+	/* maxbudg_shift = log2(max_len * max_classes_per_agg) */
+	maxbudg_shift = QFQ_MTU_SHIFT + max_cl_shift;
+	q->min_slot_shift = FRAC_BITS + maxbudg_shift - QFQ_MAX_INDEX;
+
 	for (i = 0; i <= QFQ_MAX_INDEX; i++) {
 		grp = &q->groups[i];
 		grp->index = i;
-		grp->slot_shift = QFQ_MTU_SHIFT + FRAC_BITS
-				   - (QFQ_MAX_INDEX - i);
+		grp->slot_shift = q->min_slot_shift + i;
 		for (j = 0; j < QFQ_MAX_SLOTS; j++)
 			INIT_HLIST_HEAD(&grp->slots[j]);
 	}
 
+	INIT_HLIST_HEAD(&q->nonfull_aggs);
+
 	return 0;
 }
 
 static void qfq_reset_qdisc(struct Qdisc *sch)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
-	struct qfq_group *grp;
 	struct qfq_class *cl;
-	struct hlist_node *n, *tmp;
-	unsigned int i, j;
+	struct hlist_node *n;
+	unsigned int i;
 
-	for (i = 0; i <= QFQ_MAX_INDEX; i++) {
-		grp = &q->groups[i];
-		for (j = 0; j < QFQ_MAX_SLOTS; j++) {
-			hlist_for_each_entry_safe(cl, n, tmp,
-						  &grp->slots[j], next) {
+	for (i = 0; i < q->clhash.hashsize; i++) {
+		hlist_for_each_entry(cl, n, &q->clhash.hash[i], common.hnode) {
+			if (cl->qdisc->q.qlen > 0)
 				qfq_deactivate_class(q, cl);
-			}
-		}
-	}
 
-	for (i = 0; i < q->clhash.hashsize; i++) {
-		hlist_for_each_entry(cl, n, &q->clhash.hash[i], common.hnode)
 			qdisc_reset(cl->qdisc);
+		}
 	}
 	sch->q.qlen = 0;
 }
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: Paolo Valente @ 2012-11-20 17:45 UTC (permalink / raw)
  To: David Miller; +Cc: jhs, shemminger, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <20121119.184828.628106002307042971.davem@davemloft.net>

Il 20/11/2012 00:48, David Miller ha scritto:
> From: Paolo Valente <paolo.valente@unimore.it>
> Date: Mon, 12 Nov 2012 17:48:33 +0100
>
>> [This patch received positive feedback from Stephen Hemminger ("put in
>> net-next"), but no further feedback or decision. So I am (re)sending
>> an updated version of it. The only differences with respect to the
>> previous version are the support for TSO/GSO (taken from QFQ), and a
>> hopefully improved description.]
>
> Can you rearrange the logic so that the compiler doesn't emit this
> warning?
>
> In file included from net/sched/sch_qfq.c:18:0:
> net/sched/sch_qfq.c: In function ‘qfq_dequeue’:
> include/net/sch_generic.h:480:15: warning: ‘skb’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/sched/sch_qfq.c:1007:18: note: ‘skb’ was declared here
>
> You and I both know that SKB will be initialized at this point, but
> the compiler can't see it clearly enough.
>
Unfortunately I could not reproduce the warning (with
gcc-4.7 -Wmaybe-uninitialized). I am however about to send a new version 
with skb initialized to NULL. I hope that this fix properly addresses 
this issue.
> Thanks.
>


-- 
-----------------------------------------------------------
| Paolo Valente              |                            |
| Algogroup                  |                            |
| Dip. Ing. Informazione     | tel:   +39 059 2056318     |
| Via Vignolese 905/b        | fax:   +39 059 2056129     |
| 41125 Modena - Italy       |                            |
|     home:  http://algo.ing.unimo.it/people/paolo/       |
-----------------------------------------------------------

^ permalink raw reply

* RE: question about eth_header
From: Dmitry Kravkov @ 2012-11-20 17:39 UTC (permalink / raw)
  To: Rami Rosen; +Cc: netdev@vger.kernel.org
In-Reply-To: <CAKoUArnRnx0txGWhNWpZAFBijWmtzdOe-D7Qb62-5AvBCfgHJQ@mail.gmail.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Rami Rosen
> Sent: Tuesday, November 20, 2012 4:51 PM
> To: Dmitry Kravkov
> Cc: netdev@vger.kernel.org
> Subject: Re: question about eth_header
> 
> Hi,
> I think that you should call skb_reset_mac_header()
> before calling eth_hdr() and that skb_reset_mac_header() should not be
> inside eth_hdr(). Following is explanation:
> 
> In the RX path, when we get a packet in the driver, what essentially
> happens is that at this point, we have a pointer to
> the packet buffer in skb->data, and its length in skb->len.

Actually I'm on TX path, and do not intend  to update skb, just received it 
for transmission, I want to pick mac addresess from eth header.
Now bnx2x does it by accessing skb->data (which points to the correct mac)
and I wanted to replace it with eth_hrd instead of doing ugly casting in driver code.
Moreover, in the future, when inner_mac_address will be used for tunneling
acceleration, inner_mac_address will be updated with mac_address offset, and so
may also point to the improper location.   
 
> With ethernet packets, the driver will call eth_type_trans(); please
> look at ethernet drivers code.
> eth_type_trans() indeed sets the MAC header pointer by calling
> skb_reset_mac_header() on the skb.
> 
> Afterwards, the eth_type_trans() will advance skb->data by
> 14, the size of the Ethernet header, and decrease skb->len
> by calling skb_pull_inline(skb, ETH_HLEN).
> 
> In case you are wondering why the skb_reset_mac_header() is not inside
> the eth_hdr(), I think that the reason is that, once
> skb_reset_mac_header() on the skb was called, there are cases when we
> want to make several calls eth_hdr() without each time again resetting
> the mac header, for example, in the bridging code.  There is no reason
> to doing it in terms of performance.

My question is about eth_header() function which is create() callback for
eth_header_ops.

Thanks.

^ permalink raw reply

* RE: [PATCH] ixgbe: fix broken PPTP handling
From: Keller, Jacob E @ 2012-11-20 17:27 UTC (permalink / raw)
  To: Vick, Matthew, Alan Cox, netdev@vger.kernel.org
  Cc: e1000-devel@lists.sourceforge.net
In-Reply-To: <06DFBC1E25D8024DB214DC7F41A3CD34489616B9@ORSMSX101.amr.corp.intel.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Vick, Matthew
> Sent: Tuesday, November 20, 2012 9:12 AM
> To: Alan Cox; netdev@vger.kernel.org
> Cc: e1000-devel@lists.sourceforge.net
> Subject: RE: [PATCH] ixgbe: fix broken PPTP handling
> 
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > owner@vger.kernel.org] On Behalf Of Alan Cox
> > Sent: Tuesday, November 20, 2012 8:33 AM
> > To: netdev@vger.kernel.org
> > Subject: [PATCH] ixgbe: fix broken PPTP handling
> >
> > From: Alan Cox <alan@linux.intel.com>
> >
> > Reported to the maintain 3 weeks ago so now sending a patch rather than
> > waiting. ixgbe passes a random event type to the pptp code
> >
> > Signed-off-by: Alan Cox <alan@linux.intel.com>
> > ---
> >
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    1 +
> >  1 file changed, 1 insertion(+)
> >
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> > index 01d99af..73291fe 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> > @@ -398,6 +398,7 @@ void ixgbe_ptp_check_pps_event(struct ixgbe_adapter
> > *adapter, u32 eicr)
> >
> >  	switch (hw->mac.type) {
> >  	case ixgbe_mac_X540:
> > +		event.type = PTP_CLOCK_PPS;
> >  		ptp_clock_event(adapter->ptp_clock, &event);
> >  		break;
> >  	default:
> 
> CCing e1000-devel, the mailing list for Intel Wired Ethernet support.
> 
> Sorry, Alan--I didn't see any message from you about this before or I
> would have responded. NAK on this, since I don't see how ixgbe is passing
> a random event type. The event type gets set to PTP_CLOCK_PPS just a few
> lines above (before the if (!adapter->ptp_clock) block).
> 
> Matthew

Specifically, commit 3645adbb "ixgbe: fix uninitialized event.type in ixgbe_ptp_check_pps_event"

:)

Sorry I didn't reply directly to your original email with the concern, or CC you. I will do so next time.

Thanks

- Jake

^ permalink raw reply

* Re: [RESEND PATCH net-next 1/1] sit: allow to configure 6rd tunnels via netlink
From: Nicolas Dichtel @ 2012-11-20 17:22 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20121120091748.359d95db@nehalam.linuxnetplumber.net>

Le 20/11/2012 18:17, Stephen Hemminger a écrit :
> On Tue, 20 Nov 2012 18:11:41 +0100
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
>> Le 20/11/2012 18:01, Stephen Hemminger a écrit :
>>> On Tue, 20 Nov 2012 09:41:45 +0100
>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>>>
>>>> This patch add the support of 6RD tunnels management via netlink.
>>>> Note that netdev_state_change() is now called when 6RD parameters are updated.
>>>>
>>>> 6RD parameters are updated only if there is at least one 6RD attribute.
>>>>
>>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>>> ---
>>>>    include/uapi/linux/if_tunnel.h |   4 ++
>>>>    net/ipv6/sit.c                 | 149 ++++++++++++++++++++++++++++++++++-------
>>>>    2 files changed, 128 insertions(+), 25 deletions(-)
>>>>
>>>
>>> Ok, but iproute2 git is currently for 3.6
>>> Please resubmit when 3.7 is finished during 3.8 merge window.
>> Hmm, not sure to understand. This is a kernel patch.
>
>
> I assumed you would need a user space part as well.
>
Oh yes. And I was waiting the proper time to send patches, like for netconf ;-)

^ permalink raw reply

* Re: [RESEND PATCH net-next 1/1] sit: allow to configure 6rd tunnels via netlink
From: Stephen Hemminger @ 2012-11-20 17:17 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: davem, netdev
In-Reply-To: <50ABB9CD.6080506@6wind.com>

On Tue, 20 Nov 2012 18:11:41 +0100
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:

> Le 20/11/2012 18:01, Stephen Hemminger a écrit :
> > On Tue, 20 Nov 2012 09:41:45 +0100
> > Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> >
> >> This patch add the support of 6RD tunnels management via netlink.
> >> Note that netdev_state_change() is now called when 6RD parameters are updated.
> >>
> >> 6RD parameters are updated only if there is at least one 6RD attribute.
> >>
> >> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> >> ---
> >>   include/uapi/linux/if_tunnel.h |   4 ++
> >>   net/ipv6/sit.c                 | 149 ++++++++++++++++++++++++++++++++++-------
> >>   2 files changed, 128 insertions(+), 25 deletions(-)
> >>
> >
> > Ok, but iproute2 git is currently for 3.6
> > Please resubmit when 3.7 is finished during 3.8 merge window.
> Hmm, not sure to understand. This is a kernel patch.


I assumed you would need a user space part as well.

^ permalink raw reply

* RE: [PATCH] ixgbe: fix broken PPTP handling
From: Vick, Matthew @ 2012-11-20 17:12 UTC (permalink / raw)
  To: Alan Cox, netdev@vger.kernel.org; +Cc: e1000-devel@lists.sourceforge.net
In-Reply-To: <20121120163227.11935.57062.stgit@bob.linux.org.uk>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Alan Cox
> Sent: Tuesday, November 20, 2012 8:33 AM
> To: netdev@vger.kernel.org
> Subject: [PATCH] ixgbe: fix broken PPTP handling
> 
> From: Alan Cox <alan@linux.intel.com>
> 
> Reported to the maintain 3 weeks ago so now sending a patch rather than
> waiting. ixgbe passes a random event type to the pptp code
> 
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> ---
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> index 01d99af..73291fe 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> @@ -398,6 +398,7 @@ void ixgbe_ptp_check_pps_event(struct ixgbe_adapter
> *adapter, u32 eicr)
> 
>  	switch (hw->mac.type) {
>  	case ixgbe_mac_X540:
> +		event.type = PTP_CLOCK_PPS;
>  		ptp_clock_event(adapter->ptp_clock, &event);
>  		break;
>  	default:

CCing e1000-devel, the mailing list for Intel Wired Ethernet support.

Sorry, Alan--I didn't see any message from you about this before or I would have responded. NAK on this, since I don't see how ixgbe is passing a random event type. The event type gets set to PTP_CLOCK_PPS just a few lines above (before the if (!adapter->ptp_clock) block).

Matthew

^ permalink raw reply

* Re: [RESEND PATCH net-next 1/1] sit: allow to configure 6rd tunnels via netlink
From: Nicolas Dichtel @ 2012-11-20 17:11 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20121120090113.47ce16b5@nehalam.linuxnetplumber.net>

Le 20/11/2012 18:01, Stephen Hemminger a écrit :
> On Tue, 20 Nov 2012 09:41:45 +0100
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
>> This patch add the support of 6RD tunnels management via netlink.
>> Note that netdev_state_change() is now called when 6RD parameters are updated.
>>
>> 6RD parameters are updated only if there is at least one 6RD attribute.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>>   include/uapi/linux/if_tunnel.h |   4 ++
>>   net/ipv6/sit.c                 | 149 ++++++++++++++++++++++++++++++++++-------
>>   2 files changed, 128 insertions(+), 25 deletions(-)
>>
>
> Ok, but iproute2 git is currently for 3.6
> Please resubmit when 3.7 is finished during 3.8 merge window.
Hmm, not sure to understand. This is a kernel patch.

^ permalink raw reply

* Re: [RESEND PATCH net-next 1/1] sit: allow to configure 6rd tunnels via netlink
From: Stephen Hemminger @ 2012-11-20 17:01 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: davem, netdev
In-Reply-To: <1353400905-28335-1-git-send-email-nicolas.dichtel@6wind.com>

On Tue, 20 Nov 2012 09:41:45 +0100
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:

> This patch add the support of 6RD tunnels management via netlink.
> Note that netdev_state_change() is now called when 6RD parameters are updated.
> 
> 6RD parameters are updated only if there is at least one 6RD attribute.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>  include/uapi/linux/if_tunnel.h |   4 ++
>  net/ipv6/sit.c                 | 149 ++++++++++++++++++++++++++++++++++-------
>  2 files changed, 128 insertions(+), 25 deletions(-)
> 

Ok, but iproute2 git is currently for 3.6
Please resubmit when 3.7 is finished during 3.8 merge window.

^ permalink raw reply

* Re: [PATCH v2] checkpatch: add double empty line check
From: Andy Whitcroft @ 2012-11-20 16:36 UTC (permalink / raw)
  To: Eilon Greenstein; +Cc: Joe Perches, David Rientjes, linux-kernel, netdev
In-Reply-To: <1353428544.6559.26.camel@lb-tlvb-eilong.il.broadcom.com>

On Tue, Nov 20, 2012 at 06:22:24PM +0200, Eilon Greenstein wrote:
> On Tue, 2012-11-20 at 16:14 +0000, Andy Whitcroft wrote:
> > On Tue, Nov 20, 2012 at 06:06:10PM +0200, Eilon Greenstein wrote:
> > > I'm only testing the nextline if the current line is newly added. If I
> > > got it right, when a line is newly added, the next line can be:
> > > a. another new line
> > > b. existing line (provided for context)
> > > c. Does not exist since this is the end of the file (I missed this one
> > > originally)
> > > 
> > > It cannot just jump to the next hunk and it cannot be a deleted line,
> > > right?
> > 
> > Mostly that would be true.  If the hunk is the last hunk and adds lines
> > at the bottom of a file _and_ the context around it has blank lines then
> > something.  I think that would trip up this algorithm, reporting beyond
> > the end of the hunk perhaps.
> 
> I do not want to cause any perl warning, but adding a new segment that
> ends with a new empty line above an existing empty line is something
> that I want to catch - so checking the next line (even if it is not new)
> is desired. Do you have other suggestions on how to implement something
> like that?
> 
> I'm not saying that my patch is safe - I already missed a corner case
> when adding a line at the end of the file, but I'm willing to run more
> tests and see if I hit some perl warning. So how about running it on the
> last X changes in the kernel git tree? How many tests are enough to get
> reasonable confidant level?

I have been testing the patches there with some fake files to try and
catch these indeed.  I did incldue my take on how to solve this in
previous replies.

-apw

^ permalink raw reply

* Re: [PATCH v2] checkpatch: add double empty line check
From: Andy Whitcroft @ 2012-11-20 16:36 UTC (permalink / raw)
  To: Eilon Greenstein; +Cc: Joe Perches, David Rientjes, linux-kernel, netdev
In-Reply-To: <20121120161417.GA17797@dm>

On Tue, Nov 20, 2012 at 04:14:17PM +0000, Andy Whitcroft wrote:
> On Tue, Nov 20, 2012 at 06:06:10PM +0200, Eilon Greenstein wrote:
> > I'm only testing the nextline if the current line is newly added. If I
> > got it right, when a line is newly added, the next line can be:
> > a. another new line
> > b. existing line (provided for context)
> > c. Does not exist since this is the end of the file (I missed this one
> > originally)
> > 
> > It cannot just jump to the next hunk and it cannot be a deleted line,
> > right?

Oh and in theory at least it could be a - line, though diff never
generates things in that order.

-apw

^ permalink raw reply

* [PATCH] ixgbe: fix broken PPTP handling
From: Alan Cox @ 2012-11-20 16:32 UTC (permalink / raw)
  To: netdev

From: Alan Cox <alan@linux.intel.com>

Reported to the maintain 3 weeks ago so now sending a patch rather
than waiting. ixgbe passes a random event type to the pptp code

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    1 +
 1 file changed, 1 insertion(+)


diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 01d99af..73291fe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -398,6 +398,7 @@ void ixgbe_ptp_check_pps_event(struct ixgbe_adapter *adapter, u32 eicr)
 
 	switch (hw->mac.type) {
 	case ixgbe_mac_X540:
+		event.type = PTP_CLOCK_PPS;
 		ptp_clock_event(adapter->ptp_clock, &event);
 		break;
 	default:

^ permalink raw reply related

* [PATCH] ne2000: add the right platform device
From: Alan Cox @ 2012-11-20 16:31 UTC (permalink / raw)
  To: netdev

From: Alan Cox <alan@linux.intel.com>

Without this udev doesn't have a way to key the ne device to the platform
device.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/net/ethernet/8390/ne.c |    1 +
 1 file changed, 1 insertion(+)


diff --git a/drivers/net/ethernet/8390/ne.c b/drivers/net/ethernet/8390/ne.c
index d04911d..47618e5 100644
--- a/drivers/net/ethernet/8390/ne.c
+++ b/drivers/net/ethernet/8390/ne.c
@@ -813,6 +813,7 @@ static int __init ne_drv_probe(struct platform_device *pdev)
 		dev->irq = irq[this_dev];
 		dev->mem_end = bad[this_dev];
 	}
+	SET_NETDEV_DEV(dev, &pdev->dev);
 	err = do_ne_probe(dev);
 	if (err) {
 		free_netdev(dev);

^ permalink raw reply related

* Re: [PATCH V2] xen/netfront: handle compound page fragments on transmit
From: Eric Dumazet @ 2012-11-20 16:27 UTC (permalink / raw)
  To: Ian Campbell
  Cc: netdev, xen-devel, Eric Dumazet, Konrad Rzeszutek Wilk, ANNIE LI,
	Sander Eikelenboom, Stefan Bader
In-Reply-To: <1353427257-16789-1-git-send-email-ian.campbell@citrix.com>

On Tue, 2012-11-20 at 16:00 +0000, Ian Campbell wrote:
> An SKB paged fragment can consist of a compound page with order > 0.
> However the netchannel protocol deals only in PAGE_SIZE frames.
> 
> Handle this in xennet_make_frags by iterating over the frames which
> make up the page.
> 
> This is the netfront equivalent to 6a8ed462f16b for netback.
...

> -	frags += DIV_ROUND_UP(offset + len, PAGE_SIZE);
> -	if (unlikely(frags > MAX_SKB_FRAGS + 1)) {
> -		printk(KERN_ALERT "xennet: skb rides the rocket: %d frags\n",
> -		       frags);
> -		dump_stack();
> +	slots = DIV_ROUND_UP(offset + len, PAGE_SIZE) +
> +		xennet_count_skb_frag_slots(skb);
> +	if (unlikely(slots > MAX_SKB_FRAGS + 1)) {
> +		printk(KERN_ALERT "xennet: skb rides the rocket: %d slots\n",
> +		       slots);

I think this is wrong.

You should change netfront_tx_slot_available() to stop the queue before
this can happen.

Yes, you dont hit this on your tests, but a driver should not drop a
good packet.

>  		goto drop;
>  	}
>  
>  	spin_lock_irqsave(&np->tx_lock, flags);
>  
>  	if (unlikely(!netif_carrier_ok(dev) ||
> -		     (frags > 1 && !xennet_can_sg(dev)) ||
> +		     (slots > 1 && !xennet_can_sg(dev)) ||
>  		     netif_needs_gso(skb, netif_skb_features(skb)))) {
>  		spin_unlock_irqrestore(&np->tx_lock, flags);
>  		goto drop;


diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index caa0110..cb1e605 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -215,10 +215,13 @@ static void rx_refill_timeout(unsigned long data)
        napi_schedule(&np->napi);
 }
 
+/* Considering a 64Kb packet of 16 frags, each frag can be mapped
+ * to 3 order-0 parts on pathological cases
+ */
 static int netfront_tx_slot_available(struct netfront_info *np)
 {
        return (np->tx.req_prod_pvt - np->tx.rsp_cons) <
-               (TX_MAX_TARGET - MAX_SKB_FRAGS - 2);
+               (TX_MAX_TARGET - 3*MAX_SKB_FRAGS - 2);
 }
 
 static void xennet_maybe_wake_tx(struct net_device *dev)

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox