mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 22:35:15 +00:00
Code Issues Releases Wiki Activity

learn: Fix iteration over learning specs.

Browse Source 
struct ofpact_learn_spec is variable-length.  The 'n_specs' member of
struct ofpact_learn counted the number of specs, but the iteration loops
over struct ofpact_learn_spec only iterated as far as the *minimum* length
of 'n_specs' specs.

This fixes the problem, which exhibited as consistent failures for test 431
(learning action - TCPv6 port learning), seemingly only on i386 since it
shows up for my personal development machine but appears to not happen for
anyone else.

Fixes: dfe191d5fa ("ofp-actions: Waste less memory in learn actions.")
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>
This commit is contained in:
Ben Pfaff
2016-09-02 13:26:50 -07:00
parent 784bf5d4eb
commit c65a31e262
3 changed files with 28 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
include/openvswitch/ofp-actions.h
View File

@@ -744,21 +744,34 @@ ofpact_learn_spec_next(const struct ofpact_learn_spec *spec)
* *
* Used for NXAST_LEARN. */ * Used for NXAST_LEARN. */
struct ofpact_learn { struct ofpact_learn {
struct ofpact ofpact; OFPACT_PADDED_MEMBERS(
struct ofpact ofpact;
uint16_t idle_timeout; /* Idle time before discarding (seconds). */ uint16_t idle_timeout; /* Idle time before discarding (seconds). */
uint16_t hard_timeout; /* Max time before discarding (seconds). */ uint16_t hard_timeout; /* Max time before discarding (seconds). */
uint16_t priority; /* Priority level of flow entry. */ uint16_t priority; /* Priority level of flow entry. */
uint8_t table_id; /* Table to insert flow entry. */ uint8_t table_id; /* Table to insert flow entry. */
enum nx_learn_flags flags; /* NX_LEARN_F_*. */ enum nx_learn_flags flags; /* NX_LEARN_F_*. */
ovs_be64 cookie; /* Cookie for new flow. */ ovs_be64 cookie; /* Cookie for new flow. */
uint16_t fin_idle_timeout; /* Idle timeout after FIN, if nonzero. */ uint16_t fin_idle_timeout; /* Idle timeout after FIN, if nonzero. */
uint16_t fin_hard_timeout; /* Hard timeout after FIN, if nonzero. */ uint16_t fin_hard_timeout; /* Hard timeout after FIN, if nonzero. */
);
unsigned int n_specs;
struct ofpact_learn_spec specs[]; struct ofpact_learn_spec specs[];
}; };
static inline const struct ofpact_learn_spec *
ofpact_learn_spec_end(const struct ofpact_learn *learn)
{
return ALIGNED_CAST(const struct ofpact_learn_spec *,
ofpact_next(&learn->ofpact));
}
#define OFPACT_LEARN_SPEC_FOR_EACH(SPEC, LEARN) \
for ((SPEC) = (LEARN)->specs; \
(SPEC) < ofpact_learn_spec_end(LEARN); \
(SPEC) = ofpact_learn_spec_next(SPEC))
/* Multipath link choice algorithm to apply. /* Multipath link choice algorithm to apply.
* *
* In the descriptions below, 'n_links' is max_link + 1. */ * In the descriptions below, 'n_links' is max_link + 1. */

13
lib/learn.c
View File

@@ -41,8 +41,7 @@ learn_check(const struct ofpact_learn *learn, const struct flow *flow)
struct match match; struct match match;
match_init_catchall(&match); match_init_catchall(&match);
for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
spec = ofpact_learn_spec_next(spec)) {
enum ofperr error; enum ofperr error;
/* Check the source. */ /* Check the source. */
@@ -123,8 +122,7 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow,
oft->fin_hard_timeout = learn->fin_hard_timeout; oft->fin_hard_timeout = learn->fin_hard_timeout;
} }
for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
spec = ofpact_learn_spec_next(spec)) {
struct ofpact_set_field *sf; struct ofpact_set_field *sf;
union mf_subvalue value; union mf_subvalue value;
@@ -179,8 +177,7 @@ learn_mask(const struct ofpact_learn *learn, struct flow_wildcards *wc)
union mf_subvalue value; union mf_subvalue value;
memset(&value, 0xff, sizeof value); memset(&value, 0xff, sizeof value);
for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
spec = ofpact_learn_spec_next(spec)) {
if (spec->src_type == NX_LEARN_SRC_FIELD) { if (spec->src_type == NX_LEARN_SRC_FIELD) {
mf_write_subfield_flow(&spec->src, &value, &wc->masks); mf_write_subfield_flow(&spec->src, &value, &wc->masks);
} }
@@ -386,7 +383,6 @@ learn_parse__(char *orig, char *arg, struct ofpbuf *ofpacts)
return error; return error;
} }
learn = ofpacts->header; learn = ofpacts->header;
learn->n_specs++;
} }
} }
ofpact_finish_LEARN(ofpacts, &learn); ofpact_finish_LEARN(ofpacts, &learn);
@@ -459,8 +455,7 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)
colors.param, colors.end, ntohll(learn->cookie)); colors.param, colors.end, ntohll(learn->cookie));
} }
for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
spec = ofpact_learn_spec_next(spec)) {
unsigned int n_bytes = DIV_ROUND_UP(spec->n_bits, 8); unsigned int n_bytes = DIV_ROUND_UP(spec->n_bits, 8);
ds_put_char(s, ','); ds_put_char(s, ',');

4
lib/ofp-actions.c
View File

@@ -4321,7 +4321,6 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
spec = ofpbuf_put_zeros(ofpacts, sizeof *spec); spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
learn = ofpacts->header; learn = ofpacts->header;
learn->n_specs++;
spec->src_type = header & NX_LEARN_SRC_MASK; spec->src_type = header & NX_LEARN_SRC_MASK;
spec->dst_type = header & NX_LEARN_DST_MASK; spec->dst_type = header & NX_LEARN_DST_MASK;
@@ -4421,8 +4420,7 @@ encode_LEARN(const struct ofpact_learn *learn,
nal->flags = htons(learn->flags); nal->flags = htons(learn->flags);
nal->table_id = learn->table_id; nal->table_id = learn->table_id;
for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
spec = ofpact_learn_spec_next(spec)) {
put_u16(out, spec->n_bits | spec->dst_type | spec->src_type); put_u16(out, spec->n_bits | spec->dst_type | spec->src_type);
if (spec->src_type == NX_LEARN_SRC_FIELD) { if (spec->src_type == NX_LEARN_SRC_FIELD) {