Skip to content

Commit

Permalink
learn: Avoid nested zero-sized arrays to fix build with MSVC.
Browse files Browse the repository at this point in the history 
Avoid using nested zero-sized arrays to allow compilation with MSVC.
Also, make sure the immediate data is accessed only if it exists, and
that the size is always calculated from struct learn_spec field
'n_bits'.

Fixes: dfe191d ("ofp-actions: Waste less memory in learn actions.")
Reported-by: Alin Serdean <[email protected]>
Signed-off-by: Jarno Rajahalme <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
  • Loading branch information
Jarno Rajahalme committed Sep 1, 2016
1 parent 67f0898 commit 507a9a1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 24 deletions.
39 changes: 23 additions & 16 deletions include/openvswitch/ofp-actions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -708,29 +708,36 @@ enum nx_learn_flags {

/* Part of struct ofpact_learn, below. */
struct ofpact_learn_spec {
struct mf_subfield src; /* NX_LEARN_SRC_FIELD only. */
struct mf_subfield dst; /* NX_LEARN_DST_MATCH, NX_LEARN_DST_LOAD only. */
uint16_t src_type; /* One of NX_LEARN_SRC_*. */
uint16_t dst_type; /* One of NX_LEARN_DST_*. */
uint8_t n_bits; /* Number of bits in source and dest. */
uint64_t src_imm[]; /* OFPACT_ALIGNTO (uint64_t) aligned. */
OFPACT_PADDED_MEMBERS(
struct mf_subfield src; /* NX_LEARN_SRC_FIELD only. */
struct mf_subfield dst; /* NX_LEARN_DST_MATCH,
* NX_LEARN_DST_LOAD only. */
uint16_t src_type; /* One of NX_LEARN_SRC_*. */
uint16_t dst_type; /* One of NX_LEARN_DST_*. */
uint8_t n_bits; /* Number of bits in source and dest. */
);
/* Followed by 'DIV_ROUND_UP(n_bits, 8)' bytes of immediate data for
* match 'dst_type's NX_LEARN_DST_MATCH and NX_LEARN_DST_LOAD when
* NX_LEARN_SRC_IMMEDIATE is set in 'src_type', followed by zeroes to align
* to OFPACT_ALIGNTO. */
};
BUILD_ASSERT_DECL(offsetof(struct ofpact_learn_spec, src_imm)
% OFPACT_ALIGNTO == 0);
BUILD_ASSERT_DECL(offsetof(struct ofpact_learn_spec, src_imm)
== sizeof(struct ofpact_learn_spec));
BUILD_ASSERT_DECL(sizeof(struct ofpact_learn_spec) % OFPACT_ALIGNTO == 0);

static inline const void *
ofpact_learn_spec_imm(const struct ofpact_learn_spec *spec)
{
return spec + 1;
}

static inline const struct ofpact_learn_spec *
ofpact_learn_spec_next(const struct ofpact_learn_spec *spec)
{
if (spec->src_type == NX_LEARN_SRC_IMMEDIATE) {
unsigned int n_uint64s
= OFPACT_ALIGN(DIV_ROUND_UP(spec->n_bits, 8)) / sizeof (uint64_t);
return (const struct ofpact_learn_spec *)
((const uint64_t *)(spec + 1) + n_uint64s);
} else {
return spec + 1;
unsigned int n_bytes = OFPACT_ALIGN(DIV_ROUND_UP(spec->n_bits, 8));
return ALIGNED_CAST(const struct ofpact_learn_spec *,
(const uint8_t *)(spec + 1) + n_bytes);
}
return spec + 1;
}

/* OFPACT_LEARN.
Expand Down
16 changes: 10 additions & 6 deletions lib/learn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ learn_check(const struct ofpact_learn *learn, const struct flow *flow)
if (error) {
return error;
}
mf_write_subfield_value(&spec->dst, spec->src_imm, &match);
if (spec->src_type & NX_LEARN_SRC_IMMEDIATE) {
mf_write_subfield_value(&spec->dst,
ofpact_learn_spec_imm(spec), &match);
}
break;

case NX_LEARN_DST_LOAD:
Expand Down Expand Up @@ -128,7 +131,8 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow,
if (spec->src_type == NX_LEARN_SRC_FIELD) {
mf_read_subfield(&spec->src, flow, &value);
} else {
mf_subvalue_from_value(&spec->dst, &value, spec->src_imm);
mf_subvalue_from_value(&spec->dst, &value,
ofpact_learn_spec_imm(spec));
}

switch (spec->dst_type) {
Expand Down Expand Up @@ -457,7 +461,7 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)

for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
spec = ofpact_learn_spec_next(spec)) {
unsigned int n_bytes = DIV_ROUND_UP(spec->dst.n_bits, 8);
unsigned int n_bytes = DIV_ROUND_UP(spec->n_bits, 8);
ds_put_char(s, ',');

switch (spec->src_type | spec->dst_type) {
Expand All @@ -468,15 +472,15 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)

memset(&value, 0, sizeof value);
memcpy(&value.b[spec->dst.field->n_bytes - n_bytes],
spec->src_imm, n_bytes);
ofpact_learn_spec_imm(spec), n_bytes);
ds_put_format(s, "%s%s=%s", colors.param,
spec->dst.field->name, colors.end);
mf_format(spec->dst.field, &value, NULL, s);
} else {
ds_put_format(s, "%s", colors.param);
mf_format_subfield(&spec->dst, s);
ds_put_format(s, "=%s", colors.end);
ds_put_hex(s, spec->src_imm, n_bytes);
ds_put_hex(s, ofpact_learn_spec_imm(spec), n_bytes);
}
break;
}
Expand All @@ -493,7 +497,7 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)

case NX_LEARN_SRC_IMMEDIATE | NX_LEARN_DST_LOAD:
ds_put_format(s, "%sload:%s", colors.special, colors.end);
ds_put_hex(s, spec->src_imm, n_bytes);
ds_put_hex(s, ofpact_learn_spec_imm(spec), n_bytes);
ds_put_format(s, "%s->%s", colors.special, colors.end);
mf_format_subfield(&spec->dst, s);
break;
Expand Down
5 changes: 3 additions & 2 deletions lib/ofp-actions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4431,9 +4431,10 @@ encode_LEARN(const struct ofpact_learn *learn,
} else {
size_t n_dst_bytes = 2 * DIV_ROUND_UP(spec->n_bits, 16);
uint8_t *bits = ofpbuf_put_zeros(out, n_dst_bytes);
unsigned int n_bytes = DIV_ROUND_UP(spec->dst.n_bits, 8);
unsigned int n_bytes = DIV_ROUND_UP(spec->n_bits, 8);

memcpy(bits + n_dst_bytes - n_bytes, spec->src_imm, n_bytes);
memcpy(bits + n_dst_bytes - n_bytes, ofpact_learn_spec_imm(spec),
n_bytes);
}

if (spec->dst_type == NX_LEARN_DST_MATCH ||
Expand Down

0 comments on commit 507a9a1

Please sign in to comment.