[ovs-dev] [PATCH 05/10] lib/odp: Masked set action execution and printing.

[Ben Pfaff]

On Wed, Apr 09, 2014 at 01:24:07PM -0700, Jarno Rajahalme wrote:
> 
> On Apr 9, 2014, at 10:51 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Tue, Apr 08, 2014 at 04:38:47PM -0700, Jarno Rajahalme wrote:
> >> Masked set actions add a mask, immediately following the netlink
> >> attribute data, within the netlink attribute itself.  Thus the key
> >> attribute size for a masked set action is exactly double of the
> >> non-masked set action.
> >> 
> >> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> > 
> > This adds a new restriction to key attributes: they must have a fixed
> > length (because otherwise one cannot tell whether the length is
> > doubled).  In fact, MPLS is already variable length, as documented in
> > <linux/openvswitch.h>:
> > 
> > 	OVS_KEY_ATTR_MPLS = 62, /* array of struct ovs_key_mpls.
> > 				 * The implementation may restrict
> > 				 * the accepted length of the array. */
> > 
> 
> I totally missed this, maybe due to the current userspace datapath setting only on lee, and linux kernel datapath not yet supporting MPLS.
> 
> > So I'd suggest some other way to distinguish masked set actions.
> 
> The obvious alternative is to use a second attribute of the same
> type and length within the nested attributes of the set action. It
> is currently limited to one. The additional structure will be
> somewhat repetitive, and we?ll have to enforce that the key type and
> length are the same for both. To be more explicit we could set the
> MSB of the type, but then we might also want to support mask before
> the value, etc. That may be a bit too much flexibility, though.
> 
> Thoughts?

One alternative would be to introduce OVS_ACTION_ATTR_SET_MASKED,
would use the format you proposed in this patch except that the mask
would always be present.