From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dekel Peled Subject: Re: [PATCH 2/3] app/testpmd: add actions to modify TCP header fields Date: Sun, 31 Mar 2019 13:10:41 +0000 Message-ID: References: <1553177917-43297-1-git-send-email-dekelp@mellanox.com> <1553177917-43297-3-git-send-email-dekelp@mellanox.com> <20190329135857.GI4889@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "wenzhuo.lu@intel.com" , "jingjing.wu@intel.com" , "bernard.iremonger@intel.com" , Yongseok Koh , Shahaf Shuler , "dev@dpdk.org" , Ori Kam To: Adrien Mazarguil Return-path: Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50072.outbound.protection.outlook.com [40.107.5.72]) by dpdk.org (Postfix) with ESMTP id 1EA2B2B82 for ; Sun, 31 Mar 2019 15:10:43 +0200 (CEST) In-Reply-To: <20190329135857.GI4889@6wind.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Thanks, PSB. > -----Original Message----- > From: Adrien Mazarguil > Sent: Friday, March 29, 2019 4:59 PM > To: Dekel Peled > Cc: wenzhuo.lu@intel.com; jingjing.wu@intel.com; > bernard.iremonger@intel.com; Yongseok Koh ; > Shahaf Shuler ; dev@dpdk.org; Ori Kam > > Subject: Re: [PATCH 2/3] app/testpmd: add actions to modify TCP header > fields >=20 > 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 >=20 > I suggest to merge this patch into the previous one. No reason for testpm= d > to be updated separately. >=20 I prefer it in separate patch since it is part of different SW module. > Some comments below. >=20 > > --- > > 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[] =3D { > > + ACTION_INC_TCP_SEQ_VALUE, > > + ACTION_NEXT, > > + ZERO, > > +}; > > + > > +static const enum index action_dec_tcp_seq[] =3D { > > + ACTION_DEC_TCP_SEQ_VALUE, > > + ACTION_NEXT, > > + ZERO, > > +}; > > + > > +static const enum index action_inc_tcp_ack[] =3D { > > + ACTION_INC_TCP_ACK_VALUE, > > + ACTION_NEXT, > > + ZERO, > > +}; > > + > > +static const enum index action_dec_tcp_ack[] =3D { > > + 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 =3D parse_vc_conf, > > }, > > + [ACTION_INC_TCP_SEQ] =3D { > > + .name =3D "inc_tcp_seq", > > + .help =3D "increase TCP sequence number", > > + .priv =3D PRIV_ACTION(INC_TCP_SEQ, > > + sizeof(struct rte_flow_action_modify_tcp_seq)), > > + .next =3D NEXT(action_inc_tcp_seq), > > + .call =3D parse_vc, > > + }, > > + [ACTION_INC_TCP_SEQ_VALUE] =3D { > > + .name =3D "value", > > + .help =3D "the value to increase TCP sequence number by", > > + .next =3D NEXT(action_inc_tcp_seq, NEXT_ENTRY(UNSIGNED)), > > + .args =3D ARGS(ARGS_ENTRY_HTON > > + (struct rte_flow_action_modify_tcp_seq, value)), >=20 > You may need to remove HTON here depending on the chosen endian for > the API, see my comments on previous patch. >=20 > > + .call =3D parse_vc_conf, > > + }, > > + [ACTION_DEC_TCP_SEQ] =3D { > > + .name =3D "dec_tcp_seq", > > + .help =3D "decrease TCP sequence number", > > + .priv =3D PRIV_ACTION(DEC_TCP_SEQ, > > + sizeof(struct rte_flow_action_modify_tcp_seq)), > > + .next =3D NEXT(action_dec_tcp_seq), > > + .call =3D parse_vc, > > + }, > > + [ACTION_DEC_TCP_SEQ_VALUE] =3D { > > + .name =3D "value", > > + .help =3D "the value to decrease TCP sequence number by", > > + .next =3D NEXT(action_dec_tcp_seq, > NEXT_ENTRY(UNSIGNED)), > > + .args =3D ARGS(ARGS_ENTRY_HTON > > + (struct rte_flow_action_modify_tcp_seq, value)), >=20 > Same here. >=20 > > + .call =3D parse_vc_conf, > > + }, > > + [ACTION_INC_TCP_ACK] =3D { > > + .name =3D "inc_tcp_ack", > > + .help =3D "increase TCP acknowledgment number", > > + .priv =3D PRIV_ACTION(INC_TCP_ACK, > > + sizeof(struct rte_flow_action_modify_tcp_ack)), > > + .next =3D NEXT(action_inc_tcp_ack), > > + .call =3D parse_vc, > > + }, > > + [ACTION_INC_TCP_ACK_VALUE] =3D { > > + .name =3D "value", > > + .help =3D "the value to increase TCP acknowledgment number > by", > > + .next =3D NEXT(action_inc_tcp_ack, NEXT_ENTRY(UNSIGNED)), > > + .args =3D ARGS(ARGS_ENTRY_HTON > > + (struct rte_flow_action_modify_tcp_ack, value)), >=20 > Ditto. >=20 > > + .call =3D parse_vc_conf, > > + }, > > + [ACTION_DEC_TCP_ACK] =3D { > > + .name =3D "dec_tcp_ack", > > + .help =3D "decrease TCP acknowledgment number", > > + .priv =3D PRIV_ACTION(DEC_TCP_ACK, > > + sizeof(struct rte_flow_action_modify_tcp_ack)), > > + .next =3D NEXT(action_dec_tcp_ack), > > + .call =3D parse_vc, > > + }, > > + [ACTION_DEC_TCP_ACK_VALUE] =3D { > > + .name =3D "value", > > + .help =3D "the value to decrease TCP acknowledgment number > by", > > + .next =3D NEXT(action_dec_tcp_ack, > NEXT_ENTRY(UNSIGNED)), > > + .args =3D ARGS(ARGS_ENTRY_HTON > > + (struct rte_flow_action_modify_tcp_ack, value)), >=20 > Ditto. >=20 > > + .call =3D 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 >=20 > Please add missing "." to each line. Will add. >=20 > > + > > Destroying flow rules > > ~~~~~~~~~~~~~~~~~~~~~ > > > > -- > > 1.8.3.1 > > >=20 > -- > Adrien Mazarguil > 6WIND