[{"id":1773035,"web_url":"http://patchwork.ozlabs.org/comment/1773035/","msgid":"<728f4f98-4c12-0559-19a9-1de588209ccf@gmail.com>","list_archive_url":null,"date":"2017-09-21T17:52:39","subject":"Re: [ovs-dev] [merge native tunneling and patch port 6/7]\n\tofproto-dpif-xlate: Rename apply_nested_clone_actions()","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> To patch_port_output(). The original function name does not make\n> much sense.\n\nI think the commit message is a little mangled. Perhaps like this:\n\nRename apply_nested_clone_actions() to patch_port_output(). The\noriginal function name does not make much sense.\n\nHaving the commit title extend into the commit message can be\nconfusing.\n\nOther than that nitpick the rest looks good.\n\n> \n> Signed-off-by: Andy Zhou \n> ---\n> ofproto/ofproto-dpif-xlate.c | 33 +++++++++++++++++----------------\n> 1 file changed, 17 insertions(+), 16 deletions(-)\n> \n> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c\n> index 81853c29afbf..94aa071a37cd 100644\n> --- a/ofproto/ofproto-dpif-xlate.c\n> +++ b/ofproto/ofproto-dpif-xlate.c\n> @@ -432,8 +432,8 @@ static void xlate_action_set(struct xlate_ctx *ctx);\n> static void xlate_commit_actions(struct xlate_ctx *ctx);\n> \n> static void\n> -apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,\n> - struct xport *out_dev);\n> +patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev,\n> + struct xport *out_dev);\n> \n> static void\n> ctx_trigger_freeze(struct xlate_ctx *ctx)\n> @@ -3317,7 +3317,7 @@ validate_and_combine_post_tnl_actions(struct xlate_ctx *ctx,\n> entry->tunnel_hdr.hdr_size = tnl_push_data.header_len;\n> entry->tunnel_hdr.operation = ADD;\n> \n> - apply_nested_clone_actions(ctx, xport, out_dev);\n> + patch_port_output(ctx, xport, out_dev);\n> nested_act_flag = is_tunnel_actions_clone_ready(ctx);\n> \n> if (nested_act_flag) {\n> @@ -3509,21 +3509,22 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx, const struct flow *flow, co\n> xport_in->xbundle->protected && xport_out->xbundle->protected);\n> }\n> \n> -/* Function to combine actions from following device/port with the current\n> - * device actions in openflow pipeline. Mainly used for the translation of\n> - * patch/tunnel port output actions. It pushes the openflow state into a stack\n> - * first, clear out to execute the packet through the device and finally pop\n> - * the openflow state back from the stack. This is equivalent to cloning\n> - * a packet in translation for the duration of execution.\n> +/* Function handles when a packet is sent from one bridge to another bridge.\n> *\n> - * On output to a patch port, the output action will be replaced with set of\n> - * nested actions on the peer patch port.\n> - * Similarly on output to a tunnel port, the post nested actions on\n> - * tunnel are chained up with the tunnel-push action.\n> + * The bridges are internally connected, either with patch ports or with\n> + * tunnel ports.\n> + *\n> + * The output action to another bridge causes translation to continue within\n> + * the next bridge. This process can be recursive; the next bridge can\n> + * output yet to another bridge.\n> + *\n> + * The translated actions from the second bridge onwards are enclosed within\n> + * the clone action, so that any modification to the packet will not be visible\n> + * to the remaining actions of the originating bridge.\n\nI like the improvement in the comments here. Thanks!\n\n> */\n> static void\n> -apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,\n> - struct xport *out_dev)\n> +patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev,\n> + struct xport *out_dev)\n> {\n> struct flow *flow = &ctx->xin->flow;\n> struct flow old_flow = ctx->xin->flow;\n> @@ -3760,7 +3761,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,\n> }\n> \n> if (xport->peer) {\n> - apply_nested_clone_actions(ctx, xport, xport->peer);\n> + patch_port_output(ctx, xport, xport->peer);\n> return;\n> }\n> \n> \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=\"oLkmt5W7\"; 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 3xykjD2lW4z9t42\n\tfor ;\n\tFri, 22 Sep 2017 03:52:48 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id DF42FBFF;\n\tThu, 21 Sep 2017 17:52:44 +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 403F5BDF\n\tfor ; Thu, 21 Sep 2017 17:52:44 +0000 (UTC)","from mail-pg0-f68.google.com (mail-pg0-f68.google.com\n\t[74.125.83.68])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 4229F14B\n\tfor ; Thu, 21 Sep 2017 17:52:43 +0000 (UTC)","by mail-pg0-f68.google.com with SMTP id u18so3813093pgo.1\n\tfor ; Thu, 21 Sep 2017 10:52:43 -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\te87sm4153293pfl.46.2017.09.21.10.52.40\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 21 Sep 2017 10:52:41 -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=+BAWGd1D7F8Mljl6clRAaG78ye5Y6B9rdy8uI8Crw7Y=;\n\tb=oLkmt5W708X9ilTu6bMbuGVCFdPio+afkW58R+3uNGjeWshkMPWA6gCNE9eExjtx/+\n\tboPoicpY0uTc9kJcghbBATwolR726iklcu2Rk7FQWpl8vKXtmYJzmqElOxo/5sBLNZ0W\n\tuI9NusdEzyu/bX1qr3DT1jCRJgl6EH1lZEW5AbqfINFNUW30f8cYuGQXjvHvNwkvAs2W\n\tnot56hYibrd+Xi+S6dKzia4aW4QfMOSIeh0vKEXUPUurlK9DNGj1iWt01TT3q4r/wL1C\n\tEdHK7932VIL1TOg9pE37P1OdO42FIlmIkpDea9W3LsX2A9zG9ZMYHBbsPL63vxqqGcSO\n\t5DtA==","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=+BAWGd1D7F8Mljl6clRAaG78ye5Y6B9rdy8uI8Crw7Y=;\n\tb=D/XDCQSerT6/tpVI3fWmHw/eYY724r7Jh5qJTEFwWbZ3jCRUvdH/YCO69hmlEnlw14\n\t8wAOZVjOQjSRlUmFCtThqt40ZQPa4GO8aeyrxR1f9bOy4MK+EWXzi4UeJx1fKvg9ClgP\n\typ6hk0O1r6tAQtoB7U4RU4iFLppsWQ9M0wgzLSu7WEgmu3p6JE4cKYcPRLT52l2gbxUm\n\t7O2FSz6Az/QQY3e7Qvob4kzwkPKumpr1F6xctixjluIV/J5GuwUaR0IXlzxX/fSHu20o\n\t8NEhGRqMqzSFc9xcnQl6cN4HLDHhud572t9vfHImp56EIpouPCL2DfUJBoMFeCqRTQEE\n\tzWvg==","X-Gm-Message-State":"AHPjjUgc/0vUnOXpUrexogrzjAG1F0fHfjZws4jfT9/RY7LWTC/kbBg3\n\tcTyBND0be065DFFEiCf2StIQ9KtB","X-Google-Smtp-Source":"AOwi7QDlDqlauUKD+GG9vEjvC/XwCyALAXE2+9x0PBXjfUiRIKozDJbU/qyh1ltk7zOgump+K+Z9rg==","X-Received":"by 10.84.128.73 with SMTP id 67mr6425392pla.128.1506016362473;\n\tThu, 21 Sep 2017 10:52:42 -0700 (PDT)","To":"Andy Zhou , dev@openvswitch.org","References":"<1505245749-3402-1-git-send-email-azhou@ovn.org>\n\t<1505245749-3402-6-git-send-email-azhou@ovn.org>","From":"Greg Rose ","Message-ID":"<728f4f98-4c12-0559-19a9-1de588209ccf@gmail.com>","Date":"Thu, 21 Sep 2017 10:52:39 -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-6-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 6/7]\n\tofproto-dpif-xlate: Rename apply_nested_clone_actions()","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"}}]