On 28 February 2017 at 17:17, Jarno Rajahalme <[email protected]> wrote:
> Supply the match mask to prerequisities checking when available. This
> allows checking for zero-valued matches. Non-zero valued matches
> imply the presense of corresponding mask bits, but for zero valued
> matches we must explicitly check the mask, too.
>
> This is required now only for conntrack validity checking due to the
> conntrack state having and 'invalid' bit, but not 'valid' bit. One
> way to match an valid conntrack state is to match on the 'tracked' bit
> being one and 'invalid' bit being zero. The latter requires the
> corresponding mask bit be verified.
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
> ---
> include/openvswitch/meta-flow.h | 5 ++--
> include/openvswitch/ofp-actions.h | 4 ++--
> lib/bundle.c | 4 ++--
> lib/bundle.h | 3 ++-
> lib/learn.c | 15 ++++++------
> lib/learn.h | 3 ++-
> lib/meta-flow.c | 38 ++++++++++++++++++++++-------
> lib/multipath.c | 4 ++--
> lib/multipath.h | 3 ++-
> lib/nx-match.c | 17 ++++++-------
> lib/nx-match.h | 6 ++---
> lib/ofp-actions.c | 50
> ++++++++++++++++++++-------------------
> lib/ofp-parse.c | 2 +-
> lib/ofp-util.c | 2 +-
> ofproto/ofproto-dpif-trace.c | 13 +++++-----
> ofproto/ofproto.c | 5 ++--
> utilities/ovs-ofctl.c | 6 ++---
> 17 files changed, 105 insertions(+), 75 deletions(-)
>
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
> index 83e2599..aac9945 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -1898,6 +1898,7 @@ void mf_get_mask(const struct mf_field *, const struct
> flow_wildcards *,
> /* Prerequisites. */
> bool mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow,
> struct flow_wildcards *wc);
> +bool mf_are_match_prereqs_ok(const struct mf_field *, const struct match *);
>
> static inline bool
> mf_is_l3_or_higher(const struct mf_field *mf)
> @@ -1959,8 +1960,8 @@ void mf_subfield_swap(const struct mf_subfield *,
> const struct mf_subfield *,
> struct flow *flow, struct flow_wildcards *);
>
> -enum ofperr mf_check_src(const struct mf_subfield *, const struct flow *);
> -enum ofperr mf_check_dst(const struct mf_subfield *, const struct flow *);
> +enum ofperr mf_check_src(const struct mf_subfield *, const struct match *);
> +enum ofperr mf_check_dst(const struct mf_subfield *, const struct match *);
>
> /* Parsing and formatting. */
> char *mf_parse(const struct mf_field *, const char *,
I assume that the above is OK from a library standpoint because we are
not currently guaranteeing that APIs remain stable between release
versions, only within the minor revisions of a particular release. IE,
library users compiling against libopenvswitch from 2.7 will need to
change their code to compile against libopenvswitch from 2.8.
> diff --git a/include/openvswitch/ofp-actions.h
> b/include/openvswitch/ofp-actions.h
> index 88f573d..53d6b44 100644
> --- a/include/openvswitch/ofp-actions.h
> +++ b/include/openvswitch/ofp-actions.h
> @@ -954,11 +954,11 @@ ofpacts_pull_openflow_instructions(struct ofpbuf
> *openflow,
> const struct vl_mff_map *vl_mff_map,
> struct ofpbuf *ofpacts);
> enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len,
> - struct flow *, ofp_port_t max_ports,
> + struct match *, 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,
> + struct match *, ofp_port_t max_ports,
> uint8_t table_id, uint8_t n_tables,
> enum ofputil_protocol
> usable_protocols);
> enum ofperr ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports);
> diff --git a/lib/bundle.c b/lib/bundle.c
> index 70a743b..620318e 100644
> --- a/lib/bundle.c
> +++ b/lib/bundle.c
> @@ -105,13 +105,13 @@ bundle_execute(const struct ofpact_bundle *bundle,
>
> enum ofperr
> bundle_check(const struct ofpact_bundle *bundle, ofp_port_t max_ports,
> - const struct flow *flow)
> + const struct match *match)
> {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> size_t i;
>
> if (bundle->dst.field) {
> - enum ofperr error = mf_check_dst(&bundle->dst, flow);
> + enum ofperr error = mf_check_dst(&bundle->dst, match);
> if (error) {
> return error;
> }
> diff --git a/lib/bundle.h b/lib/bundle.h
> index f5ce321..48b9b79 100644
> --- a/lib/bundle.h
> +++ b/lib/bundle.h
> @@ -29,6 +29,7 @@
> struct ds;
> struct flow;
> struct flow_wildcards;
> +struct match;
> struct ofpact_bundle;
> struct ofpbuf;
>
> @@ -43,7 +44,7 @@ ofp_port_t bundle_execute(const struct ofpact_bundle *,
> const struct flow *,
> bool (*slave_enabled)(ofp_port_t ofp_port, void
> *aux),
> void *aux);
> enum ofperr bundle_check(const struct ofpact_bundle *, ofp_port_t max_ports,
> - const struct flow *);
> + const struct match *);
> char *bundle_parse(const char *, struct ofpbuf *ofpacts)
> OVS_WARN_UNUSED_RESULT;
> char *bundle_parse_load(const char *, struct ofpbuf *ofpacts)
> OVS_WARN_UNUSED_RESULT;
> diff --git a/lib/learn.c b/lib/learn.c
> index ce52c35..1990849 100644
> --- a/lib/learn.c
> +++ b/lib/learn.c
> @@ -35,18 +35,18 @@
> /* Checks that 'learn' is a valid action on 'flow'. Returns 0 if it is
> valid,
> * otherwise an OFPERR_*. */
> enum ofperr
> -learn_check(const struct ofpact_learn *learn, const struct flow *flow)
> +learn_check(const struct ofpact_learn *learn, const struct match *src_match)
> {
> const struct ofpact_learn_spec *spec;
> - struct match match;
> + struct match dst_match;
>
> - match_init_catchall(&match);
> + match_init_catchall(&dst_match);
> OFPACT_LEARN_SPEC_FOR_EACH (spec, learn) {
> enum ofperr error;
>
> /* Check the source. */
> if (spec->src_type == NX_LEARN_SRC_FIELD) {
> - error = mf_check_src(&spec->src, flow);
> + error = mf_check_src(&spec->src, src_match);
> if (error) {
> return error;
> }
> @@ -55,18 +55,19 @@ learn_check(const struct ofpact_learn *learn, const
> struct flow *flow)
> /* Check the destination. */
> switch (spec->dst_type) {
> case NX_LEARN_DST_MATCH:
> - error = mf_check_src(&spec->dst, &match.flow);
> + error = mf_check_src(&spec->dst, &dst_match);
> if (error) {
> return error;
> }
> if (spec->src_type & NX_LEARN_SRC_IMMEDIATE) {
> mf_write_subfield_value(&spec->dst,
> - ofpact_learn_spec_imm(spec), &match);
> + ofpact_learn_spec_imm(spec),
> + &dst_match);
> }
> break;
>
> case NX_LEARN_DST_LOAD:
> - error = mf_check_dst(&spec->dst, &match.flow);
> + error = mf_check_dst(&spec->dst, &dst_match);
> if (error) {
> return error;
> }
> diff --git a/lib/learn.h b/lib/learn.h
> index adbbdf3..9dd41d3 100644
> --- a/lib/learn.h
> +++ b/lib/learn.h
> @@ -23,6 +23,7 @@
> struct ds;
> struct flow;
> struct flow_wildcards;
> +struct match;
> struct ofpbuf;
> struct ofpact_learn;
> struct ofputil_flow_mod;
> @@ -33,7 +34,7 @@ struct nx_action_learn;
> * See lib/ofp-actions.c for NXAST_LEARN specification.
> */
>
> -enum ofperr learn_check(const struct ofpact_learn *, const struct flow *);
> +enum ofperr learn_check(const struct ofpact_learn *, const struct match *);
> void learn_execute(const struct ofpact_learn *, const struct flow *,
> struct ofputil_flow_mod *, struct ofpbuf *ofpacts);
> void learn_mask(const struct ofpact_learn *, struct flow_wildcards *);
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index 40704e6..9d635a3 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -379,10 +379,12 @@ mf_is_mask_valid(const struct mf_field *mf, const union
> mf_value *mask)
> }
>
> /* Returns true if 'flow' meets the prerequisites for 'mf', false otherwise.
> + * If a non-NULL 'mask' is passed, zero-valued matches can also be verified.
> * Sets inspected bits in 'wc', if non-NULL. */
> -bool
> -mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow,
> - struct flow_wildcards *wc)
> +static bool
> +mf_are_prereqs_ok__(const struct mf_field *mf, const struct flow *flow,
> + const struct flow_wildcards *mask OVS_UNUSED,
> + struct flow_wildcards *wc)
> {
> switch (mf->prereqs) {
> case MFP_NONE:
> @@ -401,6 +403,7 @@ mf_are_prereqs_ok(const struct mf_field *mf, const struct
> flow *flow,
> case MFP_IP_ANY:
> return is_ip_any(flow);
> case MFP_TCP:
> + /* Matching !FRAG_LATER is not enforced (mask is not checked). */
> return is_tcp(flow, wc) && !(flow->nw_frag & FLOW_NW_FRAG_LATER);
> case MFP_UDP:
> return is_udp(flow, wc) && !(flow->nw_frag & FLOW_NW_FRAG_LATER);
> @@ -421,6 +424,23 @@ mf_are_prereqs_ok(const struct mf_field *mf, const
> struct flow *flow,
> OVS_NOT_REACHED();
> }
>
> +/* Returns true if 'flow' meets the prerequisites for 'mf', false otherwise.
> + * Sets inspected bits in 'wc', if non-NULL. */
> +bool
> +mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow,
> + struct flow_wildcards *wc)
> +{
> + return mf_are_prereqs_ok__(mf, flow, NULL, wc);
> +}
> +
> +/* Returns true if 'match' meets the prerequisites for 'mf', false otherwise.
> + */
> +bool
> +mf_are_match_prereqs_ok(const struct mf_field *mf, const struct match *match)
> +{
> + return mf_are_prereqs_ok__(mf, &match->flow, &match->wc, NULL);
> +}
> +
> /* Returns true if 'value' may be a valid value *as part of a masked match*,
> * false otherwise.
> *
> @@ -1948,7 +1968,7 @@ mf_set(const struct mf_field *mf,
> }
>
> static enum ofperr
> -mf_check__(const struct mf_subfield *sf, const struct flow *flow,
> +mf_check__(const struct mf_subfield *sf, const struct match *match,
> const char *type)
> {
> if (!sf->field) {
> @@ -1966,7 +1986,7 @@ mf_check__(const struct mf_subfield *sf, const struct
> flow *flow,
> "of %s field %s", sf->ofs, sf->n_bits,
> sf->field->n_bits, type, sf->field->name);
> return OFPERR_OFPBAC_BAD_SET_LEN;
> - } else if (flow && !mf_are_prereqs_ok(sf->field, flow, NULL)) {
> + } else if (match && !mf_are_match_prereqs_ok(sf->field, match)) {
> VLOG_WARN_RL(&rl, "%s field %s lacks correct prerequisites",
> type, sf->field->name);
> return OFPERR_OFPBAC_MATCH_INCONSISTENT;
> @@ -2053,18 +2073,18 @@ mf_subfield_swap(const struct mf_subfield *a,
> * 0 if so, otherwise an OpenFlow error code (e.g. as returned by
> * ofp_mkerr()). */
> enum ofperr
> -mf_check_src(const struct mf_subfield *sf, const struct flow *flow)
> +mf_check_src(const struct mf_subfield *sf, const struct match *match)
> {
> - return mf_check__(sf, flow, "source");
> + return mf_check__(sf, match, "source");
> }
>
> /* Checks whether 'sf' is valid for writing a subfield into 'flow'. Returns > 0
> * if so, otherwise an OpenFlow error code (e.g. as returned by
> * ofp_mkerr()). */
> enum ofperr
> -mf_check_dst(const struct mf_subfield *sf, const struct flow *flow)
> +mf_check_dst(const struct mf_subfield *sf, const struct match *match)
> {
> - int error = mf_check__(sf, flow, "destination");
> + int error = mf_check__(sf, match, "destination");
> if (!error && !sf->field->writable) {
> VLOG_WARN_RL(&rl, "destination field %s is not writable",
> sf->field->name);
> diff --git a/lib/multipath.c b/lib/multipath.c
> index 8a1d1fa..e855a88 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -35,9 +35,9 @@
> * OFPERR_*. */
> enum ofperr
> multipath_check(const struct ofpact_multipath *mp,
> - const struct flow *flow)
> + const struct match *match)
> {
> - return mf_check_dst(&mp->dst, flow);
> + return mf_check_dst(&mp->dst, match);
> }
>
> /* multipath_execute(). */
> diff --git a/lib/multipath.h b/lib/multipath.h
> index 6dd3785..4158302 100644
> --- a/lib/multipath.h
> +++ b/lib/multipath.h
> @@ -24,6 +24,7 @@
> struct ds;
> struct flow;
> struct flow_wildcards;
> +struct match;
> struct nx_action_multipath;
> struct ofpact_multipath;
> struct ofpbuf;
> @@ -31,7 +32,7 @@ struct ofpbuf;
> /* NXAST_MULTIPATH helper functions. */
>
> enum ofperr multipath_check(const struct ofpact_multipath *,
> - const struct flow *);
> + const struct match *);
>
> void multipath_execute(const struct ofpact_multipath *, struct flow *,
> struct flow_wildcards *);
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 91401e2..95516a1 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -539,7 +539,7 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len,
> bool strict,
> *cookie = value.be64;
> *cookie_mask = mask.be64;
> }
> - } else if (!mf_are_prereqs_ok(field, &match->flow, NULL)) {
> + } else if (!mf_are_match_prereqs_ok(field, match)) {
> error = OFPERR_OFPBMC_BAD_PREREQ;
> } else if (!mf_is_all_wild(field, &match->wc)) {
> error = OFPERR_OFPBMC_DUP_FIELD;
> @@ -1663,16 +1663,17 @@ nxm_format_reg_move(const struct ofpact_reg_move
> *move, struct ds *s)
>
>
> enum ofperr
> -nxm_reg_move_check(const struct ofpact_reg_move *move, const struct flow
> *flow)
> +nxm_reg_move_check(const struct ofpact_reg_move *move,
> + const struct match *match)
> {
> enum ofperr error;
>
> - error = mf_check_src(&move->src, flow);
> + error = mf_check_src(&move->src, match);
> if (error) {
> return error;
> }
>
> - return mf_check_dst(&move->dst, flow);
> + return mf_check_dst(&move->dst, match);
> }
>
> /* nxm_execute_reg_move(). */
> @@ -1734,16 +1735,16 @@ nxm_format_stack_pop(const struct ofpact_stack *pop,
> struct ds *s)
>
> enum ofperr
> nxm_stack_push_check(const struct ofpact_stack *push,
> - const struct flow *flow)
> + const struct match *match)
> {
> - return mf_check_src(&push->subfield, flow);
> + return mf_check_src(&push->subfield, match);
> }
>
> enum ofperr
> nxm_stack_pop_check(const struct ofpact_stack *pop,
> - const struct flow *flow)
> + const struct match *match)
> {
> - return mf_check_dst(&pop->subfield, flow);
> + return mf_check_dst(&pop->subfield, match);
> }
>
> /* nxm_execute_stack_push(), nxm_execute_stack_pop().
> diff --git a/lib/nx-match.h b/lib/nx-match.h
> index 5dca24a..631ab48 100644
> --- a/lib/nx-match.h
> +++ b/lib/nx-match.h
> @@ -116,7 +116,7 @@ char *nxm_parse_reg_move(struct ofpact_reg_move *, const
> char *)
> void nxm_format_reg_move(const struct ofpact_reg_move *, struct ds *);
>
> enum ofperr nxm_reg_move_check(const struct ofpact_reg_move *,
> - const struct flow *);
> + const struct match *);
>
> void nxm_reg_load(const struct mf_subfield *, uint64_t src_data,
> struct flow *, struct flow_wildcards *);
> @@ -128,9 +128,9 @@ void nxm_format_stack_push(const struct ofpact_stack *,
> struct ds *);
> void nxm_format_stack_pop(const struct ofpact_stack *, struct ds *);
>
> enum ofperr nxm_stack_push_check(const struct ofpact_stack *,
> - const struct flow *);
> + const struct match *);
> enum ofperr nxm_stack_pop_check(const struct ofpact_stack *,
> - const struct flow *);
> + const struct match *);
> void nx_stack_push(struct ofpbuf *stack, const void *v, uint8_t bytes);
> void nx_stack_push_bottom(struct ofpbuf *stack, const void *v, uint8_t
> bytes);
> void *nx_stack_pop(struct ofpbuf *stack, uint8_t *bytes);
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index ce80f57..2869e0f 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -7065,9 +7065,10 @@ inconsistent_match(enum ofputil_protocol
> *usable_protocols)
> * without context. */
> static enum ofperr
> ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
> - struct flow *flow, ofp_port_t max_ports,
> + struct match *match, ofp_port_t max_ports,
> uint8_t table_id, uint8_t n_tables)
> {
> + struct flow *flow = &match->flow;
> const struct ofpact_enqueue *enqueue;
> const struct mf_field *mf;
>
> @@ -7089,14 +7090,14 @@ ofpact_check__(enum ofputil_protocol
> *usable_protocols, struct ofpact *a,
> return 0;
>
> case OFPACT_OUTPUT_REG:
> - return mf_check_src(&ofpact_get_OUTPUT_REG(a)->src, flow);
> + return mf_check_src(&ofpact_get_OUTPUT_REG(a)->src, match);
>
> case OFPACT_OUTPUT_TRUNC:
> return ofpact_check_output_port(ofpact_get_OUTPUT_TRUNC(a)->port,
> max_ports);
>
> case OFPACT_BUNDLE:
> - return bundle_check(ofpact_get_BUNDLE(a), max_ports, flow);
> + return bundle_check(ofpact_get_BUNDLE(a), max_ports, match);
>
> case OFPACT_SET_VLAN_VID:
> /* Remember if we saw a vlan tag in the flow to aid translating to
> @@ -7179,7 +7180,7 @@ ofpact_check__(enum ofputil_protocol *usable_protocols,
> struct ofpact *a,
> return 0;
>
> case OFPACT_REG_MOVE:
> - return nxm_reg_move_check(ofpact_get_REG_MOVE(a), flow);
> + return nxm_reg_move_check(ofpact_get_REG_MOVE(a), match);
>
> case OFPACT_SET_FIELD:
> mf = ofpact_get_SET_FIELD(a)->field;
> @@ -7202,10 +7203,10 @@ ofpact_check__(enum ofputil_protocol
> *usable_protocols, struct ofpact *a,
> return 0;
>
> case OFPACT_STACK_PUSH:
> - return nxm_stack_push_check(ofpact_get_STACK_PUSH(a), flow);
> + return nxm_stack_push_check(ofpact_get_STACK_PUSH(a), match);
>
> case OFPACT_STACK_POP:
> - return nxm_stack_pop_check(ofpact_get_STACK_POP(a), flow);
> + return nxm_stack_pop_check(ofpact_get_STACK_POP(a), match);
>
> case OFPACT_SET_MPLS_LABEL:
> case OFPACT_SET_MPLS_TC:
> @@ -7229,13 +7230,13 @@ ofpact_check__(enum ofputil_protocol
> *usable_protocols, struct ofpact *a,
> return 0;
>
> case OFPACT_LEARN:
> - return learn_check(ofpact_get_LEARN(a), flow);
> + return learn_check(ofpact_get_LEARN(a), match);
>
> case OFPACT_CONJUNCTION:
> return 0;
>
> case OFPACT_MULTIPATH:
> - return multipath_check(ofpact_get_MULTIPATH(a), flow);
> + return multipath_check(ofpact_get_MULTIPATH(a), match);
>
> case OFPACT_NOTE:
> case OFPACT_EXIT:
> @@ -7262,7 +7263,7 @@ ofpact_check__(enum ofputil_protocol *usable_protocols,
> struct ofpact *a,
> case OFPACT_CLONE: {
> struct ofpact_nest *on = ofpact_get_CLONE(a);
> return ofpacts_check(on->actions, ofpact_nest_get_action_len(on),
> - flow, max_ports, table_id, n_tables,
> + match, max_ports, table_id, n_tables,
> usable_protocols);
> }
>
> @@ -7280,11 +7281,11 @@ ofpact_check__(enum ofputil_protocol
> *usable_protocols, struct ofpact *a,
> }
>
> if (oc->zone_src.field) {
> - return mf_check_src(&oc->zone_src, flow);
> + return mf_check_src(&oc->zone_src, match);
> }
>
> return ofpacts_check(oc->actions, ofpact_ct_get_action_len(oc),
> - flow, max_ports, table_id, n_tables,
> + match, max_ports, table_id, n_tables,
> usable_protocols);
> }
>
> @@ -7312,7 +7313,7 @@ ofpact_check__(enum ofputil_protocol *usable_protocols,
> struct ofpact *a,
> 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, max_ports, table_id, n_tables, &p);
> + match, max_ports, table_id, n_tables, &p);
> }
>
> case OFPACT_WRITE_METADATA:
> @@ -7360,32 +7361,33 @@ ofpact_check__(enum ofputil_protocol
> *usable_protocols, struct ofpact *a,
> * 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 annotate ofpacts with information gathered from the 'match'.
> *
> - * May temporarily modify 'flow', but restores the changes before returning.
> */
> + * May temporarily modify 'match', but restores the changes before
> + * returning. */
> enum ofperr
> ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
> - struct flow *flow, ofp_port_t max_ports,
> + struct match *match, 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;
> - ovs_be16 vlan_tci = flow->vlan_tci;
> - uint8_t nw_proto = flow->nw_proto;
> + ovs_be16 dl_type = match->flow.dl_type;
> + ovs_be16 vlan_tci = match->flow.vlan_tci;
> + uint8_t nw_proto = match->flow.nw_proto;
> enum ofperr error = 0;
>
> OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
> - error = ofpact_check__(usable_protocols, a, flow,
> + error = ofpact_check__(usable_protocols, a, match,
> max_ports, table_id, n_tables);
> if (error) {
> break;
> }
> }
> /* Restore fields that may have been modified. */
> - flow->dl_type = dl_type;
> - flow->vlan_tci = vlan_tci;
> - flow->nw_proto = nw_proto;
> + match->flow.dl_type = dl_type;
> + match->flow.vlan_tci = vlan_tci;
> + match->flow.nw_proto = nw_proto;
> return error;
> }
>
> @@ -7393,14 +7395,14 @@ ofpacts_check(struct ofpact ofpacts[], size_t
> ofpacts_len,
> * 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,
> + struct match *match, 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,
> + error = ofpacts_check(ofpacts, ofpacts_len, match, max_ports,
> table_id, n_tables, &p);
> return (error ? error
> : p != usable_protocols ? OFPERR_OFPBAC_MATCH_INCONSISTENT
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index a6b9fcf..7826bc5 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -546,7 +546,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command,
> char *string,
> if (!error) {
> enum ofperr err;
>
> - err = ofpacts_check(ofpacts.data, ofpacts.size, &fm->match.flow,
> + err = ofpacts_check(ofpacts.data, ofpacts.size, &fm->match,
> OFPP_MAX, fm->table_id, 255,
> usable_protocols);
> if (!err && !*usable_protocols) {
> err = OFPERR_OFPBAC_MATCH_INCONSISTENT;
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 0c9343e..7d40cbb 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -1751,7 +1751,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
> }
>
> return ofpacts_check_consistency(fm->ofpacts, fm->ofpacts_len,
> - &fm->match.flow, max_port,
> + &fm->match, max_port,
> fm->table_id, max_table, protocol);
> }
>
> diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
> index b01a131..a5cc089 100644
> --- a/ofproto/ofproto-dpif-trace.c
> +++ b/ofproto/ofproto-dpif-trace.c
> @@ -324,7 +324,7 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn,
> int argc,
> struct ofpbuf ofpacts;
> struct dp_packet *packet;
> struct ds result;
> - struct flow flow;
> + struct match match;
> uint16_t in_port;
>
> /* Three kinds of error return values! */
> @@ -354,12 +354,13 @@ ofproto_unixctl_trace_actions(struct unixctl_conn
> *conn, int argc,
> enforce_consistency = false;
> }
>
> - error = parse_flow_and_packet(argc, argv, &ofproto, &flow, &packet);
> + error = parse_flow_and_packet(argc, argv, &ofproto, &match.flow,
> &packet);
> if (error) {
> unixctl_command_reply_error(conn, error);
> free(error);
> goto exit;
> }
> + match_wc_init(&match, &match.flow);
Inside match_wc_init(), this ends up copying the entire flow from the
current memory into itself - should we avoid that?
>
> /* Do the same checks as handle_packet_out() in ofproto.c.
> *
> @@ -371,18 +372,18 @@ ofproto_unixctl_trace_actions(struct unixctl_conn
> *conn, int argc,
> *
> * We skip the "meter" check here because meter is an instruction, not an
> * action, and thus cannot appear in ofpacts. */
> - in_port = ofp_to_u16(flow.in_port.ofp_port);
> + in_port = ofp_to_u16(match.flow.in_port.ofp_port);
> if (in_port >= ofproto->up.max_ports && in_port < ofp_to_u16(OFPP_MAX)) {
> unixctl_command_reply_error(conn, "invalid in_port");
> goto exit;
> }
> if (enforce_consistency) {
> - retval = ofpacts_check_consistency(ofpacts.data, ofpacts.size, &flow,
> + retval = ofpacts_check_consistency(ofpacts.data, ofpacts.size,
> &match,
> u16_to_ofp(ofproto->up.max_ports),
> 0, ofproto->up.n_tables,
> usable_protocols);
> } else {
> - retval = ofpacts_check(ofpacts.data, ofpacts.size, &flow,
> + retval = ofpacts_check(ofpacts.data, ofpacts.size, &match,
> u16_to_ofp(ofproto->up.max_ports), 0,
> ofproto->up.n_tables, &usable_protocols);
> }
> @@ -398,7 +399,7 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn,
> int argc,
> goto exit;
> }
>
> - ofproto_trace(ofproto, &flow, packet,
> + ofproto_trace(ofproto, &match.flow, packet,
> ofpacts.data, ofpacts.size, &result);
> unixctl_command_reply(conn, ds_cstr(&result));
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index ef4b1d9..35ec31d 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -3447,6 +3447,7 @@ ofproto_packet_out_init(struct ofproto *ofproto,
> const struct ofputil_packet_out *po)
> {
> enum ofperr error;
> + struct match match;
>
> if (ofp_to_u16(po->in_port) >= ofproto->max_ports
> && ofp_to_u16(po->in_port) < ofp_to_u16(OFPP_MAX)) {
> @@ -3471,8 +3472,8 @@ ofproto_packet_out_init(struct ofproto *ofproto,
> * actions aren't in any table. This is OK as 'table_id' is only used to
> * check instructions (e.g., goto-table), which can't appear on the
> action
> * list of a packet-out. */
> - error = ofpacts_check_consistency(po->ofpacts, po->ofpacts_len,
> - opo->flow,
> + match_wc_init(&match, opo->flow);
> + error = ofpacts_check_consistency(po->ofpacts, po->ofpacts_len, &match,
> u16_to_ofp(ofproto->max_ports), 0,
> ofproto->n_tables,
> ofconn_get_protocol(ofconn));
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 77332e0..e317242 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -3880,12 +3880,12 @@ ofctl_parse_actions__(const char *version_s, bool
> instructions)
> if (!error && instructions) {
> /* Verify actions, enforce consistency. */
> enum ofputil_protocol protocol;
> - struct flow flow;
> + struct match match;
>
> - memset(&flow, 0, sizeof flow);
> + memset(&match, 0, sizeof match);
> protocol = ofputil_protocols_from_ofp_version(version);
> error = ofpacts_check_consistency(ofpacts.data, ofpacts.size,
> - &flow, OFPP_MAX,
> + &match, OFPP_MAX,
> table_id ? atoi(table_id) : 0,
> OFPTT_MAX + 1, protocol);
> }
> --
> 2.1.4
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
