Corey Hickey wrote: > This fixes the ambiguity between, for example: > tc qdisc change ... perturb 0 > tc qdisc change ... > > Without this patch, there is no way for SFQ to differentiate between > a parameter specified to be 0 and a parameter that was omitted. >
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c > index 170fd37..36197f6 100644 > --- a/net/sched/sch_sfq.c > +++ b/net/sched/sch_sfq.c > @@ -428,25 +428,31 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt) > * the previous values (sfq_change). So, overwrite the parameters as > * specified. */ > if (opt) { > - struct tc_sfq_qopt *ctl = RTA_DATA(opt); > - > - if (opt->rta_len < RTA_LENGTH(sizeof(*ctl))) > - return -EINVAL; > - > - if (ctl->quantum) > - q->quantum = ctl->quantum; > - if (ctl->perturb_period) > - q->perturb_period = ctl->perturb_period; > - if (ctl->divisor) > - q->hash_divisor = ctl->divisor; > - if (ctl->flows) > - q->depth = ctl->flows; > - if (ctl->limit) > - q->limit = ctl->limit; > - > + struct tc_sfq_qopt *ctl; > + struct rtattr *tb[TCA_SFQ_MAX]; > + > + if (rtattr_parse_nested_compat(tb, TCA_SFQ_MAX, opt, ctl, > + sizeof(*ctl))) > + goto rtattr_failure; > + > +#define GET_PARAM(dst, nest, compat) do { \ > + struct rtattr *rta = tb[(nest) - 1]; \ > + if (rta) \ > + (dst) = RTA_GET_U32(rta); \ > + else if ((compat)) \ > + (dst) = (compat); \ > +} while (0) An inline function and a comment why this is done would increase readability. > + > + GET_PARAM(q->quantum, TCA_SFQ_QUANTUM, ctl->quantum); > + GET_PARAM(q->perturb_period, TCA_SFQ_PERTURB, > + ctl->perturb_period); > + GET_PARAM(q->hash_divisor, TCA_SFQ_DIVISOR, ctl->divisor); > + GET_PARAM(q->depth, TCA_SFQ_FLOWS, ctl->flows); > + GET_PARAM(q->limit, TCA_SFQ_LIMIT, ctl->limit); > + > if (q->perturb_period > SFQ_MAX_PERTURB || > q->depth > SFQ_MAX_DEPTH) > - return -EINVAL; > + goto rtattr_failure; > } > q->limit = min_t(u32, q->limit, q->depth - 2); > q->tail = q->depth; > @@ -482,6 +488,8 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt) > for (i=0; i < q->depth; i++) > sfq_link(q, i); > return 0; > +rtattr_failure: > + return -EINVAL; > err_case: > sfq_q_destroy(q); > return -ENOBUFS; > @@ -559,17 +567,26 @@ static int sfq_dump(struct Qdisc *sch, struct sk_buff > *skb) > { > struct sfq_sched_data *q = qdisc_priv(sch); > unsigned char *b = skb_tail_pointer(skb); > + struct rtattr *nest; > struct tc_sfq_qopt opt; > > opt.quantum = q->quantum; > opt.perturb_period = q->perturb_period; > - > opt.limit = q->limit; > opt.divisor = q->hash_divisor; > opt.flows = q->depth; > > + nest = RTA_NEST_COMPAT(skb, TCA_OPTIONS, sizeof(opt), &opt); > + > + RTA_PUT_U32(skb, TCA_SFQ_QUANTUM, q->quantum); > + RTA_PUT_U32(skb, TCA_SFQ_PERTURB, q->perturb_period); > + RTA_PUT_U32(skb, TCA_SFQ_LIMIT, q->limit); > + RTA_PUT_U32(skb, TCA_SFQ_DIVISOR, q->hash_divisor); > + RTA_PUT_U32(skb, TCA_SFQ_FLOWS, q->depth); > RTA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt); This is wrong, RTA_NEST_COMPAT already dumps the structure. > > + RTA_NEST_COMPAT_END(skb, nest); > + > return skb->len; > > rtattr_failure: - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html