From: Wang Weidong <wangweidong1@huawei•com>
To: Jon Maloy <jon.maloy@ericsson•com>
Cc: Ying Xue <ying.xue@windriver•com>, <allan.stephens@windriver•com>,
<davem@davemloft•net>, <netdev@vger•kernel.org>,
<tipc-discussion@lists•sourceforge.net>
Subject: Re: [PATCH net-next 1/6] tipc: add link_kfree_skbuff helper function
Date: Sat, 7 Dec 2013 12:52:49 +0800 [thread overview]
Message-ID: <52A2A9A1.7000402@huawei.com> (raw)
In-Reply-To: <52A1DB98.60109@ericsson.com>
On 2013/12/6 22:13, Jon Maloy wrote:
> Wang,
> I am very happy to see you posting improvements to TIPC, but please synch up
> with the TIPC development team (i.e., use tipc_discussion), before posting
> it to netdev. As Ying stated,we have a patch series in the pipe that deals
> with exactly this issue, and more.
>
> Regards
> ///jon
>
Hi Jon,
Thanks for your reply. Now I am more clearly about how to do it.
Regards.
Wang
>
>
>
> On 12/06/2013 01:42 AM, Wang Weidong wrote:
>> On 2013/12/6 14:34, Ying Xue wrote:
>>> On 12/06/2013 02:23 PM, 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;
>>>> + }
>>>> +}
>>>> +
>>>
>>> Your new defined function is unnecessary, instead we already have
>>> another patch doing the same thing with kfree_skb_list(), and the patch
>>> will be to be sent out soon.
>>>
>>> Please see below link:
>>>
>>> http://article.gmane.org/gmane.network.tipc.general/5140/
>>>
>>> And the patch cleans up more things than your patch.
>>>
>>
>> Yes, You are right.
>> Thanks.
>>
>>> Regards,
>>> Ying
>>>
>>>> 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);
>>>>
>>>
>>>
>>> .
>>>
>>
>>
>
>
> .
>
next prev parent reply other threads:[~2013-12-07 4:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-06 6:23 [PATCH net-next 0/6] tipc: do some clean ups Wang Weidong
2013-12-06 6:23 ` [PATCH net-next 1/6] tipc: add link_kfree_skbuff helper function Wang Weidong
2013-12-06 6:34 ` Ying Xue
2013-12-06 6:42 ` Wang Weidong
2013-12-06 14:13 ` Jon Maloy
2013-12-07 4:52 ` Wang Weidong [this message]
2013-12-06 6:44 ` Eric Dumazet
2013-12-06 9:09 ` Daniel Borkmann
2013-12-06 9:16 ` Wang Weidong
2013-12-06 6:23 ` [PATCH net-next 2/6] tipc: remove the unnecessary variable Wang Weidong
2013-12-06 6:23 ` [PATCH net-next 3/6] tipc: use a same goto path Wang Weidong
2013-12-06 6:23 ` [PATCH net-next 4/6] tipc: Use <linux/uaccess.h> instead of <asm/uaccess.h> Wang Weidong
2013-12-06 6:23 ` [PATCH net-next 5/6] tipc: change lock_sock order in connect() Wang Weidong
2013-12-06 6:23 ` [PATCH net-next 6/6] tipc: separate the check nseq and sseq allocate failed Wang Weidong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52A2A9A1.7000402@huawei.com \
--to=wangweidong1@huawei$(echo .)com \
--cc=allan.stephens@windriver$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=jon.maloy@ericsson$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=tipc-discussion@lists$(echo .)sourceforge.net \
--cc=ying.xue@windriver$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox