Thanks Jarno and Simon, I'll apply this soon.
On Fri, Nov 15, 2013 at 03:21:36PM -0800, Jarno Rajahalme wrote:
> I like this, especially the fact that you got rid of the duplicate calls to
> of pacts_check()
>
> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
> On Nov 15, 2013, at 2:33 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> > Until now ofpacts_check() has been told either to enforce consistency or
> > not, but that means that the caller has to know exactly what protocol is
> > going to be in use (because some protocols require consistency to be
> > enforced and others don't). This commit changes ofpacts_check() to just
> > rule out protocols that require enforcement when it detects
> > inconsistencies.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > lib/ofp-actions.c | 78
> > +++++++++++++++++++++++++++++++++--------------
> > lib/ofp-actions.h | 10 ++++--
> > lib/ofp-parse.c | 48 ++++++++++-------------------
> > lib/ofp-parse.h | 12 +++-----
> > lib/ofp-util.c | 6 ++--
> > ofproto/ofproto-dpif.c | 20 ++++++++----
> > tests/learn.at | 2 --
> > tests/ofp-actions.at | 2 +-
> > tests/ovs-ofctl.at | 2 +-
> > tests/test-controller.c | 2 +-
> > utilities/ovs-ofctl.c | 30 +++++++-----------
> > 11 files changed, 113 insertions(+), 99 deletions(-)
> >
> > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> > index 4558669..e07ea1d 100644
> > --- a/lib/ofp-actions.c
> > +++ b/lib/ofp-actions.c
> > @@ -1867,14 +1867,26 @@ ofpact_check_output_port(ofp_port_t port,
> > ofp_port_t max_ports)
> > }
> > }
> >
> > +/* Removes the protocols that require consistency between match and actions
> > + * (that's everything but OpenFlow 1.0) from '*usable_protocols'.
> > + *
> > + * (An example of an inconsistency between match and actions is a flow that
> > + * does not match on an MPLS Ethertype but has an action that pops an MPLS
> > + * label.) */
> > +static void
> > +inconsistent_match(enum ofputil_protocol *usable_protocols)
> > +{
> > + *usable_protocols &= OFPUTIL_P_OF10_ANY;
> > +}
> > +
> > /* May modify flow->dl_type, flow->nw_proto and flow->vlan_tci,
> > * caller must restore them.
> > *
> > * Modifies some actions, filling in fields that could not be properly set
> > * without context. */
> > static enum ofperr
> > -ofpact_check__(struct ofpact *a, struct flow *flow,
> > - bool enforce_consistency, ofp_port_t max_ports,
> > +ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
> > + struct flow *flow, ofp_port_t max_ports,
> > uint8_t table_id, uint8_t n_tables)
> > {
> > const struct ofpact_enqueue *enqueue;
> > @@ -1910,7 +1922,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > (flow->vlan_tci & htons(VLAN_CFI)) == htons(VLAN_CFI);
> > if (!(flow->vlan_tci & htons(VLAN_CFI)) &&
> > !ofpact_get_SET_VLAN_VID(a)->push_vlan_if_needed) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > /* Temporary mark that we have a vlan tag. */
> > flow->vlan_tci |= htons(VLAN_CFI);
> > @@ -1923,7 +1935,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > (flow->vlan_tci & htons(VLAN_CFI)) == htons(VLAN_CFI);
> > if (!(flow->vlan_tci & htons(VLAN_CFI)) &&
> > !ofpact_get_SET_VLAN_PCP(a)->push_vlan_if_needed) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > /* Temporary mark that we have a vlan tag. */
> > flow->vlan_tci |= htons(VLAN_CFI);
> > @@ -1931,7 +1943,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> >
> > case OFPACT_STRIP_VLAN:
> > if (!(flow->vlan_tci & htons(VLAN_CFI))) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > /* Temporary mark that we have no vlan tag. */
> > flow->vlan_tci = htons(0);
> > @@ -1953,7 +1965,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > case OFPACT_SET_IPV4_SRC:
> > case OFPACT_SET_IPV4_DST:
> > if (flow->dl_type != htons(ETH_TYPE_IP)) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > return 0;
> >
> > @@ -1962,7 +1974,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > case OFPACT_SET_IP_TTL:
> > case OFPACT_DEC_TTL:
> > if (!is_ip_any(flow)) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > return 0;
> >
> > @@ -1970,7 +1982,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > if (!is_ip_any(flow) ||
> > (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
> > && flow->nw_proto != IPPROTO_SCTP)) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > /* Note on which transport protocol the port numbers are set.
> > * This allows this set action to be converted to an OF1.2 set field
> > @@ -1982,7 +1994,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > if (!is_ip_any(flow) ||
> > (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
> > && flow->nw_proto != IPPROTO_SCTP)) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > /* Note on which transport protocol the port numbers are set.
> > * This allows this set action to be converted to an OF1.2 set field
> > @@ -2027,7 +2039,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > case OFPACT_SET_MPLS_TTL:
> > case OFPACT_DEC_MPLS_TTL:
> > if (!eth_type_mpls(flow->dl_type)) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > return 0;
> >
> > @@ -2039,7 +2051,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> >
> > case OFPACT_FIN_TIMEOUT:
> > if (flow->nw_proto != IPPROTO_TCP) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > return 0;
> >
> > @@ -2064,7 +2076,7 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > case OFPACT_POP_MPLS:
> > flow->dl_type = ofpact_get_POP_MPLS(a)->ethertype;
> > if (!eth_type_mpls(flow->dl_type)) {
> > - goto inconsistent;
> > + inconsistent_match(usable_protocols);
> > }
> > return 0;
> >
> > @@ -2075,9 +2087,12 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > return 0;
> >
> > case OFPACT_WRITE_ACTIONS: {
> > + /* Use a temporary copy of 'usable_protocols' because we can't
> > check
> > + * consistency of an action set. */
> > struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a);
> > + enum ofputil_protocol p = *usable_protocols;
> > return ofpacts_check(on->actions, ofpact_nest_get_action_len(on),
> > - flow, false, max_ports, table_id, n_tables);
> > + flow, max_ports, table_id, n_tables, &p);
> > }
> >
> > case OFPACT_WRITE_METADATA:
> > @@ -2106,26 +2121,25 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> > default:
> > NOT_REACHED();
> > }
> > -
> > - inconsistent:
> > - if (enforce_consistency) {
> > - return OFPERR_OFPBAC_MATCH_INCONSISTENT;
> > - }
> > - return 0;
> > }
> >
> > /* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are
> > * appropriate for a packet with the prerequisites satisfied by 'flow' in a
> > * switch with no more than 'max_ports' ports.
> > *
> > + * If 'ofpacts' and 'flow' are inconsistent with one another, un-sets in
> > + * '*usable_protocols' the protocols that forbid the inconsistency. (An
> > + * example of an inconsistency between match and actions is a flow that
> > does
> > + * not match on an MPLS Ethertype but has an action that pops an MPLS
> > label.)
> > + *
> > * May annotate ofpacts with information gathered from the 'flow'.
> > *
> > * May temporarily modify 'flow', but restores the changes before
> > returning. */
> > enum ofperr
> > ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
> > - struct flow *flow, bool enforce_consistency,
> > - ofp_port_t max_ports,
> > - uint8_t table_id, uint8_t n_tables)
> > + struct flow *flow, ofp_port_t max_ports,
> > + uint8_t table_id, uint8_t n_tables,
> > + enum ofputil_protocol *usable_protocols)
> > {
> > struct ofpact *a;
> > ovs_be16 dl_type = flow->dl_type;
> > @@ -2134,7 +2148,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t
> > ofpacts_len,
> > enum ofperr error = 0;
> >
> > OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
> > - error = ofpact_check__(a, flow, enforce_consistency,
> > + error = ofpact_check__(usable_protocols, a, flow,
> > max_ports, table_id, n_tables);
> > if (error) {
> > break;
> > @@ -2147,6 +2161,24 @@ ofpacts_check(struct ofpact ofpacts[], size_t
> > ofpacts_len,
> > return error;
> > }
> >
> > +/* Like ofpacts_check(), but reports inconsistencies as
> > + * OFPERR_OFPBAC_MATCH_INCONSISTENT rather than clearing bits. */
> > +enum ofperr
> > +ofpacts_check_consistency(struct ofpact ofpacts[], size_t ofpacts_len,
> > + struct flow *flow, ofp_port_t max_ports,
> > + uint8_t table_id, uint8_t n_tables,
> > + enum ofputil_protocol usable_protocols)
> > +{
> > + enum ofputil_protocol p = usable_protocols;
> > + enum ofperr error;
> > +
> > + error = ofpacts_check(ofpacts, ofpacts_len, flow, max_ports,
> > + table_id, n_tables, &p);
> > + return (error ? error
> > + : p != usable_protocols ? OFPERR_OFPBAC_MATCH_INCONSISTENT
> > + : 0);
> > +}
> > +
> > /* Verifies that the 'ofpacts_len' bytes of actions in 'ofpacts' are
> > * in the appropriate order as defined by the OpenFlow spec. */
> > enum ofperr
> > diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
> > index 70ad4b6..470a371 100644
> > --- a/lib/ofp-actions.h
> > +++ b/lib/ofp-actions.h
> > @@ -607,9 +607,13 @@ enum ofperr ofpacts_pull_openflow_instructions(struct
> > ofpbuf *openflow,
> > enum ofp_version version,
> > struct ofpbuf *ofpacts);
> > enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len,
> > - struct flow *, bool enforce_consistency,
> > - ofp_port_t max_ports,
> > - uint8_t table_id, uint8_t n_tables);
> > + struct flow *, ofp_port_t max_ports,
> > + uint8_t table_id, uint8_t n_tables,
> > + enum ofputil_protocol *usable_protocols);
> > +enum ofperr ofpacts_check_consistency(struct ofpact[], size_t ofpacts_len,
> > + struct flow *, ofp_port_t max_ports,
> > + uint8_t table_id, uint8_t n_tables,
> > + enum ofputil_protocol
> > usable_protocols);
> > enum ofperr ofpacts_verify(const struct ofpact ofpacts[], size_t
> > ofpacts_len);
> > enum ofperr ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports);
> >
> > diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> > index 8698030..5b07d14 100644
> > --- a/lib/ofp-parse.c
> > +++ b/lib/ofp-parse.c
> > @@ -1205,8 +1205,7 @@ parse_field(const struct mf_field *mf, const char *s,
> > struct match *match,
> >
> > static char * WARN_UNUSED_RESULT
> > parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > {
> > enum {
> > F_OUT_PORT = 1 << 0,
> > @@ -1414,23 +1413,15 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int
> > command, char *string,
> > enum ofperr err;
> >
> > err = ofpacts_check(ofpacts.data, ofpacts.size, &fm->match.flow,
> > - true, OFPP_MAX, fm->table_id, 255);
> > + OFPP_MAX, fm->table_id, 255,
> > usable_protocols);
> > + if (!err && !usable_protocols) {
> > + err = OFPERR_OFPBAC_MATCH_INCONSISTENT;
> > + }
> > if (err) {
> > - if (!enforce_consistency &&
> > - err == OFPERR_OFPBAC_MATCH_INCONSISTENT) {
> > - /* Allow inconsistent actions to be used with OF 1.0.
> > */
> > - *usable_protocols &= OFPUTIL_P_OF10_ANY;
> > - /* Try again, allowing for inconsistency.
> > - * XXX: As a side effect, logging may be duplicated. */
> > - err = ofpacts_check(ofpacts.data, ofpacts.size,
> > - &fm->match.flow, false,
> > - OFPP_MAX, fm->table_id, 255);
> > - }
> > - if (err) {
> > - error = xasprintf("actions are invalid with specified
> > match "
> > - "(%s)", ofperr_to_string(err));
> > - }
> > + error = xasprintf("actions are invalid with specified
> > match "
> > + "(%s)", ofperr_to_string(err));
> > }
> > +
> > }
> > if (error) {
> > ofpbuf_uninit(&ofpacts);
> > @@ -1459,14 +1450,12 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int
> > command, char *string,
> > * error. The caller is responsible for freeing the returned string. */
> > char * WARN_UNUSED_RESULT
> > parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > {
> > char *string = xstrdup(str_);
> > char *error;
> >
> > - error = parse_ofp_str__(fm, command, string, usable_protocols,
> > - enforce_consistency);
> > + error = parse_ofp_str__(fm, command, string, usable_protocols);
> > if (error) {
> > fm->ofpacts = NULL;
> > fm->ofpacts_len = 0;
> > @@ -1813,11 +1802,9 @@ parse_ofpacts(const char *s_, struct ofpbuf *ofpacts,
> > char * WARN_UNUSED_RESULT
> > parse_ofp_flow_mod_str(struct ofputil_flow_mod *fm, const char *string,
> > uint16_t command,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > {
> > - char *error = parse_ofp_str(fm, command, string, usable_protocols,
> > - enforce_consistency);
> > + char *error = parse_ofp_str(fm, command, string, usable_protocols);
> > if (!error) {
> > /* Normalize a copy of the match. This ensures that non-normalized
> > * flows get logged but doesn't affect what gets sent to the
> > switch, so
> > @@ -1879,8 +1866,7 @@ parse_ofp_table_mod(struct ofputil_table_mod *tm,
> > const char *table_id,
> > char * WARN_UNUSED_RESULT
> > parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
> > struct ofputil_flow_mod **fms, size_t *n_fms,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > {
> > size_t allocated_fms;
> > int line_number;
> > @@ -1909,7 +1895,7 @@ parse_ofp_flow_mod_file(const char *file_name,
> > uint16_t command,
> > *fms = x2nrealloc(*fms, &allocated_fms, sizeof **fms);
> > }
> > error = parse_ofp_flow_mod_str(&(*fms)[*n_fms], ds_cstr(&s),
> > command,
> > - &usable, enforce_consistency);
> > + &usable);
> > if (error) {
> > size_t i;
> >
> > @@ -1941,14 +1927,12 @@ parse_ofp_flow_mod_file(const char *file_name,
> > uint16_t command,
> > char * WARN_UNUSED_RESULT
> > parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request *fsr,
> > bool aggregate, const char *string,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > {
> > struct ofputil_flow_mod fm;
> > char *error;
> >
> > - error = parse_ofp_str(&fm, -1, string, usable_protocols,
> > - enforce_consistency);
> > + error = parse_ofp_str(&fm, -1, string, usable_protocols);
> > if (error) {
> > return error;
> > }
> > diff --git a/lib/ofp-parse.h b/lib/ofp-parse.h
> > index 58a3e87..515ccd7 100644
> > --- a/lib/ofp-parse.h
> > +++ b/lib/ofp-parse.h
> > @@ -36,14 +36,12 @@ struct simap;
> > enum ofputil_protocol;
> >
> > char *parse_ofp_str(struct ofputil_flow_mod *, int command, const char
> > *str_,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > WARN_UNUSED_RESULT;
> >
> > char *parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string,
> > uint16_t command,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > WARN_UNUSED_RESULT;
> >
> > char *parse_ofp_table_mod(struct ofputil_table_mod *,
> > @@ -53,14 +51,12 @@ char *parse_ofp_table_mod(struct ofputil_table_mod *,
> >
> > char *parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
> > struct ofputil_flow_mod **fms, size_t *n_fms,
> > - enum ofputil_protocol *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol *usable_protocols)
> > WARN_UNUSED_RESULT;
> >
> > char *parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request *,
> > bool aggregate, const char *string,
> > - enum ofputil_protocol
> > *usable_protocols,
> > - bool enforce_consistency)
> > + enum ofputil_protocol
> > *usable_protocols)
> > WARN_UNUSED_RESULT;
> >
> > char *parse_ofpacts(const char *, struct ofpbuf *ofpacts,
> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > index ede37b0..32be2ae 100644
> > --- a/lib/ofp-util.c
> > +++ b/lib/ofp-util.c
> > @@ -1672,9 +1672,9 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
> > : OFPERR_OFPFMFC_TABLE_FULL);
> > }
> >
> > - return ofpacts_check(fm->ofpacts, fm->ofpacts_len, &fm->match.flow,
> > - oh->version > OFP10_VERSION, max_port,
> > - fm->table_id, max_table);
> > + return ofpacts_check_consistency(fm->ofpacts, fm->ofpacts_len,
> > + &fm->match.flow, max_port,
> > + fm->table_id, max_table, protocol);
> > }
> >
> > static enum ofperr
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 3391594..e0b3c70 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -5460,13 +5460,14 @@ ofproto_unixctl_trace_actions(struct unixctl_conn
> > *conn, int argc,
> > }
> >
> > /* OpenFlow 1.1 and later suggest that the switch enforces certain
> > forms of
> > - * consistency between the flow and the actions, so enforce these by
> > - * default if the actions can only work in OF1.1 or later. */
> > - enforce_consistency = !(usable_protocols & OFPUTIL_P_OF10_ANY);
> > + * consistency between the flow and the actions. With -consistent, we
> > + * enforce consistency even for a flow supported in OpenFlow 1.0. */
> > if (!strcmp(argv[1], "-consistent")) {
> > enforce_consistency = true;
> > argv++;
> > argc--;
> > + } else {
> > + enforce_consistency = false;
> > }
> >
> > error = parse_flow_and_packet(argc, argv, &ofproto, &flow, &packet);
> > @@ -5490,9 +5491,16 @@ ofproto_unixctl_trace_actions(struct unixctl_conn
> > *conn, int argc,
> > unixctl_command_reply_error(conn, "invalid in_port");
> > goto exit;
> > }
> > - retval = ofpacts_check(ofpacts.data, ofpacts.size, &flow,
> > - enforce_consistency,
> > - u16_to_ofp(ofproto->up.max_ports), 0, 0);
> > + if (enforce_consistency) {
> > + retval = ofpacts_check_consistency(ofpacts.data, ofpacts.size,
> > &flow,
> > +
> > u16_to_ofp(ofproto->up.max_ports),
> > + 0, 0, usable_protocols);
> > + } else {
> > + retval = ofpacts_check(ofpacts.data, ofpacts.size, &flow,
> > + u16_to_ofp(ofproto->up.max_ports), 0, 0,
> > + &usable_protocols);
> > + }
> > +
> > if (retval) {
> > ds_clear(&result);
> > ds_put_format(&result, "Bad actions: %s", ofperr_to_string(retval));
> > diff --git a/tests/learn.at b/tests/learn.at
> > index 491cd5e..66343d3 100644
> > --- a/tests/learn.at
> > +++ b/tests/learn.at
> > @@ -75,14 +75,12 @@ AT_CHECK([[ovs-ofctl parse-flow
> > 'actions=learn(load:5->NXM_OF_IP_DST[])']],
> > [1], [], [stderr])
> > AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0],
> > [[destination field ip_dst lacks correct prerequisites
> > -destination field ip_dst lacks correct prerequisites
> > ovs-ofctl: actions are invalid with specified match
> > (OFPBAC_MATCH_INCONSISTENT)
> > ]], [[]])
> > AT_CHECK([[ovs-ofctl parse-flow
> > 'actions=learn(load:NXM_OF_IP_DST[]->NXM_NX_REG1[])']],
> > [1], [], [stderr])
> > AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0],
> > [[source field ip_dst lacks correct prerequisites
> > -source field ip_dst lacks correct prerequisites
> > ovs-ofctl: actions are invalid with specified match
> > (OFPBAC_MATCH_INCONSISTENT)
> > ]])
> > AT_CLEANUP
> > diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
> > index 08ebccf..452bdbf 100644
> > --- a/tests/ofp-actions.at
> > +++ b/tests/ofp-actions.at
> > @@ -485,7 +485,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0
> > 'tcp actions=fin_timeout(i
> > dnl Bad: Use fin_timeout action on TCP flow that has been converted to MPLS
> > AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp
> > actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'],
> > [1], [], [dnl
> > -ovs-ofctl: actions are invalid with specified match
> > (OFPBAC_MATCH_INCONSISTENT)
> > +ovs-ofctl: none of the usable flow formats (OpenFlow10,NXM) is among the
> > allowed flow formats (OpenFlow11)
> > ])
> > OVS_VSWITCHD_STOP
> > AT_CLEANUP
> > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> > index a99a161..3434592 100644
> > --- a/tests/ovs-ofctl.at
> > +++ b/tests/ovs-ofctl.at
> > @@ -285,7 +285,7 @@ AT_CLEANUP
> >
> > AT_SETUP([ovs-ofctl action inconsistency (OpenFlow 1.1)])
> > AT_CHECK([ovs-ofctl --protocols OpenFlow11 add-flow br0 'ip
> > actions=mod_tp_dst:1234'
> > -], [1], [stdout], [ovs-ofctl: actions are invalid with specified match
> > (OFPBAC_MATCH_INCONSISTENT)
> > +], [1], [stdout], [ovs-ofctl: none of the usable flow formats
> > (OpenFlow10,NXM) is among the allowed flow formats (OpenFlow11)
> > ])
> > AT_CLEANUP
> >
> > diff --git a/tests/test-controller.c b/tests/test-controller.c
> > index 9596ad4..f487d8c 100644
> > --- a/tests/test-controller.c
> > +++ b/tests/test-controller.c
> > @@ -332,7 +332,7 @@ parse_options(int argc, char *argv[])
> > case OPT_WITH_FLOWS:
> > error = parse_ofp_flow_mod_file(optarg, OFPFC_ADD,
> > &default_flows,
> > &n_default_flows,
> > - &usable_protocols, false);
> > + &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index a0dc5c8..2bed96f 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -890,9 +890,7 @@ prepare_dump_flows(int argc, char *argv[], bool
> > aggregate,
> >
> > error = parse_ofp_flow_stats_request_str(&fsr, aggregate,
> > argc > 2 ? argv[2] : "",
> > - &usable_protocols,
> > - !(allowed_protocols
> > - & OFPUTIL_P_OF10_ANY));
> > + &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > @@ -1134,8 +1132,7 @@ ofctl_flow_mod_file(int argc OVS_UNUSED, char
> > *argv[], uint16_t command)
> > char *error;
> >
> > error = parse_ofp_flow_mod_file(argv[2], command, &fms, &n_fms,
> > - &usable_protocols,
> > - !(allowed_protocols &
> > OFPUTIL_P_OF10_ANY));
> > + &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > @@ -1154,9 +1151,7 @@ ofctl_flow_mod(int argc, char *argv[], uint16_t
> > command)
> > enum ofputil_protocol usable_protocols;
> >
> > error = parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "",
> > command,
> > - &usable_protocols,
> > - !(allowed_protocols
> > - & OFPUTIL_P_OF10_ANY));
> > + &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > @@ -2231,8 +2226,7 @@ read_flows_from_file(const char *filename, struct
> > classifier *cls, int index)
> > char *error;
> > enum ofputil_protocol usable;
> >
> > - error = parse_ofp_str(&fm, OFPFC_ADD, ds_cstr(&s), &usable,
> > - !(allowed_protocols & OFPUTIL_P_OF10_ANY));
> > + error = parse_ofp_str(&fm, OFPFC_ADD, ds_cstr(&s), &usable);
> > if (error) {
> > ovs_fatal(0, "%s:%d: %s", filename, line_number, error);
> > }
> > @@ -2651,8 +2645,7 @@ ofctl_parse_flow(int argc OVS_UNUSED, char *argv[])
> > struct ofputil_flow_mod fm;
> > char *error;
> >
> > - error = parse_ofp_flow_mod_str(&fm, argv[1], OFPFC_ADD,
> > &usable_protocols,
> > - !(allowed_protocols &
> > OFPUTIL_P_OF10_ANY));
> > + error = parse_ofp_flow_mod_str(&fm, argv[1], OFPFC_ADD,
> > &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > @@ -2670,8 +2663,7 @@ ofctl_parse_flows(int argc OVS_UNUSED, char *argv[])
> > char *error;
> >
> > error = parse_ofp_flow_mod_file(argv[1], OFPFC_ADD, &fms, &n_fms,
> > - &usable_protocols,
> > - !(allowed_protocols &
> > OFPUTIL_P_OF10_ANY));
> > + &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > @@ -3077,9 +3069,10 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED,
> > char *argv[] OVS_UNUSED)
> > /* Verify actions, enforce consistency. */
> > struct flow flow;
> > memset(&flow, 0, sizeof flow);
> > - error = ofpacts_check(ofpacts.data, ofpacts.size, &flow,
> > - true, OFPP_MAX,
> > - table_id ? atoi(table_id) : 0, 255);
> > + error = ofpacts_check_consistency(ofpacts.data, ofpacts.size,
> > + &flow, OFPP_MAX,
> > + table_id ? atoi(table_id) :
> > 0,
> > + 255, OFPUTIL_P_OF11_STD);
> > }
> > if (error) {
> > printf("bad OF1.1 instructions: %s\n\n",
> > ofperr_get_name(error));
> > @@ -3177,8 +3170,7 @@ ofctl_check_vlan(int argc OVS_UNUSED, char *argv[])
> > string_s = match_to_string(&match, OFP_DEFAULT_PRIORITY);
> > printf("%s -> ", string_s);
> > fflush(stdout);
> > - error_s = parse_ofp_str(&fm, -1, string_s, &usable_protocols,
> > - !(allowed_protocols & OFPUTIL_P_OF10_ANY));
> > + error_s = parse_ofp_str(&fm, -1, string_s, &usable_protocols);
> > if (error_s) {
> > ovs_fatal(0, "%s", error_s);
> > }
> > --
> > 1.7.10.4
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
