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

Do not perform validation in learn_parse();

Browse Source 
I believe this is consistent with the handling of all other action
parsing called from parse_named_action().

Verification of all actions, including learn actions, occurs separately
in ofpact_check__(). It also occurs via in a call to ofpacts_check()
in parse_ofp_str(),

This patch is larger than might otherwise be expected as the flow argument
of learn_parse() is now unused and thus removed.  This propagates up the
call-chain some way.

This implementation was suggested by Jesse Gross in response to an
enhancement I made to the validation performed during parsing learn actions
to allow it to correctly account for changes to the dl_type due to MPLS
push and pop actions.

Tests have also been updated to check for the less specific messages
generated by the call to ofpacts_check() in parse_ofp_str() which at the
suggestion of Ben Pfaff was added by a prior patch for this purpose.

Cc: Jesse Gross <jesse@nicira.com>
Cc: Ben Pfaff <blp@nicira.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit is contained in:
Simon Horman
2013-05-08 10:50:15 +09:00
committed by Ben Pfaff
parent b019d34da6
commit dd43a55859
4 changed files with 19 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
lib/learn.c
View File

@@ -512,14 +512,13 @@ learn_parse_spec(const char *orig, char *name, char *value,
*
* Modifies 'arg'. */
void
learn_parse(char *arg, const struct flow *flow, struct ofpbuf *ofpacts)
learn_parse(char *arg, struct ofpbuf *ofpacts)
{
char *orig = xstrdup(arg);
char *name, *value;
struct ofpact_learn *learn;
struct match match;
enum ofperr error;
learn = ofpact_put_LEARN(ofpacts);
learn->idle_timeout = OFP_FLOW_PERMANENT;
@@ -556,21 +555,6 @@ learn_parse(char *arg, const struct flow *flow, struct ofpbuf *ofpacts)
learn_parse_spec(orig, name, value, spec);
/* Check prerequisites. */
if (spec->src_type == NX_LEARN_SRC_FIELD
&& flow && !mf_are_prereqs_ok(spec->src.field, flow)) {
ovs_fatal(0, "%s: cannot specify source field %s because "
"prerequisites are not satisfied",
orig, spec->src.field->name);
}
if ((spec->dst_type == NX_LEARN_DST_MATCH
|| spec->dst_type == NX_LEARN_DST_LOAD)
&& !mf_are_prereqs_ok(spec->dst.field, &match.flow)) {
ovs_fatal(0, "%s: cannot specify destination field %s because "
"prerequisites are not satisfied",
orig, spec->dst.field->name);
}
/* Update 'match' to allow for satisfying destination
* prerequisites. */
if (spec->src_type == NX_LEARN_SRC_IMMEDIATE
@@ -581,13 +565,6 @@ learn_parse(char *arg, const struct flow *flow, struct ofpbuf *ofpacts)
}
ofpact_update_len(ofpacts, &learn->ofpact);
/* In theory the above should have caught any errors, but... */
if (flow) {
error = learn_check(learn, flow);
if (error) {
ovs_fatal(0, "%s: %s", orig, ofperr_to_string(error));
}
}
free(orig);
}

2
lib/learn.h
View File

@@ -39,7 +39,7 @@ void learn_to_nxast(const struct ofpact_learn *, struct ofpbuf *openflow);
void learn_execute(const struct ofpact_learn *, const struct flow *,
struct ofputil_flow_mod *, struct ofpbuf *ofpacts);
void learn_parse(char *, const struct flow *, struct ofpbuf *ofpacts);
void learn_parse(char *, struct ofpbuf *ofpacts);
void learn_format(const struct ofpact_learn *, struct ds *);
#endif /* learn.h */

20
lib/ofp-parse.c
View File

@@ -418,7 +418,7 @@ parse_sample(struct ofpbuf *b, char *arg)
}
static void
parse_named_action(enum ofputil_action_code code, const struct flow *flow,
parse_named_action(enum ofputil_action_code code,
char *arg, struct ofpbuf *ofpacts)
{
struct ofpact_tunnel *tunnel;
@@ -579,7 +579,7 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
NOT_REACHED();
case OFPUTIL_NXAST_LEARN:
learn_parse(arg, flow, ofpacts);
learn_parse(arg, ofpacts);
break;
case OFPUTIL_NXAST_EXIT:
@@ -634,12 +634,12 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
}
static bool
str_to_ofpact__(const struct flow *flow, char *pos, char *act, char *arg,
str_to_ofpact__(char *pos, char *act, char *arg,
struct ofpbuf *ofpacts, int n_actions)
{
int code = ofputil_action_code_from_name(act);
if (code >= 0) {
parse_named_action(code, flow, arg, ofpacts);
parse_named_action(code, arg, ofpacts);
} else if (!strcasecmp(act, "drop")) {
if (n_actions) {
ovs_fatal(0, "Drop actions must not be preceded by other "
@@ -662,7 +662,7 @@ str_to_ofpact__(const struct flow *flow, char *pos, char *act, char *arg,
}
static void
str_to_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
str_to_ofpacts(char *str, struct ofpbuf *ofpacts)
{
char *pos, *act, *arg;
enum ofperr error;
@@ -671,7 +671,7 @@ str_to_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
pos = str;
n_actions = 0;
while (ofputil_parse_key_value(&pos, &act, &arg)) {
if (!str_to_ofpact__(flow, pos, act, arg, ofpacts, n_actions)) {
if (!str_to_ofpact__(pos, act, arg, ofpacts, n_actions)) {
break;
}
n_actions++;
@@ -729,7 +729,7 @@ parse_named_instruction(enum ovs_instruction_type type,
}
static void
str_to_inst_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
str_to_inst_ofpacts(char *str, struct ofpbuf *ofpacts)
{
char *pos, *inst, *arg;
int type;
@@ -741,7 +741,7 @@ str_to_inst_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
while (ofputil_parse_key_value(&pos, &inst, &arg)) {
type = ofpact_instruction_type_from_name(inst);
if (type < 0) {
if (!str_to_ofpact__(flow, pos, inst, arg, ofpacts, n_actions)) {
if (!str_to_ofpact__(pos, inst, arg, ofpacts, n_actions)) {
break;
}
@@ -1007,7 +1007,7 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
enum ofperr err;
ofpbuf_init(&ofpacts, 32);
str_to_inst_ofpacts(&fm->match.flow, act_str, &ofpacts);
str_to_inst_ofpacts(act_str, &ofpacts);
fm->ofpacts_len = ofpacts.size;
fm->ofpacts = ofpbuf_steal_data(&ofpacts);
@@ -1096,7 +1096,7 @@ void
parse_ofpacts(const char *s_, struct ofpbuf *ofpacts)
{
char *s = xstrdup(s_);
str_to_ofpacts(NULL, s, ofpacts);
str_to_ofpacts(s, ofpacts);
free(s);
}

12
tests/learn.at
View File

@@ -50,12 +50,14 @@ AT_CLEANUP
AT_SETUP([learning action - invalid prerequisites])
AT_CHECK([[ovs-ofctl parse-flow 'actions=learn(load:5->NXM_OF_IP_DST[])']],
[1], [],
[[ovs-ofctl: load:5->NXM_OF_IP_DST[]: cannot specify destination field ip_dst because prerequisites are not satisfied
]])
[1], [], [stderr])
AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0],
[[destination field ip_dst lacks correct prerequisites
]], [[]])
AT_CHECK([[ovs-ofctl parse-flow 'actions=learn(load:NXM_OF_IP_DST[]->NXM_NX_REG1[])']],
[1], [],
[[ovs-ofctl: load:NXM_OF_IP_DST[]->NXM_NX_REG1[]: cannot specify source field ip_dst because prerequisites are not satisfied
[1], [], [stderr])
AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0],
[[source field ip_dst lacks correct prerequisites
]])
AT_CLEANUP