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: dfe191d5faa6 ("ofp-actions: Waste less memory in learn actions.")
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
include/openvswitch/ofp-actions.h | 33 +++++++++++++++++++++++----------
lib/learn.c | 13 ++++---------
lib/ofp-actions.c | 4 +---
3 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index bf7d62f..6759201 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -744,21 +744,34 @@ ofpact_learn_spec_next(const struct ofpact_learn_spec
*spec)
*
* Used for NXAST_LEARN. */
struct ofpact_learn {
- struct ofpact ofpact;
+ OFPACT_PADDED_MEMBERS(
+ struct ofpact ofpact;
- uint16_t idle_timeout; /* Idle time before discarding (seconds). */
- uint16_t hard_timeout; /* Max time before discarding (seconds). */
- uint16_t priority; /* Priority level of flow entry. */
- uint8_t table_id; /* Table to insert flow entry. */
- enum nx_learn_flags flags; /* NX_LEARN_F_*. */
- ovs_be64 cookie; /* Cookie for new flow. */
- uint16_t fin_idle_timeout; /* Idle timeout after FIN, if nonzero. */
- uint16_t fin_hard_timeout; /* Hard timeout after FIN, if nonzero. */
+ uint16_t idle_timeout; /* Idle time before discarding (seconds). */
+ uint16_t hard_timeout; /* Max time before discarding (seconds). */
+ uint16_t priority; /* Priority level of flow entry. */
+ uint8_t table_id; /* Table to insert flow entry. */
+ enum nx_learn_flags flags; /* NX_LEARN_F_*. */
+ ovs_be64 cookie; /* Cookie for new flow. */
+ uint16_t fin_idle_timeout; /* Idle 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[];
};
+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.
*
* In the descriptions below, 'n_links' is max_link + 1. */
diff --git a/lib/learn.c b/lib/learn.c
index bb57f7d..9cab759 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -41,8 +41,7 @@ learn_check(const struct ofpact_learn *learn, const struct
flow *flow)
struct match match;
match_init_catchall(&match);
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
- spec = ofpact_learn_spec_next(spec)) {
+ OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
enum ofperr error;
/* 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;
}
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
- spec = ofpact_learn_spec_next(spec)) {
+ OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
struct ofpact_set_field *sf;
union mf_subvalue value;
@@ -179,8 +177,7 @@ learn_mask(const struct ofpact_learn *learn, struct
flow_wildcards *wc)
union mf_subvalue value;
memset(&value, 0xff, sizeof value);
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
- spec = ofpact_learn_spec_next(spec)) {
+ OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
if (spec->src_type == NX_LEARN_SRC_FIELD) {
mf_write_subfield_flow(&spec->src, &value, &wc->masks);
}
@@ -386,7 +383,6 @@ learn_parse__(char *orig, char *arg, struct ofpbuf *ofpacts)
return error;
}
learn = ofpacts->header;
- learn->n_specs++;
}
}
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));
}
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
- spec = ofpact_learn_spec_next(spec)) {
+ OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
unsigned int n_bytes = DIV_ROUND_UP(spec->n_bits, 8);
ds_put_char(s, ',');
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index f27ff21..22c7b16 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -4321,7 +4321,6 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
learn = ofpacts->header;
- learn->n_specs++;
spec->src_type = header & NX_LEARN_SRC_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->table_id = learn->table_id;
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
- spec = ofpact_learn_spec_next(spec)) {
+ OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
put_u16(out, spec->n_bits | spec->dst_type | spec->src_type);
if (spec->src_type == NX_LEARN_SRC_FIELD) {
--
2.1.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
