netdev - Re: net-sched 05/05: sch_htb: use dynamic class hash helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080702213102.GB2476@ami.dom.local>
Date:	Wed, 2 Jul 2008 23:31:02 +0200
From:	Jarek Poplawski <jarkao2@...il.com>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	netdev@...r.kernel.org, devik@....cz
Subject: Re: net-sched 05/05: sch_htb: use dynamic class hash helpers

Patrick McHardy wrote, On 07/01/2008 04:34 PM:

> net-sched: sch_htb: use dynamic class hash helpers
> 
> Signed-off-by: Patrick McHardy <kaber@...sh.net>

This patch also looks OK too me, except 2 tiny doubts below:

> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
> index 879f0b6..d5e4fdb 100644
> --- a/net/sched/sch_htb.c
> +++ b/net/sched/sch_htb.c
> @@ -51,7 +51,6 @@
>      one less than their parent.
>  */
>  
> -#define HTB_HSIZE 16		/* classid hash size */
>  static int htb_hysteresis __read_mostly = 0; /* whether to use mode hysteresis for speedup */
>  #define HTB_VER 0x30011		/* major must be matched with number suplied by TC as version */
>  
> @@ -72,8 +71,8 @@ enum htb_cmode {
>  
>  /* interior & leaf nodes; props specific to leaves are marked L: */
>  struct htb_class {
> +	struct Qdisc_class_common common;

Hmm... isn't this variable name too "common"? Why not something more
meaningful like: id, node, head, info etc?

...
> @@ -975,13 +957,14 @@ static unsigned int htb_drop(struct Qdisc *sch)
>  static void htb_reset(struct Qdisc *sch)
>  {
>  	struct htb_sched *q = qdisc_priv(sch);
> -	int i;
> -
> -	for (i = 0; i < HTB_HSIZE; i++) {
> -		struct hlist_node *p;
> -		struct htb_class *cl;
> +	struct Qdisc_class_common *clc;
> +	struct hlist_node *n;
> +	struct htb_class *cl;
> +	unsigned int i;
>  
> -		hlist_for_each_entry(cl, p, q->hash + i, hlist) {
> +	for (i = 0; i < q->clhash.hashsize; i++) {
> +		hlist_for_each_entry(clc, n, &q->clhash.hash[i], hnode) {
> +			cl = container_of(clc, struct htb_class, common);

Why not?:
		hlist_for_each_entry(cl, n, &q->clhash.hash[i], common.hnode) {

Of course, this is probably a matter of taste, so I don't persist...

Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html