public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash•net>
To: Corey Hickey <bugfood-ml@fatooh•org>
Cc: netdev@vger•kernel.org
Subject: Re: [PATCH 10/10] Use nested compat attributes to pass parameters.
Date: Mon, 01 Oct 2007 15:54:38 +0200	[thread overview]
Message-ID: <4700FC1E.9040309@trash.net> (raw)
In-Reply-To: <11910199782593-git-send-email-bugfood-ml@fatooh.org>

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:


  reply	other threads:[~2007-10-01 13:55 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-28 22:52 SFQ: backport some features from ESFQ (try 4) Corey Hickey
2007-09-28 22:52 ` [PATCH 01/10] Preparatory refactoring part 1 Corey Hickey
2007-10-01 13:25   ` Patrick McHardy
2007-10-01 20:46     ` Corey Hickey
2007-10-02  3:17       ` Patrick McHardy
2007-09-28 22:52 ` [PATCH 02/10] Preparatory refactoring part 2 Corey Hickey
2007-10-01 13:33   ` Patrick McHardy
2007-10-01 20:46     ` Corey Hickey
2007-10-02  3:19       ` Patrick McHardy
2007-09-28 22:52 ` [PATCH 03/10] Move two functions Corey Hickey
2007-10-01 13:27   ` Patrick McHardy
2007-10-01 20:46     ` Corey Hickey
2007-09-28 22:52 ` [PATCH 04/10] Make "depth" (number of queues) user-configurable: Corey Hickey
2007-09-28 22:52 ` [PATCH 05/10] Add divisor Corey Hickey
2007-10-01 13:38   ` Patrick McHardy
2007-10-01 20:47     ` Corey Hickey
2007-09-28 22:52 ` [PATCH 06/10] Make qdisc changeable Corey Hickey
2007-10-01 13:43   ` Patrick McHardy
2007-10-01 20:47     ` Corey Hickey
2007-09-28 22:52 ` [PATCH 07/10] Remove comments about hardcoded values Corey Hickey
2007-09-28 22:52 ` [PATCH 08/10] Multiply perturb_period by HZ when used rather than when assigned Corey Hickey
2007-09-28 22:52 ` [PATCH 09/10] Change perturb_period to unsigned Corey Hickey
2007-10-01 13:45   ` Patrick McHardy
2007-10-01 20:47     ` Corey Hickey
2007-09-28 22:52 ` [PATCH 10/10] Use nested compat attributes to pass parameters Corey Hickey
2007-10-01 13:54   ` Patrick McHardy [this message]
2007-10-01 20:47     ` Corey Hickey
2007-10-02  3:21       ` Patrick McHardy
2007-09-28 22:54 ` sfq Corey Hickey
2007-09-28 22:57   ` sfq (iproute2 patches) Corey Hickey
2007-09-28 22:54 ` [PATCH 1/3] SFQ: Support changing depth and divisor Corey Hickey
2007-09-28 22:54 ` [PATCH 2/3] Change perturb_period to unsigned Corey Hickey
2007-09-28 22:54 ` [PATCH 3/3] Use nested compat attributes for passing parameters to the kernel Corey Hickey
2007-09-29 15:38 ` SFQ: backport some features from ESFQ (try 4) Patrick McHardy
2007-09-29 17:36   ` Corey Hickey
  -- strict thread matches above, loose matches on Subject: below --
2007-08-25 22:26 [PATCH 00/10] SFQ: backport some features from ESFQ (try 3) Corey Hickey
2007-08-25 22:27 ` [PATCH 10/10] Use nested compat attributes to pass parameters Corey Hickey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4700FC1E.9040309@trash.net \
    --to=kaber@trash$(echo .)net \
    --cc=bugfood-ml@fatooh$(echo .)org \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox