On Fri, Feb 15, 2013 at 04:17:21PM -0800, Jesse Gross wrote:
> On Mon, Feb 4, 2013 at 4:01 PM, Ben Pfaff <b...@nicira.com> wrote:
> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index 04a5e7f..2a542da 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> > @@ -678,7 +678,6 @@ static int validate_userspace(const struct nlattr *attr)
> > {
> > static const struct nla_policy
> > userspace_policy[OVS_USERSPACE_ATTR_MAX + 1] = {
> > [OVS_USERSPACE_ATTR_PID] = {.type = NLA_U32 },
> > - [OVS_USERSPACE_ATTR_USERDATA] = {.type = NLA_U64 },
>
> It seems like it is better to keep this with type NLA_UNSPEC rather
> than remove it completely. There isn't any functional difference but
> it seems to better reflect the intention.
>
> Otherwise, this looks good to me.
> Acked-by: Jesse Gross <je...@nicira.com>
Thanks for the review.
I made the change you suggested.
I tested this just now and found a bug: OVS_USERSPACE_ATTR_USERDATA in
the action has to be translated to OVS_PACKET_ATTR_USERDATA in the
kernel->user Netlink message, otherwise it gets interpreted as
OVS_PACKET_ATTR_KEY. So I applied the following incremental:
diff --git a/datapath/datapath.c b/datapath/datapath.c
index bc0b906..0920a30 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -392,8 +392,9 @@ static int queue_userspace_packet(struct net *net, int
dp_ifindex,
nla_nest_end(user_skb, nla);
if (upcall_info->userdata)
- nla_append(user_skb, NLA_ALIGN(upcall_info->userdata->nla_len),
- upcall_info->userdata);
+ __nla_put(user_skb, OVS_PACKET_ATTR_USERDATA,
+ upcall_info->userdata->nla_len - NLA_HDRLEN,
+ nla_data(upcall_info->userdata));
nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
and I will push it to master in a moment.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
