On Wed, Aug 08, 2018 at 12:14:30PM -0700, David Miller wrote:
> From: Doron Roberts-Kedes <doro...@fb.com>
> Date: Tue, 7 Aug 2018 11:09:39 -0700
>
> > +static int __skb_nsg(struct sk_buff *skb, int offset, int len,
> > + unsigned int recursion_level)
> > +{
> > + int start = skb_headlen(skb);
> > + int i, copy = start - offset;
> > + struct sk_buff *frag_iter;
> > + int elt = 0;
> > +
> > + if (unlikely(recursion_level >= 24))
> > + return -EMSGSIZE;
>
> This recursion is kinda crazy.
>
> Even skb_cow_data() doesn't recurse like this (of course because it copies
> into linear buffers).
>
> There has to be a way to simplify this. Fragment lists are such a rarely
> used SKB geometry, and few if any devices support it for transmission
> (so the fraglist will get undone at transmit time anyways).
>
Interesting. Just wanted to clarify whether the issue is the use of
recursion or the fact that the function is handling the frag_list at
all. This is the rx path, so my understanding was that we need to handle
the frag_list. Please let me know if I'm misunderstanding your point
about the rare use of fragment lists.
If the issue is the recursion, I can rewrite the function to not use
recursion, but skb_to_sgvec uses a similar pattern and is invoked
immediately afterwards.
Taking a step back, is there an existing solution for what this function
is trying to do? I was surprised to find that there did not seem to
exist a function for determining the number of scatterlist elements
required to map an skb without COW.
