On Sat, Nov 30, 2013 at 4:21 AM, Thomas Graf <[email protected]> wrote: > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c > index 8eaa39a..867edf1 100644 > --- a/net/openvswitch/datapath.c > +++ b/net/openvswitch/datapath.c > +static int queue_gso_packets(struct datapath *, struct net *, int dp_ifindex, > + struct sk_buff *, const struct dp_upcall_info *); > +static int queue_userspace_packet(struct datapath *, struct net *, > + int dp_ifindex, struct sk_buff *, > const struct dp_upcall_info *);
Should we drop the dp_ifindex arguments from these functions? It should be trivially derivable from struct datapath. > static size_t upcall_msg_size(const struct sk_buff *skb, > - const struct nlattr *userdata) > + const struct nlattr *userdata, > + unsigned int hdrlen) I think that 'skb' is now unused. > @@ -427,7 +429,21 @@ static int queue_userspace_packet(struct net *net, int > dp_ifindex, > goto out; > } > > - len = upcall_msg_size(skb, upcall_info->userdata); > + /* Complete checksum if needed */ > + if (skb->ip_summed == CHECKSUM_PARTIAL && > + (err = skb_checksum_help(skb))) > + goto out; I think that we can remove the hardware features argument to __skb_gso_segment() in queue_gso_packet(). It was there to take advantage of the copy and checksum optimization but that's no longer present. > @@ -447,13 +463,17 @@ static int queue_userspace_packet(struct net *net, int > dp_ifindex, > nla_len(upcall_info->userdata), > nla_data(upcall_info->userdata)); > > - nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len); > + /* Only reserve room for attribute header, packet data is added > + * in skb_zerocopy() */ > + if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0))) > + goto out; Does this initialized 'err' on failure? _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
