netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [patch net-next 3/7] htb: remove pointless first initialization of buffer and cbuffer
From: Eric Dumazet @ 2013-02-08 16:35 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, edumazet, jhs, kuznet
In-Reply-To: <1360328327-7144-4-git-send-email-jiri@resnulli.us>

On Fri, 2013-02-08 at 13:58 +0100, Jiri Pirko wrote:
> these are initialized correctly couple of lines later in the function.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  net/sched/sch_htb.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
> index 14a83dc..547912e9 100644
> --- a/net/sched/sch_htb.c
> +++ b/net/sched/sch_htb.c
> @@ -1503,9 +1503,6 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
>  			cl->prio = TC_HTB_NUMPRIO - 1;
>  	}
>  
> -	cl->buffer = hopt->buffer;
> -	cl->cbuffer = hopt->cbuffer;
> -
>  	cl->rate.rate_bps = (u64)hopt->rate.rate << 3;
>  	cl->ceil.rate_bps = (u64)hopt->ceil.rate << 3;
>  

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH] drivers: net: Remove remaining alloc/OOM messages
From: John W. Linville @ 2013-02-08 16:19 UTC (permalink / raw)
  To: Joe Perches
  Cc: netdev, linux-can, linux-kernel, e1000-devel, linux-usb,
	linux-wireless, ath9k-devel, wil6210, brcm80211-dev-list,
	devicetree-discuss
In-Reply-To: <1360273587.27863.15.camel@joe-AO722>

On Thu, Feb 07, 2013 at 01:46:27PM -0800, Joe Perches wrote:
> alloc failures already get standardized OOM
> messages and a dump_stack.
> 
> For the affected mallocs around these OOM messages:
> 
> Converted kmallocs with multiplies to kmalloc_array.
> Converted a kmalloc/memcpy to kmemdup.
> Removed now unused stack variables.
> Removed unnecessary parentheses.
> Neatened alignment.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

No objection...

Acked-by: John W. Linville <linville@tuxdriver.com>

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
From: Daniel Borkmann @ 2013-02-08 16:02 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: davem, linux-sctp, netdev
In-Reply-To: <51151EAA.2090302@gmail.com>

On 02/08/2013 04:50 PM, Vlad Yasevich wrote:
> On 02/08/2013 08:04 AM, Daniel Borkmann wrote:
>> On sctp_endpoint_destroy, previously used sensitive keying material
>> should be zeroed out before the memory is returned, as we already do
>> with e.g. auth keys when released.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>
> I'll ack this, but the whole multiple cookie keys code is completely unused and has been all this time.  Noone uses anything other then the secret_key[0] since there is no changeover support anywhere.  It might be nice to clean that up too.

Put on my todo list for follow-up patches, thanks.

> Acked-by: Vlad Yasevich <vyasevic@redhat.com>
>
> -vlad
>
>> ---
>>   net/sctp/endpointola.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
>> index 17a001b..1a9c5fb 100644
>> --- a/net/sctp/endpointola.c
>> +++ b/net/sctp/endpointola.c
>> @@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
>>   /* Final destructor for endpoint.  */
>>   static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>>   {
>> +    int i;
>> +
>>       SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
>>
>>       /* Free up the HMAC transform. */
>> @@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>>       sctp_inq_free(&ep->base.inqueue);
>>       sctp_bind_addr_free(&ep->base.bind_addr);
>>
>> +    for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
>> +        memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
>> +
>>       /* Remove and free the port */
>>       if (sctp_sk(ep->base.sk)->bind_hash)
>>           sctp_put_port(ep->base.sk);
>>
>
>
> c
> If

^ permalink raw reply

* Re: [patch net-next 2/7] htb: fix values in opt dump
From: Jiri Pirko @ 2013-02-08 16:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem, edumazet, jhs, kuznet
In-Reply-To: <1360338849.28557.98.camel@edumazet-glaptop>

Fri, Feb 08, 2013 at 04:54:09PM CET, eric.dumazet@gmail.com wrote:
>On Fri, 2013-02-08 at 13:58 +0100, Jiri Pirko wrote:
>> in htb_change_class() cl->buffer and cl->buffer are stored in ns.
>> So in dump, convert them back to psched ticks.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  net/sched/sch_htb.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
>> index 476992c..14a83dc 100644
>> --- a/net/sched/sch_htb.c
>> +++ b/net/sched/sch_htb.c
>> @@ -1135,9 +1135,9 @@ static int htb_dump_class(struct Qdisc *sch, unsigned long arg,
>>  	memset(&opt, 0, sizeof(opt));
>>  
>>  	opt.rate.rate = cl->rate.rate_bps >> 3;
>> -	opt.buffer = cl->buffer;
>> +	opt.buffer = PSCHED_NS2TICKS(cl->buffer);
>>  	opt.ceil.rate = cl->ceil.rate_bps >> 3;
>> -	opt.cbuffer = cl->cbuffer;
>> +	opt.cbuffer = PSCHED_NS2TICKS(cl->cbuffer);
>>  	opt.quantum = cl->quantum;
>>  	opt.prio = cl->prio;
>>  	opt.level = cl->level;
>
>Acked-by: Eric Dumazet <edumazet@google.com>
>
>Isnt it a patch for net tree (and stable) ?

That may be.

>
>A bug origin (commit id/title) is welcomed to ease stable team work.


Okay. I will repost the patchset anyway because I misspelled "peak"
(and I will probably add similar patches I did for tbf for act_police).
I will include your acks and commit id/title.

Jiri
>
>
>

^ permalink raw reply

* Re: [patch net-next 2/7] htb: fix values in opt dump
From: Eric Dumazet @ 2013-02-08 15:54 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, edumazet, jhs, kuznet
In-Reply-To: <1360328327-7144-3-git-send-email-jiri@resnulli.us>

On Fri, 2013-02-08 at 13:58 +0100, Jiri Pirko wrote:
> in htb_change_class() cl->buffer and cl->buffer are stored in ns.
> So in dump, convert them back to psched ticks.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  net/sched/sch_htb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
> index 476992c..14a83dc 100644
> --- a/net/sched/sch_htb.c
> +++ b/net/sched/sch_htb.c
> @@ -1135,9 +1135,9 @@ static int htb_dump_class(struct Qdisc *sch, unsigned long arg,
>  	memset(&opt, 0, sizeof(opt));
>  
>  	opt.rate.rate = cl->rate.rate_bps >> 3;
> -	opt.buffer = cl->buffer;
> +	opt.buffer = PSCHED_NS2TICKS(cl->buffer);
>  	opt.ceil.rate = cl->ceil.rate_bps >> 3;
> -	opt.cbuffer = cl->cbuffer;
> +	opt.cbuffer = PSCHED_NS2TICKS(cl->cbuffer);
>  	opt.quantum = cl->quantum;
>  	opt.prio = cl->prio;
>  	opt.level = cl->level;

Acked-by: Eric Dumazet <edumazet@google.com>

Isnt it a patch for net tree (and stable) ?

A bug origin (commit id/title) is welcomed to ease stable team work.

^ permalink raw reply

* Re: [patch net-next 1/7] htb: use PSCHED_TICKS2NS()
From: Eric Dumazet @ 2013-02-08 15:51 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, edumazet, jhs, kuznet
In-Reply-To: <1360328327-7144-2-git-send-email-jiri@resnulli.us>

On Fri, 2013-02-08 at 13:58 +0100, Jiri Pirko wrote:
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  net/sched/sch_htb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
> index 51561ea..476992c 100644
> --- a/net/sched/sch_htb.c
> +++ b/net/sched/sch_htb.c
> @@ -1512,8 +1512,8 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
>  	htb_precompute_ratedata(&cl->rate);
>  	htb_precompute_ratedata(&cl->ceil);
>  
> -	cl->buffer = hopt->buffer << PSCHED_SHIFT;
> -	cl->cbuffer = hopt->buffer << PSCHED_SHIFT;
> +	cl->buffer = PSCHED_TICKS2NS(hopt->buffer);
> +	cl->cbuffer = PSCHED_TICKS2NS(hopt->buffer);
>  
>  	sch_tree_unlock(sch);
>  

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
From: Vlad Yasevich @ 2013-02-08 15:50 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev
In-Reply-To: <cf390ae999888aafe57914bcd87e5207be154557.1360327229.git.dborkman@redhat.com>

On 02/08/2013 08:04 AM, Daniel Borkmann wrote:
> On sctp_endpoint_destroy, previously used sensitive keying material
> should be zeroed out before the memory is returned, as we already do
> with e.g. auth keys when released.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

I'll ack this, but the whole multiple cookie keys code is completely 
unused and has been all this time.  Noone uses anything other then the 
secret_key[0] since there is no changeover support anywhere.  It might 
be nice to clean that up too.

Acked-by: Vlad Yasevich <vyasevic@redhat.com>

-vlad

> ---
>   net/sctp/endpointola.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> index 17a001b..1a9c5fb 100644
> --- a/net/sctp/endpointola.c
> +++ b/net/sctp/endpointola.c
> @@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
>   /* Final destructor for endpoint.  */
>   static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>   {
> +	int i;
> +
>   	SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
>
>   	/* Free up the HMAC transform. */
> @@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>   	sctp_inq_free(&ep->base.inqueue);
>   	sctp_bind_addr_free(&ep->base.bind_addr);
>
> +	for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
> +		memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
> +
>   	/* Remove and free the port */
>   	if (sctp_sk(ep->base.sk)->bind_hash)
>   		sctp_put_port(ep->base.sk);
>


c
If

^ permalink raw reply

* Re: [PATCH] netpoll: cleanup sparse warnings
From: Neil Horman @ 2013-02-08 15:47 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, fengguang.wu, David Miller
In-Reply-To: <1360293933.28557.92.camel@edumazet-glaptop>

On Thu, Feb 07, 2013 at 07:25:33PM -0800, Eric Dumazet wrote:
> On Thu, 2013-02-07 at 13:37 -0500, Neil Horman wrote:
> > On Thu, Feb 07, 2013 at 07:52:56AM -0800, Eric Dumazet wrote:
> > > On Thu, 2013-02-07 at 09:56 -0500, Neil Horman wrote:
> > > > With my recent commit I introduced two sparse warnings.  Looking closer there
> > > > were a few more in the same file, so I fixed them all up.  Basic rcu pointer
> > > > dereferencing suff
> > > 
> > > > -	npinfo = np->dev->npinfo;
> > > > +	/* rtnl_dereference would be preferable here but
> > > > +	 * rcu_cleanup_netpoll path can put us in here safely without
> > > > +	 * holding the rtnl, so plain rcu_dereference it is
> > > > +	 */
> > > > +	npinfo = rcu_dereference(np->dev->npinfo);
> > > >  	if (!npinfo)
> > > >  		return;
> > > >  
> > > 
> > > Are you sure it wont trigger a LOCKDEP complain (CONFIG_PROVE_RCU=y) ?
> > > 
> > Hm, looking at it, you're probably right.  We're not holding the rcu_read_lock,
> > and I'd forgotten that rcu_dereference implicitly checks that rcu_read_lock is
> > held.  I guess, since the only paths that we get here on are in a bh rcu
> > quiescence point or with the rtnl held we should probably make this:
> > 
> > rcu_dereference_protected(np->dev->npinfo, rtnl_locked() || in_interrupt());
> > 
> > Although, thinking about this further somewhat begs the question as to how we
> > prevent one context from calling __netpoll_cleanup in a path holding rtnl, while
> > in parallel calling __netpoll_cleanup from the rcu callback.  That might not be
> > a huge deal as __netpoll_cleanup uses spinlocks to do list modification, and an
> > atomic_dec_and_test to gate the free, but it still seems ugly.
> > 
> > What do you think?
> 
> I think you could use 
> 
>  rcu_dereference_check(p, lockdep_rtnl_is_held() || something)
> 
Actually, I think all we need to do is take the rcu_read_lock and use
rcu_dereference directly.  __netpoll_cleanup doesn't actually change the
dev->npinfo pointer itself, thats handled by the caller, always under protection
of the rtnl, so here we just need to ensure that no one changes npifo while
we're using it
Neil

> 
> 

^ permalink raw reply

* Re: [PATCH v5 00/45] CPU hotplug: stop_machine()-free CPU hotplug
From: Russell King - ARM Linux @ 2013-02-08 15:41 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: Rusty Russell, tglx, peterz, tj, oleg, paulmck, mingo, linux-arch,
	nikunj, linux-pm, fweisbec, linux-doc, linux-kernel, rostedt,
	xiaoguangrong, rjw, sbw, wangyun, netdev, namhyung, akpm, walken,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <51134596.4080106@linux.vnet.ibm.com>

On Thu, Feb 07, 2013 at 11:41:34AM +0530, Srivatsa S. Bhat wrote:
> On 02/07/2013 09:44 AM, Rusty Russell wrote:
> > "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> writes:
> >> On 01/22/2013 01:03 PM, Srivatsa S. Bhat wrote:
> >>                  Avg. latency of 1 CPU offline (ms) [stop-cpu/stop-m/c latency]
> >>
> >> # online CPUs    Mainline (with stop-m/c)       This patchset (no stop-m/c)
> >>
> >>       8                 17.04                          7.73
> >>
> >>      16                 18.05                          6.44
> >>
> >>      32                 17.31                          7.39
> >>
> >>      64                 32.40                          9.28
> >>
> >>     128                 98.23                          7.35
> > 
> > Nice!
> 
> Thank you :-)
> 
> >  I wonder how the ARM guys feel with their quad-cpu systems...
> > 
> 
> That would be definitely interesting to know :-)

That depends what exactly you'd like tested (and how) and whether you'd
like it to be a test-chip based quad core, or an OMAP dual-core SoC.

^ permalink raw reply

* Please reply for details
From: KI SANG JUNG @ 2013-02-08 15:01 UTC (permalink / raw)




Dear Friend,
I find it pleasurable to request your partnership in business worth USD$48.8M. I'm soliciting your assistance and honesty to receive money onmy behalf since my position would not allow me to do this alone. This is legitimate and may be of appeal to you. If interested, contact me directly via my E-mail: y4593-sangjung@yahoo.com for more information.

Sincerely Yours
Ki

^ permalink raw reply

* Re: [PATCH net-next] net: sctp: sctp_auth_make_key_vector: use sctp_auth_create_key
From: Vlad Yasevich @ 2013-02-08 15:01 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev
In-Reply-To: <783e1129996c3f09c7da121f5dd91c8ac7ba5baa.1360314390.git.dborkman@redhat.com>

On 02/08/2013 04:22 AM, Daniel Borkmann wrote:
> In sctp_auth_make_key_vector, we allocate a temporary sctp_auth_bytes
> structure with kmalloc instead of the sctp_auth_create_key allocator.
> Change this to sctp_auth_create_key as it is the case everywhere else,
> so that we also can properly free it via sctp_auth_key_put. This makes
> it easier for future code changes in the structure and allocator itself,
> since a single API is consistently used for this purpose. Also, by
> using sctp_auth_create_key we're doing sanity checks over the arguments.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

Acked-by: Vlad Yasevich <vyasevich@gmail.com>

-vlad

> ---
>   net/sctp/auth.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index 94a12de..5ec7509 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -209,12 +209,10 @@ static struct sctp_auth_bytes *sctp_auth_make_key_vector(
>
>   	len = random_len + hmacs_len + chunks_len;
>
> -	new = kmalloc(sizeof(struct sctp_auth_bytes) + len, gfp);
> +	new = sctp_auth_create_key(len, gfp);
>   	if (!new)
>   		return NULL;
>
> -	new->len = len;
> -
>   	memcpy(new->data, random, random_len);
>   	offset += random_len;
>
> @@ -353,8 +351,8 @@ static struct sctp_auth_bytes *sctp_auth_asoc_create_secret(
>   	secret = sctp_auth_asoc_set_secret(ep_key, first_vector, last_vector,
>   					    gfp);
>   out:
> -	kfree(local_key_vector);
> -	kfree(peer_key_vector);
> +	sctp_auth_key_put(local_key_vector);
> +	sctp_auth_key_put(peer_key_vector);
>
>   	return secret;
>   }
>

^ permalink raw reply

* Re: [PATCH] atm/iphase: rename fregt_t -> ffreg_t
From: chas williams - CONTRACTOR @ 2013-02-08 14:58 UTC (permalink / raw)
  To: Heiko Carstens; +Cc: David Miller, netdev, linux-kernel
In-Reply-To: <20130208101911.GA12195@osiris>

On Fri, 8 Feb 2013 11:19:11 +0100
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> We have conflicting type qualifiers for "freg_t" in s390's ptrace.h and the
> iphase atm device driver, which causes the compile error below.
> Unfortunately the s390 typedef can't be renamed, since it's a user visible api,
> nor can I change the include order in s390 code to avoid the conflict.
> 
> So simply rename the iphase typedef to a new name. Fixes this compile error:
> 
> In file included from drivers/atm/iphase.c:66:0:
> drivers/atm/iphase.h:639:25: error: conflicting type qualifiers for 'freg_t'
> In file included from next/arch/s390/include/asm/ptrace.h:9:0,
>                  from next/arch/s390/include/asm/lowcore.h:12,
>                  from next/arch/s390/include/asm/thread_info.h:30,
>                  from include/linux/thread_info.h:54,
>                  from include/linux/preempt.h:9,
>                  from include/linux/spinlock.h:50,
>                  from include/linux/seqlock.h:29,
>                  from include/linux/time.h:5,
>                  from include/linux/stat.h:18,
>                  from include/linux/module.h:10,
>                  from drivers/atm/iphase.c:43:
> next/arch/s390/include/uapi/asm/ptrace.h:197:3: note: previous declaration of 'freg_t' was here
> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>

seems like a fine idea.

Acked-by: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil>

^ permalink raw reply

* [PATCH] linux-3.8-rc6 Fix Missing Allocation Failure Checks
From: syrine tlili @ 2013-02-08 14:35 UTC (permalink / raw)
  To: matt.fleming, sage, isdn, megaraidlinux; +Cc: linux-kernel, netdev

From: Syrine Tlili <syrine.tl@gmail.com>

Check for memory allocation failures to avoid
NULL pointer dereferencing.

Signed-off-by: Syrine Tlili <syrine.tl@gmail.com>
---


diff -uprN -X linux-3.8-rc6-vanilla/Documentation/dontdiff
linux-3.8-rc6-vanilla/arch/x86/platform/efi/efi.c
b/arch/x86/platform/efi/efi.c
--- linux-3.8-rc6-vanilla/arch/x86/platform/efi/efi.c	2013-02-01
02:08:14.000000000 +0100
+++ b/arch/x86/platform/efi/efi.c	2013-02-06 20:33:36.390643500 +0100
@@ -924,6 +924,8 @@ void __init efi_enter_virtual_mode(void)
 		new_memmap = krealloc(new_memmap,
 				      (count + 1) * memmap.desc_size,
 				      GFP_KERNEL);
+		if (!new_memmap)
+			return -ENOMEM;
 		memcpy(new_memmap + (count * memmap.desc_size), md,
 		       memmap.desc_size);
 		count++;
diff -uprN -X linux-3.8-rc6-vanilla/Documentation/dontdiff
linux-3.8-rc6-vanilla/drivers/isdn/mISDN/fsm.c
b/drivers/isdn/mISDN/fsm.c
--- linux-3.8-rc6-vanilla/drivers/isdn/mISDN/fsm.c	2013-02-01
02:08:14.000000000 +0100
+++ b/drivers/isdn/mISDN/fsm.c	2013-02-06 20:33:01.772644948 +0100
@@ -34,6 +34,8 @@ mISDN_FsmNew(struct Fsm *fsm,

 	fsm->jumpmatrix = kzalloc(sizeof(FSMFNPTR) * fsm->state_count *
 				  fsm->event_count, GFP_KERNEL);
+	if (!fsm->jumpmatrix)
+		return -ENOMEM;

 	for (i = 0; i < fncount; i++)
 		if ((fnlist[i].state >= fsm->state_count) ||
diff -uprN -X linux-3.8-rc6-vanilla/Documentation/dontdiff
linux-3.8-rc6-vanilla/drivers/scsi/megaraid.c
b/drivers/scsi/megaraid.c
--- linux-3.8-rc6-vanilla/drivers/scsi/megaraid.c	2013-02-01
02:08:14.000000000 +0100
+++ b/drivers/scsi/megaraid.c	2013-02-06 21:22:23.051521099 +0100
@@ -4437,6 +4437,8 @@ mega_internal_command(adapter_t *adapter
 	memset(scb, 0, sizeof(scb_t));

 	sdev = kzalloc(sizeof(struct scsi_device), GFP_KERNEL);
+	if (!sdev)
+		goto err_out;
 	scmd->device = sdev;

 	memset(adapter->int_cdb, 0, sizeof(adapter->int_cdb));
@@ -4482,6 +4484,11 @@ mega_internal_command(adapter_t *adapter
 	scsi_free_command(GFP_KERNEL, scmd);

 	return rval;
+
+err_out:
+	mutex_unlock(&adapter->int_mtx);
+	scsi_free_command(GFP_KERNEL, scmd);
+	return -ENOMEM;
 }


diff -uprN -X linux-3.8-rc6-vanilla/Documentation/dontdiff
linux-3.8-rc6-vanilla/fs/ceph/mdsmap.c b/fs/ceph/mdsmap.c
--- linux-3.8-rc6-vanilla/fs/ceph/mdsmap.c	2013-02-01 02:08:14.000000000 +0100
+++ b/fs/ceph/mdsmap.c	2013-02-06 20:34:02.934642390 +0100
@@ -132,6 +132,8 @@ struct ceph_mdsmap *ceph_mdsmap_decode(v
 				m->m_info[mds].export_targets =
 					kcalloc(num_export_targets, sizeof(u32),
 						GFP_NOFS);
+				if (m->m_info[mds].export_targets == NULL)
+					goto badmem;
 				for (j = 0; j < num_export_targets; j++)
 					m->m_info[mds].export_targets[j] =
 					       ceph_decode_32(&pexport_targets);

^ permalink raw reply

* Re: [RFC PATCH] xfrm: fix handling of XFRM policies mark and mask.
From: Emmanuel Thierry @ 2013-02-08 14:16 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: jamal, Romain KUNTZ, netdev@vger.kernel.org, davem@davemloft.net,
	herbert, linux-kernel@vger.kernel.org, Jamal Hadi Salim
In-Reply-To: <20130207125437.GC17794@secunet.com>

Hello,

Le 7 févr. 2013 à 13:54, Steffen Klassert <steffen.klassert@secunet.com> a écrit :

> On Thu, Feb 07, 2013 at 12:08:22PM +0100, Emmanuel Thierry wrote:
>> 
>> This is a nice idea, however you keep the insertion asymmetric. The usage of xfrm marks in non-conflicting cases will be made possible, but it stays disturbing for a user as the initial example will still have the same behavior:
>> * Inserting the marked one then the unmarked will succeed
>> * Inserting the unmarked then the marked one will fail
>> This gives to the user the feeling of an indeterministic behavior of the xfrm module.
> 
> This was intended. Inserting the marked one then the unmarked
> is a working scenario. Some users might rely on it, so we can't
> change this as you proposed.
> 
> On the other hand, inserting the unmarked one then the marked
> might result in a wrong policy lookup, so we can't allow this.
> The only possibility we have, is inserting with different
> priorites and that's what I'm proposing.
> 
> I fear we have to live with that asymmetric behaviour if
> both policies have the same priority.
> 

Ok, actually i understand the concern of backward compatibility you expose. It is true that users might be disturbed if we change such a behavior they would rely on.
Anyway, i'm ok with your patch.

Best regards
Emmanuel Thierry--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* [PATCH v2 1/5] iproute2: add a missing return statement
From: Kees van Reeuwijk @ 2013-02-08 13:32 UTC (permalink / raw)
  To: netdev

From: Kees van Reeuwijk <reeuwijk@few.vu.nl>

Since do_help() has to return an int to fit in the table of commands,
it should actually return an int. This patch lets it do so.

Signed-off-by: Kees van Reeuwijk <reeuwijk@few.vu.nl>

---


 ip.c             |    1 +
diff --git a/ip/ip.c b/ip/ip.c
index 632d271..c3ff627 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -57,6 +57,7 @@ static void usage(void)
 static int do_help(int argc, char **argv)
 {
 	usage();
+        return 0;
 }
 
 static const struct cmd {

^ permalink raw reply related

* [PATCH v2 4/5] iproute2: clarification of various man8 pages
From: Kees van Reeuwijk @ 2013-02-08 13:32 UTC (permalink / raw)
  To: netdev

From: Kees van Reeuwijk <reeuwijk@few.vu.nl>

Rephrasing for clarity.

Note that in ip-rule.8 I rephrased a sentence to "The RPDB is scanned
in order of decreasing priority." The original version talked about
*in*creasing priority, but from the context that didn't make sense.


Signed-off-by: Kees van Reeuwijk <reeuwijk@few.vu.nl>

---


 arpd.8         |   38 +++++++++++++++++++-------------------
 bridge.8       |   10 +++++-----
 ip-addrlabel.8 |   16 ++++++++--------
 ip-maddress.8  |   10 +++++-----
 ip-monitor.8   |   25 ++++++++++++++++---------
 ip-mroute.8    |    4 ++--
 ip-neighbour.8 |    2 +-
 ip-netconf.8   |    6 +++---
 ip-netns.8     |    4 ++--
 ip-rule.8      |    6 +++---
 ip.8           |   20 ++++++++++----------
 tc.8           |    6 +++---
 12 files changed, 77 insertions(+), 70 deletions(-)

diff --git a/man/man8/arpd.8 b/man/man8/arpd.8
index a14044b..6b9a43a 100644
--- a/man/man8/arpd.8
+++ b/man/man8/arpd.8
@@ -4,12 +4,12 @@
 arpd \- userspace arp daemon.
 
 .SH SYNOPSIS
-Usage: arpd [ -lkh? ] [ -a N ] [ -b dbase ] [ -B number ] [ -f file ] [-p interval ] [ -n time ] [ -R rate ] [ interfaces ]
+Usage: arpd [ -lkh? ] [ -a N ] [ -b dbase ] [ -B number ] [ -f file ] [-p interval ] [ -n time ] [ -R rate ] [ <INTERFACES> ]
 
 .SH DESCRIPTION
 The
 .B arpd
-daemon collects gratuitous ARP information, saving it on local disk and feeding it to kernel on demand to avoid redundant broadcasting due to limited size of kernel ARP cache.
+daemon collects gratuitous ARP information, saving it on local disk and feeding it to the kernel on demand to avoid redundant broadcasting due to limited size of the kernel ARP cache.
 
 .SH OPTIONS
 .TP
@@ -17,41 +17,41 @@ daemon collects gratuitous ARP information, saving it on local disk and feeding
 Print help
 .TP
 -l
-Dump arpd database to stdout and exit. Output consists of three columns: interface index, IP address and MAC address. Negative entries for dead hosts are also shown, in this case MAC address is replaced by word FAILED followed by colon and time when the fact that host is dead was proven the last time.
+Dump the arpd database to stdout and exit. The output consists of three columns: the interface index, the IP address of the interface, and the MAC address of the interface. Negative entries for dead hosts are also shown, in this case the MAC address is replaced by the word FAILED followed by a colon and the most recent time when the fact that the host is dead was proven.
 .TP
 -f <FILE>
-Read and load arpd database from FILE in text format similar dumped by option -l. Exit after load, probably listing resulting database, if option -l is also given. If FILE is -, stdin is read to get ARP table.
+Read and load an arpd database from FILE in a text format similar to that dumped by option -l. Exit after load, possibly listing resulting database, if option -l is also given. If FILE is -, stdin is read to get the ARP table.
 .TP
 -b <DATABASE>
-location of database file. Default location is /var/lib/arpd/arpd.db
+the location of the database file. The default location is /var/lib/arpd/arpd.db
 .TP
 -a <NUMBER>
-arpd not only passively listens ARP on wire, but also send brodcast queries itself. NUMBER is number of such queries to make before destination is considered as dead. When arpd is started as kernel helper (i.e. with app_solicit enabled in sysctl or even with option -k) without this option and still did not learn enough information, you can observe 1 second gaps in service. Not fatal, but not good.
+With this option, arpd not only passively listens for ARP packets on the interface, but also sends brodcast queries itself. NUMBER is the number of such queries to make before a destination is considered dead. When arpd is started as kernel helper (i.e. with app_solicit enabled in sysctl or even with option -k) without this option and still did not learn enough information, you can observe 1 second gaps in service. Not fatal, but not good.
 .TP
 -k
-Suppress sending broadcast queries by kernel. It takes sense together with option -a.
+Suppress sending broadcast queries by the kernel. This option only makes sense together with option -a.
 .TP
 -n <TIME>
-Timeout of negative cache. When resolution fails arpd suppresses further attempts to resolve for this period. It makes sense only together with option -k This timeout should not be too much longer than boot time of a typical host not supporting gratuitous ARP. Default value is 60 seconds.
+Specifies the timeout of the negative cache. When resolution fails, arpd suppresses further attempts to resolve for this period. This option only makes sense together with option '-k'. This timeout should not be too much longer than the boot time of a typical host not supporting gratuitous ARP. Default value is 60 seconds.
 .TP
 -p <TIME>
-Time to wait in seconds between polling attempts to the kernel ARP table. TIME may be a floating point number.  The default value is 30.
+The time to wait in seconds between polling attempts to the kernel ARP table. TIME may be a floating point number.  The default value is 30.
 .TP
 -R <RATE>
 Maximal steady rate of broadcasts sent by arpd in packets per second. Default value is 1.
 .TP
 -B <NUMBER>
-Number of broadcasts sent by <tt/arpd/ back to back. Default value is 3. Together with option <tt/-R/ this option allows to police broadcasting not to exceed B+R*T over any interval of time T.
+The number of broadcasts sent by arpd back to back. Default value is 3. Together with the -R option, this option ensures that the number of ARP queries that are broadcast does not exceed B+R*T over any interval of time T.
 .P
-<INTERFACE> is the name of networking interface to watch. If no interfaces given, arpd monitors all the interfaces. In this case arpd does not adjust sysctl parameters, it is supposed user does this himself after arpd is started.
+<INTERFACES> is a list of names of networking interfaces to watch. If no interfaces are given, arpd monitors all the interfaces. In this case arpd does not adjust sysctl parameters, it is assumed that the user does this himself after arpd is started.
 .P
-Signals
-.br
-arpd exits gracefully syncing database and restoring adjusted sysctl parameters, when receives SIGINT or SIGTERM. SIGHUP syncs database to disk. SIGUSR1 sends some statistics to syslog. Effect of another signals is undefined, they may corrupt database and leave sysctl praameters in an unpredictable state.
+.SH SIGNALS
+.TP
+When arpd receives a SIGINT or SIGTERM signal, it exits gracefully, syncing the database and restoring adjusted sysctl parameters. On a SIGHUP it syncs the database to disk. With SIGUSR1 it sends some statistics to syslog. The effect of any other signals is undefined. In particular, they may corrupt the database and leave the sysctl parameters in an unpredictable state.
 .P
-Note
-.br
-In order for arpd to be able to serve as ARP resolver, kernel must be compiled with the option CONFIG_ARPD and, in the case when interface list in not given on command line, variable app_solicit on interfaces of interest should be in /proc/sys/net/ipv4/neigh/*. If this is not made arpd still collects gratuitous ARP information in its database.
+.SH NOTE
+.TP
+In order for arpd to be able to serve as ARP resolver, the kernel must be compiled with the option CONFIG_ARPD and, in the case when interface list in not given on command line, variable app_solicit on interfaces of interest should be in /proc/sys/net/ipv4/neigh/*. If this is not made arpd still collects gratuitous ARP information in its database.
 .SH EXAMPLES
 .TP
 arpd -b /var/tmp/arpd.db
@@ -64,6 +64,6 @@ arpd -b /var/tmp/arpd.db -a 1 eth0 eth1
 Enable kernel helper, leaving leading role to kernel.
 .TP
 arpd -b /var/tmp/arpd.db -a 3 -k eth0 eth1
-Completely replace kernel resolution on interfaces eth0 and eth1. In this case kernel still does unicast probing to validate entries, but all the broadcast activity is suppressed and made under authority of arpd.
+Completely replace kernel resolution on interfaces eth0 and eth1. In this case the kernel still does unicast probing to validate entries, but all the broadcast activity is suppressed and made under authority of arpd.
 .PP
-This is mode which arpd is supposed to work normally. It is not default just to prevent occasional enabling of too aggressive mode occasionally.
+This is the mode in which arpd normally is supposed to work. It is not the default to prevent occasional enabling of too aggressive a mode.
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 5ce8219..fd91618 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -47,8 +47,8 @@ utility and exit.
 
 .TP
 .BR "\-s" , " \-stats", " \-statistics"
-output more information.  If the option
-appears twice or more, the amount of information increases.
+output more information.  If this option
+is given multiple times, the amount of information increases.
 As a rule, the information is statistics or some time values.
 
 
@@ -135,7 +135,7 @@ The arguments are the same as with
 
 .SS bridge fdb show - list forwarding entries.
 
-This commands displays current forwarding table.
+This command displays the current forwarding table.
 
 .PP
 With the
@@ -154,7 +154,7 @@ Namely, the
 command is the first in the command line and then the object list follows:
 
 .BR "bridge monitor" " [ " all " |"
-.IR LISTofOBJECTS " ]"
+.IR OBJECT-LIST " ]"
 
 .I OBJECT-LIST
 is the list of object types that we want to monitor.
@@ -186,7 +186,7 @@ based on the bridge to which the coresponding ethernet device is attached.
 
 .SH SEE ALSO
 .BR ip (8)
-.br
+.SH BUGS
 .RB "Please direct bugreports and patches to: " <netdev@vger.kernel.org>
 
 .SH AUTHOR
diff --git a/man/man8/ip-addrlabel.8 b/man/man8/ip-addrlabel.8
index 3252bd2..fefc3ef 100644
--- a/man/man8/ip-addrlabel.8
+++ b/man/man8/ip-addrlabel.8
@@ -34,12 +34,12 @@ ip-addrlabel \- protocol address label management
 .BR "ip addrlabel" " { " list " | " flush " }"
 
 .SH "DESCRIPTION"
-IPv6 address label is used for address selection
-described in RFC 3484.  Precedence is managed by userspace,
-and only label is stored in kernel.
+IPv6 address labels are used for address selection;
+they are described in RFC 3484.  Precedence is managed by userspace,
+and only the label itself is stored in the kernel.
 
 .SS ip addrlabel add - add an address label
-the command adds an address label entry to the kernel.
+add an address label entry to the kernel.
 .TP
 .BI prefix " PREFIX"
 .TP
@@ -50,15 +50,15 @@ the outgoing interface.
 the label for the prefix.
 0xffffffff is reserved.
 .SS ip addrlabel del - delete an address label
-the command deletes an address label entry in the kernel.
+delete an address label entry from the kernel.
 .B Arguments:
 coincide with the arguments of
 .B ip addrlabel add
-but label is not required.
+but the label is not required.
 .SS ip addrlabel list - list address labels
-the command show contents of address labels.
+list the current address label entries in the kernel.
 .SS ip addrlabel flush - flush address labels
-the command flushes the contents of address labels and it does not restore default settings.
+flush all address labels in the kernel. This does not restore any default settings.
 
 .SH SEE ALSO
 .br
diff --git a/man/man8/ip-maddress.8 b/man/man8/ip-maddress.8
index afae551..e0bad47 100644
--- a/man/man8/ip-maddress.8
+++ b/man/man8/ip-maddress.8
@@ -15,11 +15,11 @@ ip-maddress \- multicast addresses management
 .ti -8
 
 .BR "ip maddress" " [ " add " | " del " ]"
-.IB MULTIADDR " dev " STRING
+.IB MULTIADDR " dev " NAME
 
 .ti -8
 .BR "ip maddress show" " [ " dev
-.IR STRING " ]"
+.IR NAME " ]"
 
 .SH DESCRIPTION
 .B maddress
@@ -33,14 +33,14 @@ the device name.
 
 .SS ip maddress add - add a multicast address
 .SS ip maddress delete - delete a multicast address
-these commands attach/detach a static link layer multicast address
+these commands attach/detach a static link-layer multicast address
 to listen on the interface.
 Note that it is impossible to join protocol multicast groups
-statically.  This command only manages link layer addresses.
+statically.  This command only manages link-layer addresses.
 
 .TP
 .BI address " LLADDRESS " (default)
-the link layer multicast address.
+the link-layer multicast address.
 
 .TP
 .BI dev " NAME"
diff --git a/man/man8/ip-monitor.8 b/man/man8/ip-monitor.8
index b07cb0e..b6e8d1d 100644
--- a/man/man8/ip-monitor.8
+++ b/man/man8/ip-monitor.8
@@ -6,8 +6,12 @@ ip-monitor, rtmon \- state monitoring
 .ad l
 .in +8
 .ti -8
-.BR "ip " " [ ip-OPTIONS ] " "monitor" " [ " all " |"
-.IR LISTofOBJECTS " ] [ file " FILENAME " ]
+.BR "ip " " [ "
+.IR ip-OPTIONS " ]"
+.BR  "monitor" " [ " all " |"
+.IR OBJECT-LIST " ] ["
+.BI file " FILENAME "
+]
 .sp
 
 .SH DESCRIPTION
@@ -20,7 +24,9 @@ Namely, the
 command is the first in the command line and then the object list follows:
 
 .BR "ip monitor" " [ " all " |"
-.IR LISTofOBJECTS " ] [ file " FILENAME " ]
+.IR OBJECT-LIST " ] ["
+.BI file " FILENAME "
+]
 
 .I OBJECT-LIST
 is the list of object types that we want to monitor.
@@ -35,11 +41,12 @@ opens RTNETLINK, listens on it and dumps state changes in the format
 described in previous sections.
 
 .P
-If a
-.I FILENAME
-is given, it does not listen on RTNETLINK,
-but opens the file containing RTNETLINK messages saved in binary format
-and dumps them.  Such a history file can be generated with the
+If the
+.BI file
+option is given, the program does not listen on RTNETLINK,
+but opens the given file, and dumps its contents. The file
+should contain RTNETLINK messages saved in binary format.
+Such a file can be generated with the
 .B rtmon
 utility.  This utility has a command line syntax similar to
 .BR "ip monitor" .
@@ -56,7 +63,7 @@ in a startup script, you will be able to view the full history
 later.
 
 .P
-Certainly, it is possible to start
+Nevertheless, it is possible to start
 .B rtmon
 at any time.
 It prepends the history with the state snapshot dumped at the moment
diff --git a/man/man8/ip-mroute.8 b/man/man8/ip-mroute.8
index 870df5e..3b708cf 100644
--- a/man/man8/ip-mroute.8
+++ b/man/man8/ip-mroute.8
@@ -18,7 +18,7 @@ ip-mroute \- multicast routing cache management
 
 .SH DESCRIPTION
 .B mroute
-objects are multicast routing cache entries created by a user level
+objects are multicast routing cache entries created by a user-level
 mrouting daemon (f.e.
 .B pimd
 or
@@ -28,7 +28,7 @@ or
 Due to the limitations of the current interface to the multicast routing
 engine, it is impossible to change
 .B mroute
-objects administratively, so we may only display them.  This limitation
+objects administratively, so we can only display them.  This limitation
 will be removed in the future.
 
 .SS ip mroute show - list mroute cache entries
diff --git a/man/man8/ip-neighbour.8 b/man/man8/ip-neighbour.8
index 34980c5..5d9768f 100644
--- a/man/man8/ip-neighbour.8
+++ b/man/man8/ip-neighbour.8
@@ -40,7 +40,7 @@ command manipulates
 objects that establish bindings between protocol addresses and
 link layer addresses for hosts sharing the same link.
 Neighbour entries are organized into tables. The IPv4 neighbour table
-is known by another name - the ARP table.
+is also known by another name - the ARP table.
 
 .P
 The corresponding commands display neighbour bindings
diff --git a/man/man8/ip-netconf.8 b/man/man8/ip-netconf.8
index 8041ea2..2718258 100644
--- a/man/man8/ip-netconf.8
+++ b/man/man8/ip-netconf.8
@@ -8,7 +8,7 @@ ip-netconf \- network configuration monitoring
 .ti -8
 .BR "ip " " [ ip-OPTIONS ] " "netconf show" " [ "
 .B dev
-.IR STRING " ]"
+.IR NAME " ]"
 
 .SH DESCRIPTION
 The
@@ -25,8 +25,8 @@ is displayed.
 .SS ip netconf show - display network parameters
 
 .TP
-.BI dev " STRING"
-the name of the device to display network parameters.
+.BI dev " NAME"
+the name of the device to display network parameters for.
 
 .SH SEE ALSO
 .br
diff --git a/man/man8/ip-netns.8 b/man/man8/ip-netns.8
index d06ccef..87534be 100644
--- a/man/man8/ip-netns.8
+++ b/man/man8/ip-netns.8
@@ -48,8 +48,8 @@ descriptor can be used with the
 .B setns(2)
 system call to change the network namespace associated with a task.
 
-The convention for network namespace aware applications is to look
-for global network configuration files first in
+For applications that are aware of network namespaces, the convention
+is to look for global network configuration files first in
 .BR "/etc/netns/" NAME "/"
 then in
 .BR "/etc/".
diff --git a/man/man8/ip-rule.8 b/man/man8/ip-rule.8
index 0f62a53..36f461b 100644
--- a/man/man8/ip-rule.8
+++ b/man/man8/ip-rule.8
@@ -75,16 +75,16 @@ Each policy routing rule consists of a
 .B selector
 and an
 .B action predicate.
-The RPDB is scanned in the order of increasing priority. The selector
+The RPDB is scanned in order of decreasing priority. The selector
 of each rule is applied to {source address, destination address, incoming
 interface, tos, fwmark} and, if the selector matches the packet,
 the action is performed.  The action predicate may return with success.
 In this case, it will either give a route or failure indication
 and the RPDB lookup is terminated. Otherwise, the RPDB program
-continues on the next rule.
+continues with the next rule.
 
 .P
-Semantically, natural action is to select the nexthop and the output device.
+Semantically, the natural action is to select the nexthop and the output device.
 
 .P
 At startup time the kernel configures the default RPDB consisting of three
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 9063049..881e1ff 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -36,24 +36,24 @@ print the version of the
 utility and exit.
 
 .TP
-.BR "\-s" , " \-stats", " \-statistics"
+.BR "\-s" , " \-stats" , " \-statistics"
 output more information.  If the option
 appears twice or more, the amount of information increases.
 As a rule, the information is statistics or some time values.
 
 .TP
-.BR "\-l" , " \-loops"
+.BR "\-l" , " \-loops " <COUNT>
 Specify maximum number of loops the 'ip addr flush' logic
 will attempt before giving up.  The default is 10.
 Zero (0) means loop until all addresses are removed.
 
 .TP
-.BR "\-f" , " \-family"
-followed by protocol family identifier:
-.BR "inet" , " inet6" , "bridge" , "ipx" , "dnet"
+.BR "\-f" , " \-family " <FAMILY>
+Specifies the protocol family to use. The protocol family identifier can be one of
+.BR "inet" , " inet6" , " bridge" , " ipx" , " dnet"
 or
-.BR link ,
-enforce the protocol family to use.  If the option is not present,
+.BR link .
+If this option is not present,
 the protocol family is guessed from other arguments.  If the rest
 of the command line does not give enough information to guess the
 family,
@@ -178,9 +178,9 @@ host addresses.
 
 .PP
 The names of all objects may be written in full or
-abbreviated form, f.e.
+abbreviated form, for exampe
 .B address
-is abbreviated as
+can be abbreviated as
 .B addr
 or just
 .B a.
@@ -230,7 +230,7 @@ was written by Alexey N. Kuznetsov and added in Linux 2.2.
 .br
 .RB "IP Command reference " ip-cref.ps
 .SH REPORTING BUGS
-Report bug to the Network Developers mailing list
+Report any bugs to the Network Developers mailing list
 .B <netdev@vger.kernel.org>
 where the development and maintenance is primarily done.
 You do not have to be subscribed to the list to send a message there.
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index e9a7229..bbc6790 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -75,7 +75,7 @@ is also called prioritizing, and happens only on egress.
 
 .TP
 POLICING
-Where shaping deals with transmission of traffic, policing pertains to traffic
+Whereas shaping deals with transmission of traffic, policing pertains to traffic
 arriving. Policing thus occurs on ingress.
 
 .TP
@@ -203,7 +203,7 @@ Furthermore, each class contains a
 .B leaf qdisc
 which by default has
 .B pfifo
-behaviour though another qdisc can be attached in place. This qdisc may again
+behaviour, although another qdisc can be attached in place. This qdisc may again
 contain classes, but each class can have only one leaf qdisc.
 
 When a packet enters a classful qdisc it can be
@@ -233,7 +233,7 @@ attached to that class. Check qdisc specific manpages for details, however.
 All qdiscs, classes and filters have IDs, which can either be specified
 or be automatically assigned.
 
-ID's consist of a major number and a minor number, separated by a colon.
+IDs consist of a major number and a minor number, separated by a colon.
 Both major and minor number are limited to 16 bits. There are two special
 values: root is signified by major and minor of all ones, and unspecified
 is all zeros.

^ permalink raw reply related

* [PATCH v2 3/5] iproute2: clarifications in the tc-hfsc.7 man page
From: Kees van Reeuwijk @ 2013-02-08 13:32 UTC (permalink / raw)
  To: netdev

From: Kees van Reeuwijk <reeuwijk@few.vu.nl>

Improved man page as follows:
- Use more `mainstream' english
- Rephrased for clarity
- Use standard notation for units



Signed-off-by: Kees van Reeuwijk <reeuwijk@few.vu.nl>

---


 tc-hfsc.7 |  247 +++++++++++++++++++++++++++++++------------------------------
 1 files changed, 124 insertions(+), 123 deletions(-)

diff --git a/man/man7/tc-hfsc.7 b/man/man7/tc-hfsc.7
index d4e63f2..ca04961 100644
--- a/man/man7/tc-hfsc.7
+++ b/man/man7/tc-hfsc.7
@@ -4,13 +4,12 @@ tc-hfcs \- Hierarchical Fair Service Curve
 .
 .SH "HISTORY & INTRODUCTION"
 .
-HFSC \- \fBHierarchical Fair Service Curve\fR was first presented at
+HFSC (Hierarchical Fair Service Curve) is a network packet scheduling algorithm that was first presented at
 SIGCOMM'97. Developed as a part of ALTQ (ALTernative Queuing) on NetBSD, found
 its way quickly to other BSD systems, and then a few years ago became part of
 the linux kernel. Still, it's not the most popular scheduling algorithm \-
-especially if compared to HTB \- and it's not well documented from enduser's
-perspective. This introduction aims to explain how HFSC works without
-going to deep into math side of things (although some if it will be
+especially if compared to HTB \- and it's not well documented for the enduser. This introduction aims to explain how HFSC works without using
+too much math (although some math it will be
 inevitable).
 
 In short HFSC aims to:
@@ -30,10 +29,10 @@ service provided during linksharing
 .
 The main "selling" point of HFSC is feature \fB(1)\fR, which is achieved by
 using nonlinear service curves (more about what it actually is later). This is
-particularly useful in VoIP or games, where not only guarantee of consistent
-bandwidth is important, but initial delay of a data stream as well. Note that
+particularly useful in VoIP or games, where not only a guarantee of consistent
+bandwidth is important, but also limiting the initial delay of a data stream. Note that
 it matters only for leaf classes (where the actual queues are) \- thus class
-hierarchy is ignored in realtime case.
+hierarchy is ignored in the realtime case.
 
 Feature \fB(2)\fR is well, obvious \- any algorithm featuring class hierarchy
 (such as HTB or CBQ) strives to achieve that. HFSC does that well, although
@@ -44,8 +43,8 @@ Feature \fB(3)\fR is mentioned due to the nature of the problem. There may be
 situations where it's either not possible to guarantee service of all curves at
 the same time, and/or it's impossible to do so fairly. Both will be explained
 later. Note that this is mainly related to interior (aka aggregate) classes, as
-the leafs are already handled by \fB(1)\fR. Still \- it's perfectly possible to
-create a leaf class w/o realtime service, and in such case \- the caveats will
+the leafs are already handled by \fB(1)\fR. Still, it's perfectly possible to
+create a leaf class without realtime service, and in such a case the caveats will
 naturally extend to leaf classes as well.
 
 .SH ABBREVIATIONS
@@ -62,21 +61,22 @@ SC \- service curve
 .SH "BASICS OF HFSC"
 .
 To understand how HFSC works, we must first introduce a service curve.
-Overall, it's a nondecreasing function of some time unit, returning amount of
-service (allowed or allocated amount of bandwidth) by some specific point in
-time. The purpose of it should be subconsciously obvious \- if a class was
-allowed to transfer not less than the amount specified by its service curve \-
-then service curve is not violated.
-
-Still \- we need more elaborate criterion than just the above (although in
-most generic case it can be reduced to it). The criterion has to take two
+Overall, it's a nondecreasing function of some time unit, returning the amount
+of
+service (an allowed or allocated amount of bandwidth) at some specific point in
+time. The purpose of it should be subconsciously obvious: if a class was
+allowed to transfer not less than the amount specified by its service curve,
+then the service curve is not violated.
+
+Still, we need more elaborate criterion than just the above (although in
+the most generic case it can be reduced to it). The criterion has to take two
 things into account:
 .
 .RS 4
 .IP \(bu 4
 idling periods
 .IP \(bu
-ability to "look back", so if during current active period service curve is violated, maybe it
+the ability to "look back", so if during current active period the service curve is violated, maybe it
 isn't if we count excess bandwidth received during earlier active period(s)
 .RE
 .PP
@@ -102,9 +102,9 @@ as in (a), but with a larger gap
 .RE
 .
 .PP
-Consider \fB(a)\fR \- if the service received during both periods meets
-\fB(1)\fR, then all is good. But what if it doesn't do so during the 2nd
-period ? If the amount of service received during the 1st period is bigger
+Consider \fB(a)\fR: if the service received during both periods meets
+\fB(1)\fR, then all is well. But what if it doesn't do so during the 2nd
+period? If the amount of service received during the 1st period is larger
 than the service curve, then it might compensate for smaller service during
 the 2nd period \fIand\fR the gap \- if the gap is small enough.
 
@@ -172,42 +172,43 @@ curves and the above "utility" functions.
 .SH "REALTIME CRITERION"
 .
 RT criterion \fIignores class hierarchy\fR and guarantees precise bandwidth and
-delay allocation. We say that packet is eligible for sending, when current real
-time is bigger than eligible time. From all packets eligible, the one most
-suited for sending, is the one with the smallest deadline time. Sounds simply,
-but consider following example:
+delay allocation. We say that a packet is eligible for sending, when the
+current real
+time is later than the eligible time of the packet. From all eligible packets, the one most
+suited for sending is the one with the shortest deadline time. This sounds
+simple, but consider the following example:
 
-Interface 10mbit, two classes, both with two\-piece linear service curves:
+Interface 10Mbit, two classes, both with two\-piece linear service curves:
 .RS 4
 .IP \(bu 4
-1st class \- 2mbit for 100ms, then 7mbit (convex \- 1st slope < 2nd slope)
+1st class \- 2Mbit for 100ms, then 7Mbit (convex \- 1st slope < 2nd slope)
 .IP \(bu
-2nd class \- 7mbit for 100ms, then 2mbit (concave \- 1st slope > 2nd slope)
+2nd class \- 7Mbit for 100ms, then 2Mbit (concave \- 1st slope > 2nd slope)
 .RE
 .PP
 Assume for a moment, that we only use D() for both finding eligible packets,
 and choosing the most fitting one, thus eligible time would be computed as
 D^(\-1)(w) and deadline time would be computed as D^(\-1)(w+l). If the 2nd
 class starts sending packets 1 second after the 1st class, it's of course
-impossible to guarantee 14mbit, as the interface capability is only 10mbit.
+impossible to guarantee 14Mbit, as the interface capability is only 10Mbit.
 The only workaround in this scenario is to allow the 1st class to send the
 packets earlier that would normally be allowed. That's where separate E() comes
 to help. Putting all the math aside (see HFSC paper for details), E() for RT
 concave service curve is just like D(), but for the RT convex service curve \-
 it's constructed using \fIonly\fR RT service curve's 2nd slope (in our example
-\- 7mbit).
+ 7Mbit).
 
 The effect of such E() \- packets will be sent earlier, and at the same time
-D() \fIwill\fR be updated \- so current deadline time calculated from it will
-be bigger. Thus, when the 2nd class starts sending packets later, both the 1st
-and the 2nd class will be eligible, but the 2nd session's deadline time will be
-smaller and its packets will be sent first. When the 1st class becomes idle at
-some later point, the 2nd class will be able to "buffer" up again for later
-active period of the 1st class.
+D() \fIwill\fR be updated \- so the current deadline time calculated from it
+will be later. Thus, when the 2nd class starts sending packets later, both
+the 1st and the 2nd class will be eligible, but the 2nd session's deadline
+time will be smaller and its packets will be sent first. When the 1st class
+becomes idle at some later point, the 2nd class will be able to "buffer" up
+again for later active period of the 1st class.
 
 A short remark \- in a situation, where the total amount of bandwidth
-available on the interface is bigger than the allocated total realtime parts
-(imagine interface 10 mbit, but 1mbit/2mbit and 2mbit/1mbit classes), the sole
+available on the interface is larger than the allocated total realtime parts
+(imagine a 10 Mbit interface, but 1Mbit/2Mbit and 2Mbit/1Mbit classes), the sole
 speed of the interface could suffice to guarantee the times.
 
 Important part of RT criterion is that apart from updating its D() and E(),
@@ -233,18 +234,18 @@ real time and virtual time \- the decision is based solely on direct comparison
 of virtual times of all active subclasses \- the one with the smallest vt wins
 and gets scheduled. One immediate conclusion from this fact is that absolute
 values don't matter \- only ratios between them (so for example, two children
-classes with simple linear 1mbit service curves will get the same treatment
-from LS criterion's perspective, as if they were 5mbit). The other conclusion
+classes with simple linear 1Mbit service curves will get the same treatment
+from LS criterion's perspective, as if they were 5Mbit). The other conclusion
 is, that in perfectly fluid system with linear curves, all virtual times across
 whole class hierarchy would be equal.
 
-Why is VC defined in term of virtual time (and what is it) ?
+Why is VC defined in term of virtual time (and what is it)?
 
 Imagine an example: class A with two children \- A1 and A2, both with let's say
-10mbit SCs. If A2 is idle, A1 receives all the bandwidth of A (and update its
+10Mbit SCs. If A2 is idle, A1 receives all the bandwidth of A (and update its
 V() in the process). When A2 becomes active, A1's virtual time is already
-\fIfar\fR bigger than A2's one. Considering the type of decision made by LS
-criterion, A1 would become idle for a lot of time. We can workaround this
+\fIfar\fR later than A2's one. Considering the type of decision made by LS
+criterion, A1 would become idle for a long time. We can workaround this
 situation by adjusting virtual time of the class becoming active \- we do that
 by getting such time "up to date". HFSC uses a mean of the smallest and the
 biggest virtual time of currently active children fit for sending. As it's not
@@ -259,20 +260,20 @@ either it's impossible to guarantee service curves and satisfy fairness
 during certain time periods:
 
 .RS 4
-Recall the example from RT section, slightly modified (with 3mbit slopes
-instead of 2mbit ones):
+Recall the example from RT section, slightly modified (with 3Mbit slopes
+instead of 2Mbit ones):
 
 .IP \(bu 4
-1st class \- 3mbit for 100ms, then 7mbit (convex \- 1st slope < 2nd slope)
+1st class \- 3Mbit for 100ms, then 7Mbit (convex \- 1st slope < 2nd slope)
 .IP \(bu
-2nd class \- 7mbit for 100ms, then 3mbit (concave \- 1st slope > 2nd slope)
+2nd class \- 7Mbit for 100ms, then 3Mbit (concave \- 1st slope > 2nd slope)
 
 .PP
-They sum up nicely to 10mbit \- interface's capacity. But if we wanted to only
+They sum up nicely to 10Mbit \- the interface's capacity. But if we wanted to only
 use LS for guarantees and fairness \- it simply won't work. In LS context,
 only V() is used for making decision which class to schedule. If the 2nd class
 becomes active when the 1st one is in its second slope, the fairness will be
-preserved \- ratio will be 1:1 (7mbit:7mbit), but LS itself is of course
+preserved \- ratio will be 1:1 (7Mbit:7Mbit), but LS itself is of course
 unable to guarantee the absolute values themselves \- as it would have to go
 beyond of what the interface is capable of.
 .RE
@@ -287,28 +288,28 @@ This is similar to the above case, but a bit more subtle. We will consider two
 subtrees, arbitrated by their common (root here) parent:
 
 .nf
-R (root) -\ 10mbit
+R (root) -\ 10Mbit
 
-A  \- 7mbit, then 3mbit
-A1 \- 5mbit, then 2mbit
-A2 \- 2mbit, then 1mbit
+A  \- 7Mbit, then 3Mbit
+A1 \- 5Mbit, then 2Mbit
+A2 \- 2Mbit, then 1Mbit
 
-B  \- 3mbit, then 7mbit
+B  \- 3Mbit, then 7Mbit
 .fi
 
 R arbitrates between left subtree (A) and right (B). Assume that A2 and B are
 constantly backlogged, and at some later point A1 becomes backlogged (when all
 other classes are in their 2nd linear part).
 
-What happens now ? B (choice made by R) will \fIalways\fR get 7 mbit as R is
+What happens now? B (choice made by R) will \fIalways\fR get 7 Mbit as R is
 only (obviously) concerned with the ratio between its direct children. Thus A
-subtree gets 3mbit, but its children would want (at the point when A1 became
-backlogged) 5mbit + 1mbit. That's of course impossible, as they can only get
-3mbit due to interface limitation.
+subtree gets 3Mbit, but its children would want (at the point when A1 became
+backlogged) 5Mbit + 1Mbit. That's of course impossible, as they can only get
+3Mbit due to interface limitation.
 
 In the left subtree \- we have the same situation as previously (fair split
 between A1 and A2, but violated guarantees), but in the whole tree \- there's
-no fairness (B got 7mbit, but A1 and A2 have to fit together in 3mbit) and
+no fairness (B got 7Mbit, but A1 and A2 have to fit together in 3Mbit) and
 there's no guarantees for all classes (only B got what it wanted). Even if we
 violated fairness in the A subtree and set A2's service curve to 0, A1 would
 still not get the required bandwidth.
@@ -317,83 +318,83 @@ still not get the required bandwidth.
 .SH "UPPERLIMIT CRITERION"
 .
 UL criterion is an extensions to LS one, that permits sending packets only
-if current real time is bigger than fit\-time ('ft'). So the modified LS
+if current real time is later than fit\-time ('ft'). So the modified LS
 criterion becomes: choose the smallest virtual time from all active children,
 such that fit\-time < current real time also holds. Fit\-time is calculated
 from F(), which is based on UL service curve. As you can see, its role is
 kinda similar to E() used in RT criterion. Also, for obvious reasons \- you
 can't specify UL service curve without LS one.
 
-Main purpose of UL service curve is to limit HFSC to bandwidth available on the
+The main purpose of the UL service curve is to limit HFSC to bandwidth available on the
 upstream router (think adsl home modem/router, and linux server as
-nat/firewall/etc. with 100mbit+ connection to mentioned modem/router).
+NAT/firewall/etc. with 100Mbit+ connection to mentioned modem/router).
 Typically, it's used to create a single class directly under root, setting
-linear UL service curve to available bandwidth \- and then creating your class
-structure from that class downwards. Of course, you're free to add UL service
-(linear or not) curve to any class with LS criterion.
+a linear UL service curve to available bandwidth \- and then creating your class
+structure from that class downwards. Of course, you're free to add a UL service
+curve (linear or not) to any class with LS criterion.
 
-Important part about UL service curve is, that whenever at some point in time
+An important part about the UL service curve is that whenever at some point in time
 a class doesn't qualify for linksharing due to its fit\-time, the next time it
-does qualify, it will update its virtual time to the smallest virtual time of
-all active children fit for linksharing. This way, one of the main things LS
+does qualify it will update its virtual time to the smallest virtual time of
+all active children fit for linksharing. This way, one of the main things the LS
 criterion tries to achieve \- equality of all virtual times across whole
 hierarchy \- is preserved (in perfectly fluid system with only linear curves,
 all virtual times would be equal).
 
 Without that, 'vt' would lag behind other virtual times, and could cause
-problems. Consider interface with capacity 10mbit, and following leaf classes
+problems. Consider an interface with a capacity of 10Mbit, and the following leaf classes
 (just in case you're skipping this text quickly \- this example shows behavior
 that \f(BIdoesn't happen\fR):
 
 .nf
-A \- ls 5.0mbit
-B \- ls 2.5mbit
-C \- ls 2.5mbit, ul 2.5mbit
+A \- ls 5.0Mbit
+B \- ls 2.5Mbit
+C \- ls 2.5Mbit, ul 2.5Mbit
 .fi
 
-If B was idle, while A and C were constantly backlogged, they would normally
+If B was idle, while A and C were constantly backlogged, A and C would normally
 (as far as LS criterion is concerned) divide bandwidth in 2:1 ratio. But due
-to UL service curve in place, C would get at most 2.5mbit, and A would get the
-remaining 7.5mbit. The longer the backlogged period, the more virtual times of
+to UL service curve in place, C would get at most 2.5Mbit, and A would get the
+remaining 7.5Mbit. The longer the backlogged period, the more the virtual times of
 A and C would drift apart. If B became backlogged at some later point in time,
 its virtual time would be set to (A's\~vt\~+\~C's\~vt)/2, thus blocking A from
-sending any traffic, until B's virtual time catches up with A.
+sending any traffic until B's virtual time catches up with A.
 .
 .SH "SEPARATE LS / RT SCs"
 .
-Another difference from original HFSC paper, is that RT and LS SCs can be
-specified separately. Moreover \- leaf classes are allowed to have only either
-RT SC or LS SC. For interior classes, only LS SCs make sense \- Any RT SC will
+Another difference from the original HFSC paper is that RT and LS SCs can be
+specified separately. Moreover, leaf classes are allowed to have only either
+RT SC or LS SC. For interior classes, only LS SCs make sense: any RT SC will
 be ignored.
 .
 .SH "CORNER CASES"
 .
-Separate service curves for LS and RT criteria can lead to certain traps,
+Separate service curves for LS and RT criteria can lead to certain traps
 that come from "fighting" between ideal linksharing and enforced realtime
 guarantees. Those situations didn't exist in original HFSC paper, where
 specifying separate LS / RT service curves was not discussed.
 
-Consider interface with capacity 10mbit, with following leaf classes:
+Consider an interface with a 10Mbit capacity, with the following leaf classes:
 
 .nf
-A \- ls 5.0mbit, rt 8mbit
-B \- ls 2.5mbit
-C \- ls 2.5mbit
+A \- ls 5.0Mbit, rt 8Mbit
+B \- ls 2.5Mbit
+C \- ls 2.5Mbit
 .fi
 
 Imagine A and C are constantly backlogged. As B is idle, A and C would divide
 bandwidth in 2:1 ratio, considering LS service curve (so in theory \- 6.66 and
-3.33). Alas RT criterion takes priority, so A will get 8mbit and LS will be
-able to compensate class C for only 2 mbit \- this will cause discrepancy
+3.33). Alas RT criterion takes priority, so A will get 8Mbit and LS will be
+able to compensate class C for only 2 Mbit \- this will cause discrepancy
 between virtual times of A and C.
 
-Assume this situation lasts for a lot of time with no idle periods, and
+Assume this situation lasts for a long time with no idle periods, and
 suddenly B becomes active. B's virtual time will be updated to
 (A's\~vt\~+\~C's\~vt)/2, effectively landing in the middle between A's and C's
 virtual time. The effect \- B, having no RT guarantees, will be punished and
 will not be allowed to transfer until C's virtual time catches up.
 
-If the interface had higher capacity \- for example 100mbit, this example
+If the interface had a higher capacity, for example 100Mbit, this example
 would behave perfectly fine though.
 
 Let's look a bit closer at the above example \- it "cleverly" invalidates one
@@ -401,8 +402,8 @@ of the basic things LS criterion tries to achieve \- equality of all virtual
 times across class hierarchy. Leaf classes without RT service curves are
 literally left to their own fate (governed by messed up virtual times).
 
-Also - it doesn't make much sense. Class A will always be guaranteed up to
-8mbit, and this is more than any absolute bandwidth that could happen from its
+Also, it doesn't make much sense. Class A will always be guaranteed up to
+8Mbit, and this is more than any absolute bandwidth that could happen from its
 LS criterion (excluding trivial case of only A being active). If the bandwidth
 taken by A is smaller than absolute value from LS criterion, the unused part
 will be automatically assigned to other active classes (as A has idling periods
@@ -411,7 +412,7 @@ average, bursts would be handled at the speed defined by RT criterion. Still,
 if extra speed is needed (e.g. due to latency), non linear service curves
 should be used in such case.
 
-In the other words - LS criterion is meaningless in the above example.
+In the other words: the LS criterion is meaningless in the above example.
 
 You can quickly "workaround" it by making sure each leaf class has RT service
 curve assigned (thus guaranteeing all of them will get some bandwidth), but it
@@ -422,13 +423,13 @@ happen \fIonly\fR in the first segment, then there's little wrong with
 "overusing" RT curve a bit:
 
 .nf
-A \- ls 5.0mbit, rt 9mbit/30ms, then 1mbit
-B \- ls 2.5mbit
-C \- ls 2.5mbit
+A \- ls 5.0Mbit, rt 9Mbit/30ms, then 1Mbit
+B \- ls 2.5Mbit
+C \- ls 2.5Mbit
 .fi
 
 Here, the vt of A will "spike" in the initial period, but then A will never get more
-than 1mbit, until B & C catch up. Then everything will be back to normal.
+than 1Mbit until B & C catch up. Then everything will be back to normal.
 .
 .SH "LINUX AND TIMER RESOLUTION"
 .
@@ -457,43 +458,43 @@ or aren't available.
 
 This is important to keep those settings in mind, as in scenario like: no
 tickless, no HR timers, frequency set to 100hz \- throttling accuracy would be
-at 10ms. It doesn't automatically mean you would be limited to ~0.8mbit/s
+at 10ms. It doesn't automatically mean you would be limited to ~0.8Mbit/s
 (assuming packets at ~1KB) \- as long as your queues are prepared to cover for
-timer inaccuracy. Of course, in case of e.g. locally generated udp traffic \-
+timer inaccuracy. Of course, in case of e.g. locally generated UDP traffic \-
 appropriate socket size is needed as well. Short example to make it more
 understandable (assume hardcore anti\-schedule settings \- HZ=100, no HR
 timers, no tickless):
 
 .nf
 tc qdisc add dev eth0 root handle 1:0 hfsc default 1
-tc class add dev eth0 parent 1:0 classid 1:1 hfsc rt m2 10mbit
+tc class add dev eth0 parent 1:0 classid 1:1 hfsc rt m2 10Mbit
 .fi
 
-Assuming packet of ~1KB size and HZ=100, that averages to ~0.8mbit \- anything
-beyond it (e.g. the above example with specified rate over 10x bigger) will
+Assuming packet of ~1KB size and HZ=100, that averages to ~0.8Mbit \- anything
+beyond it (e.g. the above example with specified rate over 10x larger) will
 require appropriate queuing and cause bursts every ~10 ms. As you can
 imagine, any HFSC's RT guarantees will be seriously invalidated by that.
 Aforementioned example is mainly important if you deal with old hardware \- as
-it's particularly popular for home server chores. Even then, you can easily
+is particularly popular for home server chores. Even then, you can easily
 set HZ=1000 and have very accurate scheduling for typical adsl speeds.
 
 Anything modern (apic or even hpet msi based timers + \&'tickless system')
-will provide enough accuracy for superb 1gbit scheduling. For example, on one
-of basically cheap dual core AMD boards I have with following settings:
+will provide enough accuracy for superb 1Gbit scheduling. For example, on one
+of my cheap dual-core AMD boards I have the following settings:
 
 .nf
 tc qdisc add dev eth0 parent root handle 1:0 hfsc default 1
-tc class add dev eth0 paretn 1:0 classid 1:1 hfsc rt m2 300mbit
+tc class add dev eth0 parent 1:0 classid 1:1 hfsc rt m2 300mbit
 .fi
 
-And simple:
+And a simple:
 
 .nf
 nc \-u dst.host.com 54321 </dev/zero
 nc \-l \-p 54321 >/dev/null
 .fi
 
-\&...will yield following effects over period of ~10 seconds (taken from
+\&...will yield the following effects over a period of ~10 seconds (taken from
 /proc/interrupts):
 
 .nf
@@ -502,16 +503,16 @@ nc \-l \-p 54321 >/dev/null
 .fi
 
 That's roughly 31000/s. Now compare it with HZ=1000 setting. The obvious
-drawback of it is that cpu load can be rather extensive with servicing that
-many timer interrupts. Example with 300mbit RT service curve on 1gbit link is
+drawback of it is that cpu load can be rather high with servicing that
+many timer interrupts. The example with 300Mbit RT service curve on 1Gbit link is
 particularly ugly, as it requires a lot of throttling with minuscule delays.
 
-Also note that it's just an example showing capability of current hardware.
-The above example (essentially 300mbit TBF emulator) is pointless on internal
-interface to begin with \- you will pretty much always want regular LS service
-curve there, and in such scenario HFSC simply doesn't throttle at all.
+Also note that it's just an example showing the capabilities of current hardware.
+The above example (essentially a 300Mbit TBF emulator) is pointless on an internal
+interface to begin with: you will pretty much always want a regular LS service
+curve there, and in such a scenario HFSC simply doesn't throttle at all.
 
-300mbit RT service curve (selected columns from mpstat \-P ALL 1):
+300Mbit RT service curve (selected columns from mpstat \-P ALL 1):
 
 .nf
 10:56:43 PM  CPU  %sys     %irq   %soft   %idle
@@ -520,28 +521,28 @@ curve there, and in such scenario HFSC simply doesn't throttle at all.
 10:56:44 PM    1   4.95   12.87    6.93   73.27
 .fi
 
-So, in rare case you need those speeds with only RT service curve, or with UL
-service curve \- remember about drawbacks.
+So, in the rare case you need those speeds with only a RT service curve, or with a UL
+service curve: remember the drawbacks.
 .
 .SH "CAVEAT: RANDOM ONLINE EXAMPLES"
 .
 For reasons unknown (though well guessed), many examples you can google love to
 overuse UL criterion and stuff it in every node possible. This makes no sense
 and works against what HFSC tries to do (and does pretty damn well). Use UL
-where it makes sense - on the uppermost node to match upstream router's uplink
-capacity. Or - in special cases, such as testing (limit certain subtree to some
-speed) or customers that must never get more than certain speed. In the last
-case you can usually achieve the same by just using RT criterion without LS+UL
+where it makes sense: on the uppermost node to match upstream router's uplink
+capacity. Or in special cases, such as testing (limit certain subtree to some
+speed), or customers that must never get more than certain speed. In the last
+case you can usually achieve the same by just using a RT criterion without LS+UL
 on leaf nodes.
 
-As for router case - remember it's good to differentiate between "traffic to
+As for the router case - remember it's good to differentiate between "traffic to
 router" (remote console, web config, etc.) and "outgoing traffic", so for
 example:
 
 .nf
 tc qdisc add dev eth0 root handle 1:0 hfsc default 0x8002
-tc class add dev eth0 parent 1:0 classid 1:999 hfsc rt m2 50mbit
-tc class add dev eth0 parent 1:0 classid 1:1 hfsc ls m2 2mbit ul m2 2mbit
+tc class add dev eth0 parent 1:0 classid 1:999 hfsc rt m2 50Mbit
+tc class add dev eth0 parent 1:0 classid 1:1 hfsc ls m2 2Mbit ul m2 2Mbit
 .fi
 
 \&... so "internet" tree under 1:1 and "router itself" as 1:999

^ permalink raw reply related

* [PATCH v2 5/5] iproute2: improved error messages
From: Kees van Reeuwijk @ 2013-02-08 13:32 UTC (permalink / raw)
  To: netdev

From: Kees van Reeuwijk <reeuwijk@few.vu.nl>

This patch improves many error messages as follows:
- For incorrect parameters, show the value of the offending parameter, rather than just say that it is incorrect
- Rephrased messages for clarity
- Rephrased to more `mainstream' english


Signed-off-by: Kees van Reeuwijk <reeuwijk@few.vu.nl>

---


 ip6tunnel.c      |    8 ++++----
 ipaddress.c      |    2 +-
 ipl2tp.c         |    2 +-
 iplink.c         |   28 ++++++++++++++--------------
 iplink_can.c     |    6 +++---
 iplink_ipoib.c   |    2 +-
 iplink_macvlan.c |    2 +-
 iplink_macvtap.c |    8 ++++----
 iplink_vlan.c    |   12 ++++++------
 iplink_vxlan.c   |    2 +-
 ipnetns.c        |   20 ++++++++++----------
 ipprefix.c       |    2 +-
 iproute.c        |    4 ++--
 iptunnel.c       |   30 +++++++++++++++---------------
 iptuntap.c       |    2 +-
 ipxfrm.c         |    2 +-
 link_gre.c       |    8 ++++----
 link_vti.c       |   10 +++++-----
 tcp_metrics.c    |    2 +-
 tunnel.c         |    6 +++---
 20 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index fcc9f33..216e982 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -128,7 +128,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm *p)
 				 strcmp(*argv, "any") == 0)
 				p->proto = 0;
 			else {
-                                fprintf(stderr,"Cannot guess tunnel mode.\n");
+                                fprintf(stderr,"Unknown tunnel mode \"%s\"\n", *argv);
                                 exit(-1);
                         }
                 } else if (strcmp(*argv, "remote") == 0) {
@@ -293,7 +293,7 @@ static int do_tunnels_list(struct ip6_tnl_parm *p)
 		buf[sizeof(buf) - 1] = '\0';
 		if ((ptr = strchr(buf, ':')) == NULL ||
 		    (*ptr++ = 0, sscanf(buf, "%s", name) != 1)) {
-			fprintf(stderr, "Wrong format of /proc/net/dev. Sorry.\n");
+			fprintf(stderr, "Wrong format for /proc/net/dev. Giving up.\n");
 			goto end;
 		}
 		if (sscanf(ptr, "%ld%ld%ld%ld%ld%ld%ld%*d%ld%ld%ld%ld%ld%ld%ld",
@@ -309,7 +309,7 @@ static int do_tunnels_list(struct ip6_tnl_parm *p)
 			continue;
 		type = ll_index_to_type(index);
 		if (type == -1) {
-			fprintf(stderr, "Failed to get type of [%s]\n", name);
+			fprintf(stderr, "Failed to get type of \"%s\"\n", name);
 			continue;
 		}
 		if (type != ARPHRD_TUNNEL6)
@@ -402,7 +402,7 @@ int do_ip6tunnel(int argc, char **argv)
 	case AF_INET6:
 		break;
 	default:
-		fprintf(stderr, "Unsupported family:%d\n", preferred_family);
+		fprintf(stderr, "Unsupported protocol family: %d\n", preferred_family);
 		exit(-1);
 	}
 
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 297506d..0bd8a76 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -785,7 +785,7 @@ static int ipadd_save_prep(void)
 	int ret;
 
 	if (isatty(STDOUT_FILENO)) {
-		fprintf(stderr, "Not sending binary stream to stdout\n");
+		fprintf(stderr, "Not sending a binary stream to stdout\n");
 		return -1;
 	}
 
diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index f6e264a..2d22317 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -494,7 +494,7 @@ static int parse_args(int argc, char **argv, int cmd, struct l2tp_parm *p)
 			} else if (strcmp(*argv, "udp") == 0) {
 				p->encap = L2TP_ENCAPTYPE_UDP;
 			} else {
-				fprintf(stderr, "Unknown tunnel encapsulation.\n");
+				fprintf(stderr, "Unknown tunnel encapsulation \"%s\"\n", *argv);
 				exit(-1);
 			}
 		} else if (strcmp(*argv, "name") == 0) {
diff --git a/ip/iplink.c b/ip/iplink.c
index 5ff8f85..c24f0ce 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -94,9 +94,9 @@ static void usage(void)
 	iplink_usage();
 }
 
-static int on_off(char *msg)
+static int on_off(const char *msg, const char *realval)
 {
-	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\"\n", msg);
+	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n", msg, realval);
 	return -1;
 }
 
@@ -348,7 +348,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			} else if (strcmp(*argv, "off") == 0) {
 				req->i.ifi_flags &= ~IFF_MULTICAST;
 			} else
-				return on_off("multicast");
+				return on_off("multicast", *argv);
 		} else if (strcmp(*argv, "allmulticast") == 0) {
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_ALLMULTI;
@@ -357,7 +357,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			} else if (strcmp(*argv, "off") == 0) {
 				req->i.ifi_flags &= ~IFF_ALLMULTI;
 			} else
-				return on_off("allmulticast");
+				return on_off("allmulticast", *argv);
 		} else if (strcmp(*argv, "promisc") == 0) {
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_PROMISC;
@@ -366,7 +366,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			} else if (strcmp(*argv, "off") == 0) {
 				req->i.ifi_flags &= ~IFF_PROMISC;
 			} else
-				return on_off("promisc");
+				return on_off("promisc", *argv);
 		} else if (strcmp(*argv, "trailers") == 0) {
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_NOTRAILERS;
@@ -375,7 +375,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			} else if (strcmp(*argv, "on") == 0) {
 				req->i.ifi_flags &= ~IFF_NOTRAILERS;
 			} else
-				return on_off("trailers");
+				return on_off("trailers", *argv);
 		} else if (strcmp(*argv, "arp") == 0) {
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_NOARP;
@@ -384,7 +384,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			} else if (strcmp(*argv, "off") == 0) {
 				req->i.ifi_flags |= IFF_NOARP;
 			} else
-				return on_off("noarp");
+				return on_off("noarp", *argv);
 		} else if (strcmp(*argv, "vf") == 0) {
 			struct rtattr *vflist;
 			NEXT_ARG();
@@ -417,7 +417,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			} else if (strcmp(*argv, "off") == 0) {
 				req->i.ifi_flags &= ~IFF_DYNAMIC;
 			} else
-				return on_off("dynamic");
+				return on_off("dynamic", *argv);
 		} else if (matches(*argv, "type") == 0) {
 			NEXT_ARG();
 			*type = *argv;
@@ -852,7 +852,7 @@ static int do_set(int argc, char **argv)
 			} else if (strcmp(*argv, "off") == 0) {
 				flags &= ~IFF_MULTICAST;
 			} else
-				return on_off("multicast");
+				return on_off("multicast", *argv);
 		} else if (strcmp(*argv, "allmulticast") == 0) {
 			NEXT_ARG();
 			mask |= IFF_ALLMULTI;
@@ -861,7 +861,7 @@ static int do_set(int argc, char **argv)
 			} else if (strcmp(*argv, "off") == 0) {
 				flags &= ~IFF_ALLMULTI;
 			} else
-				return on_off("allmulticast");
+				return on_off("allmulticast", *argv);
 		} else if (strcmp(*argv, "promisc") == 0) {
 			NEXT_ARG();
 			mask |= IFF_PROMISC;
@@ -870,7 +870,7 @@ static int do_set(int argc, char **argv)
 			} else if (strcmp(*argv, "off") == 0) {
 				flags &= ~IFF_PROMISC;
 			} else
-				return on_off("promisc");
+				return on_off("promisc", *argv);
 		} else if (strcmp(*argv, "trailers") == 0) {
 			NEXT_ARG();
 			mask |= IFF_NOTRAILERS;
@@ -879,7 +879,7 @@ static int do_set(int argc, char **argv)
 			} else if (strcmp(*argv, "on") == 0) {
 				flags &= ~IFF_NOTRAILERS;
 			} else
-				return on_off("trailers");
+				return on_off("trailers", *argv);
 		} else if (strcmp(*argv, "arp") == 0) {
 			NEXT_ARG();
 			mask |= IFF_NOARP;
@@ -888,7 +888,7 @@ static int do_set(int argc, char **argv)
 			} else if (strcmp(*argv, "off") == 0) {
 				flags |= IFF_NOARP;
 			} else
-				return on_off("noarp");
+				return on_off("noarp", *argv);
 		} else if (matches(*argv, "dynamic") == 0) {
 			NEXT_ARG();
 			mask |= IFF_DYNAMIC;
@@ -897,7 +897,7 @@ static int do_set(int argc, char **argv)
 			} else if (strcmp(*argv, "off") == 0) {
 				flags &= ~IFF_DYNAMIC;
 			} else
-				return on_off("dynamic");
+				return on_off("dynamic", *argv);
 		} else {
                         if (strcmp(*argv, "dev") == 0) {
 				NEXT_ARG();
diff --git a/ip/iplink_can.c b/ip/iplink_can.c
index c8af4bc..5674358 100644
--- a/ip/iplink_can.c
+++ b/ip/iplink_can.c
@@ -68,8 +68,8 @@ static void set_ctrlmode(char* name, char *arg,
 		cm->flags |= flags;
 	} else if (strcmp(arg, "off") != 0) {
 		fprintf(stderr,
-			"Error: argument of \"%s\" must be \"on\" or \"off\"\n",
-			name);
+			"Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n",
+			name, arg);
 		exit(-1);
 	}
 	cm->mask |= flags;
@@ -169,7 +169,7 @@ static int can_parse_opt(struct link_util *lu, int argc, char **argv,
 			usage();
 			return -1;
 		} else {
-			fprintf(stderr, "can: what is \"%s\"?\n", *argv);
+			fprintf(stderr, "can: unknown option \"%s\"\n", *argv);
 			usage();
 			return -1;
 		}
diff --git a/ip/iplink_ipoib.c b/ip/iplink_ipoib.c
index ae372bc..5c1c68c 100644
--- a/ip/iplink_ipoib.c
+++ b/ip/iplink_ipoib.c
@@ -66,7 +66,7 @@ static int ipoib_parse_opt(struct link_util *lu, int argc, char **argv,
 			explain();
 			return -1;
 		} else {
-			fprintf(stderr, "ipoib: what is \"%s\"?\n", *argv);
+			fprintf(stderr, "ipoib: unknown option \"%s\"?\n", *argv);
 			explain();
 			return -1;
 		}
diff --git a/ip/iplink_macvlan.c b/ip/iplink_macvlan.c
index ed0e34b..5b4b868 100644
--- a/ip/iplink_macvlan.c
+++ b/ip/iplink_macvlan.c
@@ -58,7 +58,7 @@ static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			explain();
 			return -1;
 		} else {
-			fprintf(stderr, "macvlan: what is \"%s\"?\n", *argv);
+			fprintf(stderr, "macvlan: unknown option \"%s\"?\n", *argv);
 			explain();
 			return -1;
 		}
diff --git a/ip/iplink_macvtap.c b/ip/iplink_macvtap.c
index 6c7fe1f..bea9f0c 100644
--- a/ip/iplink_macvtap.c
+++ b/ip/iplink_macvtap.c
@@ -24,10 +24,10 @@ static void explain(void)
 	);
 }
 
-static int mode_arg(void)
+static int mode_arg(const char *arg)
 {
         fprintf(stderr, "Error: argument of \"mode\" must be \"private\", "
-		"\"vepa\", \"bridge\" or \"passthru\" \n");
+		"\"vepa\", \"bridge\" or \"passthru\", not \"%s\"\n", arg);
         return -1;
 }
 
@@ -48,14 +48,14 @@ static int macvtap_parse_opt(struct link_util *lu, int argc, char **argv,
 			else if (strcmp(*argv, "passthru") == 0)
 				mode = MACVLAN_MODE_PASSTHRU;
 			else
-				return mode_arg();
+				return mode_arg(*argv);
 
 			addattr32(n, 1024, IFLA_MACVLAN_MODE, mode);
 		} else if (matches(*argv, "help") == 0) {
 			explain();
 			return -1;
 		} else {
-			fprintf(stderr, "macvtap: what is \"%s\"?\n", *argv);
+			fprintf(stderr, "macvtap: unknown command \"%s\"?\n", *argv);
 			explain();
 			return -1;
 		}
diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 97af8d6..26ceb8d 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -33,9 +33,9 @@ static void explain(void)
 	);
 }
 
-static int on_off(char *msg)
+static int on_off(const char *msg, const char *arg)
 {
-	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\"\n", msg);
+	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n", msg, arg);
 	return -1;
 }
 
@@ -93,7 +93,7 @@ static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			else if (strcmp(*argv, "off") == 0)
 				flags.flags &= ~VLAN_FLAG_REORDER_HDR;
 			else
-				return on_off("reorder_hdr");
+				return on_off("reorder_hdr", *argv);
 		} else if (matches(*argv, "gvrp") == 0) {
 			NEXT_ARG();
 			flags.mask |= VLAN_FLAG_GVRP;
@@ -102,7 +102,7 @@ static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			else if (strcmp(*argv, "off") == 0)
 				flags.flags &= ~VLAN_FLAG_GVRP;
 			else
-				return on_off("gvrp");
+				return on_off("gvrp", *argv);
 		} else if (matches(*argv, "loose_binding") == 0) {
 			NEXT_ARG();
 			flags.mask |= VLAN_FLAG_LOOSE_BINDING;
@@ -111,7 +111,7 @@ static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			else if (strcmp(*argv, "off") == 0)
 				flags.flags &= ~VLAN_FLAG_LOOSE_BINDING;
 			else
-				return on_off("loose_binding");
+				return on_off("loose_binding", *argv);
 		} else if (matches(*argv, "ingress-qos-map") == 0) {
 			NEXT_ARG();
 			if (vlan_parse_qos_map(&argc, &argv, n,
@@ -128,7 +128,7 @@ static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			explain();
 			return -1;
 		} else {
-			fprintf(stderr, "vlan: what is \"%s\"?\n", *argv);
+			fprintf(stderr, "vlan: unknown command \"%s\"?\n", *argv);
 			explain();
 			return -1;
 		}
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 4b72361..1025326 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -149,7 +149,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			explain();
 			return -1;
 		} else {
-			fprintf(stderr, "vxlan: what is \"%s\"?\n", *argv);
+			fprintf(stderr, "vxlan: unknown command \"%s\"?\n", *argv);
 			explain();
 			return -1;
 		}
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 51b1c5e..b047b97 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -130,7 +130,7 @@ static int netns_exec(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 	if (argc < 2) {
-		fprintf(stderr, "No cmd specified\n");
+		fprintf(stderr, "No command specified\n");
 		return EXIT_FAILURE;
 	}
 	name = argv[0];
@@ -138,13 +138,13 @@ static int netns_exec(int argc, char **argv)
 	snprintf(net_path, sizeof(net_path), "%s/%s", NETNS_RUN_DIR, name);
 	netns = open(net_path, O_RDONLY);
 	if (netns < 0) {
-		fprintf(stderr, "Cannot open network namespace %s: %s\n",
+		fprintf(stderr, "Cannot open network namespace \"%s\": %s\n",
 			name, strerror(errno));
 		return EXIT_FAILURE;
 	}
 	if (setns(netns, CLONE_NEWNET) < 0) {
-		fprintf(stderr, "seting the network namespace failed: %s\n",
-			strerror(errno));
+		fprintf(stderr, "seting the network namespace \"%s\" failed: %s\n",
+			name, strerror(errno));
 		return EXIT_FAILURE;
 	}
 
@@ -154,7 +154,7 @@ static int netns_exec(int argc, char **argv)
 	}
 	/* Don't let any mounts propogate back to the parent */
 	if (mount("", "/", "none", MS_SLAVE | MS_REC, NULL)) {
-		fprintf(stderr, "mount --make-rslave / failed: %s\n",
+		fprintf(stderr, "\"mount --make-rslave /\" failed: %s\n",
 			strerror(errno));
 		return EXIT_FAILURE;
 	}
@@ -172,7 +172,7 @@ static int netns_exec(int argc, char **argv)
 	bind_etc(name);
 
 	if (execvp(cmd, argv + 1)  < 0)
-		fprintf(stderr, "exec of %s failed: %s\n",
+		fprintf(stderr, "exec of \"%s\" failed: %s\n",
 			cmd, strerror(errno));
 	return EXIT_FAILURE;
 }
@@ -330,7 +330,7 @@ static int netns_delete(int argc, char **argv)
 	snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name);
 	umount2(netns_path, MNT_DETACH);
 	if (unlink(netns_path) < 0) {
-		fprintf(stderr, "Cannot remove %s: %s\n",
+		fprintf(stderr, "Cannot remove namespace file \"%s\": %s\n",
 			netns_path, strerror(errno));
 		return EXIT_FAILURE;
 	}
@@ -389,14 +389,14 @@ static int netns_add(int argc, char **argv)
 	/* Create the filesystem state */
 	fd = open(netns_path, O_RDONLY|O_CREAT|O_EXCL, 0);
 	if (fd < 0) {
-		fprintf(stderr, "Could not create %s: %s\n",
+		fprintf(stderr, "Cannot not create namespace file \"%s\": %s\n",
 			netns_path, strerror(errno));
 		return EXIT_FAILURE;
 	}
 	close(fd);
 	if (unshare(CLONE_NEWNET) < 0) {
-		fprintf(stderr, "Failed to create a new network namespace: %s\n",
-			strerror(errno));
+		fprintf(stderr, "Failed to create a new network namespace \"%s\": %s\n",
+			name, strerror(errno));
 		goto out_delete;
 	}
 
diff --git a/ip/ipprefix.c b/ip/ipprefix.c
index ee276b3..cf68f89 100644
--- a/ip/ipprefix.c
+++ b/ip/ipprefix.c
@@ -60,7 +60,7 @@ int print_prefix(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		return 0;
 
 	if (prefix->prefix_family != AF_INET6) {
-		fprintf(stderr, "wrong family %d\n", prefix->prefix_family);
+		fprintf(stderr, "incorrect protocol family: %d\n", prefix->prefix_family);
 		return 0;
 	}
 	if (prefix->prefix_type != ND_OPT_PREFIX_INFORMATION) {
diff --git a/ip/iproute.c b/ip/iproute.c
index c60156f..309cc87 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1099,7 +1099,7 @@ static int save_route_prep(void)
 	int ret;
 
 	if (isatty(STDOUT_FILENO)) {
-		fprintf(stderr, "Not sending binary stream to stdout\n");
+		fprintf(stderr, "Not sending a binary stream to stdout\n");
 		return -1;
 	}
 
@@ -1445,7 +1445,7 @@ int iproute_get(int argc, char **argv)
 	}
 
 	if (req.r.rtm_dst_len == 0) {
-		fprintf(stderr, "need at least destination address\n");
+		fprintf(stderr, "need at least a destination address\n");
 		exit(1);
 	}
 
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index f8b91ba..4275f26 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -102,7 +102,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 				p->iph.protocol = IPPROTO_IPIP;
 				p->i_flags |= VTI_ISVTI;
 			} else {
-				fprintf(stderr,"Cannot guess tunnel mode.\n");
+				fprintf(stderr,"Unknown tunnel mode \"%s\"\n", *argv);
 				exit(-1);
 			}
 		} else if (strcmp(*argv, "key") == 0) {
@@ -114,7 +114,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 				p->i_key = p->o_key = get_addr32(*argv);
 			else {
 				if (get_unsigned(&uval, *argv, 0)<0) {
-					fprintf(stderr, "invalid value of \"key\"\n");
+					fprintf(stderr, "invalid value for \"key\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				p->i_key = p->o_key = htonl(uval);
@@ -127,7 +127,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 				p->i_key = get_addr32(*argv);
 			else {
 				if (get_unsigned(&uval, *argv, 0)<0) {
-					fprintf(stderr, "invalid value of \"ikey\"\n");
+					fprintf(stderr, "invalid value for \"ikey\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				p->i_key = htonl(uval);
@@ -140,7 +140,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 				p->o_key = get_addr32(*argv);
 			else {
 				if (get_unsigned(&uval, *argv, 0)<0) {
-					fprintf(stderr, "invalid value of \"okey\"\n");
+					fprintf(stderr, "invalid value for \"okey\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				p->o_key = htonl(uval);
@@ -242,7 +242,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 
 	if (p->iph.protocol == IPPROTO_IPIP || p->iph.protocol == IPPROTO_IPV6) {
 		if ((p->i_flags & GRE_KEY) || (p->o_flags & GRE_KEY)) {
-			fprintf(stderr, "Keys are not allowed with ipip and sit.\n");
+			fprintf(stderr, "Keys are not allowed with ipip and sit tunnels\n");
 			return -1;
 		}
 	}
@@ -262,7 +262,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 		p->o_flags |= GRE_KEY;
 	}
 	if (IN_MULTICAST(ntohl(p->iph.daddr)) && !p->iph.saddr) {
-		fprintf(stderr, "Broadcast tunnel requires a source address.\n");
+		fprintf(stderr, "A broadcast tunnel requires a source address\n");
 		return -1;
 	}
 	if (isatap)
@@ -444,7 +444,7 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
 		buf[sizeof(buf) - 1] = 0;
 		if ((ptr = strchr(buf, ':')) == NULL ||
 		    (*ptr++ = 0, sscanf(buf, "%s", name) != 1)) {
-			fprintf(stderr, "Wrong format of /proc/net/dev. Sorry.\n");
+			fprintf(stderr, "Wrong format for /proc/net/dev. Giving up.\n");
 			fclose(fp);
 			return -1;
 		}
@@ -461,7 +461,7 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
 			continue;
 		type = ll_index_to_type(index);
 		if (type == -1) {
-			fprintf(stderr, "Failed to get type of [%s]\n", name);
+			fprintf(stderr, "Failed to get type of \"%s\"\n", name);
 			continue;
 		}
 		if (type != ARPHRD_TUNNEL && type != ARPHRD_IPGRE && type != ARPHRD_SIT)
@@ -558,17 +558,17 @@ static int do_prl(int argc, char **argv)
 			strncpy(medium, *argv, IFNAMSIZ-1);
 			devname++;
 		} else {
-			fprintf(stderr,"%s: Invalid PRL parameter.\n", *argv);
+			fprintf(stderr,"Invalid PRL parameter \"%s\"\n", *argv);
 			exit(-1);
 		}
 		if (count > 1) {
-			fprintf(stderr,"One PRL entry at a time.\n");
+			fprintf(stderr,"One PRL entry at a time\n");
 			exit(-1);
 		}
 		argc--; argv++;
 	}
 	if (devname == 0) {
-		fprintf(stderr, "Must specify dev.\n");
+		fprintf(stderr, "Must specify device\n");
 		exit(-1);
 	}
 
@@ -608,13 +608,13 @@ static int do_6rd(int argc, char **argv)
 			strncpy(medium, *argv, IFNAMSIZ-1);
 			devname++;
 		} else {
-			fprintf(stderr,"%s: Invalid 6RD parameter.\n", *argv);
+			fprintf(stderr,"Invalid 6RD parameter \"%s\"\n", *argv);
 			exit(-1);
 		}
 		argc--; argv++;
 	}
 	if (devname == 0) {
-		fprintf(stderr, "Must specify dev.\n");
+		fprintf(stderr, "Must specify device\n");
 		exit(-1);
 	}
 
@@ -637,7 +637,7 @@ int do_iptunnel(int argc, char **argv)
 	case AF_INET6:
 		return do_ip6tunnel(argc, argv);
 	default:
-		fprintf(stderr, "Unsupported family:%d\n", preferred_family);
+		fprintf(stderr, "Unsupported protocol family: %d\n", preferred_family);
 		exit(-1);
 	}
 
@@ -661,6 +661,6 @@ int do_iptunnel(int argc, char **argv)
 	} else
 		return do_show(0, NULL);
 
-	fprintf(stderr, "Command \"%s\" is unknown, try \"ip tunnel help\".\n", *argv);
+	fprintf(stderr, "Command \"%s\" is unknown, try \"ip tunnel help\"\n", *argv);
 	exit(-1);
 }
diff --git a/ip/iptuntap.c b/ip/iptuntap.c
index 20914e1..3d9ad7d 100644
--- a/ip/iptuntap.c
+++ b/ip/iptuntap.c
@@ -128,7 +128,7 @@ static int parse_args(int argc, char **argv, struct ifreq *ifr, uid_t *uid, gid_
 				}
 				ifr->ifr_flags |= IFF_TAP;
 			} else {
-				fprintf(stderr,"Cannot guess tunnel mode.\n");
+				fprintf(stderr,"Unknown tunnel mode \"%s\"\n", *argv);
 				exit(-1);
 			}
 		} else if (uid && matches(*argv, "user") == 0) {
diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index dda4a7a..5a0f215 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -1231,7 +1231,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 				uval = htonl(get_addr32(*argv));
 			else {
 				if (get_unsigned(&uval, *argv, 0)<0) {
-					fprintf(stderr, "invalid value of \"key\"\n");
+					fprintf(stderr, "invalid value for \"key\"; it should be an unsigned integer\n");
 					exit(-1);
 				}
 			}
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 839fb29..7e0b896 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -141,7 +141,7 @@ get_failed:
 			else {
 				if (get_unsigned(&uval, *argv, 0) < 0) {
 					fprintf(stderr,
-						"Invalid value for \"key\"\n");
+						"Invalid value for \"key\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				uval = htonl(uval);
@@ -157,7 +157,7 @@ get_failed:
 				uval = get_addr32(*argv);
 			else {
 				if (get_unsigned(&uval, *argv, 0)<0) {
-					fprintf(stderr, "invalid value of \"ikey\"\n");
+					fprintf(stderr, "invalid value for \"ikey\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				uval = htonl(uval);
@@ -172,7 +172,7 @@ get_failed:
 				uval = get_addr32(*argv);
 			else {
 				if (get_unsigned(&uval, *argv, 0)<0) {
-					fprintf(stderr, "invalid value of \"okey\"\n");
+					fprintf(stderr, "invalid value for \"okey\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				uval = htonl(uval);
@@ -247,7 +247,7 @@ get_failed:
 		oflags |= GRE_KEY;
 	}
 	if (IN_MULTICAST(ntohl(daddr)) && !saddr) {
-		fprintf(stderr, "Broadcast tunnel requires a source address.\n");
+		fprintf(stderr, "A broadcast tunnel requires a source address.\n");
 		return -1;
 	}
 
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 385f435..77a7482 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -117,7 +117,7 @@ get_failed:
 			else {
 				if (get_unsigned(&uval, *argv, 0) < 0) {
 					fprintf(stderr,
-						"Invalid value for \"key\"\n");
+						"Invalid value for \"key\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				uval = htonl(uval);
@@ -132,7 +132,7 @@ get_failed:
 				uval = get_addr32(*argv);
 			else {
 				if (get_unsigned(&uval, *argv, 0) < 0) {
-					fprintf(stderr, "invalid value of \"ikey\"\n");
+					fprintf(stderr, "invalid value for \"ikey\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				uval = htonl(uval);
@@ -146,7 +146,7 @@ get_failed:
 				uval = get_addr32(*argv);
 			else {
 				if (get_unsigned(&uval, *argv, 0) < 0) {
-					fprintf(stderr, "invalid value of \"okey\"\n");
+					fprintf(stderr, "invalid value for \"okey\": \"%s\"; it should be an unsigned integer\n", *argv);
 					exit(-1);
 				}
 				uval = htonl(uval);
@@ -155,7 +155,7 @@ get_failed:
 		} else if (!matches(*argv, "remote")) {
 			NEXT_ARG();
 			if (!strcmp(*argv, "any")) {
-				fprintf(stderr, "invalid value of \"remote\"\n");
+				fprintf(stderr, "invalid value for \"remote\": \"%s\"\n", *argv);
 				exit(-1);
 			} else {
 				daddr = get_addr32(*argv);
@@ -163,7 +163,7 @@ get_failed:
 		} else if (!matches(*argv, "local")) {
 			NEXT_ARG();
 			if (!strcmp(*argv, "any")) {
-				fprintf(stderr, "invalid value of \"local\"\n");
+				fprintf(stderr, "invalid value for \"local\": \"%s\"\n", *argv);
 				exit(-1);
 			} else {
 				saddr = get_addr32(*argv);
diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c
index 34e1d8e..a01e1fb 100644
--- a/ip/tcp_metrics.c
+++ b/ip/tcp_metrics.c
@@ -268,7 +268,7 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 	case AF_INET6:
 		break;
 	default:
-		fprintf(stderr, "Unsupported family:%d\n", preferred_family);
+		fprintf(stderr, "Unsupported protocol family: %d\n", preferred_family);
 		return -1;
 	}
 
diff --git a/ip/tunnel.c b/ip/tunnel.c
index b176d3f..a6a2846 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -74,7 +74,7 @@ int tnl_get_ioctl(const char *basedev, void *p)
 	fd = socket(preferred_family, SOCK_DGRAM, 0);
 	err = ioctl(fd, SIOCGETTUNNEL, &ifr);
 	if (err)
-		fprintf(stderr, "get tunnel %s failed: %s\n", basedev, 
+		fprintf(stderr, "get tunnel \"%s\" failed: %s\n", basedev,
 			strerror(errno));
 
 	close(fd);
@@ -95,7 +95,7 @@ int tnl_add_ioctl(int cmd, const char *basedev, const char *name, void *p)
 	fd = socket(preferred_family, SOCK_DGRAM, 0);
 	err = ioctl(fd, cmd, &ifr);
 	if (err)
-		fprintf(stderr, "add tunnel %s failed: %s\n", ifr.ifr_name,
+		fprintf(stderr, "add tunnel \"%s\" failed: %s\n", ifr.ifr_name,
 			strerror(errno));
 	close(fd);
 	return err;
@@ -116,7 +116,7 @@ int tnl_del_ioctl(const char *basedev, const char *name, void *p)
 	fd = socket(preferred_family, SOCK_DGRAM, 0);
 	err = ioctl(fd, SIOCDELTUNNEL, &ifr);
 	if (err)
-		fprintf(stderr, "delete tunnel %s failed: %s\n",
+		fprintf(stderr, "delete tunnel \"%s\" failed: %s\n",
 			ifr.ifr_name, strerror(errno));
 	close(fd);
 	return err;

^ permalink raw reply related

* [PATCH v2 2/5] iproute2: clarifications in the libnetlink.3 man page
From: Kees van Reeuwijk @ 2013-02-08 13:32 UTC (permalink / raw)
  To: netdev

From: Kees van Reeuwijk <reeuwijk@few.vu.nl>

Rephrasing for clarity.


Signed-off-by: Kees van Reeuwijk <reeuwijk@few.vu.nl>

 ---

 libnetlink.3 |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/man/man3/libnetlink.3 b/man/man3/libnetlink.3
index d114169..e999bd6 100644
--- a/man/man3/libnetlink.3
+++ b/man/man3/libnetlink.3
@@ -100,7 +100,7 @@ in case of error. NULL for
 means to not use a filter.
 .B junk
 is used to filter messages not destined to the local socket.
-Only one message bundle is received. Unless there is no message 
+Only one message bundle is received. If there is a message
 pending, this function does not block.
 
 .TP
@@ -112,7 +112,7 @@ is a callback that gets the message source address, the message itself,
 and the
 .B jarg
 cookie as arguments. It will get called for all received messages.
-Only one message bundle is received. Unless there is no message 
+Only one message bundle is received. If there is a message
 pending this function does not block.
 
 .TP
@@ -123,7 +123,7 @@ but reads a netlink message bundle from the file
 .B file
 and passes the messages to
 .B handler
-for parsing. The file contains raw data as received from a rtnetlink socket.
+for parsing. The file should contain raw data as received from a rtnetlink socket.
 .PP
 The following functions are useful to construct custom rtnetlink messages. For
 simple database dumping with filtering it is better to use the higher level

^ permalink raw reply related

* [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev
In-Reply-To: <cover.1360327229.git.dborkman@redhat.com>

On sctp_endpoint_destroy, previously used sensitive keying material
should be zeroed out before the memory is returned, as we already do
with e.g. auth keys when released.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/endpointola.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 17a001b..1a9c5fb 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
 /* Final destructor for endpoint.  */
 static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
 {
+	int i;
+
 	SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
 
 	/* Free up the HMAC transform. */
@@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
 	sctp_inq_free(&ep->base.inqueue);
 	sctp_bind_addr_free(&ep->base.bind_addr);
 
+	for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
+		memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
+
 	/* Remove and free the port */
 	if (sctp_sk(ep->base.sk)->bind_hash)
 		sctp_put_port(ep->base.sk);
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net 1/2] net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev
In-Reply-To: <cover.1360327229.git.dborkman@redhat.com>

In sctp_setsockopt_auth_key, we create a temporary copy of the user
passed shared auth key for the endpoint or association and after
internal setup, we free it right away. Since it's sensitive data, we
should zero out the key before returning the memory back to the
allocator. Thus, use kzfree instead of kfree, just as we do in
sctp_auth_key_put().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9e65758..cedd9bf 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3390,7 +3390,7 @@ static int sctp_setsockopt_auth_key(struct sock *sk,
 
 	ret = sctp_auth_set_key(sctp_sk(sk)->ep, asoc, authkey);
 out:
-	kfree(authkey);
+	kzfree(authkey);
 	return ret;
 }
 
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net 0/2] two small fixes for sctp key usage
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev

Cryptographically used keys should be zeroed out when our session
ends resp. memory is freed, thus do not leave them somewhere in the
memory.

Daniel Borkmann (2):
  net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
  net: sctp: sctp_endpoint_free: zero out secret key data

 net/sctp/endpointola.c | 5 +++++
 net/sctp/socket.c      | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
1.7.11.7

^ permalink raw reply

* [patch net-next 7/7] tbf: ignore max_size check for gso skbs
From: Jiri Pirko @ 2013-02-08 12:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, jhs, kuznet
In-Reply-To: <1360328327-7144-1-git-send-email-jiri@resnulli.us>

This check made bigger packets incorrectly dropped. Remove this
limitation for gso skbs.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/sched/sch_tbf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 35bfd49..7ee5bff 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -121,7 +121,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	struct tbf_sched_data *q = qdisc_priv(sch);
 	int ret;
 
-	if (qdisc_pkt_len(skb) > q->max_size)
+	if (qdisc_pkt_len(skb) > q->max_size && !skb_is_gso(skb))
 		return qdisc_reshape_fail(skb, sch);
 
 	ret = qdisc_enqueue(skb, q->qdisc);
-- 
1.8.1.2

^ permalink raw reply related

* [patch net-next 6/7] tbf: improved accuracy at high rates
From: Jiri Pirko @ 2013-02-08 12:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, jhs, kuznet
In-Reply-To: <1360328327-7144-1-git-send-email-jiri@resnulli.us>

Current TBF uses rate table computed by the "tc" userspace program,
which has the following issue:

The rate table has 256 entries to map packet lengths to
token (time units).  With TSO sized packets, the 256 entry granularity
leads to loss/gain of rate, making the token bucket inaccurate.

Thus, instead of relying on rate table, this patch explicitly computes
the time and accounts for packet transmission times with nanosecond
granularity.

This is a followup to 56b765b79e9a78dc7d3f8850ba5e5567205a3ecd

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/sched/sch_tbf.c | 60 ++++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 4b056c15..35bfd49 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -19,6 +19,7 @@
 #include <linux/errno.h>
 #include <linux/skbuff.h>
 #include <net/netlink.h>
+#include <net/sch_generic.h>
 #include <net/pkt_sched.h>
 
 
@@ -100,23 +101,21 @@
 struct tbf_sched_data {
 /* Parameters */
 	u32		limit;		/* Maximal length of backlog: bytes */
-	u32		buffer;		/* Token bucket depth/rate: MUST BE >= MTU/B */
+	s64		buffer;		/* Token bucket depth/rate: MUST BE >= MTU/B */
 	u32		mtu;
 	u32		max_size;
-	struct qdisc_rate_table	*R_tab;
-	struct qdisc_rate_table	*P_tab;
+	struct psched_ratecfg rate;
+	struct psched_ratecfg peek;
+	bool peek_present;
 
 /* Variables */
-	long	tokens;			/* Current number of B tokens */
-	long	ptokens;		/* Current number of P tokens */
+	s64	tokens;			/* Current number of B tokens */
+	s64	ptokens;		/* Current number of P tokens */
 	psched_time_t	t_c;		/* Time check-point */
 	struct Qdisc	*qdisc;		/* Inner qdisc, default - bfifo queue */
 	struct qdisc_watchdog watchdog;	/* Watchdog timer */
 };
 
-#define L2T(q, L)   qdisc_l2t((q)->R_tab, L)
-#define L2T_P(q, L) qdisc_l2t((q)->P_tab, L)
-
 static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
 	struct tbf_sched_data *q = qdisc_priv(sch);
@@ -157,23 +156,23 @@ static struct sk_buff *tbf_dequeue(struct Qdisc *sch)
 
 	if (skb) {
 		psched_time_t now;
-		long toks;
-		long ptoks = 0;
+		s64 toks;
+		s64 ptoks = 0;
 		unsigned int len = qdisc_pkt_len(skb);
 
-		now = psched_get_time();
-		toks = psched_tdiff_bounded(now, q->t_c, q->buffer);
+		now = ktime_to_ns(ktime_get());
+		toks = min_t(s64, now - q->t_c, q->buffer);
 
-		if (q->P_tab) {
+		if (q->peek_present) {
 			ptoks = toks + q->ptokens;
 			if (ptoks > (long)q->mtu)
 				ptoks = q->mtu;
-			ptoks -= L2T_P(q, len);
+			ptoks -= (s64) psched_l2t_ns(&q->peek, len);
 		}
 		toks += q->tokens;
-		if (toks > (long)q->buffer)
+		if (toks > q->buffer)
 			toks = q->buffer;
-		toks -= L2T(q, len);
+		toks -= (s64) psched_l2t_ns(&q->rate, len);
 
 		if ((toks|ptoks) >= 0) {
 			skb = qdisc_dequeue_peeked(q->qdisc);
@@ -214,7 +213,7 @@ static void tbf_reset(struct Qdisc *sch)
 
 	qdisc_reset(q->qdisc);
 	sch->q.qlen = 0;
-	q->t_c = psched_get_time();
+	q->t_c = ktime_to_ns(ktime_get());
 	q->tokens = q->buffer;
 	q->ptokens = q->mtu;
 	qdisc_watchdog_cancel(&q->watchdog);
@@ -295,12 +294,17 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
 	q->limit = qopt->limit;
 	q->mtu = qopt->mtu;
 	q->max_size = max_size;
-	q->buffer = qopt->buffer;
+	q->buffer = PSCHED_TICKS2NS(qopt->buffer);
 	q->tokens = q->buffer;
 	q->ptokens = q->mtu;
 
-	swap(q->R_tab, rtab);
-	swap(q->P_tab, ptab);
+	psched_ratecfg_precompute(&q->rate, rtab->rate.rate);
+	if (ptab) {
+		psched_ratecfg_precompute(&q->peek, ptab->rate.rate);
+		q->peek_present = true;
+	} else {
+		q->peek_present = false;
+	}
 
 	sch_tree_unlock(sch);
 	err = 0;
@@ -319,7 +323,7 @@ static int tbf_init(struct Qdisc *sch, struct nlattr *opt)
 	if (opt == NULL)
 		return -EINVAL;
 
-	q->t_c = psched_get_time();
+	q->t_c = ktime_to_ns(ktime_get());
 	qdisc_watchdog_init(&q->watchdog, sch);
 	q->qdisc = &noop_qdisc;
 
@@ -331,12 +335,6 @@ static void tbf_destroy(struct Qdisc *sch)
 	struct tbf_sched_data *q = qdisc_priv(sch);
 
 	qdisc_watchdog_cancel(&q->watchdog);
-
-	if (q->P_tab)
-		qdisc_put_rtab(q->P_tab);
-	if (q->R_tab)
-		qdisc_put_rtab(q->R_tab);
-
 	qdisc_destroy(q->qdisc);
 }
 
@@ -352,13 +350,13 @@ static int tbf_dump(struct Qdisc *sch, struct sk_buff *skb)
 		goto nla_put_failure;
 
 	opt.limit = q->limit;
-	opt.rate = q->R_tab->rate;
-	if (q->P_tab)
-		opt.peakrate = q->P_tab->rate;
+	opt.rate.rate = psched_ratecfg_getrate(&q->rate);
+	if (q->peek_present)
+		opt.peakrate.rate = psched_ratecfg_getrate(&q->peek);
 	else
 		memset(&opt.peakrate, 0, sizeof(opt.peakrate));
 	opt.mtu = q->mtu;
-	opt.buffer = q->buffer;
+	opt.buffer = PSCHED_NS2TICKS(q->buffer);
 	if (nla_put(skb, TCA_TBF_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
 
-- 
1.8.1.2

^ permalink raw reply related

* [patch net-next 5/7] sch: make htb_rate_cfg and functions around that generic
From: Jiri Pirko @ 2013-02-08 12:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, jhs, kuznet
In-Reply-To: <1360328327-7144-1-git-send-email-jiri@resnulli.us>

As it is going to be used in tbf as well, push these to generic code.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/net/sch_generic.h | 18 +++++++++++++
 net/sched/sch_generic.c   | 37 +++++++++++++++++++++++++++
 net/sched/sch_htb.c       | 65 +++++++----------------------------------------
 3 files changed, 64 insertions(+), 56 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 2d06c2a..c7e5512 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -679,4 +679,22 @@ static inline struct sk_buff *skb_act_clone(struct sk_buff *skb, gfp_t gfp_mask,
 }
 #endif
 
+struct psched_ratecfg {
+	u64 rate_bps;
+	u32 mult;
+	u32 shift;
+};
+
+static inline u64 psched_l2t_ns(struct psched_ratecfg *r, unsigned int len)
+{
+	return ((u64)len * r->mult) >> r->shift;
+}
+
+extern void psched_ratecfg_precompute(struct psched_ratecfg *r, u32 rate);
+
+static inline u32 psched_ratecfg_getrate(struct psched_ratecfg *r)
+{
+	return r->rate_bps >> 3;
+}
+
 #endif
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 5d81a44..ffad481 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -25,6 +25,7 @@
 #include <linux/rcupdate.h>
 #include <linux/list.h>
 #include <linux/slab.h>
+#include <net/sch_generic.h>
 #include <net/pkt_sched.h>
 #include <net/dst.h>
 
@@ -896,3 +897,39 @@ void dev_shutdown(struct net_device *dev)
 
 	WARN_ON(timer_pending(&dev->watchdog_timer));
 }
+
+void psched_ratecfg_precompute(struct psched_ratecfg *r, u32 rate)
+{
+	u64 factor;
+	u64 mult;
+	int shift;
+
+	r->rate_bps = rate << 3;
+	r->shift = 0;
+	r->mult = 1;
+	/*
+	 * Calibrate mult, shift so that token counting is accurate
+	 * for smallest packet size (64 bytes).  Token (time in ns) is
+	 * computed as (bytes * 8) * NSEC_PER_SEC / rate_bps.  It will
+	 * work as long as the smallest packet transfer time can be
+	 * accurately represented in nanosec.
+	 */
+	if (r->rate_bps > 0) {
+		/*
+		 * Higher shift gives better accuracy.  Find the largest
+		 * shift such that mult fits in 32 bits.
+		 */
+		for (shift = 0; shift < 16; shift++) {
+			r->shift = shift;
+			factor = 8LLU * NSEC_PER_SEC * (1 << r->shift);
+			mult = div64_u64(factor, r->rate_bps);
+			if (mult > UINT_MAX)
+				break;
+		}
+
+		r->shift = shift - 1;
+		factor = 8LLU * NSEC_PER_SEC * (1 << r->shift);
+		r->mult = div64_u64(factor, r->rate_bps);
+	}
+}
+EXPORT_SYMBOL(psched_ratecfg_precompute);
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 2b22544..03c2692 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -38,6 +38,7 @@
 #include <linux/workqueue.h>
 #include <linux/slab.h>
 #include <net/netlink.h>
+#include <net/sch_generic.h>
 #include <net/pkt_sched.h>
 
 /* HTB algorithm.
@@ -71,12 +72,6 @@ enum htb_cmode {
 	HTB_CAN_SEND		/* class can send */
 };
 
-struct htb_rate_cfg {
-	u64 rate_bps;
-	u32 mult;
-	u32 shift;
-};
-
 /* interior & leaf nodes; props specific to leaves are marked L: */
 struct htb_class {
 	struct Qdisc_class_common common;
@@ -124,8 +119,8 @@ struct htb_class {
 	int filter_cnt;
 
 	/* token bucket parameters */
-	struct htb_rate_cfg rate;
-	struct htb_rate_cfg ceil;
+	struct psched_ratecfg rate;
+	struct psched_ratecfg ceil;
 	s64 buffer, cbuffer;	/* token bucket depth/rate */
 	psched_tdiff_t mbuffer;	/* max wait time */
 	s64 tokens, ctokens;	/* current number of tokens */
@@ -168,45 +163,6 @@ struct htb_sched {
 	struct work_struct work;
 };
 
-static u64 l2t_ns(struct htb_rate_cfg *r, unsigned int len)
-{
-	return ((u64)len * r->mult) >> r->shift;
-}
-
-static void htb_precompute_ratedata(struct htb_rate_cfg *r)
-{
-	u64 factor;
-	u64 mult;
-	int shift;
-
-	r->shift = 0;
-	r->mult = 1;
-	/*
-	 * Calibrate mult, shift so that token counting is accurate
-	 * for smallest packet size (64 bytes).  Token (time in ns) is
-	 * computed as (bytes * 8) * NSEC_PER_SEC / rate_bps.  It will
-	 * work as long as the smallest packet transfer time can be
-	 * accurately represented in nanosec.
-	 */
-	if (r->rate_bps > 0) {
-		/*
-		 * Higher shift gives better accuracy.  Find the largest
-		 * shift such that mult fits in 32 bits.
-		 */
-		for (shift = 0; shift < 16; shift++) {
-			r->shift = shift;
-			factor = 8LLU * NSEC_PER_SEC * (1 << r->shift);
-			mult = div64_u64(factor, r->rate_bps);
-			if (mult > UINT_MAX)
-				break;
-		}
-
-		r->shift = shift - 1;
-		factor = 8LLU * NSEC_PER_SEC * (1 << r->shift);
-		r->mult = div64_u64(factor, r->rate_bps);
-	}
-}
-
 /* find class in global hash table using given handle */
 static inline struct htb_class *htb_find(u32 handle, struct Qdisc *sch)
 {
@@ -632,7 +588,7 @@ static inline void htb_accnt_tokens(struct htb_class *cl, int bytes, s64 diff)
 
 	if (toks > cl->buffer)
 		toks = cl->buffer;
-	toks -= (s64) l2t_ns(&cl->rate, bytes);
+	toks -= (s64) psched_l2t_ns(&cl->rate, bytes);
 	if (toks <= -cl->mbuffer)
 		toks = 1 - cl->mbuffer;
 
@@ -645,7 +601,7 @@ static inline void htb_accnt_ctokens(struct htb_class *cl, int bytes, s64 diff)
 
 	if (toks > cl->cbuffer)
 		toks = cl->cbuffer;
-	toks -= (s64) l2t_ns(&cl->ceil, bytes);
+	toks -= (s64) psched_l2t_ns(&cl->ceil, bytes);
 	if (toks <= -cl->mbuffer)
 		toks = 1 - cl->mbuffer;
 
@@ -1134,9 +1090,9 @@ static int htb_dump_class(struct Qdisc *sch, unsigned long arg,
 
 	memset(&opt, 0, sizeof(opt));
 
-	opt.rate.rate = cl->rate.rate_bps >> 3;
+	opt.rate.rate = psched_ratecfg_getrate(&cl->rate);
 	opt.buffer = PSCHED_NS2TICKS(cl->buffer);
-	opt.ceil.rate = cl->ceil.rate_bps >> 3;
+	opt.ceil.rate = psched_ratecfg_getrate(&cl->ceil);
 	opt.cbuffer = PSCHED_NS2TICKS(cl->cbuffer);
 	opt.quantum = cl->quantum;
 	opt.prio = cl->prio;
@@ -1503,11 +1459,8 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 			cl->prio = TC_HTB_NUMPRIO - 1;
 	}
 
-	cl->rate.rate_bps = (u64)hopt->rate.rate << 3;
-	cl->ceil.rate_bps = (u64)hopt->ceil.rate << 3;
-
-	htb_precompute_ratedata(&cl->rate);
-	htb_precompute_ratedata(&cl->ceil);
+	psched_ratecfg_precompute(&cl->rate, hopt->rate.rate);
+	psched_ratecfg_precompute(&cl->ceil, hopt->ceil.rate);
 
 	cl->buffer = PSCHED_TICKS2NS(hopt->buffer);
 	cl->cbuffer = PSCHED_TICKS2NS(hopt->buffer);
-- 
1.8.1.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).