RE: [PATCH net-next 1/2] net: permit skb_segment on head_frag frag_list skb

[Yuan, Linyu (NSB - CN/Shanghai)]

> -----Original Message-----
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of Yonghong Song
> Sent: Tuesday, March 20, 2018 1:16 PM
> To: eduma...@google.com; a...@fb.com; dan...@iogearbox.net;
> dipt...@fb.com; netdev@vger.kernel.org
> Cc: kernel-t...@fb.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.
> +                             } else {
> +                                     BUG_ON(skb_headlen(list_skb) && 
> check_list_skb ==
> list_skb);
Just according code logic, no need BUG_ON, right? 
> 
> -                             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