On Apr 9, 2014, at 1:26 PM, Ben Pfaff <b...@nicira.com> wrote: > On Wed, Apr 09, 2014 at 01:24:07PM -0700, Jarno Rajahalme wrote: >> >> On Apr 9, 2014, at 10:51 AM, Ben Pfaff <b...@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 <jrajaha...@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.
Thanks, I like this, as this will also make feature checking more explicit. Jarno
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
