[{"id":1773021,"web_url":"http://patchwork.ozlabs.org/comment/1773021/","msgid":"","list_archive_url":null,"date":"2017-09-21T17:33:59","subject":"Re: [ovs-dev] [merge native tunneling and patch port 4/7]\n\tofproto-dpif-xlate: Keep track of the last action","submitter":{"id":69140,"url":"http://patchwork.ozlabs.org/api/people/69140/","name":"Gregory Rose","email":"gvrose8192@gmail.com"},"content":"On 09/12/2017 12:49 PM, Andy Zhou wrote:\n> When Openflow clone is last action of an action list, it does not\n> make sense for xlated actions to generate datapath clone action.\n> \n> This patch attemps to Keep track of last clone action, but not\n> necessarily for every case.\n> \n> Signed-off-by: Andy Zhou \n> ---\n> include/openvswitch/ofp-actions.h | 7 ++\n> ofproto/ofproto-dpif-xlate.c | 214 +++++++++++++++++++++++---------------\n> 2 files changed, 140 insertions(+), 81 deletions(-)\n> \n> diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h\n> index ad8e1bec06ba..03c1d11dc9a2 100644\n> --- a/include/openvswitch/ofp-actions.h\n> +++ b/include/openvswitch/ofp-actions.h\n> @@ -216,6 +216,13 @@ ofpact_end(const struct ofpact *ofpacts, size_t ofpacts_len)\n> return ALIGNED_CAST(struct ofpact *, (uint8_t *) ofpacts + ofpacts_len);\n> }\n> \n> +static inline bool\n> +ofpact_last(const struct ofpact *a, const struct ofpact *ofpacts,\n> + size_t ofpact_len)\n> +{\n> + return ofpact_next(a) == ofpact_end(ofpacts, ofpact_len);\n> +}\n> +\n> static inline const struct ofpact *\n> ofpact_find_type_flattened(const struct ofpact *a, enum ofpact_type type,\n> const struct ofpact * const end)\n> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c\n> index 223313d4ecba..9e39a4ff7f49 100644\n> --- a/ofproto/ofproto-dpif-xlate.c\n> +++ b/ofproto/ofproto-dpif-xlate.c\n> @@ -507,11 +507,12 @@ static struct xlate_cfg *new_xcfg = NULL;\n> \n> static bool may_receive(const struct xport *, struct xlate_ctx *);\n> static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len,\n> - struct xlate_ctx *);\n> + struct xlate_ctx *, bool);\n> static void xlate_normal(struct xlate_ctx *);\n> static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,\n> uint8_t table_id, bool may_packet_in,\n> - bool honor_table_miss, bool with_ct_orig);\n> + bool honor_table_miss, bool with_ct_orig,\n> + bool is_last_action);\n> static bool input_vid_is_valid(const struct xlate_ctx *,\n> uint16_t vid, struct xbundle *);\n> static void xvlan_copy(struct xvlan *dst, const struct xvlan *src);\n> @@ -536,7 +537,8 @@ struct xlate_bond_recirc {\n> };\n> \n> static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port,\n> - const struct xlate_bond_recirc *xr);\n> + const struct xlate_bond_recirc *xr,\n> + bool is_last_action);\n> \n> static struct xbridge *xbridge_lookup(struct xlate_cfg *,\n> const struct ofproto_dpif *);\n> @@ -2218,7 +2220,8 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,\n> memcpy(&old_vlans, &ctx->xin->flow.vlans, sizeof(old_vlans));\n> xvlan_put(&ctx->xin->flow, &out_xvlan);\n> \n> - compose_output_action(ctx, xport->ofp_port, use_recirc ? &xr : NULL);\n> + compose_output_action(ctx, xport->ofp_port, use_recirc ? &xr : NULL,\n> + false);\n> memcpy(&ctx->xin->flow.vlans, &old_vlans, sizeof(old_vlans));\n> }\n> \n> @@ -3557,7 +3560,7 @@ apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,\n> if (xport_stp_forward_state(out_dev) &&\n> xport_rstp_forward_state(out_dev)) {\n> xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true,\n> - false);\n> + false, true);\n> if (!ctx->freezing) {\n> xlate_action_set(ctx);\n> }\n> @@ -3572,7 +3575,7 @@ apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,\n> mirror_mask_t old_mirrors2 = ctx->mirrors;\n> \n> xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true,\n> - false);\n> + false, true);\n> ctx->mirrors = old_mirrors2;\n> ctx->base_flow = old_base_flow;\n> ctx->odp_actions->size = old_size;\n> @@ -3716,7 +3719,8 @@ terminate_native_tunnel(struct xlate_ctx *ctx, ofp_port_t ofp_port,\n> \n> static void\n> compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,\n> - const struct xlate_bond_recirc *xr, bool check_stp)\n> + const struct xlate_bond_recirc *xr, bool check_stp,\n> + bool is_last_action OVS_UNUSED)\n> {\n> const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);\n> struct flow_wildcards *wc = ctx->wc;\n> @@ -3882,13 +3886,15 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,\n> \n> static void\n> compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port,\n> - const struct xlate_bond_recirc *xr)\n> + const struct xlate_bond_recirc *xr,\n> + bool is_last_action)\n> {\n> - compose_output_action__(ctx, ofp_port, xr, true);\n> + compose_output_action__(ctx, ofp_port, xr, true, is_last_action);\n> }\n> \n> static void\n> -xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule, bool deepens)\n> +xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule,\n> + bool deepens, bool is_last_action)\n> {\n> struct rule_dpif *old_rule = ctx->rule;\n> ovs_be64 old_cookie = ctx->rule_cookie;\n> @@ -3904,7 +3910,8 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule, bool deepens)\n> ctx->rule = rule;\n> ctx->rule_cookie = rule->up.flow_cookie;\n> actions = rule_get_actions(&rule->up);\n> - do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx);\n> + do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx,\n> + is_last_action);\n> ctx->rule_cookie = old_cookie;\n> ctx->rule = old_rule;\n> ctx->depth -= deepens;\n> @@ -3979,7 +3986,7 @@ tuple_swap(struct flow *flow, struct flow_wildcards *wc)\n> static void\n> xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,\n> bool may_packet_in, bool honor_table_miss,\n> - bool with_ct_orig)\n> + bool with_ct_orig, bool is_last_action)\n> {\n> /* Check if we need to recirculate before matching in a table. */\n> if (ctx->was_mpls) {\n> @@ -4030,7 +4037,8 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,\n> \n> struct ovs_list *old_trace = ctx->xin->trace;\n> xlate_report_table(ctx, rule, table_id);\n> - xlate_recursively(ctx, rule, table_id <= old_table_id);\n> + xlate_recursively(ctx, rule, table_id <= old_table_id,\n> + is_last_action);\n> ctx->xin->trace = old_trace;\n> }\n> \n> @@ -4057,7 +4065,8 @@ xlate_group_stats(struct xlate_ctx *ctx, struct group_dpif *group,\n> }\n> \n> static void\n> -xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket)\n> +xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket,\n> + bool is_last_action)\n> {\n> uint64_t action_list_stub[1024 / 8];\n> struct ofpbuf action_list = OFPBUF_STUB_INITIALIZER(action_list_stub);\n> @@ -4068,7 +4077,7 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket)\n> \n> ofpacts_execute_action_set(&action_list, &action_set);\n> ctx->depth++;\n> - do_xlate_actions(action_list.data, action_list.size, ctx);\n> + do_xlate_actions(action_list.data, action_list.size, ctx, is_last_action);\n> ctx->depth--;\n> \n> ofpbuf_uninit(&action_list);\n> @@ -4106,23 +4115,26 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket)\n> }\n> \n> static void\n> -xlate_all_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> +xlate_all_group(struct xlate_ctx *ctx, struct group_dpif *group,\n> + bool is_last_action)\n> {\n> struct ofputil_bucket *bucket;\n> LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {\n> - xlate_group_bucket(ctx, bucket);\n> + bool last = is_last_action && !bucket->list_node.next;\n> + xlate_group_bucket(ctx, bucket, last);\n> }\n> xlate_group_stats(ctx, group, NULL);\n> }\n> \n> static void\n> -xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> +xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group,\n> + bool is_last_action)\n> {\n> struct ofputil_bucket *bucket;\n> \n> bucket = group_first_live_bucket(ctx, group, 0);\n> if (bucket) {\n> - xlate_group_bucket(ctx, bucket);\n> + xlate_group_bucket(ctx, bucket, is_last_action);\n> xlate_group_stats(ctx, group, bucket);\n> } else if (ctx->xin->xcache) {\n> ofproto_group_unref(&group->up);\n> @@ -4130,7 +4142,8 @@ xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> }\n> \n> static void\n> -xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> +xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group,\n> + bool is_last_action)\n> {\n> struct flow_wildcards *wc = ctx->wc;\n> struct ofputil_bucket *bucket;\n> @@ -4140,7 +4153,7 @@ xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> flow_mask_hash_fields(&ctx->xin->flow, wc, NX_HASH_FIELDS_SYMMETRIC_L4);\n> bucket = group_best_live_bucket(ctx, group, basis);\n> if (bucket) {\n> - xlate_group_bucket(ctx, bucket);\n> + xlate_group_bucket(ctx, bucket, is_last_action);\n> xlate_group_stats(ctx, group, bucket);\n> } else if (ctx->xin->xcache) {\n> ofproto_group_unref(&group->up);\n> @@ -4148,7 +4161,8 @@ xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> }\n> \n> static void\n> -xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> +xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group,\n> + bool is_last_action)\n> {\n> const struct field_array *fields = &group->up.props.fields;\n> const uint8_t *mask_values = fields->values;\n> @@ -4186,7 +4200,7 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> \n> struct ofputil_bucket *bucket = group_best_live_bucket(ctx, group, basis);\n> if (bucket) {\n> - xlate_group_bucket(ctx, bucket);\n> + xlate_group_bucket(ctx, bucket, is_last_action);\n> xlate_group_stats(ctx, group, bucket);\n> } else if (ctx->xin->xcache) {\n> ofproto_group_unref(&group->up);\n> @@ -4194,7 +4208,8 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> }\n> \n> static void\n> -xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> +xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group,\n> + bool is_last_action)\n> {\n> struct ofputil_bucket *bucket;\n> \n> @@ -4218,7 +4233,7 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> ctx->wc->masks.dp_hash |= mask;\n> bucket = group_best_live_bucket(ctx, group, basis);\n> if (bucket) {\n> - xlate_group_bucket(ctx, bucket);\n> + xlate_group_bucket(ctx, bucket, is_last_action);\n> xlate_group_stats(ctx, group, bucket);\n> }\n> }\n> @@ -4226,7 +4241,8 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> }\n> \n> static void\n> -xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> +xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group,\n> + bool is_last_action)\n> {\n> const char *selection_method = group->up.props.selection_method;\n> \n> @@ -4238,11 +4254,11 @@ xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> }\n> \n> if (selection_method[0] == '\\0') {\n> - xlate_default_select_group(ctx, group);\n> + xlate_default_select_group(ctx, group, is_last_action);\n> } else if (!strcasecmp(\"hash\", selection_method)) {\n> - xlate_hash_fields_select_group(ctx, group);\n> + xlate_hash_fields_select_group(ctx, group, is_last_action);\n> } else if (!strcasecmp(\"dp_hash\", selection_method)) {\n> - xlate_dp_hash_select_group(ctx, group);\n> + xlate_dp_hash_select_group(ctx, group, is_last_action);\n> } else {\n> /* Parsing of groups should ensure this never happens */\n> OVS_NOT_REACHED();\n> @@ -4250,7 +4266,8 @@ xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)\n> }\n> \n> static void\n> -xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)\n> +xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group,\n> + bool is_last_action)\n> {\n> bool was_in_group = ctx->in_group;\n> ctx->in_group = true;\n> @@ -4258,13 +4275,13 @@ xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)\n> switch (group->up.type) {\n> case OFPGT11_ALL:\n> case OFPGT11_INDIRECT:\n> - xlate_all_group(ctx, group);\n> + xlate_all_group(ctx, group, is_last_action);\n> break;\n> case OFPGT11_SELECT:\n> - xlate_select_group(ctx, group);\n> + xlate_select_group(ctx, group, is_last_action);\n> break;\n> case OFPGT11_FF:\n> - xlate_ff_group(ctx, group);\n> + xlate_ff_group(ctx, group, is_last_action);\n> break;\n> default:\n> OVS_NOT_REACHED();\n> @@ -4274,7 +4291,8 @@ xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)\n> }\n> \n> static bool\n> -xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)\n> +xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id,\n> + bool is_last_action)\n> {\n> if (xlate_resubmit_resource_check(ctx)) {\n> struct group_dpif *group;\n> @@ -4288,7 +4306,7 @@ xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)\n> group_id);\n> return true;\n> }\n> - xlate_group_action__(ctx, group);\n> + xlate_group_action__(ctx, group, is_last_action);\n> }\n> \n> return false;\n> @@ -4296,7 +4314,8 @@ xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)\n> \n> static void\n> xlate_ofpact_resubmit(struct xlate_ctx *ctx,\n> - const struct ofpact_resubmit *resubmit)\n> + const struct ofpact_resubmit *resubmit,\n> + bool is_last_action)\n> {\n> ofp_port_t in_port;\n> uint8_t table_id;\n> @@ -4321,26 +4340,47 @@ xlate_ofpact_resubmit(struct xlate_ctx *ctx,\n> }\n> \n> xlate_table_action(ctx, in_port, table_id, may_packet_in,\n> - honor_table_miss, resubmit->with_ct_orig);\n> + honor_table_miss, resubmit->with_ct_orig,\n> + is_last_action);\n> }\n> \n> static void\n> -flood_packets(struct xlate_ctx *ctx, bool all)\n> +flood_packet_to_port(struct xlate_ctx *ctx, const struct xport *xport,\n> + bool all, bool is_last_action)\n> {\n> - const struct xport *xport;\n> + if (!xport) {\n> + return;\n> + }\n> +\n> + if (all) {\n> + compose_output_action__(ctx, xport->ofp_port, NULL, false,\n> + is_last_action);\n> + } else {\n> + compose_output_action(ctx, xport->ofp_port, NULL, is_last_action);\n> + } > +}\n> +\n> +static void\n> +flood_packets(struct xlate_ctx *ctx, bool all, bool is_last_action)\n> +{\n> + const struct xport *xport, *last = NULL;\n> \n> + /* Use 'last' the keep track of the last output port. */\n> HMAP_FOR_EACH (xport, ofp_node, &ctx->xbridge->xports) {\n> if (xport->ofp_port == ctx->xin->flow.in_port.ofp_port) {\n> continue;\n> }\n> \n> - if (all) {\n> - compose_output_action__(ctx, xport->ofp_port, NULL, false);\n> - } else if (!(xport->config & OFPUTIL_PC_NO_FLOOD)) {\n> - compose_output_action(ctx, xport->ofp_port, NULL);\n> + if (all || !(xport->config & OFPUTIL_PC_NO_FLOOD)) {\n> + /* 'last' is not the last port, send a packet out, and\n> + * update 'last'. */\n> + flood_packet_to_port(ctx, last, all, false);\n> + last = xport;\n> }\n> }\n> \n> + /* Send the packet to the 'last' port. */\n> + flood_packet_to_port(ctx, last, all, is_last_action);\n> ctx->nf_output_iface = NF_OUT_FLOOD;\n> }\n> \n> @@ -4834,7 +4874,8 @@ compose_dec_mpls_ttl_action(struct xlate_ctx *ctx)\n> \n> static void\n> xlate_output_action(struct xlate_ctx *ctx,\n> - ofp_port_t port, uint16_t max_len, bool may_packet_in)\n> + ofp_port_t port, uint16_t max_len, bool may_packet_in,\n> + bool is_last_action)\n> {\n> ofp_port_t prev_nf_output_iface = ctx->nf_output_iface;\n> \n> @@ -4842,20 +4883,21 @@ xlate_output_action(struct xlate_ctx *ctx,\n> \n> switch (port) {\n> case OFPP_IN_PORT:\n> - compose_output_action(ctx, ctx->xin->flow.in_port.ofp_port, NULL);\n> + compose_output_action(ctx, ctx->xin->flow.in_port.ofp_port, NULL,\n> + is_last_action);\n> break;\n> case OFPP_TABLE:\n> xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,\n> - 0, may_packet_in, true, false);\n> + 0, may_packet_in, true, false, is_last_action);\n> break;\n> case OFPP_NORMAL:\n> xlate_normal(ctx);\n> break;\n> case OFPP_FLOOD:\n> - flood_packets(ctx, false);\n> + flood_packets(ctx, false, is_last_action);\n> break;\n> case OFPP_ALL:\n> - flood_packets(ctx, true);\n> + flood_packets(ctx, true, is_last_action);\n> break;\n> case OFPP_CONTROLLER:\n> execute_controller_action(ctx, max_len,\n> @@ -4870,7 +4912,7 @@ xlate_output_action(struct xlate_ctx *ctx,\n> case OFPP_LOCAL:\n> default:\n> if (port != ctx->xin->flow.in_port.ofp_port) {\n> - compose_output_action(ctx, port, NULL);\n> + compose_output_action(ctx, port, NULL, is_last_action);\n> } else {\n> xlate_report(ctx, OFT_WARN, \"skipping output to input port\");\n> }\n> @@ -4889,7 +4931,8 @@ xlate_output_action(struct xlate_ctx *ctx,\n> \n> static void\n> xlate_output_reg_action(struct xlate_ctx *ctx,\n> - const struct ofpact_output_reg *or)\n> + const struct ofpact_output_reg *or,\n> + bool is_last_action)\n> {\n> uint64_t port = mf_get_subfield(&or->src, &ctx->xin->flow);\n> if (port <= UINT16_MAX) {\n> @@ -4899,7 +4942,8 @@ xlate_output_reg_action(struct xlate_ctx *ctx,\n> \n> memset(&value, 0xff, sizeof value);\n> mf_write_subfield_flow(&or->src, &value, &ctx->wc->masks);\n> - xlate_output_action(ctx, u16_to_ofp(port), or->max_len, false);\n> + xlate_output_action(ctx, u16_to_ofp(port), or->max_len, false,\n> + is_last_action);\n> } else {\n> xlate_report(ctx, OFT_WARN, \"output port %\"PRIu64\" is out of range\",\n> port);\n> @@ -4908,7 +4952,8 @@ xlate_output_reg_action(struct xlate_ctx *ctx,\n> \n> static void\n> xlate_output_trunc_action(struct xlate_ctx *ctx,\n> - ofp_port_t port, uint32_t max_len)\n> + ofp_port_t port, uint32_t max_len,\n> + bool is_last_action)\n> {\n> bool support_trunc = ctx->xbridge->support.trunc;\n> struct ovs_action_trunc *trunc;\n> @@ -4945,7 +4990,7 @@ xlate_output_trunc_action(struct xlate_ctx *ctx,\n> OVS_ACTION_ATTR_TRUNC,\n> sizeof *trunc);\n> trunc->max_len = max_len;\n> - xlate_output_action(ctx, port, max_len, false);\n> + xlate_output_action(ctx, port, max_len, false, is_last_action);\n> if (!support_trunc) {\n> ctx->xout->slow |= SLOW_ACTION;\n> }\n> @@ -4958,7 +5003,8 @@ xlate_output_trunc_action(struct xlate_ctx *ctx,\n> \n> static void\n> xlate_enqueue_action(struct xlate_ctx *ctx,\n> - const struct ofpact_enqueue *enqueue)\n> + const struct ofpact_enqueue *enqueue,\n> + bool is_last_action)\n> {\n> ofp_port_t ofp_port = enqueue->port;\n> uint32_t queue_id = enqueue->queue;\n> @@ -4969,7 +5015,7 @@ xlate_enqueue_action(struct xlate_ctx *ctx,\n> error = dpif_queue_to_priority(ctx->xbridge->dpif, queue_id, &priority);\n> if (error) {\n> /* Fall back to ordinary output action. */\n> - xlate_output_action(ctx, enqueue->port, 0, false);\n> + xlate_output_action(ctx, enqueue->port, 0, false, is_last_action);\n> return;\n> }\n> \n> @@ -4983,7 +5029,7 @@ xlate_enqueue_action(struct xlate_ctx *ctx,\n> /* Add datapath actions. */\n> flow_priority = ctx->xin->flow.skb_priority;\n> ctx->xin->flow.skb_priority = priority;\n> - compose_output_action(ctx, ofp_port, NULL);\n> + compose_output_action(ctx, ofp_port, NULL, is_last_action);\n> ctx->xin->flow.skb_priority = flow_priority;\n> \n> /* Update NetFlow output port. */\n> @@ -5031,7 +5077,8 @@ slave_enabled_cb(ofp_port_t ofp_port, void *xbridge_)\n> \n> static void\n> xlate_bundle_action(struct xlate_ctx *ctx,\n> - const struct ofpact_bundle *bundle)\n> + const struct ofpact_bundle *bundle,\n> + bool is_last_action)\n> {\n> ofp_port_t port;\n> \n> @@ -5041,7 +5088,7 @@ xlate_bundle_action(struct xlate_ctx *ctx,\n> nxm_reg_load(&bundle->dst, ofp_to_u16(port), &ctx->xin->flow, ctx->wc);\n> xlate_report_subfield(ctx, &bundle->dst);\n> } else {\n> - xlate_output_action(ctx, port, 0, false);\n> + xlate_output_action(ctx, port, 0, false, is_last_action);\n> }\n> }\n> \n> @@ -5335,7 +5382,7 @@ reversible_actions(const struct ofpact *ofpacts, size_t ofpacts_len)\n> \n> static void\n> clone_xlate_actions(const struct ofpact *actions, size_t actions_len,\n> - struct xlate_ctx *ctx)\n> + struct xlate_ctx *ctx, bool is_last_action)\n> {\n> struct ofpbuf old_stack = ctx->stack;\n> union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];\n> @@ -5350,9 +5397,9 @@ clone_xlate_actions(const struct ofpact *actions, size_t actions_len,\n> size_t offset, ac_offset;\n> struct flow old_flow = ctx->xin->flow;\n> \n> - if (reversible_actions(actions, actions_len)) {\n> + if (reversible_actions(actions, actions_len) || is_last_action) {\n> old_flow = ctx->xin->flow;\n> - do_xlate_actions(actions, actions_len, ctx);\n> + do_xlate_actions(actions, actions_len, ctx, is_last_action);\n> if (ctx->freezing) {\n> finish_freezing(ctx);\n> }\n> @@ -5373,7 +5420,7 @@ clone_xlate_actions(const struct ofpact *actions, size_t actions_len,\n> if (ctx->xbridge->support.clone) { /* Use clone action */\n> /* Use clone action as datapath clone. */\n> offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_CLONE);\n> - do_xlate_actions(actions, actions_len, ctx);\n> + do_xlate_actions(actions, actions_len, ctx, true);\n> if (ctx->freezing) {\n> finish_freezing(ctx);\n> }\n> @@ -5386,7 +5433,7 @@ clone_xlate_actions(const struct ofpact *actions, size_t actions_len,\n> offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE);\n> ac_offset = nl_msg_start_nested(ctx->odp_actions,\n> OVS_SAMPLE_ATTR_ACTIONS);\n> - do_xlate_actions(actions, actions_len, ctx);\n> + do_xlate_actions(actions, actions_len, ctx, true);\n> if (ctx->freezing) {\n> finish_freezing(ctx);\n> }\n> @@ -5425,11 +5472,12 @@ xlate_done:\n> }\n> \n> static void\n> -compose_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc)\n> +compose_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc,\n> + bool is_last_action)\n> {\n> size_t oc_actions_len = ofpact_nest_get_action_len(oc);\n> \n> - clone_xlate_actions(oc->actions, oc_actions_len, ctx);\n> + clone_xlate_actions(oc->actions, oc_actions_len, ctx, is_last_action);\n> }\n> \n> static void\n> @@ -5511,7 +5559,7 @@ xlate_action_set(struct xlate_ctx *ctx)\n> struct ovs_list *old_trace = ctx->xin->trace;\n> ctx->xin->trace = xlate_report(ctx, OFT_TABLE,\n> \"--. Executing action set:\");\n> - do_xlate_actions(action_list.data, action_list.size, ctx);\n> + do_xlate_actions(action_list.data, action_list.size, ctx, true);\n> ctx->xin->trace = old_trace;\n> \n> ctx->in_action_set = false;\n> @@ -5735,7 +5783,8 @@ put_ct_nat(struct xlate_ctx *ctx)\n> }\n> \n> static void\n> -compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)\n> +compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc,\n> + bool is_last_action)\n> {\n> ovs_u128 old_ct_label_mask = ctx->wc->masks.ct_label;\n> uint32_t old_ct_mark_mask = ctx->wc->masks.ct_mark;\n> @@ -5750,7 +5799,8 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)\n> ctx->ct_nat_action = NULL;\n> ctx->wc->masks.ct_mark = 0;\n> ctx->wc->masks.ct_label = OVS_U128_ZERO;\n> - do_xlate_actions(ofc->actions, ofpact_ct_get_action_len(ofc), ctx);\n> + do_xlate_actions(ofc->actions, ofpact_ct_get_action_len(ofc), ctx,\n> + is_last_action);\n> \n> if (ofc->zone_src.field) {\n> zone = mf_get_subfield(&ofc->zone_src, &ctx->xin->flow);\n> @@ -6146,7 +6196,7 @@ xlate_ofpact_unroll_xlate(struct xlate_ctx *ctx,\n> \n> static void\n> do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> - struct xlate_ctx *ctx)\n> + struct xlate_ctx *ctx, bool is_last_action)\n> {\n> struct flow_wildcards *wc = ctx->wc;\n> struct flow *flow = &ctx->xin->flow;\n> @@ -6167,6 +6217,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> const struct ofpact_metadata *metadata;\n> const struct ofpact_set_field *set_field;\n> const struct mf_field *mf;\n> + bool last = is_last_action && ofpact_last(a, ofpacts, ofpacts_len)\n> + && ctx->action_set.size;\n> \n> if (ctx->error) {\n> break;\n> @@ -6194,11 +6246,11 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> switch (a->type) {\n> case OFPACT_OUTPUT:\n> xlate_output_action(ctx, ofpact_get_OUTPUT(a)->port,\n> - ofpact_get_OUTPUT(a)->max_len, true);\n> + ofpact_get_OUTPUT(a)->max_len, true, last);\n> break;\n> \n> case OFPACT_GROUP:\n> - if (xlate_group_action(ctx, ofpact_get_GROUP(a)->group_id)) {\n> + if (xlate_group_action(ctx, ofpact_get_GROUP(a)->group_id, last)) {\n> /* Group could not be found. */\n> \n> /* XXX: Terminates action list translation, but does not\n> @@ -6227,7 +6279,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> case OFPACT_ENQUEUE:\n> memset(&wc->masks.skb_priority, 0xff,\n> sizeof wc->masks.skb_priority);\n> - xlate_enqueue_action(ctx, ofpact_get_ENQUEUE(a));\n> + xlate_enqueue_action(ctx, ofpact_get_ENQUEUE(a), last);\n> break;\n> \n> case OFPACT_SET_VLAN_VID:\n> @@ -6339,7 +6391,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> * to avoid that, only adding any actions that follow the resubmit\n> * to the frozen actions.\n> */\n> - xlate_ofpact_resubmit(ctx, ofpact_get_RESUBMIT(a));\n> + xlate_ofpact_resubmit(ctx, ofpact_get_RESUBMIT(a), last);\n> continue;\n> \n> case OFPACT_SET_TUNNEL:\n> @@ -6437,16 +6489,16 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> break;\n> \n> case OFPACT_BUNDLE:\n> - xlate_bundle_action(ctx, ofpact_get_BUNDLE(a));\n> + xlate_bundle_action(ctx, ofpact_get_BUNDLE(a), last);\n> break;\n> \n> case OFPACT_OUTPUT_REG:\n> - xlate_output_reg_action(ctx, ofpact_get_OUTPUT_REG(a));\n> + xlate_output_reg_action(ctx, ofpact_get_OUTPUT_REG(a), last);\n> break;\n> \n> case OFPACT_OUTPUT_TRUNC:\n> xlate_output_trunc_action(ctx, ofpact_get_OUTPUT_TRUNC(a)->port,\n> - ofpact_get_OUTPUT_TRUNC(a)->max_len);\n> + ofpact_get_OUTPUT_TRUNC(a)->max_len, last);\n> break;\n> \n> case OFPACT_LEARN:\n> @@ -6502,7 +6554,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> ovs_assert(ctx->table_id < ogt->table_id);\n> \n> xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,\n> - ogt->table_id, true, true, false);\n> + ogt->table_id, true, true, false, last);\n> break;\n> }\n> \n> @@ -6511,7 +6563,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> break;\n> \n> case OFPACT_CLONE:\n> - compose_clone(ctx, ofpact_get_CLONE(a));\n> + compose_clone(ctx, ofpact_get_CLONE(a), last);\n> break;\n> \n> case OFPACT_ENCAP:\n> @@ -6531,7 +6583,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,\n> }\n> \n> case OFPACT_CT:\n> - compose_conntrack_action(ctx, ofpact_get_CT(a));\n> + compose_conntrack_action(ctx, ofpact_get_CT(a), last);\n> break;\n> \n> case OFPACT_CT_CLEAR:\n> @@ -7099,7 +7151,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)\n> }\n> \n> mirror_ingress_packet(&ctx);\n> - do_xlate_actions(ofpacts, ofpacts_len, &ctx);\n> + do_xlate_actions(ofpacts, ofpacts_len, &ctx, true);\n> if (ctx.error) {\n> goto exit;\n> }\n> @@ -7127,7 +7179,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)\n> && xbridge->has_in_band\n> && in_band_must_output_to_local_port(flow)\n> && !actions_output_to_local_port(&ctx)) {\n> - compose_output_action(&ctx, OFPP_LOCAL, NULL);\n> + compose_output_action(&ctx, OFPP_LOCAL, NULL, false);\n> }\n> \n> if (user_cookie_offset) {\n> \n\nThat's a large patch but I couldn't find any problems with it, so...\n\nTested-by: Greg Rose \nReviewed-by: Greg Rose ","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"cRJpxZhT\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xykHp5yW7z9s3T\n\tfor ;\n\tFri, 22 Sep 2017 03:34:14 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id DA617BE0;\n\tThu, 21 Sep 2017 17:34:06 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 0E7548E2\n\tfor ; Thu, 21 Sep 2017 17:34:05 +0000 (UTC)","from mail-pf0-f193.google.com (mail-pf0-f193.google.com\n\t[209.85.192.193])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 757E81AE\n\tfor ; Thu, 21 Sep 2017 17:34:03 +0000 (UTC)","by mail-pf0-f193.google.com with SMTP id i23so2738066pfi.2\n\tfor ; Thu, 21 Sep 2017 10:34:03 -0700 (PDT)","from gizo.bigblue.kilchis.com (67-5-132-83.ptld.qwest.net.\n\t[67.5.132.83]) by smtp.gmail.com with ESMTPSA id\n\tf13sm4250025pfj.127.2017.09.21.10.34.00\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 21 Sep 2017 10:34:00 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=subject:to:references:from:message-id:date:user-agent:mime-version\n\t:in-reply-to:content-language:content-transfer-encoding;\n\tbh=726e5rlksagnCeqlJn6ZV2z8mVrkFXJzo+DCbQiW8KA=;\n\tb=cRJpxZhTr+2HZ0OaLJYPlUoiKPczwGnlExq7pT55HScphJoA3znLZtXVCpMSuZKVN7\n\to5RE2V61E5SmlEICTI2fUitGtYjldzpN4WTsBkIR5T1GbchysiGpXmoGBjRk/DTAkpAP\n\trvpTP55I18zbKw03KUqJsOw+yVsw1KCAK/ftby0S8yMqjjLhQH8UuK/4hAa2tgBFeqSd\n\t+W1ByOUeiSrbhgQnB62ZEFQSKW1qyaUAYtWirwQa4+v5k93ZSqHDRgjN+20YnDlCtrGA\n\tuveyezguroeqB3GKWDxahxnXFxYYMOdN13kjpOMiiIxjkIt1pV0ZOzpfz4GcGQdnURF7\n\tJslQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=726e5rlksagnCeqlJn6ZV2z8mVrkFXJzo+DCbQiW8KA=;\n\tb=MCt/+qOY3IaiQcySooTZCJ7aq8FIPAdeLyqqNMECg/20aQOzQ7d8k05ufeFWSFzOE3\n\tsiIfx+vbUqHt6yaC07Gz9Oxm8/WfOAwDJoqi0tcrCjG87q50rXNil3e6fZEsE9LDauN0\n\tKQSgTPuHWrlAg5wPAs1OqrU3wD9qNZsu9aPS+izH9pQdm1reuEcd/zyptAPOAPQqvM7S\n\tJAx1709VuBADMBJwoU8ZnY9edz2dhXUJbHuaxtSHG70T8BLqzjE5DoQuV+eIS0QmDgLx\n\tHHvbgtVaR2vqEj1pZqKsaazsE0WXWyMsXxS+DloArDTwS5P3fivXP3kbkxpwmqYRYVYo\n\tUI8Q==","X-Gm-Message-State":"AHPjjUhRIvBdAVFtMOaIEpvEYiOdPmwzbHi1sRF0eUqh1hjxGiXhIpTK\n\tPv+fgPAKV6M4S8NFiDXOl1uixn5G","X-Google-Smtp-Source":"AOwi7QCWipLmDKQnV0gnQUJuX0LmzodYYNEqkwzTDwvO/if6Irpw78dpTlD+QGqQMC5vHI59JSbFBg==","X-Received":"by 10.84.232.72 with SMTP id f8mr6203632pln.269.1506015242078;\n\tThu, 21 Sep 2017 10:34:02 -0700 (PDT)","To":"Andy Zhou , dev@openvswitch.org","References":"<1505245749-3402-1-git-send-email-azhou@ovn.org>\n\t<1505245749-3402-4-git-send-email-azhou@ovn.org>","From":"Greg Rose ","Message-ID":"","Date":"Thu, 21 Sep 2017 10:33:59 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.1.0","MIME-Version":"1.0","In-Reply-To":"<1505245749-3402-4-git-send-email-azhou@ovn.org>","Content-Language":"en-US","X-Spam-Status":"No, score=0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tDKIM_VALID_AU, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_NONE, \n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Subject":"Re: [ovs-dev] [merge native tunneling and patch port 4/7]\n\tofproto-dpif-xlate: Keep track of the last action","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}}]