[ovs-dev] [PATCH 1/2] ofp-actions: Make ofpacts_check() report consistency for all protocols.

Ben Pfaff blp at nicira.com
Wed Nov 20 17:37:36 UTC 2013


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 <jrajahalme at nicira.com>
> 
> On Nov 15, 2013, at 2:33 PM, Ben Pfaff <blp at 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 <blp at 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 at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 



More information about the dev mailing list