On Thu, Sep 29, 2011 at 4:07 PM, Jesse Gross <[email protected]> wrote: > 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]> > Pushed to master.
Thanks. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
