[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dc924e3a-8925-d24b-e608-38bb0c3b2925@fb.com>
Date: Mon, 19 Mar 2018 22:54:03 -0700
From: Yonghong Song <yhs@...com>
To: "Yuan, Linyu (NSB - CN/Shanghai)" <linyu.yuan@...ia-sbell.com>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"ast@...com" <ast@...com>,
"daniel@...earbox.net" <daniel@...earbox.net>,
"diptanu@...com" <diptanu@...com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "kernel-team@...com" <kernel-team@...com>
Subject: Re: [PATCH net-next 1/2] net: permit skb_segment on head_frag
frag_list skb
On 3/19/18 10:30 PM, Yuan, Linyu (NSB - CN/Shanghai) wrote:
>
>
>> -----Original Message-----
>> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
>> On Behalf Of Yonghong Song
>> Sent: Tuesday, March 20, 2018 1:16 PM
>> To: edumazet@...gle.com; ast@...com; daniel@...earbox.net;
>> diptanu@...com; netdev@...r.kernel.org
>> Cc: kernel-team@...com
>> Subject: [PATCH net-next 1/2] net: permit skb_segment on head_frag frag_list
>> skb
>>
>>
>> while (pos < offset + len) {
>> if (i >= nfrags) {
>> - BUG_ON(skb_headlen(list_skb));
>> + if (skb_headlen(list_skb) && check_list_skb == list_skb) {
> Here cause next BUG_ON always false.
The idea is since in this branch, we did not do list_skb =
list_skb->next. So we update check_list_skb. Next time, when the
control reaches here, list_skb may still be the same, but check_list_skb
is not, so we proceed to process list_skb->frags in the else branch.
In the else branch, we have
list_skb = list_skb->next;
check_list_skb = list_skb;
So when the current frags are processed and ready for the list_skb.
list_skb will be equal to check_list_skb and it will be processed again.
It is a little bit convoluted. Please let me know you have better idea.
>> + } else {
>> + BUG_ON(skb_headlen(list_skb) && check_list_skb ==
>> list_skb);
> Just according code logic, no need BUG_ON, right?
Oh, yes, we do not need this. Will remove in the next version.
>>
>> - i = 0;
>> - nfrags = skb_shinfo(list_skb)->nr_frags;
>> - frag = skb_shinfo(list_skb)->frags;
>> - frag_skb = list_skb;
>> + i = 0;
>> + nfrags = skb_shinfo(list_skb)->nr_frags;
>> + frag = skb_shinfo(list_skb)->frags;
>> + frag_skb = list_skb;
>>
>> - BUG_ON(!nfrags);
>> + BUG_ON(!nfrags);
>>
>> - if (skb_orphan_frags(frag_skb, GFP_ATOMIC) ||
>> - skb_zerocopy_clone(nskb, frag_skb,
>> - GFP_ATOMIC))
>> - goto err;
>> + if (skb_orphan_frags(frag_skb, GFP_ATOMIC) ||
>> + skb_zerocopy_clone(nskb, frag_skb,
>> GFP_ATOMIC))
>> + goto err;
>>
>> - list_skb = list_skb->next;
>> + list_skb = list_skb->next;
>> + check_list_skb = list_skb;
>> + }
>> }
>>
>> if (unlikely(skb_shinfo(nskb)->nr_frags >=
>> --
>> 2.9.5
>
Powered by blists - more mailing lists