From 9c2a44dcdcecec56255b89d0175b843de3e69711 Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Mon, 17 Jul 2017 17:39:11 -0700 Subject: [PATCH] xlate: Refactor compose_clone() API Create a new function that hides the details of netlink encoding for the translated clone action. Signed-off-by: Andy Zhou Acked-by: Ben Pfaff --- ofproto/ofproto-dpif-xlate.c | 76 +++++++++++++++--------------------- tests/ofproto-dpif.at | 7 ++++ 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 06c9b9afd0a..d97f55191d9 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5249,35 +5249,39 @@ xlate_sample_action(struct xlate_ctx *ctx, tunnel_out_port, false); } -/* Use datapath 'clone' or sample to enclose the translation of 'oc'. */ +/* For datapath that does not support 'clone' action, but have a suitable + * sample action implementation for clone. The upstream Linux kernel + * version 4.11 or greater, and kernel module from OVS version 2.8 or + * greater version have suitable sample action implementations. */ static void -compose_clone_action(struct xlate_ctx *ctx, const struct ofpact_nest *oc) +compose_clone(struct xlate_ctx *ctx, struct flow *old_base_flow, + const struct ofpact_nest *oc) { - size_t clone_offset = nl_msg_start_nested(ctx->odp_actions, - OVS_ACTION_ATTR_CLONE); - do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); - nl_msg_end_non_empty_nested(ctx->odp_actions, clone_offset); -} - -/* Use datapath 'sample' action to translate clone. */ -static void -compose_clone_action_using_sample(struct xlate_ctx *ctx, - const struct ofpact_nest *oc) -{ - size_t offset = nl_msg_start_nested(ctx->odp_actions, - OVS_ACTION_ATTR_SAMPLE); - - size_t ac_offset = nl_msg_start_nested(ctx->odp_actions, - OVS_SAMPLE_ATTR_ACTIONS); - - do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); + size_t offset, ac_offset; - if (nl_msg_end_non_empty_nested(ctx->odp_actions, ac_offset)) { - nl_msg_cancel_nested(ctx->odp_actions, offset); - } else { - nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, - UINT32_MAX); /* 100% probability. */ - nl_msg_end_nested(ctx->odp_actions, offset); + if (ctx->xbridge->support.clone) { + /* Use clone action as datapath clone. */ + offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_CLONE); + do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); + nl_msg_end_non_empty_nested(ctx->odp_actions, offset); + ctx->base_flow = *old_base_flow; + } else if (ctx->xbridge->support.sample_nesting > 3) { + /* Use sample action as datapath clone. */ + offset = nl_msg_start_nested(ctx->odp_actions, + OVS_ACTION_ATTR_SAMPLE); + ac_offset = nl_msg_start_nested(ctx->odp_actions, + OVS_SAMPLE_ATTR_ACTIONS); + do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); + if (nl_msg_end_non_empty_nested(ctx->odp_actions, ac_offset)) { + nl_msg_cancel_nested(ctx->odp_actions, offset); + } else { + nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, + UINT32_MAX); /* 100% probability. */ + nl_msg_end_nested(ctx->odp_actions, offset); + } + ctx->base_flow = *old_base_flow; + } else { /* Use actions. */ + do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); } } @@ -5298,27 +5302,11 @@ xlate_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc) ofpbuf_use_stub(&ctx->action_set, actset_stub, sizeof actset_stub); ofpbuf_put(&ctx->action_set, old_action_set.data, old_action_set.size); - /* Datapath clone action will make sure the pre clone packets - * are used for actions after clone. Save and restore - * ctx->base_flow to reflect this for the openflow pipeline. */ - if (ctx->xbridge->support.clone) { - struct flow old_base_flow = ctx->base_flow; - compose_clone_action(ctx, oc); - ctx->base_flow = old_base_flow; - } else if (ctx->xbridge->support.sample_nesting > 3) { - /* Avoid generate sample action if datapath - * only allow small number of nesting. Deeper nesting - * can cause the datapath to reject the generated flow. */ - struct flow old_base_flow = ctx->base_flow; - compose_clone_action_using_sample(ctx, oc); - ctx->base_flow = old_base_flow; - } else { - do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); - } + struct flow old_base = ctx->base_flow; + compose_clone(ctx, &old_base, oc); ofpbuf_uninit(&ctx->action_set); ctx->action_set = old_action_set; - ofpbuf_uninit(&ctx->stack); ctx->stack = old_stack; diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 5304cd935ba..3c36c8d0425 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -6821,6 +6821,13 @@ AT_CHECK([tail -1 stdout], [0], [dnl Datapath actions: sample(sample=100.0%,actions(set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2)),sample(sample=100.0%,actions(set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3)),4 ]) +AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], [ignore]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) + +AT_CHECK([tail -1 stdout], [0], [dnl +Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +]) + OVS_VSWITCHD_STOP AT_CLEANUP