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

[Jarek Poplawski <jarkao2@gmail.com>]

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@trash.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.