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

Jarno Rajahalme jrajahalme at nicira.com
Fri Nov 15 23:21:36 UTC 2013


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