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

[Ben Pfaff]

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. */

So I'd suggest some other way to distinguish masked set actions.

In general the code looks good but it has more repetition than I
usually like.  Is there a good way to avoid some of that?  Maybe some
pattern like this (not tested):

void
memcpy_masked(void *dst_, const void *src_, const void *mask_, size_t size)
{
    uint8_t *dst = dst_;
    const uint8_t *src = src_;
    const uint8_t *mask = mask_;
    size_t i;

    if (mask) {
        for (i = 0; i < size; i++) {
            dst[i] = src[i] | (dst[i] & ~mask[i]);
        }
    } else {
        memcpy(dst, src, size);
    }
}

#define copy_masked(dst, src, mask)             \
    ((void) sizeof((dst) == (src)),             \
     (void) sizeof((dst) == (mask)),            \
     memcpy_masked(dst, src, mask))
#define get_mask(a, type)                       \
    (nl_attr_get_size(a) >= 2 * sizeof(type)    \
     ? (const type *) nl_attr_get(a) + 1        \
     : NULL)

...
    switch (type) {
    case OVS_KEY_ATTR_PRIORITY:
        copy_masked(&md->skb_priority,
                    nl_attr_get_u32(a), get_mask(a, uint32_t));
        break;