From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B7B3FA034F; Wed, 31 Mar 2021 14:07:51 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7CFEF140E88; Wed, 31 Mar 2021 14:07:51 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id E1D46406A3 for ; Wed, 31 Mar 2021 14:07:49 +0200 (CEST) IronPort-SDR: R+XdOhSaR3zhL5xPaNWJqnESSaIMhM4KUuihMpGmoEIyFnMEyCqXiC6YOq0N4FrPra3TdTLN3A +DX/X3FRghbQ== X-IronPort-AV: E=McAfee;i="6000,8403,9939"; a="253324805" X-IronPort-AV: E=Sophos;i="5.81,293,1610438400"; d="scan'208";a="253324805" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Mar 2021 05:07:48 -0700 IronPort-SDR: Zfm0FQNG9EUCB5g6xuoxnlpGImxrLkxVvQHiNbTMauS8FF9LblR27TrkfVc/SZJ36fs2D2LA0m MXRNpsUl86Qw== X-IronPort-AV: E=Sophos;i="5.81,293,1610438400"; d="scan'208";a="445657123" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.249.9]) ([10.213.249.9]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Mar 2021 05:07:47 -0700 To: Salem Sol , dev@dpdk.org Cc: Jiawei Wang , Ori Kam , Xiaoyun Li References: <20210317092610.71000-1-salems@nvidia.com> <20210317092610.71000-2-salems@nvidia.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <5eff804f-baf3-7dce-7cb8-6189454f0665@intel.com> Date: Wed, 31 Mar 2021 13:07:46 +0100 MIME-Version: 1.0 In-Reply-To: <20210317092610.71000-2-salems@nvidia.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 3/17/2021 9:26 AM, Salem Sol wrote: > From: Jiawei Wang > > With the current code the VXLAN/NVGRE parsing routine > stored the configuration of the header on stack, this > might lead to overwriting the data on the stack. > > This patch stores the external data of vxlan and nvgre encap > into global data as a pre-step to supporting vxlan and nvgre > encap as a sample actions. > I didn't get what was on stack and moved in to the global data, can you please elaborate. For example for nvgre, 'action_nvgre_encap_data' is pointer in stack but the actual object is 'ctx->object', so it is not really in the stack. Tough, OK to refactor and split the function as preparation to support the sample action. > Signed-off-by: Jiawei Wang <...> > -/** Parse VXLAN encap action. */ > +/** Setup VXLAN encap configuration. */ > static int > -parse_vc_action_vxlan_encap(struct context *ctx, const struct token *token, > - const char *str, unsigned int len, > - void *buf, unsigned int size) > +parse_setup_vxlan_encap_data > + (struct action_vxlan_encap_data *action_vxlan_encap_data) Can you please fix the syntax, I guess this is done to keep within in 80 column limit, but from readability perspective I think better to go over the 80 column a little instead of breaking the line like this. <...> > +/** Setup NVGRE encap configuration. */ > +static int > +parse_setup_nvgre_encap_data > + (struct action_nvgre_encap_data *action_nvgre_encap_data) > +{ > + if (!action_nvgre_encap_data) > + return -1; This is a static function, and the input of it is in our control, so not sure if we should verify the input here. But if we need to, can you please check the return value of this function where it is called.