DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Salem Sol <salems@nvidia.com>, dev@dpdk.org
Cc: Jiawei Wang <jiaweiw@nvidia.com>, Ori Kam <orika@nvidia.com>,
	Xiaoyun Li <xiaoyun.li@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally
Date: Wed, 31 Mar 2021 13:07:46 +0100	[thread overview]
Message-ID: <5eff804f-baf3-7dce-7cb8-6189454f0665@intel.com> (raw)
In-Reply-To: <20210317092610.71000-2-salems@nvidia.com>

On 3/17/2021 9:26 AM, Salem Sol wrote:
> From: Jiawei Wang <jiaweiw@nvidia.com>
> 
> 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 <jiaweiw@nvidia.com>

<...>

> -/** 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.

  parent reply	other threads:[~2021-03-31 12:07 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17  9:26 [dpdk-dev] [PATCH v3 0/8] Add support for VXLAN and NVGRE encap as a sample actions Salem Sol
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally Salem Sol
2021-03-17 10:06   ` Slava Ovsiienko
2021-03-31 12:07   ` Ferruh Yigit [this message]
2021-04-01  4:13     ` Jiawei(Jonny) Wang
2021-04-06 14:44       ` Ferruh Yigit
2021-04-07  8:19         ` Salem Sol
2021-04-07  8:23           ` Ferruh Yigit
2021-04-07  8:35             ` Salem Sol
2021-04-07 11:30               ` Ferruh Yigit
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 2/8] net/mlx5: support VXLAN encap action in sample Salem Sol
2021-03-17 10:06   ` Slava Ovsiienko
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 3/8] net/mlx5: support NVGRE " Salem Sol
2021-03-17 10:07   ` Slava Ovsiienko
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 4/8] app/testpmd: support VXLAN encap for sample action Salem Sol
2021-03-17 10:07   ` Slava Ovsiienko
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 5/8] app/testpmd: support NVGRE " Salem Sol
2021-03-17 10:07   ` Slava Ovsiienko
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide Salem Sol
2021-03-17 10:07   ` Slava Ovsiienko
2021-03-31 12:05   ` Ferruh Yigit
2021-04-01 10:39     ` Salem Sol
2021-04-01 10:43       ` Ferruh Yigit
2021-04-01 10:48         ` Salem Sol
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 7/8] doc: update sample actions support in mlx5 guide Salem Sol
2021-03-17 10:08   ` Slava Ovsiienko
2021-03-17  9:26 ` [dpdk-dev] [PATCH v3 8/8] doc: update dpdk 21.05 release notes Salem Sol
2021-03-17 10:08   ` Slava Ovsiienko
2021-03-31 12:08   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5eff804f-baf3-7dce-7cb8-6189454f0665@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=jiaweiw@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=salems@nvidia.com \
    --cc=xiaoyun.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).