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

[Jesse Gross]

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>