On 02-06-2009 23:10, Jarek Poplawski wrote:
quoted text
> On Tue, Jun 02, 2009 at 09:16:27AM -0400, jamal wrote:
...
quoted text
> 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.

After the second look I have some questions:
- if it's really aimed to skip checking by ->get() tp's before they're
configured in ->change(), maybe instead of using tp_c to check this it
would be simpler to generally skip calling ->get() for newly created
tp's?
- otherwise the current method probably needs adding a tp_c check for
NULL in u32_destroy()?
- it seems this method would also need adding a 'handle' lookup to
the u32_change(); otherwise its 'handle' parameter isn't controlled
for for uniqueness, unless I miss something?

Cheers,
Jarek P.

quoted text
> 
>> 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;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html