[ovs-dev] [PATCH] Allow OVS_USERSPACE_ATTR_USERDATA to be variable length.

Ben Pfaff blp at nicira.com
Sat Feb 16 00:48:24 UTC 2013


On Fri, Feb 15, 2013 at 04:17:21PM -0800, Jesse Gross wrote:
> On Mon, Feb 4, 2013 at 4:01 PM, Ben Pfaff <blp at 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 <jesse at 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.



More information about the dev mailing list