From patchwork Mon Jul 17 03:11:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Yang, Yi" X-Patchwork-Id: 789191 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 3x9pHM58Qsz9sBR for ; Mon, 17 Jul 2017 13:11:59 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 1D7B08A1; Mon, 17 Jul 2017 03:11:51 +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 807C8890 for ; Mon, 17 Jul 2017 03:11:50 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id BF05A1BB for ; Mon, 17 Jul 2017 03:11:49 +0000 (UTC) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jul 2017 20:11:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,372,1496127600"; d="scan'208";a="127456537" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga005.fm.intel.com with ESMTP; 16 Jul 2017 20:11:49 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 16 Jul 2017 20:11:48 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by FMSMSX154.amr.corp.intel.com (10.18.116.70) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 16 Jul 2017 20:11:48 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.116]) by SHSMSX104.ccr.corp.intel.com ([10.239.4.70]) with mapi id 14.03.0319.002; Mon, 17 Jul 2017 11:11:47 +0800 From: "Yang, Yi Y" To: Jan Scheurich , "dev@openvswitch.org" Thread-Topic: [ovs-dev] [PATCH 0/6] Generic Encap & Decap based NSH implementation Thread-Index: AQHS9UGiuwB4EyRSG06SJbxPIaCNQKJWWKkAgAERodA= Date: Mon, 17 Jul 2017 03:11:46 +0000 Message-ID: <79BBBFE6CB6C9B488C1A45ACD284F51961BFDAAD@SHSMSX103.ccr.corp.intel.com> References: <1499226343-123663-1-git-send-email-yi.y.yang@intel.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: Re: [ovs-dev] [PATCH 0/6] Generic Encap & Decap based NSH implementation 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: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Thanks Jan, I have folded it to next version. -----Original Message----- From: Jan Scheurich [mailto:jan.scheurich@ericsson.com] Sent: Monday, July 17, 2017 2:52 AM To: Yang, Yi Y ; dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH 0/6] Generic Encap & Decap based NSH implementation Hi Yi, The following incremental clarifies and cleans up the error handling during translation of encap and decap actions for NSH: - Rephrased the xlate_report_debug() messages when dropping packets during translation of encap/decap actions. - Encap and decap translation error handling should be complete. Removed any TODO from the comments. - As all encap/decap errors are checked during translation, the function commit_packet_type_change() should throw an abort when it encounters an unexpected combination of old and new packet types. diff --git a/lib/odp-util.c b/lib/odp-util.c index 02c9859..57c6ec8 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -6765,24 +6765,31 @@ commit_packet_type_change(const struct flow *flow, /* TODO: Do we need to copy all the other fields too? */ break; default: - /* Only the above protocols are supported for encap. The check - * is done at action decoding. */ + /* Only the above protocols are supported for encap. + * The check is done at action translation. */ OVS_NOT_REACHED(); } } else { - /* This is a decap case. It can only happen after translation of a - * decap action. */ + /* This is an explicit or implicit decap case. */ if (pt_ns(flow->packet_type) == OFPHTN_ETHERTYPE && base_flow->packet_type == htonl(PT_ETH)) { - /* pop_eth */ + /* Generate pop_eth and continue without recirculation. */ odp_put_pop_eth_action(odp_actions); base_flow->packet_type = flow->packet_type; base_flow->dl_src = eth_addr_zero; base_flow->dl_dst = eth_addr_zero; - } else if (ntohl(base_flow->packet_type) == PT_NSH) { + } else { + /* All other decap cases require recirculation. + * No need to update the base flow here. */ + switch (ntohl(base_flow->packet_type)) { + case PT_NSH: + /* decap_nsh. */ odp_put_decap_nsh_action(odp_actions); - base_flow->packet_type = flow->packet_type; - memcpy(&base_flow->nsh, &flow->nsh, sizeof(base_flow->nsh)); + break; + default: + /* Checks are done during translation. */ + OVS_NOT_REACHED(); + } } } diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 41a2dd1..9ebfd30 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5459,10 +5459,10 @@ rewrite_flow_encap_ethernet(struct xlate_ctx *ctx, flow->dl_dst = eth_addr_zero; flow->dl_type = ethertype; } else { + /* Error handling: drop packet. */ xlate_report_debug(ctx, OFT_ACTION, - "encap(ethernet) unsupported for packet type " - "ethernet"); - /* TODO: Error handling: drop packet. */ + "Dropping packet as encap(ethernet) is not " + "supported for packet type ethernet."); ctx->error = 1; } } @@ -5532,9 +5532,11 @@ rewrite_flow_encap_nsh(struct xlate_ctx *ctx, np = NSH_P_NSH; break; default: + /* Error handling: drop packet. */ xlate_report_debug(ctx, OFT_ACTION, - "encap(nsh) for unsupported packet type %x", - ntohl(packet_type)); + "Dropping packet as encap(nsh) is not " + "supported for packet type (%d,0x%x)", + pt_ns(packet_type), + pt_ns_type(packet_type)); ctx->error = 1; return; } @@ -5584,6 +5586,7 @@ xlate_generic_encap_action(struct xlate_ctx *ctx, rewrite_flow_encap_nsh(ctx, encap, flow, wc); break; default: + /* New packet type was checked during decoding. */ OVS_NOT_REACHED(); break; } @@ -5632,10 +5635,10 @@ xlate_generic_decap_action(struct xlate_ctx *ctx, flow->packet_type = htonl(PT_NSH); break; default: + /* Error handling: drop packet. */ xlate_report_debug(ctx, OFT_ACTION, - "decap() NSH Next Protocol not supported" - " %x", flow->nsh.np); - /* TODO: Error handling: drop packet. */ + "Dropping packet as NSH next protocol %d " + "is not supported", flow->nsh.np); ctx->error = 1; return false; break; @@ -5644,10 +5647,12 @@ xlate_generic_decap_action(struct xlate_ctx *ctx, /* Trigger recirculation. */ return true; default: - xlate_report_debug(ctx, OFT_ACTION, - "decap() for unsupported packet type %x", - ntohl(flow->packet_type)); - /* TODO: Error handling: drop packet. */ + /* Error handling: drop packet. */ + xlate_report_debug( + ctx, OFT_ACTION, + "Dropping packet as the decap() does not support " + "packet type (%d,0x%x)", + pt_ns(flow->packet_type), + pt_ns_type(flow->packet_type)); ctx->error = 1; return false; }