From patchwork Fri Jul 21 22:46:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Zhou X-Patchwork-Id: 792363 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xDmGT0Rzmz9ryk for ; Sat, 22 Jul 2017 08:51:29 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id B6D27BD7; Fri, 21 Jul 2017 22:47:37 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id E5604BC3 for ; Fri, 21 Jul 2017 22:47:30 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f193.google.com (mail-pf0-f193.google.com [209.85.192.193]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 04D7617E for ; Fri, 21 Jul 2017 22:47:29 +0000 (UTC) Received: by mail-pf0-f193.google.com with SMTP id c23so5705839pfe.5 for ; Fri, 21 Jul 2017 15:47:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:content-transfer-encoding; bh=kmepF5SQ1lqBmhCnl1uHyoi1z811LoZyuILfoc8helQ=; b=sqi4+8WILumPN8ZHOTxXuhCTpQT8w8BKTmuRq81AJiXVoLBgFFc5eH7deUSQY86CyN xo0jhGO46K+9YblmKIUI4W88FmWLdFhoY1gJCAKdd182QYZ7scxAfaTENLVF21V8ZtBW tNCY4i+N68LsQNh+LKpaVkKTnj0BrPF253c6EOxHAcH09r//IMtDfTzgy5JWy6rV3Wnp 8xO7mE0T3sA/Cxzp9XLA54lv9AVP5C65qkAxBdGZFl/WCcpWRlMKB4MFDZf7ge5mKDNM S+igIbDjKi7P1mGkSa7bhewR6R6viGkBzMWMCBDo3WRVsfgkH8rWSMvzCW/pquoMtqRk etdg== X-Gm-Message-State: AIVw111/7HCkAC9snpPnrYV4wcMO3F8Bkno+/qhkAsd5L665YpWKFu/Z 8QyfikoHp9clYyct X-Received: by 10.98.141.23 with SMTP id z23mr9056351pfd.34.1500677249299; Fri, 21 Jul 2017 15:47:29 -0700 (PDT) Received: from centos.eng.vmware.com ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id q3sm10019886pgf.69.2017.07.21.15.47.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 21 Jul 2017 15:47:28 -0700 (PDT) From: Andy Zhou To: dev@openvswitch.org Date: Fri, 21 Jul 2017 15:46:33 -0700 Message-Id: <1500677193-28512-8-git-send-email-azhou@ovn.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1500677193-28512-1-git-send-email-azhou@ovn.org> References: <1500677193-28512-1-git-send-email-azhou@ovn.org> X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [clone optmization v2 7/7] xlate: Emit datapath clone only when necessary. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Currently the open flow 'clone' action is always translated into datapath clone. While this is valid translation, the datapath 'clone' action is more expensive and has more restrictions than not using them. This patch optimizing the open flow 'clone' translation. Whenever the open flow actions within the 'clone' is reversible, i.e. any datapath actions that modifies a packet can be reversed by using another datapath action. Reversible actions can be translated without emitting datapath clone. This patch combines xlate_clone() and compose_clone() into a single compose_clone() API, since the layering boundary is not obvious. Signed-off-by: Andy Zhou --- ofproto/ofproto-dpif-xlate.c | 169 ++++++++++++++++++++++++++++++++----------- tests/ofproto-dpif.at | 39 +++++++++- 2 files changed, 164 insertions(+), 44 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 9b7a6a4f7620..66df4693dd3e 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5245,49 +5245,83 @@ xlate_sample_action(struct xlate_ctx *ctx, tunnel_out_port, false); } -/* 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(struct xlate_ctx *ctx, struct flow *old_base_flow, - const struct ofpact_nest *oc) +/* Determine if an datapath action translated from the openflow action + * can be reversed by another datapath action. + * + * Openflow actions that do not emit datapath actions are trivially + * reversible. Reversiblity of other actions depends on nature of + * action and their translation. */ +static bool +reversible_actions(const struct ofpact *ofpacts, size_t ofpacts_len) { - size_t offset, ac_offset; + const struct ofpact *a; - 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); + OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { + switch (a->type) { + case OFPACT_BUNDLE: + case OFPACT_CLEAR_ACTIONS: + case OFPACT_CLONE: + case OFPACT_CONJUNCTION: + case OFPACT_CONTROLLER: + case OFPACT_CT_CLEAR: + case OFPACT_DEBUG_RECIRC: + case OFPACT_DEC_MPLS_TTL: + case OFPACT_DEC_TTL: + case OFPACT_ENQUEUE: + case OFPACT_EXIT: + case OFPACT_FIN_TIMEOUT: + case OFPACT_GOTO_TABLE: + case OFPACT_GROUP: + case OFPACT_LEARN: + case OFPACT_MULTIPATH: + case OFPACT_NOTE: + case OFPACT_OUTPUT: + case OFPACT_OUTPUT_REG: + case OFPACT_POP_MPLS: + case OFPACT_POP_QUEUE: + case OFPACT_PUSH_MPLS: + case OFPACT_PUSH_VLAN: + case OFPACT_REG_MOVE: + case OFPACT_RESUBMIT: + case OFPACT_SAMPLE: + case OFPACT_SET_ETH_DST: + case OFPACT_SET_ETH_SRC: + case OFPACT_SET_FIELD: + case OFPACT_SET_IP_DSCP: + case OFPACT_SET_IP_ECN: + case OFPACT_SET_IP_TTL: + case OFPACT_SET_IPV4_DST: + case OFPACT_SET_IPV4_SRC: + case OFPACT_SET_L4_DST_PORT: + case OFPACT_SET_L4_SRC_PORT: + case OFPACT_SET_MPLS_LABEL: + case OFPACT_SET_MPLS_TC: + case OFPACT_SET_MPLS_TTL: + case OFPACT_SET_QUEUE: + case OFPACT_SET_TUNNEL: + case OFPACT_SET_VLAN_PCP: + case OFPACT_SET_VLAN_VID: + case OFPACT_STACK_POP: + case OFPACT_STACK_PUSH: + case OFPACT_STRIP_VLAN: + case OFPACT_UNROLL_XLATE: + case OFPACT_WRITE_ACTIONS: + case OFPACT_WRITE_METADATA: + break; + + case OFPACT_CT: + case OFPACT_METER: + case OFPACT_NAT: + case OFPACT_OUTPUT_TRUNC: + return false; } - ctx->base_flow = *old_base_flow; - } else { /* Use actions. */ - do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); } + return true; } static void -xlate_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc) +compose_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc) { - bool old_was_mpls = ctx->was_mpls; - bool old_conntracked = ctx->conntracked; - struct flow old_flow = ctx->xin->flow; - struct ofpbuf old_stack = ctx->stack; union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)]; ofpbuf_use_stub(&ctx->stack, new_stack, sizeof new_stack); @@ -5298,16 +5332,57 @@ 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); + size_t offset, ac_offset; + size_t oc_actions_len = ofpact_nest_get_action_len(oc); + struct flow old_flow = ctx->xin->flow; + + if (reversible_actions(oc->actions, oc_actions_len)) { + old_flow = ctx->xin->flow; + do_xlate_actions(oc->actions, oc_actions_len, ctx); + goto xlate_done; + } + + /* Commit datapath actions before emitting the clone action to + * avoid emitting those actions twice. Once inside + * the clone, another time for the action after clone. */ + xlate_commit_actions(ctx); struct flow old_base = ctx->base_flow; - compose_clone(ctx, &old_base, oc); + bool old_was_mpls = ctx->was_mpls; + bool old_conntracked = ctx->conntracked; - ofpbuf_uninit(&ctx->action_set); - ctx->action_set = old_action_set; - ofpbuf_uninit(&ctx->stack); - ctx->stack = old_stack; + /* The actions are not reversible, a datapath clone action is + * required to encode the translation. Select the clone action + * based on datapath capabilities. */ + if (ctx->xbridge->support.clone) { /* Use clone action */ + /* Use clone action as datapath clone. */ + offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_CLONE); + ac_offset = ctx->odp_actions->size; + do_xlate_actions(oc->actions, oc_actions_len, ctx); + nl_msg_end_non_empty_nested(ctx->odp_actions, offset); + goto dp_clone_done; + } - ctx->xin->flow = old_flow; + 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, oc_actions_len, 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); + } + goto dp_clone_done; + } + + /* Datapath does not support clone, skip xlate 'oc' and + * report an error */ + xlate_report_error(ctx, "Failed to compose clone action"); +dp_clone_done: /* The clone's conntrack execution should have no effect on the original * packet. */ ctx->conntracked = old_conntracked; @@ -5315,6 +5390,16 @@ xlate_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc) /* Popping MPLS from the clone should have no effect on the original * packet. */ ctx->was_mpls = old_was_mpls; + + /* Restore the 'base_flow' for the next action. */ + ctx->base_flow = old_base; + +xlate_done: + ofpbuf_uninit(&ctx->action_set); + ctx->action_set = old_action_set; + ofpbuf_uninit(&ctx->stack); + ctx->stack = old_stack; + ctx->xin->flow = old_flow; } static void @@ -6166,7 +6251,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_CLONE: - xlate_clone(ctx, ofpact_get_CLONE(a)); + compose_clone(ctx, ofpact_get_CLONE(a)); break; case OFPACT_CT: diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index ceff93014dc2..fc81e1ebfa6d 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -6772,6 +6772,7 @@ AT_SETUP([ofproto-dpif - clone action]) OVS_VSWITCHD_START add_of_ports br0 1 2 3 4 +dnl Reversible open flow clone actions, no datapath clone action should be generated. AT_DATA([flows.txt], [dnl in_port=1, ip, actions=clone(set_field:192.168.3.3->ip_src),clone(set_field:192.168.4.4->ip_dst,output:2),clone(mod_dl_src:80:81:81:81:81:81,set_field:192.168.5.5->ip_dst,output:3),output:4 ]) @@ -6780,7 +6781,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [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: clone(set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2),clone(set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3),4 +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 ]) dnl Test flow xlate openflow clone action without using datapath clone action. @@ -6789,7 +6790,7 @@ AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [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: 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 +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 ]) AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], [ignore]) @@ -6799,6 +6800,40 @@ 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 ]) +dnl Mixing reversible and irreversible open flow clone actions. Datapath clone action +dnl should be generated when necessary. + +dnl Restore the datapath support level. +AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone true], [0], []) +AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 10], [0], []) + +AT_DATA([flows.txt], [dnl +in_port=1, ip, actions=clone(set_field:192.168.3.3->ip_src),clone(set_field:192.168.4.4->ip_dst,output:2),clone(ct(commit),output:3),output:4 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [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(ipv4(src=10.10.10.2,dst=10.10.10.1)),clone(ct(commit),3),4 +]) + +AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [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(ipv4(src=10.10.10.2,dst=10.10.10.1)),sample(sample=100.0%,actions(ct(commit),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(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +]) +AT_CHECK([grep "Failed to compose clone action" stdout], [0], [ignore]) + OVS_VSWITCHD_STOP AT_CLEANUP