[ovs-dev] [PATCH] Avoid skb-clone in upcall

[Pravin Shelar]

On Thu, Sep 29, 2011 at 4:07 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Thu, Sep 29, 2011 at 3:41 PM, Pravin Shelar <pshelar at nicira.com> wrote:
>> There is not need to clone skb while sending packet to user-space.
>> Since data is only read from packet skb.
>>
>> Signed-off-by: Pravin Shelar <pshelar at nicira.com>
>
> Please use tabs instead of spaces for indentation.  This causes a
> bunch of whitespace warnings but please check for other uses as well:
> Applying: Avoid skb-clone in upcall
> /home/jesse/openvswitch/.git/rebase-apply/patch:29: space before tab in indent.
>                        kfree_skb(skb);
> /home/jesse/openvswitch/.git/rebase-apply/patch:74: space before tab in indent.
>                        next = nskb->next;
> /home/jesse/openvswitch/.git/rebase-apply/patch:75: space before tab in indent.
>                        kfree_skb(nskb);
> /home/jesse/openvswitch/.git/rebase-apply/patch:76: space before tab in indent.
>                } while ((nskb = next) != NULL);
> warning: 4 lines add whitespace errors.
>
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index 63a3932..83c147b 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> +int dp_upcall(struct datapath *dp, struct sk_buff *skb,
>> +             const struct dp_upcall_info *upcall_info)
>>  {
>> +       struct sk_buff *nskb = NULL;
>
> Now that this variable has been moved to a larger scope, can you
> please give it a more descriptive name?
>
>> @@ -477,20 +476,12 @@ static int queue_userspace_packets(struct datapath *dp, u32 pid,
>>
>>                err = genlmsg_unicast(&init_net, user_skb, pid);
>>                if (err)
>> -                       goto err_kfree_skbs;
>> +                       return err;
>>
>> -               consume_skb(skb);
>> -               skb = nskb;
>> +               skb = skb->next;
>>        } while (skb);
>
> You can move the assignment of skb = skb->next in the while loop -
> that's the slightly more canonical way to do it.
>
> Otherwise, this looks good to me:
> Acked-by: Jesse Gross <jesse at nicira.com>
>
Pushed to master.

Thanks.