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. */
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;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
