lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 28 Sep 2013 10:06:37 -0700 From: Stephen Hemminger <stephen@...workplumber.org> To: Vijay Subramanian <subramanian.vijay@...il.com> Cc: netdev@...r.kernel.org Subject: Re: [PATCH v1 net-next] net: pkt_sched: PIE AQM scheme Thanks for submitting this, it is in ok shape, but I still see several issues. > diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h > index f2624b5..2fb6e6d 100644 > --- a/include/uapi/linux/pkt_sched.h > +++ b/include/uapi/linux/pkt_sched.h > @@ -787,4 +787,30 @@ struct tc_fq_qd_stats { > __u32 throttled_flows; > __u32 pad; > }; > + > +/*PIE*/ > +enum { > + TCA_PIE_UNSPEC, > + TCA_PIE_TARGET, > + TCA_PIE_LIMIT, > + TCA_PIE_TUPDATE, > + TCA_PIE_ALPHA, > + TCA_PIE_BETA, > + TCA_PIE_ECN, > + TCA_PIE_BYTEMODE, > + __TCA_PIE_MAX > +}; > +#define TCA_PIE_MAX (__TCA_PIE_MAX - 1) > + > +struct tc_pie_xstats { > + __u32 prob; /* current probability */ > + __u32 delay; /* current delay in ms */ > + __u32 avg_dq_rate; /* current average dq_rate in bits/pie_time */ > + __u32 packets_in; /* total number of packets enqueued */ > + __u32 dropped; /* packets dropped due to pie_action */ > + __u32 overlimit; /* dropped due to lack of space in queue */ > + __u32 maxq; /* maximum queue size */ > + __u32 ecn_mark; /* packets marked with ecn*/ > +}; > + > #endif > diff --git a/net/sched/Kconfig b/net/sched/Kconfig > index c03a32a..7b32e58 100644 > --- a/net/sched/Kconfig > +++ b/net/sched/Kconfig > @@ -286,6 +286,17 @@ config NET_SCH_FQ > > If unsure, say N. > > +config NET_SCH_PIE > + tristate "Proportianal Enhanced Controller AQM (PIE)" Spelling should be: Proportional > diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c > new file mode 100644 > index 0000000..cfcfde9 > --- /dev/null > +++ b/net/sched/sch_pie.c > @@ -0,0 +1,623 @@ > +/* Copyright (C) 2013 Cisco Systems, Inc, 2013. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, > + * USA. > + * > + * Author: Vijay Subramanian <vijaynsu@...co.com> > + * Author: Mythili Prabhu <mysuryan@...co.com> > + * > + * ECN support is added by Naeem Khademi <naeemk@....uio.no> > + * University of Oslo, Norway. > + */ > + > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/types.h> > +#include <linux/kernel.h> > +#include <linux/errno.h> > +#include <linux/skbuff.h> > +#include <net/pkt_sched.h> > +#include <net/inet_ecn.h> > + > +#define PIE_DEFAULT_QUEUE_LIMIT 200 /* in packets */ > +#define QUEUE_THRESHOLD (5000) Useless paren's here > +#define DQCOUNT_INVALID -1 > +#define THRESHOLD_PKT_SIZE 1500 > +#define MAX_INT_VALUE 0xffffffff > +#define MAX_INT_VALUE_CAP (0xffffffff >> 8) > + > +typedef u32 pie_time_t; > +typedef s32 pie_tdiff_t; > +#define PIE_SHIFT 10 > +#define MS2PIETIME(a) ((a * NSEC_PER_MSEC) >> PIE_SHIFT) > +#define PIE_TIME_PER_SEC ((NSEC_PER_SEC >> PIE_SHIFT)) > + I would prefer that all packet schedulers use the same set of clock routines (psched), rather than inventing own wrapper for high resolution clock. > +static inline pie_time_t pie_get_time(void) > +{ > + u64 ns = ktime_to_ns(ktime_get()); > + return ns >> PIE_SHIFT; > +} > + > +static inline u32 pie_time_to_ms(pie_time_t val) > +{ > + u64 valms = ((u64) val << PIE_SHIFT); > + > + do_div(valms, NSEC_PER_MSEC); > + return (u32) valms; > +} Psched has all this. > +/* parameters used*/ > +struct pie_params { > + pie_time_t target; /* user specified target delay in pietime*/ > + pie_time_t tupdate; /* frequency with which the timer fires*/ ^ space before end of comment > + u32 limit; /* number of packets that can be enqueued */ > + u32 alpha; /* alpha and beta are between -4 and 4 */ > + u32 beta; /* and are used for shift relative to 1 */ > + bool ecn; /* true if ecn is enabled */ > + bool bytemode; /* to scale drop early prob based on pkt size */ > +}; > + > +/* variables used*/ > +struct pie_vars { > + u32 prob; /* probability but scaled by u32 limit. */ > + pie_time_t burst_time; > + pie_time_t qdelay; > + pie_time_t qdelay_old; > + u32 dq_count; /* measured in bytes */ > + pie_time_t dq_tstamp; /* drain rate */ > + u32 avg_dq_rate; /* bytes per pietime tick, scaled by 8 */ > + u32 qlen_old; /* in bytes */ > +}; > + > +struct pie_stats { > + u32 packets_in; /* total number of packets enqueued */ > + u32 dropped; /* packets dropped due to pie_action */ > + u32 overlimit; /* dropped due to lack of space in queue */ > + u32 maxq; /* maximum queue size */ > + u32 ecn_mark; /* packets marked with ECN */ > +}; > + > +static void pie_params_init(struct pie_params *params) > +{ > + memset(params, 0, sizeof(*params)); Unnecessary, already zero'd when created by qdisc_alloc() Call chain qdisc_create qdisc_alloc ops->init => pie_init pie_params_init > + params->alpha = 2; > + params->beta = 20; > + params->tupdate = MS2PIETIME(30); /* 30 ms */ > + params->limit = PIE_DEFAULT_QUEUE_LIMIT; > + params->target = MS2PIETIME(20); /* 20 ms */ > + params->ecn = false; > + params->bytemode = false; > +} > + > +static void pie_vars_init(struct pie_vars *vars) > +{ > + memset(vars, 0, sizeof(*vars)); ditto, already zero'd > + vars->dq_count = DQCOUNT_INVALID; > + vars->avg_dq_rate = 0; > + /* default of 100 ms in pietime */ > + vars->burst_time = MS2PIETIME(100); > +} > + > +static void pie_stats_init(struct pie_stats *stats) > +{ > + memset(stats, 0, sizeof(*stats)); > +} > + ditto, already zero'd > +struct pie_sched_data { > + struct pie_params params; > + struct pie_vars vars; > + struct pie_stats stats; > + struct timer_list adapt_timer; > +}; Standard practice is to put data structures before code. > + > +static inline bool drop_early(struct Qdisc *sch, u32 packet_size) Inline is not needed if static. Let compiler decide. > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + u32 rnd; > + u32 local_prob = q->vars.prob; > + > + > + /* If there is still burst allowance left or delay is much below target > + * not due to heavy dropping, skip random early drop > + */ > + if (q->vars.burst_time > 0) > + return false; > + > + /* If current delay is less than half of target, and > + * if drop prob is low already, disable early_drop > + */ > + if ((q->vars.qdelay < q->params.target / 2) > + && (q->vars.prob < MAX_INT_VALUE / 5)) > + return false; > + > + /* If we have fewer than 2 packets, disable drop_early, > + * similar to min_th in RED > + */ > + if (sch->qstats.backlog < 2 * 1500) > + return false; > + > + /* If bytemode is turned on, use packet size to compute new > + * probablity. Smaller packets will have lower drop prob in this case > + */ > + if (q->params.bytemode) { > + /* If packet_size is greater than THRESHOLD_PKT_SIZE, > + * we cap the probability to the maximum value > + */ > + if (packet_size <= THRESHOLD_PKT_SIZE) { > + local_prob = > + (local_prob / THRESHOLD_PKT_SIZE) * packet_size; > + } else { > + local_prob = MAX_INT_VALUE_CAP; > + } > + > + } else { > + local_prob = q->vars.prob; > + } > + > + rnd = net_random() % MAX_INT_VALUE_CAP; > + if (rnd < local_prob) > + return true; > + > + return false; > +} > + > +static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + > + if (unlikely(qdisc_qlen(sch) >= sch->limit)) > + goto out; > + > + if (!drop_early(sch, skb->len)) { > + /* we can enqueue the packet */ > + q->stats.packets_in++; > + > + if (qdisc_qlen(sch) > q->stats.maxq) > + q->stats.maxq = qdisc_qlen(sch); > + > + return qdisc_enqueue_tail(skb, sch); > + } else if (q->params.ecn && INET_ECN_set_ce(skb) && > + (q->vars.prob <= MAX_INT_VALUE / 10)) { > + /* If packet is ecn capable, mark it if drop probability > + * is lower than 10%, else drop it. > + */ > + q->stats.ecn_mark++; > + return qdisc_enqueue_tail(skb, sch); > + } > +out: > + q->stats.overlimit++; > + qdisc_drop(skb, sch); > + return NET_XMIT_CN; /*indicate congestion*/ > +} > + > +static const struct nla_policy pie_policy[TCA_PIE_MAX + 1] = { > + [TCA_PIE_TARGET] = {.type = NLA_U32}, ^ space before } > + [TCA_PIE_LIMIT] = {.type = NLA_U32}, > + [TCA_PIE_TUPDATE] = {.type = NLA_U32}, > + [TCA_PIE_ALPHA] = {.type = NLA_U32}, > + [TCA_PIE_BETA] = {.type = NLA_U32}, > + [TCA_PIE_ECN] = {.type = NLA_U32}, > + [TCA_PIE_BYTEMODE] = {.type = NLA_U32}, Looks prettier if all = are aligned > +}; > + > +static int pie_change(struct Qdisc *sch, struct nlattr *opt) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + struct nlattr *tb[TCA_PIE_MAX + 1]; > + unsigned int qlen; > + int err; > + > + if (!opt) > + return -EINVAL; > + > + err = nla_parse_nested(tb, TCA_PIE_MAX, opt, pie_policy); > + if (err < 0) > + return err; > + > + sch_tree_lock(sch); > + > + /* convert from microseconds to pietime */ > + if (tb[TCA_PIE_TARGET]) { > + /* target is in us */ > + u32 target = nla_get_u32(tb[TCA_PIE_TARGET]); > + /* convert to pietime */ > + q->params.target = ((u64) target * NSEC_PER_USEC) >> PIE_SHIFT; > + } > + > + if (tb[TCA_PIE_TUPDATE]) { > + /* tupdate is in us */ > + u32 tupdate = nla_get_u32(tb[TCA_PIE_TUPDATE]); > + /* convert to pietime */ > + q->params.tupdate = > + ((u64) tupdate * NSEC_PER_USEC) >> PIE_SHIFT; > + } > + > + if (tb[TCA_PIE_LIMIT]) { > + u32 limit = nla_get_u32(tb[TCA_PIE_LIMIT]); > + q->params.limit = limit; > + sch->limit = limit; > + } > + > + if (tb[TCA_PIE_ALPHA]) > + q->params.alpha = nla_get_u32(tb[TCA_PIE_ALPHA]); > + > + if (tb[TCA_PIE_BETA]) > + q->params.beta = nla_get_u32(tb[TCA_PIE_BETA]); > + > + if (tb[TCA_PIE_ECN]) > + q->params.ecn = nla_get_u32(tb[TCA_PIE_ECN]); > + > + if (tb[TCA_PIE_BYTEMODE]) > + q->params.bytemode = nla_get_u32(tb[TCA_PIE_BYTEMODE]); > + > + /* Drop excess packets if new limit is lower */ > + qlen = sch->q.qlen; > + while (sch->q.qlen > sch->limit) { > + struct sk_buff *skb = __skb_dequeue(&sch->q); > + > + sch->qstats.backlog -= qdisc_pkt_len(skb); > + qdisc_drop(skb, sch); > + } > + qdisc_tree_decrease_qlen(sch, qlen - sch->q.qlen); > + > + sch_tree_unlock(sch); > + return 0; > +} > + > +static int pie_process_dequeue(struct Qdisc *sch, struct sk_buff *skb) > +{ > + > + struct pie_sched_data *q = qdisc_priv(sch); > + int qlen = sch->qstats.backlog; /* current queue size in bytes */ > + > + /* If current queue is about 10 packets or more and dq_count is unset > + * we have enough packets to calculate the drain rate. Save > + * current time as dq_tstamp and start measurement cycle. > + */ > + > + if (qlen >= QUEUE_THRESHOLD && q->vars.dq_count == DQCOUNT_INVALID) { > + q->vars.dq_tstamp = pie_get_time(); > + q->vars.dq_count = 0; > + } > + > + /* Calculate the average drain rate from this value. If queue length > + * has receded to a small value viz., <= QUEUE_THRESHOLD bytes,reset > + * the dq_count to -1 as we don't have enough packets to calculate the > + * drain rate anymore The following if block is entered only when we > + * have a substantial queue built up (QUEUE_THRESHOLD bytes or more) > + * and we calculate the drain rate for the threshold here. dq_count is > + * in bytes, time difference in pie_time, hence rate is in > + * bytes/pie_time. > + */ > + > + if (q->vars.dq_count != DQCOUNT_INVALID) { I prefer not having blank line after each block comment when it refers directly to the code below (save screen space when reading). > + > + q->vars.dq_count += skb->len; > + > + if (q->vars.dq_count >= QUEUE_THRESHOLD) { > + pie_time_t now = pie_get_time(); > + pie_tdiff_t dtime = now - q->vars.dq_tstamp; > + u64 count = q->vars.dq_count << 3; /* scale by 8*/ This shift doesn't do what you expect. Since dq_count is 32 bit, the compiler does a 32 bit shift and then assigns it to count. So either just make count 32 bit and skip all the expensive 64 bit divide, or cast q->vars.dq_count to 64 bit first. > + > + if (dtime == 0) > + return 0; > + > + /* dtime has overflowed */ > + if (dtime < 0) > + dtime = -dtime; > + > + do_div(count, dtime); > + > + if (q->vars.avg_dq_rate == 0) > + q->vars.avg_dq_rate = count; > + else > + q->vars.avg_dq_rate = > + (q->vars.avg_dq_rate - > + (q->vars.avg_dq_rate >> 3)) + (count >> 3); > + > + /* If the queue has receded below the threshold, we hold > + * on to the last drain rate calculated, else we reset > + * dq_count to 0 to re-enter the if block when the next > + * packet is dequeued > + */ > + > + if (qlen < QUEUE_THRESHOLD) > + q->vars.dq_count = DQCOUNT_INVALID; > + else { > + q->vars.dq_count = 0; > + q->vars.dq_tstamp = pie_get_time(); > + } > + > + if (q->vars.burst_time > 0) { > + if (q->vars.burst_time > dtime) > + q->vars.burst_time -= dtime; > + else > + q->vars.burst_time = 0; > + } > + } > + > + } > + return 0; > +} > + > +static void calculate_probability(struct Qdisc *sch) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + int qlen = sch->qstats.backlog; /* queue size in bytes */ > + pie_time_t qdelay = 0; /* in pietime */ > + pie_time_t qdelay_old = q->vars.qdelay; /* in pietime */ > + s32 delta = 0; /* signed difference */ > + u32 oldprob; > + u32 alpha, beta; > + bool update_prob = true; /* Should probability be updated? */ > + Big problem, this is called without locks?? timer -> pie_timer -> calculate_probability When you add locks you also have to worry about deadlock on shutdown. > + q->vars.qdelay_old = q->vars.qdelay; > + > + if (q->vars.avg_dq_rate > 0) > + qdelay = (qlen << 3) / q->vars.avg_dq_rate; > + else > + qdelay = 0; > + > + /* If qdelay is zero and qlen is not, it means qlen is very small, less > + * than dequeue_rate, so we do not update probabilty in this round > + */ > + if (qdelay == 0 && qlen != 0) > + update_prob = false; > + > + /* Add ranges for alpha and beta, more aggressive for high dropping > + * mode and gentle steps for light dropping mode > + * In light dropping mode, take gentle steps; in medium dropping mode, > + * take medium steps; in high dropping mode, take big steps. > + */ > + if (q->vars.prob < MAX_INT_VALUE / 100) { > + alpha = > + (q->params.alpha * (MAX_INT_VALUE / PIE_TIME_PER_SEC)) >> 7; > + beta = > + (q->params.beta * (MAX_INT_VALUE / PIE_TIME_PER_SEC)) >> 7; > + } else if (q->vars.prob < MAX_INT_VALUE / 10) { > + alpha = > + (q->params.alpha * (MAX_INT_VALUE / PIE_TIME_PER_SEC)) >> 5; > + beta = > + (q->params.beta * (MAX_INT_VALUE / PIE_TIME_PER_SEC)) >> 5; > + } else { > + alpha = > + (q->params.alpha * (MAX_INT_VALUE / PIE_TIME_PER_SEC)) >> 4; > + beta = > + (q->params.beta * (MAX_INT_VALUE / PIE_TIME_PER_SEC)) >> 4; > + } > + > + /* alpha and beta should be between 0 and 32, in multiples of 1/16 > + */ > + delta += alpha * ((qdelay - q->params.target)); > + delta += beta * ((qdelay - qdelay_old)); > + > + oldprob = q->vars.prob; > + > + /* addition to ensure we increase probability in steps of no > + * more than 2% > + */ > + > + if (delta > (s32) (MAX_INT_VALUE * 2 / 100) > + && q->vars.prob >= MAX_INT_VALUE / 10) { > + delta = MAX_INT_VALUE * 2 / 100; > + } > + > + /* Non-linear drop > + * Tune drop probability to increase quickly for high delays > + * (250ms and above) > + * 250ms is derived through experiments and provides error protection > + */ > + > + if (qdelay > (MS2PIETIME(250))) > + delta += (2 * MAX_INT_VALUE) / 100; > + > + q->vars.prob += delta; > + > + if (delta > 0) { > + /* prevent overflow */ > + if (q->vars.prob < oldprob) { > + q->vars.prob = MAX_INT_VALUE; > + /* Prevent normalization error > + * If probability is the maximum value already, > + * we normalize it here, and skip the > + * check to do a non-linear drop in the next section > + */ > + update_prob = false; > + } > + } else { > + /* prevent underflow */ > + if (q->vars.prob > oldprob) > + q->vars.prob = 0; > + } > + > + /* Non-linear drop in probability */ > + /* Reduce drop probability quickly if delay is 0 for 2 consecutive > + * Tupdate periods > + */ > + if ((qdelay == 0) && (qdelay_old == 0) && update_prob) > + q->vars.prob = (q->vars.prob * 98) / 100; > + > + q->vars.qdelay = qdelay; > + q->vars.qlen_old = qlen; > + > + /* we restart the measurement cycle if the following conditions are met > + * 1. If the delay has been low for 2 consecutive Tupdate periods > + * 2. Calculated drop probability is zero > + * 3. We have atleast one estimate for the avg_dq_rate ie., > + * is a non-zero value > + */ > + if ((q->vars.qdelay < q->params.target / 2) > + && (q->vars.qdelay_old < q->params.target / 2) > + && (q->vars.prob == 0) > + && q->vars.avg_dq_rate > 0) > + pie_vars_init(&q->vars); > + > + return; > +} Don't do empty return at end of function. It is unnecessary. > + > +static inline void pie_timer(unsigned long arg) Since this is a callback it can't be inlined anyway > +{ > + struct Qdisc *sch = (struct Qdisc *)arg; > + struct pie_sched_data *q = qdisc_priv(sch); > + u64 tup; Does this really have to be 64 bit? > + > + calculate_probability(sch); > + /* reset the timer to fire after 'tupdate' us, > + * tupdate is currently in pie_time > + * mod_timer expects time to be in jiffies > + */ > + /* convert from pietime to nsecs to ms*/ > + tup = pie_time_to_ms(q->params.tupdate); > + tup = (tup * HZ) / (1000); /* and then to jiffies */ Useless paren around (1000) > + > + mod_timer(&q->adapt_timer, jiffies + tup); > + > + return; > + > +} > + > +static int pie_init(struct Qdisc *sch, struct nlattr *opt) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + > + pie_params_init(&q->params); > + pie_vars_init(&q->vars); > + pie_stats_init(&q->stats); > + sch->limit = q->params.limit; > + setup_timer(&q->adapt_timer, pie_timer, (unsigned long)sch); > + add_timer(&q->adapt_timer); This is wrong, you haven't set expiration timer (will be 0), so timer will fire. > + > + if (opt) { > + int err = pie_change(sch, opt); > + > + if (err) > + return err; > + } > + > + return 0; > +} > + > +static int pie_dump(struct Qdisc *sch, struct sk_buff *skb) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + struct nlattr *opts; > + > + opts = nla_nest_start(skb, TCA_OPTIONS); > + if (opts == NULL) > + goto nla_put_failure; > + > + /* convert target and tupdate from pietime to us */ > + if (nla_put_u32(skb, TCA_PIE_TARGET, > + pie_time_to_ms(q->params.target) * 1000L) || > + nla_put_u32(skb, TCA_PIE_LIMIT, > + sch->limit) || > + nla_put_u32(skb, TCA_PIE_TUPDATE, > + pie_time_to_ms(q->params.tupdate) * 1000L) || > + nla_put_u32(skb, TCA_PIE_ALPHA, > + q->params.alpha) || > + nla_put_u32(skb, TCA_PIE_BETA, q->params.beta) || > + nla_put_u32(skb, TCA_PIE_ECN, q->params.ecn) || > + nla_put_u32(skb, TCA_PIE_BYTEMODE, q->params.bytemode)) > + goto nla_put_failure; > + > + return nla_nest_end(skb, opts); > + > +nla_put_failure: > + nla_nest_cancel(skb, opts); > + return -1; > + > +} > + > +static int pie_dump_stats(struct Qdisc *sch, struct gnet_dump *d) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + struct tc_pie_xstats st = { > + .prob = q->vars.prob, > + .delay = pie_time_to_ms(q->vars.qdelay) * 1000, /* in us*/ > + /* unscale and return dq_rate in bytes per sec*/ > + .avg_dq_rate = q->vars.avg_dq_rate * (PIE_TIME_PER_SEC/8), > + .packets_in = q->stats.packets_in, > + .overlimit = q->stats.overlimit, > + .maxq = q->stats.maxq, > + .dropped = q->stats.dropped, > + .ecn_mark = q->stats.ecn_mark, > + }; Personal preference, I prefer a aligned initialization style. > + > + return gnet_stats_copy_app(d, &st, sizeof(st)); > +} > + > +static inline struct sk_buff *pie_qdisc_dequeue(struct Qdisc *sch) > +{ > + struct sk_buff *skb; > + skb = __qdisc_dequeue_head(sch, &sch->q); > + > + if (!skb) > + return NULL; > + > + pie_process_dequeue(sch, skb); > + > + return skb; > +} > + > +static void pie_reset(struct Qdisc *sch) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + qdisc_reset_queue(sch); > + pie_vars_init(&q->vars); > + > + return; > +} > + > +static void pie_destroy(struct Qdisc *sch) > +{ > + struct pie_sched_data *q = qdisc_priv(sch); > + > + del_timer_sync(&q->adapt_timer); > +} > + > +static struct Qdisc_ops pie_qdisc_ops __read_mostly = { > + .id = "pie", > + .priv_size = sizeof(struct pie_sched_data), > + > + .enqueue = pie_qdisc_enqueue, > + .dequeue = pie_qdisc_dequeue, > + .peek = qdisc_peek_dequeued, > + .init = pie_init, > + .destroy = pie_destroy, > + .reset = pie_reset, > + .change = pie_change, > + .dump = pie_dump, > + .dump_stats = pie_dump_stats, > + .owner = THIS_MODULE, > +}; > + > +static int __init pie_module_init(void) > +{ > + return register_qdisc(&pie_qdisc_ops); > +} > + > +static void __exit pie_module_exit(void) > +{ > + unregister_qdisc(&pie_qdisc_ops); > +} > + > +module_init(pie_module_init); > +module_exit(pie_module_exit); > + > +MODULE_DESCRIPTION > + ("PIE (Proportional Intergal controller Enhanced) scheduler"); Keep this on one line, ignore any complaints from checkpatch about it. > +MODULE_AUTHOR("Vijay Subramanian"); > +MODULE_AUTHOR("Mythili Prabhu"); > +MODULE_LICENSE("GPL"); Please fix and resubmit. This is just a first pass review, there are probably more detailed issues that others will see. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists