[ovs-dev] [PATCH] stt: linearize for CONFIG_SLUB case

[Jesse Gross]

On Mon, Apr 4, 2016 at 7:57 PM, pravin shelar <pshelar at ovn.org> wrote:
> On Mon, Apr 4, 2016 at 1:56 PM, Jesse Gross <jesse at kernel.org> wrote:
>> On Fri, Apr 1, 2016 at 4:58 PM, pravin shelar <pshelar at ovn.org> wrote:
>>> On Thu, Mar 31, 2016 at 9:06 PM, Jesse Gross <jesse at kernel.org> wrote:
>>>> On Thu, Mar 31, 2016 at 2:30 PM, Pravin B Shelar <pshelar at ovn.org> wrote:
>>>>> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
>>>>> index eb397e8..ae33d5e 100644
>>>>> --- a/datapath/linux/compat/stt.c
>>>>> +++ b/datapath/linux/compat/stt.c
>>>>>  static int coalesce_skb(struct sk_buff **headp)
>>>>>  {
>>>>> -       struct sk_buff *frag, *head, *prev;
>>>>> +       struct sk_buff *frag, *head;
>>>>> +#ifndef SKIP_ZERO_COPY
>>>>> +       struct sk_buff *prev;
>>>>> +#endif
>>>>>         int err;
>>>>>
>>>>>         err = straighten_frag_list(headp);
>>>>
>>>> I don't think that we need to straighten the frag list in the
>>>> SKIP_ZERO_COPY case. It's basically just undoing what the for loop
>>>> that forms the frag list will do. __skb_linearize() will be able to
>>>> handle it in any case.
>>>>
>>> I can skip straightening and work on skb with frag-list, but I do not
>>> want to complicate the code.  This case is pretty rare after the
>>> change in reassemble where complete skb is allocated on first set
>>> segment.
>>
>> I don't think that it should increase complexity - I believe that we
>> can just skip the call to straighten_frag_list() and everything else
>> will continue to work.
>>
> One difference is that the fragments iterator reads the list from
> skb->next, which is what straighten_frag_list() generate. If we skip
> the call the list is in shinfo-frag-list.
> Let me see if this can be simplified a bit.

I think it's actually still OK as is. When there is a frag list, the
expectation is that the length, data_len, and truesize for all of the
individual fragments are summed in the main skb at the head of the
list. normalize_frag_list() actually needs to undo this. As a result,
the fact that the frag_list iterator won't descend into the inner
frag_lists shouldn't be a problem. I think when we get to the
assignment of skb_shinfo(head)->frag_list, we can just stick
head->next onto the end of the list if there is any existing
frag_list.