* 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
* [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: 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
* 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® Ethernet, visit http://communities.intel.com/community/wired
^ 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] 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
* 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
* /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: 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
* 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: /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: [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: [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: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: Stephen Hemminger @ 2012-11-20 18:44 UTC (permalink / raw)
To: David Miller; +Cc: paolo.valente, jhs, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <20121120.131516.311189144173456123.davem@davemloft.net>
On Tue, 20 Nov 2012 13:15:16 -0500 (EST)
David Miller <davem@davemloft.net> wrote:
> 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.
Probably. that's it. Plus Fedora has more gcc maintainers.
^ permalink raw reply
* Re: [PATCH] sctp: send abort chunk when max_retrans exceeded
From: Vlad Yasevich @ 2012-11-20 18:44 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller, linux-sctp
In-Reply-To: <1353434344-17864-1-git-send-email-nhorman@tuxdriver.com>
On 11/20/2012 12:59 PM, Neil Horman wrote:
> 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;
> +}
Neil
You ended dropping the parameter information of the parameter that
caused the violation. Was that intentional?
-vlad
> +
> +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));
>
>
^ permalink raw reply
* Re: [RESEND PATCH net-next 1/1] sit: allow to configure 6rd tunnels via netlink
From: David Miller @ 2012-11-20 18:45 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev, shemminger
In-Reply-To: <1353400905-28335-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 20 Nov 2012 09:41:45 +0100
> 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>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/1] bnx2x: Remove duplicate inclusion of bnx2x_hsi.h
From: David Miller @ 2012-11-20 18:47 UTC (permalink / raw)
To: sachin.kamat; +Cc: netdev, eilong, patches
In-Reply-To: <1353325931-1803-1-git-send-email-sachin.kamat@linaro.org>
From: Sachin Kamat <sachin.kamat@linaro.org>
Date: Mon, 19 Nov 2012 17:22:11 +0530
> bnx2x_hsi.h was included twice.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] ne2000: add the right platform device
From: David Miller @ 2012-11-20 18:49 UTC (permalink / raw)
To: alan; +Cc: netdev
In-Reply-To: <20121120163157.11874.74810.stgit@bob.linux.org.uk>
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Tue, 20 Nov 2012 16:31:57 +0000
> 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>
Applied and queued up for -stable, thanks Alan.
^ permalink raw reply
* Re: [PATCH] pkt_sched: QFQ Plus: fair-queueing service at DRR cost
From: David Miller @ 2012-11-20 18:54 UTC (permalink / raw)
To: paolo.valente; +Cc: jhs, shemminger, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <20121120174513.GA21618@paolo-ThinkPad-W520>
From: Paolo Valente <paolo.valente@unimore.it>
Date: Tue, 20 Nov 2012 18:45:13 +0100
> - struct sk_buff *skb;
> + struct sk_buff *skb = NULL;
This is not really an improvement, now the compiler can think
that NULL is passed eventually into qdisc_bstats_update().
Please make the logic easier for the compiler to digest.
For example, restructure the top-level logic into something like:
skb = NULL;
if (!list_empty(&in_serv_agg->active))
skb = qfq_peek_skb(in_serv_agg, &cl, &len);
else
len = 0; /* no more active classes in the in-service agg */
if (len == 0 || in_serv_agg->budget < len) {
...
/*
* 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;
That way it is clearer, to both humans and the compiler, what is
going on here.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next] sockopt: Change getsockopt() of SO_BINDTODEVICE to return an interface name
From: David Miller @ 2012-11-20 18:58 UTC (permalink / raw)
To: brian.haley; +Cc: xemul, eric.dumazet, netdev
In-Reply-To: <50A6A8FB.3050901@hp.com>
From: Brian Haley <brian.haley@hp.com>
Date: Fri, 16 Nov 2012 15:58:35 -0500
> Instead of having the getsockopt() of SO_BINDTODEVICE return an index, which
> will then require another call like if_indextoname() to get the actual interface
> name, have it return the name directly.
>
> This also matches the existing man page description on socket(7) which mentions
> the argument being an interface name.
>
> If the value has not been set, zero is returned and optlen will be set to zero
> to indicate there is no interface name present.
>
> Added a seqlock to protect this code path, and dev_ifname(), from someone
> changing the device name via dev_change_name().
>
> v2: Added seqlock protection while copying device name.
>
> Signed-off-by: Brian Haley <brian.haley@hp.com>
Brian I was going to apply this, but something about how you email
patches results in them being corrupted.
Go to:
http://patchwork.ozlabs.org/patch/199732/
Click on Download "mbox", and try to apply that to the net-next tree
to see what I mean.
^ permalink raw reply
* Re: [PATCH v2] checkpatch: add double empty line check
From: Eilon Greenstein @ 2012-11-20 19:10 UTC (permalink / raw)
To: Andy Whitcroft
Cc: Joe Perches, David Rientjes, linux-kernel@vger.kernel.org, netdev
In-Reply-To: <20121120163607.GB17797@dm>
On Tue, 2012-11-20 at 18:36 +0200, Andy Whitcroft wrote:
> 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.
>
Andy - I could not find any other perl warnings. In the current
suggestion, only $line and $prevline are being accessed unconditionally
and $rawlines[$linenr] is accessed only after checking it exists - so it
seems safe.
About the logic - true, if diff will show deleted lines after newly
added lines, some new double line segments will be missed. However, it
seems like few other things will break if diff will start acting out
like that. The suggestion you posted earlier will miss those as well,
and starting to check for this weird case (of deleted lines after the
added lines) does not seem right.
So this patch seems safe, it does not generate false positives and it
does catch all the cases we can currently generate with diff - can you
please consider applying it to checkpatch?
I'm posting it again below for easier reference, please let me know if
you would like me to send it in a clean email separately.
Thanks,
Eilon
>From 403038375a9c09046631f674d14c221758a0de61 Mon Sep 17 00:00:00 2001
From: Eilon Greenstein <eilong@broadcom.com>
Date: Tue, 20 Nov 2012 21:05:30 +0200
Subject: [PATCH] checkpatch: add double empty line check
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
scripts/checkpatch.pl | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 21a9f5d..c0c610c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3579,6 +3579,19 @@ sub process {
WARN("EXPORTED_WORLD_WRITABLE",
"Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr);
}
+
+# check for double empty lines
+ if ($line =~ /^\+\s*$/) {
+ my $nextline = "";
+ if (defined($rawlines[$linenr])) {
+ $nextline = $rawlines[$linenr];
+ }
+ if ($nextline =~ /^\s*$/ ||
+ $prevline =~ /^\+?\s*$/ && $nextline !~ /^\+\s*$/) {
+ CHK("DOUBLE_EMPTY_LINE",
+ "One empty line should be sufficient. Consider removing this one.\n" . $herecurr);
+ }
+ }
}
# If we have no input at all, then there is nothing to report on
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] sctp: send abort chunk when max_retrans exceeded
From: Neil Horman @ 2012-11-20 19:15 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, David S. Miller, linux-sctp
In-Reply-To: <50ABCF87.2090901@gmail.com>
On Tue, Nov 20, 2012 at 01:44:23PM -0500, Vlad Yasevich wrote:
> On 11/20/2012 12:59 PM, Neil Horman wrote:
> >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;
> >+}
>
> Neil
>
> You ended dropping the parameter information of the parameter that
> caused the violation. Was that intentional?
>
Yes, it was, because theres not really IMO a specific parameter that causes this
abort condition. If a chunk needs to be resent more than max_retrans times, we
abort the connection, theres no specific parameter that we can point to that
says "this caused the problem", we're just aborting because we can't get a SACK
from the peer. Likewise, I can't think of any information that we can include
that would give the peer, or the anyone tcpdumping the connection an improved
view as to why the abort happened, beyond the string this patch currently
includes.
I know I had privately sent you an early version of the patch as a rough draft
which did include space for a param header, but that patch never filled that
space out, since we don't have any valid information to fill it out with.
Thanks & Regards
Neil
^ permalink raw reply
* re: team: add broadcast mode
From: Dan Carpenter @ 2012-11-20 19:23 UTC (permalink / raw)
To: jpirko; +Cc: netdev
Hello Jiri Pirko,
The patch 5fc889911a99: "team: add broadcast mode" from Jul 11, 2012,
leads to the following Smatch warning:
drivers/net/team/team_mode_broadcast.c:46 bc_transmit()
warn: signedness bug returning '18446744073709551516'
The error message sucks because 18446744073709551516 is -100 as an int,
and I'm not sure how it figures this returns -100...
But actually there is a signedness bug in bc_transmit(). We return
error codes from team_dev_queue_xmit() and cast them to 1 as bool. This
function is supposed to return 1 on success but instead it returns zero.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] sctp: send abort chunk when max_retrans exceeded
From: Vlad Yasevich @ 2012-11-20 19:28 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller, linux-sctp
In-Reply-To: <20121120191514.GA10287@shamino.rdu.redhat.com>
On 11/20/2012 02:15 PM, Neil Horman wrote:
> On Tue, Nov 20, 2012 at 01:44:23PM -0500, Vlad Yasevich wrote:
>> On 11/20/2012 12:59 PM, Neil Horman wrote:
>>> 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;
>>> +}
>>
>> Neil
>>
>> You ended dropping the parameter information of the parameter that
>> caused the violation. Was that intentional?
>>
> Yes, it was, because theres not really IMO a specific parameter that causes this
> abort condition.
Sure there is. You changed sctp_make_violation_paramlen() which is
called when we receive a protocol parameter which has an invalid length.
This triggers a violation and the parameter is report back. This has
nothing to do with max_rtx overflow.
The new code doesn't have to include parameter information and I am fine
with that.
-vlad
> If a chunk needs to be resent more than max_retrans times, we
> abort the connection, theres no specific parameter that we can point to that
> says "this caused the problem", we're just aborting because we can't get a SACK
> from the peer. Likewise, I can't think of any information that we can include
> that would give the peer, or the anyone tcpdumping the connection an improved
> view as to why the abort happened, beyond the string this patch currently
> includes.
>
> I know I had privately sent you an early version of the patch as a rough draft
> which did include space for a param header, but that patch never filled that
> space out, since we don't have any valid information to fill it out with.
>
> Thanks & Regards
> Neil
>
Neil
^ permalink raw reply
* Re: [PATCH v2] checkpatch: add double empty line check
From: Andy Whitcroft @ 2012-11-20 19:32 UTC (permalink / raw)
To: Eilon Greenstein
Cc: Joe Perches, David Rientjes, linux-kernel@vger.kernel.org, netdev
In-Reply-To: <1353438635.10779.10.camel@lb-tlvb-eilong.il.broadcom.com>
On Tue, Nov 20, 2012 at 09:10:35PM +0200, Eilon Greenstein wrote:
> About the logic - true, if diff will show deleted lines after newly
> added lines, some new double line segments will be missed. However, it
> seems like few other things will break if diff will start acting out
> like that. The suggestion you posted earlier will miss those as well,
> and starting to check for this weird case (of deleted lines after the
> added lines) does not seem right.
Actually the version I sent should indeed cope with the deleted lines
regardless of order. It was cirtainly intended to.
-apw
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox