From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id C1EE9A05D3
	for <public@inbox.dpdk.org>; Fri, 29 Mar 2019 14:59:02 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id E0DAD4C80;
	Fri, 29 Mar 2019 14:59:00 +0100 (CET)
Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com
 [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id 46C984C80
 for <dev@dpdk.org>; Fri, 29 Mar 2019 14:58:59 +0100 (CET)
Received: by mail-wm1-f65.google.com with SMTP id q16so2629174wmj.3
 for <dev@dpdk.org>; Fri, 29 Mar 2019 06:58:59 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google;
 h=date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:in-reply-to;
 bh=s0s0ekEMDzjAWghu59BPglAFg7Dk/3bPdnqtSJcD0wc=;
 b=DbsptRlOqejT1wk9AokH6isfcvP1h9046KVkCLreWk1nqdL61wTGoD/BaGkQltOVWr
 4o5rSiJG17EqvCfr9r/qOTaJajR/dpKaR/vxr56UDWgbuody1Nrl4g1V82qvJ4dKk+4I
 1u5yKFf1rW/Ivk063vbqSk7eTjqfcY8XxYV1WbBSQ/ULtgrKCC6w+aJSlhp8q5mgMaXf
 /aX7bZzi2MNd3+Kp6JEiI2QK4i8r3SJR2oxkhzvwBnPU5Ki8FTuAzIY/jBrzcES7VV6G
 WyU/JRuHQq8nZm/hd+wZGWMpXPKhyXXRV5LiySsSMEnNGVr5fw9y82441IHXjY3OHi4z
 7EGA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:references
 :mime-version:content-disposition:in-reply-to;
 bh=s0s0ekEMDzjAWghu59BPglAFg7Dk/3bPdnqtSJcD0wc=;
 b=Ao4iRGA5biBZJk3QOhS7pCDGtHCE/QZkD79ZBp4VlhSxuUOmSpnaRKXgIaMF1nvvlR
 54uPytdw+Ci22nhAYTkbqQrKNktEt49TlkqHbkuxcHYVj6PPwcxXXdC0NK4TnghFmklO
 X2vu6x9xeMmwaOgvsN7dIejUQk0wM2+HIruFpZplQetl5QZfNpUtBBbpE7/MMGl5bCEA
 QorsfS3NyCCW/pH34c+SFn33ORq2NulgZWBokL9PEE9avuWMfrTWmgkNo20UIlAEPvIm
 uY50tEVbwCs43ue0FlivL/vsnvQXTeI6M0/1XAEtqddZOYIpDmKfwREJFJQmQceRgepm
 lx2Q==
X-Gm-Message-State: APjAAAWIjnsvH/q7SB5C2kD7vPpp7eyJzBQlgMJuUHRqlxRZ6ylZbagz
 WjvCIetawoSTVl5bgqF/r69tvg==
X-Google-Smtp-Source: APXvYqzMIhDOS4jzHf2zP4q9wVcGW4p76KhB1lQGKMF65rDxWvqB5wbWITBTv1KgPxeojlbAhT/6uQ==
X-Received: by 2002:a1c:6783:: with SMTP id b125mr3927972wmc.41.1553867938979; 
 Fri, 29 Mar 2019 06:58:58 -0700 (PDT)
Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78])
 by smtp.gmail.com with ESMTPSA id k4sm2515210wro.33.2019.03.29.06.58.58
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Fri, 29 Mar 2019 06:58:58 -0700 (PDT)
Date: Fri, 29 Mar 2019 14:58:57 +0100
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Dekel Peled <dekelp@mellanox.com>
Cc: wenzhuo.lu@intel.com, jingjing.wu@intel.com,
 bernard.iremonger@intel.com, yskoh@mellanox.com,
 shahafs@mellanox.com, dev@dpdk.org, orika@mellanox.com
Message-ID: <20190329135857.GI4889@6wind.com>
References: <1553177917-43297-1-git-send-email-dekelp@mellanox.com>
 <1553177917-43297-3-git-send-email-dekelp@mellanox.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Disposition: inline
In-Reply-To: <1553177917-43297-3-git-send-email-dekelp@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH 2/3] app/testpmd: add actions to modify TCP
	header fields
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190329135857.r3C4KpBvHdEp-7dquL3hMrruRM-nnPD8gC_4_i3hwss@z>

On Thu, Mar 21, 2019 at 04:18:36PM +0200, Dekel Peled wrote:
> Add actions:
> - INC_TCP_SEQ - Increase sequence number in the outermost TCP header.
> - DEC_TCP_SEQ - Decrease sequence number in the outermost TCP header.
> - INC_TCP_ACK - Increase acknowledgment number in the outermost TCP
>                 header.
> - DEC_TCP_ACK - Decrease acknowledgment number in the outermost TCP
>                 header.
> 
> Original work by Xiaoyu Min.
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>

I suggest to merge this patch into the previous one. No reason for testpmd
to be updated separately.

Some comments below.

> ---
>  app/test-pmd/cmdline_flow.c                 | 100 ++++++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  16 +++++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 36659a6..5cd4ceb 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -271,6 +271,14 @@ enum index {
>  	ACTION_SET_MAC_SRC_MAC_SRC,
>  	ACTION_SET_MAC_DST,
>  	ACTION_SET_MAC_DST_MAC_DST,
> +	ACTION_INC_TCP_SEQ,
> +	ACTION_INC_TCP_SEQ_VALUE,
> +	ACTION_DEC_TCP_SEQ,
> +	ACTION_DEC_TCP_SEQ_VALUE,
> +	ACTION_INC_TCP_ACK,
> +	ACTION_INC_TCP_ACK_VALUE,
> +	ACTION_DEC_TCP_ACK,
> +	ACTION_DEC_TCP_ACK_VALUE,
>  };
>  
>  /** Maximum size for pattern in struct rte_flow_item_raw. */
> @@ -884,6 +892,10 @@ struct parse_action_priv {
>  	ACTION_SET_TTL,
>  	ACTION_SET_MAC_SRC,
>  	ACTION_SET_MAC_DST,
> +	ACTION_INC_TCP_SEQ,
> +	ACTION_DEC_TCP_SEQ,
> +	ACTION_INC_TCP_ACK,
> +	ACTION_DEC_TCP_ACK,
>  	ZERO,
>  };
>  
> @@ -1046,6 +1058,30 @@ struct parse_action_priv {
>  	ZERO,
>  };
>  
> +static const enum index action_inc_tcp_seq[] = {
> +	ACTION_INC_TCP_SEQ_VALUE,
> +	ACTION_NEXT,
> +	ZERO,
> +};
> +
> +static const enum index action_dec_tcp_seq[] = {
> +	ACTION_DEC_TCP_SEQ_VALUE,
> +	ACTION_NEXT,
> +	ZERO,
> +};
> +
> +static const enum index action_inc_tcp_ack[] = {
> +	ACTION_INC_TCP_ACK_VALUE,
> +	ACTION_NEXT,
> +	ZERO,
> +};
> +
> +static const enum index action_dec_tcp_ack[] = {
> +	ACTION_DEC_TCP_ACK_VALUE,
> +	ACTION_NEXT,
> +	ZERO,
> +};
> +
>  static int parse_init(struct context *, const struct token *,
>  		      const char *, unsigned int,
>  		      void *, unsigned int);
> @@ -2843,6 +2879,70 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
>  			     (struct rte_flow_action_set_mac, mac_addr)),
>  		.call = parse_vc_conf,
>  	},
> +	[ACTION_INC_TCP_SEQ] = {
> +		.name = "inc_tcp_seq",
> +		.help = "increase TCP sequence number",
> +		.priv = PRIV_ACTION(INC_TCP_SEQ,
> +			sizeof(struct rte_flow_action_modify_tcp_seq)),
> +		.next = NEXT(action_inc_tcp_seq),
> +		.call = parse_vc,
> +	},
> +	[ACTION_INC_TCP_SEQ_VALUE] = {
> +		.name = "value",
> +		.help = "the value to increase TCP sequence number by",
> +		.next = NEXT(action_inc_tcp_seq, NEXT_ENTRY(UNSIGNED)),
> +		.args = ARGS(ARGS_ENTRY_HTON
> +			(struct rte_flow_action_modify_tcp_seq, value)),

You may need to remove HTON here depending on the chosen endian for the
API, see my comments on previous patch.

> +		.call = parse_vc_conf,
> +	},
> +	[ACTION_DEC_TCP_SEQ] = {
> +		.name = "dec_tcp_seq",
> +		.help = "decrease TCP sequence number",
> +		.priv = PRIV_ACTION(DEC_TCP_SEQ,
> +			sizeof(struct rte_flow_action_modify_tcp_seq)),
> +		.next = NEXT(action_dec_tcp_seq),
> +		.call = parse_vc,
> +	},
> +	[ACTION_DEC_TCP_SEQ_VALUE] = {
> +		.name = "value",
> +		.help = "the value to decrease TCP sequence number by",
> +		.next = NEXT(action_dec_tcp_seq, NEXT_ENTRY(UNSIGNED)),
> +		.args = ARGS(ARGS_ENTRY_HTON
> +			(struct rte_flow_action_modify_tcp_seq, value)),

Same here.

> +		.call = parse_vc_conf,
> +	},
> +	[ACTION_INC_TCP_ACK] = {
> +		.name = "inc_tcp_ack",
> +		.help = "increase TCP acknowledgment number",
> +		.priv = PRIV_ACTION(INC_TCP_ACK,
> +			sizeof(struct rte_flow_action_modify_tcp_ack)),
> +		.next = NEXT(action_inc_tcp_ack),
> +		.call = parse_vc,
> +	},
> +	[ACTION_INC_TCP_ACK_VALUE] = {
> +		.name = "value",
> +		.help = "the value to increase TCP acknowledgment number by",
> +		.next = NEXT(action_inc_tcp_ack, NEXT_ENTRY(UNSIGNED)),
> +		.args = ARGS(ARGS_ENTRY_HTON
> +			(struct rte_flow_action_modify_tcp_ack, value)),

Ditto.

> +		.call = parse_vc_conf,
> +	},
> +	[ACTION_DEC_TCP_ACK] = {
> +		.name = "dec_tcp_ack",
> +		.help = "decrease TCP acknowledgment number",
> +		.priv = PRIV_ACTION(DEC_TCP_ACK,
> +			sizeof(struct rte_flow_action_modify_tcp_ack)),
> +		.next = NEXT(action_dec_tcp_ack),
> +		.call = parse_vc,
> +	},
> +	[ACTION_DEC_TCP_ACK_VALUE] = {
> +		.name = "value",
> +		.help = "the value to decrease TCP acknowledgment number by",
> +		.next = NEXT(action_dec_tcp_ack, NEXT_ENTRY(UNSIGNED)),
> +		.args = ARGS(ARGS_ENTRY_HTON
> +			(struct rte_flow_action_modify_tcp_ack, value)),

Ditto.

> +		.call = parse_vc_conf,
> +	},
>  };
>  
>  /** Remove and return last entry from argument stack. */
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 1a12da4..c6f8b2c 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3961,6 +3961,22 @@ This section lists supported actions and their attributes, if any.
>  
>    - ``mac_addr {MAC-48}``: new destination MAC address
>  
> +- ``inc_tcp_seq``: Increase sequence number in the outermost TCP header
> +
> +  - ``value {unsigned}``: Value to increase TCP sequence number by
> +
> +- ``dec_tcp_seq``: Decrease sequence number in the outermost TCP header
> +
> +  - ``value {unsigned}``: Value to decrease TCP sequence number by
> +
> +- ``inc_tcp_ack``: Increase acknowledgment number in the outermost TCP header
> +
> +  - ``value {unsigned}``: Value to increase TCP acknowledgment number by
> +
> +- ``dec_tcp_ack``: Decrease acknowledgment number in the outermost TCP header
> +
> +  - ``value {unsigned}``: Value to decrease TCP acknowledgment number by

Please add missing "." to each line.

> +
>  Destroying flow rules
>  ~~~~~~~~~~~~~~~~~~~~~
>  
> -- 
> 1.8.3.1
> 

-- 
Adrien Mazarguil
6WIND