Re: [PATCH net-next 1/6] tipc: add link_kfree_skbuff helper function

[Wang Weidong <wangweidong1@huawei.com>]

On 2013/12/6 17:09, Daniel Borkmann wrote:
> On 12/06/2013 07:23 AM, Wang Weidong wrote:
>> replaces some chunks of code that kfree the sk_buff.
>> This is just code simplification, no functional changes.
>>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>   net/tipc/link.c | 58 +++++++++++++++++++--------------------------------------
>>   1 file changed, 19 insertions(+), 39 deletions(-)
>>
>> diff --git a/net/tipc/link.c b/net/tipc/link.c
>> index 69cd9bf..1c27d7b 100644
>> --- a/net/tipc/link.c
>> +++ b/net/tipc/link.c
>> @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i)
>>       return (i + 3) & ~3u;
>>   }
>>
>> +static void link_kfree_skbuff(struct sk_buff *buf)
>> +{
>> +    struct sk_buff *next;
>> +
>> +    while (buf) {
>> +        next = buf->next;
>> +        kfree_skb(buf);
>> +        buf = next;
>> +    }
>> +}
> 
> So kfree_skb_list() does not work for you ?
> 
> Also, in case we do not have such generic functions, they should
> go to skbuff.c instead ...
> 
No, It is work for me. I just din't not the kfree_skb_list() and I found
that use a helper function can make code more simplification. So I do that.

Regards.
Wang


>>   static void link_init_max_pkt(struct tipc_link *l_ptr)
>>   {
>>       u32 max_pkt;
>> @@ -387,13 +398,8 @@ exit:
>>   static void link_release_outqueue(struct tipc_link *l_ptr)
>>   {
>>       struct sk_buff *buf = l_ptr->first_out;
>> -    struct sk_buff *next;
>>
>> -    while (buf) {
>> -        next = buf->next;
>> -        kfree_skb(buf);
>> -        buf = next;
>> -    }
>> +    link_kfree_skbuff(buf);
>>       l_ptr->first_out = NULL;
>>       l_ptr->out_queue_size = 0;
>>   }
>> @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr)
>>   void tipc_link_stop(struct tipc_link *l_ptr)
>>   {
>>       struct sk_buff *buf;
>> -    struct sk_buff *next;
>>
>>       buf = l_ptr->oldest_deferred_in;
>> -    while (buf) {
>> -        next = buf->next;
>> -        kfree_skb(buf);
>> -        buf = next;
>> -    }
>> +    link_kfree_skbuff(buf);
>>
>>       buf = l_ptr->first_out;
>> -    while (buf) {
>> -        next = buf->next;
>> -        kfree_skb(buf);
>> -        buf = next;
>> -    }
>> +    link_kfree_skbuff(buf);
>>
>>       tipc_link_reset_fragments(l_ptr);
>>
>> @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr)
>>       kfree_skb(l_ptr->proto_msg_queue);
>>       l_ptr->proto_msg_queue = NULL;
>>       buf = l_ptr->oldest_deferred_in;
>> -    while (buf) {
>> -        struct sk_buff *next = buf->next;
>> -        kfree_skb(buf);
>> -        buf = next;
>> -    }
>> +    link_kfree_skbuff(buf);
>>       if (!list_empty(&l_ptr->waiting_ports))
>>           tipc_link_wakeup_ports(l_ptr, 1);
>>
>> @@ -1127,10 +1120,7 @@ again:
>>           if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) {
>>               res = -EFAULT;
>>   error:
>> -            for (; buf_chain; buf_chain = buf) {
>> -                buf = buf_chain->next;
>> -                kfree_skb(buf_chain);
>> -            }
>> +            link_kfree_skbuff(buf_chain);
>>               return res;
>>           }
>>           sect_crs += sz;
>> @@ -1180,18 +1170,12 @@ error:
>>           if (l_ptr->max_pkt < max_pkt) {
>>               sender->max_pkt = l_ptr->max_pkt;
>>               tipc_node_unlock(node);
>> -            for (; buf_chain; buf_chain = buf) {
>> -                buf = buf_chain->next;
>> -                kfree_skb(buf_chain);
>> -            }
>> +            link_kfree_skbuff(buf_chain);
>>               goto again;
>>           }
>>       } else {
>>   reject:
>> -        for (; buf_chain; buf_chain = buf) {
>> -            buf = buf_chain->next;
>> -            kfree_skb(buf_chain);
>> -        }
>> +        link_kfree_skbuff(buf_chain);
>>           return tipc_port_reject_sections(sender, hdr, msg_sect,
>>                            len, TIPC_ERR_NO_NODE);
>>       }
>> @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf)
>>           fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE);
>>           if (fragm == NULL) {
>>               kfree_skb(buf);
>> -            while (buf_chain) {
>> -                buf = buf_chain;
>> -                buf_chain = buf_chain->next;
>> -                kfree_skb(buf);
>> -            }
>> +            link_kfree_skbuff(buf_chain);
>>               return -ENOMEM;
>>           }
>>           msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE);
>>
> 
> .
>