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

From: Daniel Borkmann <[email-masked]>
Date: Fri, 06 Dec 2013 10:09:07 +0100

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 <[email-masked]>
> ---
>   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 ...

>   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);
>