Re: [BUG] net_cls: Panic occured when net_cls subsystem use

[Jarek Poplawski <jarkao2@gmail.com>]

On Tue, Jun 02, 2009 at 09:16:27AM -0400, jamal wrote:
...
> Thanks for the courtesy.
> I note Dave already swallowed Minoru's patch; so lets move from there.
> Yes, there's a possibility of a regression - I (and so are you) are only
> recently evolved humans; we are not perfect... yet ;->
> So i would agree with Minoru testing your patch as plan B in case the
> applied one starts causing trouble.
> BTW, ok - here's a quick untested, uncompiled fix to the u32 classifier
> to fix the first rock (which you already worked around in your changes
> to the included patch). No rush to submit for now..

Thanks for your courtesy as well. Alas, I'm not sure I can fully
understand the current patch. You seem to redefine the ->get() method
usage, so it looks for handle only for configured tp's. It might be
right but I need more time to check this.

> 
> On the second rock you threw so violently, after some reflection, I
> think it is ok to send a replace twice with different priorities.
> The second one will be added and the old not deleted, but if the user
> has chosen the correct priority, then things will work out just fine.
> And if they want they have to explicitly delete the one they dont want.
> It is also not illegal to do a "replace" for installing instead of
> "add".
> 
> So the only other things left to do from this exercise are (no rush in
> any of them):
> a) remove all "buckets" from underneath other classifiers
> b) get consistency across all classifiers in usage of setup API
> 
> If you want to do this - go ahead; else i plan on tackling it probably
> when stable 2.6.31 kicks in.

I definitely prefer you doing this and me asking "rude" questions.;-)

Cheers,
Jarek P.

> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index 07372f6..5ad0b98 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -249,6 +249,9 @@ static unsigned long u32_get(struct tcf_proto *tp, u32 handle)
>  	struct tc_u_hnode *ht;
>  	struct tc_u_common *tp_c = tp->data;
>  
> +	if (!tp_c)
> +		return 0;
> +
>  	if (TC_U32_HTID(handle) == TC_U32_ROOT)
>  		ht = tp->root;
>  	else
> @@ -311,7 +314,6 @@ static int u32_init(struct tcf_proto *tp)
>  	root_ht->tp_c = tp_c;
>  
>  	tp->root = root_ht;
> -	tp->data = tp_c;
>  	return 0;
>  }
>  
> @@ -524,7 +526,7 @@ static int u32_change(struct tcf_proto *tp, unsigned long base, u32 handle,
>  		      struct nlattr **tca,
>  		      unsigned long *arg)
>  {
> -	struct tc_u_common *tp_c = tp->data;
> +	struct tc_u_common *tp_c = tp->root->tp_c;
>  	struct tc_u_hnode *ht;
>  	struct tc_u_knode *n;
>  	struct tc_u32_sel *s;
> @@ -540,6 +542,7 @@ static int u32_change(struct tcf_proto *tp, unsigned long base, u32 handle,
>  	if (err < 0)
>  		return err;
>  
> +	tp->data = tp_c;
>  	if ((n = (struct tc_u_knode*)*arg) != NULL) {
>  		if (TC_U32_KEY(n->handle) == 0)
>  			return -EINVAL;