On Thu, Sep 29, 2011 at 3:41 PM, Pravin Shelar <[email protected]> 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 <[email protected]>
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 <[email protected]>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
