From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dekelp@mellanox.com>
Received: from EUR03-VE1-obe.outbound.protection.outlook.com
(mail-eopbgr50062.outbound.protection.outlook.com [40.107.5.62])
by dpdk.org (Postfix) with ESMTP id 6BFFD11A4
for <dev@dpdk.org>; Sun, 31 Mar 2019 15:09:48 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
s=selector1;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=U326GmMIDKWqfHhw9V0C8Xo0Lp4BhlOJI3si9bpm0Xc=;
b=ukYvZMK6RUbQ7yFraRYsX1muiBpV6y/RnaphNkt1O9cgt+yZiYOQCWGUrT5NNI18e9ErePHEZ9QeAy+aKx7vAb7HwqWSZVtUNOYbzACeAuBxQhlB1bf6DSFqUc5fmZR0x6kb+artGxmCoxbKboxL4Eaae8LII8pmFSN0JMaeqK4=
Received: from VI1PR05MB4224.eurprd05.prod.outlook.com (52.133.12.13) by
VI1PR05MB3502.eurprd05.prod.outlook.com (10.170.239.32) with Microsoft SMTP
Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
15.20.1730.18; Sun, 31 Mar 2019 13:09:47 +0000
Received: from VI1PR05MB4224.eurprd05.prod.outlook.com
([fe80::bcb0:ed58:d76:cac]) by VI1PR05MB4224.eurprd05.prod.outlook.com
([fe80::bcb0:ed58:d76:cac%4]) with mapi id 15.20.1750.017; Sun, 31 Mar 2019
13:09:47 +0000
From: Dekel Peled <dekelp@mellanox.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
CC: "wenzhuo.lu@intel.com" <wenzhuo.lu@intel.com>, "jingjing.wu@intel.com"
<jingjing.wu@intel.com>, "bernard.iremonger@intel.com"
<bernard.iremonger@intel.com>, Yongseok Koh <yskoh@mellanox.com>, Shahaf
Shuler <shahafs@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>, Ori Kam
<orika@mellanox.com>
Thread-Topic: [PATCH 1/3] ethdev: add actions to modify TCP header fields
Thread-Index: AQHU3/FKRE77gvlxWk+bdOA5g+04ZKYiry8AgAL8YuA=
Date: Sun, 31 Mar 2019 13:09:46 +0000
Message-ID: <VI1PR05MB4224DE370723C0ADC4B01342B6540@VI1PR05MB4224.eurprd05.prod.outlook.com>
References: <1553177917-43297-1-git-send-email-dekelp@mellanox.com>
<1553177917-43297-2-git-send-email-dekelp@mellanox.com>
<20190329135850.GH4889@6wind.com>
In-Reply-To: <20190329135850.GH4889@6wind.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: spf=none (sender IP is )
smtp.mailfrom=dekelp@mellanox.com;
x-originating-ip: [193.47.165.251]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: df4ce010-0c3f-4869-b827-08d6b5da2671
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0; PCL:0;
RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020);
SRVR:VI1PR05MB3502;
x-ms-traffictypediagnostic: VI1PR05MB3502:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <VI1PR05MB3502AFAA6CDCEF0668562F71B6540@VI1PR05MB3502.eurprd05.prod.outlook.com>
x-forefront-prvs: 0993689CD1
x-forefront-antispam-report: SFV:NSPM;
SFS:(10009020)(39860400002)(366004)(346002)(396003)(136003)(376002)(189003)(199004)(13464003)(26005)(81166006)(5660300002)(9686003)(107886003)(66066001)(4326008)(86362001)(71190400001)(76176011)(55016002)(476003)(102836004)(305945005)(6436002)(53936002)(54906003)(7736002)(316002)(3846002)(25786009)(105586002)(6246003)(11346002)(256004)(14454004)(6506007)(6116002)(97736004)(81156014)(8936002)(6916009)(229853002)(52536014)(74316002)(68736007)(99286004)(33656002)(186003)(486006)(478600001)(71200400001)(446003)(8676002)(7696005)(106356001)(2906002)(53546011);
DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR05MB3502;
H:VI1PR05MB4224.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en;
PTR:InfoNoRecords; MX:1; A:1;
received-spf: None (protection.outlook.com: mellanox.com does not designate
permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: yT4XX0FJmgGWBpDOYPPOWteaaiBPhwFiWy9EmLvIcF+hdc1UTnjedPLXYfzxYzpgi4K7oVWBHg6+5TWomrSuvF5d+GlvuwR1QoituuraRJmLIGxQCQ+tTRZchzhBe+SMs5QXfOb+TgEQmGH6yX+CAFdk3nEWMWT6ucmpOKy5bcngN3Z9zEwhfLKOWXKpY47qf6z9jNlhxNwwgRCu8EiLi/ti3IobjGfML6rWtlJfW1lpnNfFDVvAZYWWJzI5yOIC1t6TvP4Cms++RhXhu0FiZiRUBdrWle4/wHVkpmtI6IRBvj11GAhfbaDfPl58sBYgG4jWiuQpGyAdMyb+n3b6JS0YpE4DwBE5brAEAQzijGqiWcc2e+Yp3MIHGu+i+zZfxRMuhgVjMD7ndIUl5gQ31THIZ6SwM5FBHmCwzhHUFxM=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: df4ce010-0c3f-4869-b827-08d6b5da2671
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Mar 2019 13:09:47.0187 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR05MB3502
Subject: Re: [dpdk-dev] [PATCH 1/3] ethdev: 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>
X-List-Received-Date: Sun, 31 Mar 2019 13:09:48 -0000
Thanks, PSB.
> -----Original Message-----
> From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Sent: Friday, March 29, 2019 4:59 PM
> To: Dekel Peled <dekelp@mellanox.com>
> Cc: wenzhuo.lu@intel.com; jingjing.wu@intel.com;
> bernard.iremonger@intel.com; Yongseok Koh <yskoh@mellanox.com>;
> Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org; Ori Kam
> <orika@mellanox.com>
> Subject: Re: [PATCH 1/3] ethdev: add actions to modify TCP header fields
>=20
> Hi Derek,
It's Dekel, not Derek.
>=20
> It's been a while since I last reviewed something, sorry it took so long.
> Some comments below.
>=20
> On Thu, Mar 21, 2019 at 04:18:35PM +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.
>=20
> Out of curiosity, are these upcoming/existing OpenFlow actions? If so you
> should consider prefixing them with "OF_", otherwise leave them as is.
Not related to OpenFlow.
>=20
> > Original work by Xiaoyu Min.
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> > doc/guides/prog_guide/rte_flow.rst | 72
> ++++++++++++++++++++++++++++++++++++++
> > lib/librte_ethdev/rte_flow.c | 8 +++++
> > lib/librte_ethdev/rte_flow.h | 60
> +++++++++++++++++++++++++++++++
> > 3 files changed, 140 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 0203f4f..bdb817a 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2345,6 +2345,78 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
> error will be returned.
> > | ``mac_addr`` | MAC address |
> > +--------------+---------------+
> >
> > +Action: ``INC_TCP_SEQ``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Increase sequence number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> I know this is already present for various SET* actions, but please do no=
t tie
> actions to the presence of specific pattern items at the API level. You c=
an
> describe that the lack of a TCP header in matched *traffic* results in
> unspecified behavior though.
I accept, will change it in all relevant locations.
>=20
> Then PMD documentation can describe that the lack of a TCP pattern item
> with this action results in RTE_FLOW_ERROR_TYPE_ACTION as a PMD-specific
> constraint.
>=20
> > +
> > +.. _table_rte_flow_action_inc_tcp_seq:
> > +
> > +.. table:: INC_TCP_SEQ
> > +
> > + +-----------+--------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D+
> > + | ``value`` | Value to increase TCP sequence number by |
> > + +-----------+--------------------------------------------+
>=20
> Nit: unnecessary empty space after "by".
Will remove.
>=20
> > +
> > +Action: ``DEC_TCP_SEQ``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Decrease sequence number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> Same comment as above.
>=20
> > +
> > +.. _table_rte_flow_action_dec_tcp_seq:
> > +
> > +.. table:: DEC_TCP_SEQ
> > +
> > + +-----------+--------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D+
> > + | ``value`` | Value to decrease TCP sequence number by |
> > + +-----------+--------------------------------------------+
> > +
> > +Action: ``INC_TCP_ACK``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Increase acknowledgment number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> Ditto.
>=20
> > +
> > +.. _table_rte_flow_action_inc_tcp_ack:
> > +
> > +.. table:: INC_TCP_ACK
> > +
> > + +-----------+--------------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D=3D+
> > + | ``value`` | Value to increase TCP acknowledgment number by |
> > + +-----------+--------------------------------------------------+
> > +
> > +Action: ``DEC_TCP_ACK``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Decrease acknowledgment number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> Ditto.
>=20
> > +
> > +.. _table_rte_flow_action_dec_tcp_ack:
> > +
> > +.. table:: DEC_TCP_ACK
> > +
> > + +-----------+--------------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D=3D+
> > + | ``value`` | Value to decrease TCP acknowledgment number by |
> > + +-----------+--------------------------------------------------+
> > +
> > Negative types
> > ~~~~~~~~~~~~~~
> >
> > diff --git a/lib/librte_ethdev/rte_flow.c
> > b/lib/librte_ethdev/rte_flow.c index 3277be1..589d0b9 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -143,6 +143,14 @@ struct rte_flow_desc_data {
> > MK_FLOW_ACTION(SET_TTL, sizeof(struct rte_flow_action_set_ttl)),
> > MK_FLOW_ACTION(SET_MAC_SRC, sizeof(struct
> rte_flow_action_set_mac)),
> > MK_FLOW_ACTION(SET_MAC_DST, sizeof(struct
> rte_flow_action_set_mac)),
> > + MK_FLOW_ACTION(INC_TCP_SEQ,
> > + sizeof(struct rte_flow_action_modify_tcp_seq)),
> > + MK_FLOW_ACTION(DEC_TCP_SEQ,
> > + sizeof(struct rte_flow_action_modify_tcp_seq)),
> > + MK_FLOW_ACTION(INC_TCP_ACK,
> > + sizeof(struct rte_flow_action_modify_tcp_ack)),
> > + MK_FLOW_ACTION(DEC_TCP_ACK,
> > + sizeof(struct rte_flow_action_modify_tcp_ack)),
> > };
> >
> > static int
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index c0fe879..74cd03e 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -1651,6 +1651,46 @@ enum rte_flow_action_type {
> > * See struct rte_flow_action_set_mac.
> > */
> > RTE_FLOW_ACTION_TYPE_SET_MAC_DST,
> > +
> > + /**
> > + * Increase sequence number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
>=20
> Ditto.
>=20
> > + *
> > + * See struct rte_flow_action_modify_tcp_seq
> > + */
> > + RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ,
> > +
> > + /**
> > + * Decrease sequence number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
>=20
> Ditto.
>=20
> > + *
> > + * See struct rte_flow_action_modify_tcp_seq
> > + */
> > + RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ,
> > +
> > + /**
> > + * Increase acknowledgment number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
> > + *
>=20
> Ditto.
>=20
> > + * See struct rte_flow_action_modify_tcp_ack
> > + */
> > + RTE_FLOW_ACTION_TYPE_INC_TCP_ACK,
> > +
> > + /**
> > + * Decrease acknowledgment number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
> > + *
>=20
> Ditto.
>=20
> > + * See struct rte_flow_action_modify_tcp_ack
> > + */
> > + RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK,
> > };
> >
> > /**
> > @@ -2122,6 +2162,26 @@ struct rte_flow_action_set_mac {
> > uint8_t mac_addr[ETHER_ADDR_LEN];
> > };
> >
> > +/**
>=20
> Experimental tag is missing.
Will add it here and elsewhere.
>=20
> > + * RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> > + *
> > + * Increase/Decrease outermost TCP's sequence number
>=20
> Suggestion:
>=20
> Increase/decrease outermost TCP sequence number.
Accepted, will change here and elsewhere.
>=20
> > + */
> > +struct rte_flow_action_modify_tcp_seq {
> > + rte_be32_t value;
>=20
> Field documentation is mandatory, e.g.:
>=20
> rte_be32_t value; /**< Value to add/subtract. */
Will add it.
>=20
> Beside, I'm not sure this value should be big endian since it's not store=
d as is
> in the TCP header; it's used by the host system to compute a new value.
It isn't used by the host, but sent to NIC as part of action.
>=20
> Also what about having another field to specify what needs to be done wit=
h
> this value (e.g. add/sub/set - in which case big endian makes sense) to
> reduce the number of new actions? Something like:
>=20
> struct rte_flow_action_mod_tcp_seq {
> enum rte_flow_action_mod_tcp_seq_op {
> RTE_FLOW_ACTION_MOD_TCP_SEQ_OP_ADD,
> RTE_FLOW_ACTION_MOD_TCP_SEQ_OP_SUB,
> RTE_FLOW_ACTION_MOD_TCP_SEQ_OP_SET,
> } op; /**< Operation to perform. */
> rte_be32_t value; /**< Value to use with operation. */ };
>=20
> > +};
> > +
> > +/**
>=20
> Experimental tag also missing.
>=20
> > + * RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> > + *
> > + * Increase/Decrease TCP's acknowledgment number.
>=20
> Suggestion:
>=20
> Increase/decrease outermost TCP acknowledgment number.
Accepted, will change.
>=20
> > + */
> > +struct rte_flow_action_modify_tcp_ack {
> > + rte_be32_t value;
>=20
> Field documentation also missing.
>=20
> > +};
> > +
> > /*
> > * Definition of a single action.
> > *
> > --
> > 1.8.3.1
> >
>=20
> --
> Adrien Mazarguil
> 6WIND
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 D7C60A00B9
for <public@inbox.dpdk.org>; Sun, 31 Mar 2019 15:09:50 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 0CA532B82;
Sun, 31 Mar 2019 15:09:50 +0200 (CEST)
Received: from EUR03-VE1-obe.outbound.protection.outlook.com
(mail-eopbgr50062.outbound.protection.outlook.com [40.107.5.62])
by dpdk.org (Postfix) with ESMTP id 6BFFD11A4
for <dev@dpdk.org>; Sun, 31 Mar 2019 15:09:48 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
s=selector1;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=U326GmMIDKWqfHhw9V0C8Xo0Lp4BhlOJI3si9bpm0Xc=;
b=ukYvZMK6RUbQ7yFraRYsX1muiBpV6y/RnaphNkt1O9cgt+yZiYOQCWGUrT5NNI18e9ErePHEZ9QeAy+aKx7vAb7HwqWSZVtUNOYbzACeAuBxQhlB1bf6DSFqUc5fmZR0x6kb+artGxmCoxbKboxL4Eaae8LII8pmFSN0JMaeqK4=
Received: from VI1PR05MB4224.eurprd05.prod.outlook.com (52.133.12.13) by
VI1PR05MB3502.eurprd05.prod.outlook.com (10.170.239.32) with Microsoft SMTP
Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
15.20.1730.18; Sun, 31 Mar 2019 13:09:47 +0000
Received: from VI1PR05MB4224.eurprd05.prod.outlook.com
([fe80::bcb0:ed58:d76:cac]) by VI1PR05MB4224.eurprd05.prod.outlook.com
([fe80::bcb0:ed58:d76:cac%4]) with mapi id 15.20.1750.017; Sun, 31 Mar 2019
13:09:47 +0000
From: Dekel Peled <dekelp@mellanox.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
CC: "wenzhuo.lu@intel.com" <wenzhuo.lu@intel.com>, "jingjing.wu@intel.com"
<jingjing.wu@intel.com>, "bernard.iremonger@intel.com"
<bernard.iremonger@intel.com>, Yongseok Koh <yskoh@mellanox.com>, Shahaf
Shuler <shahafs@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>, Ori Kam
<orika@mellanox.com>
Thread-Topic: [PATCH 1/3] ethdev: add actions to modify TCP header fields
Thread-Index: AQHU3/FKRE77gvlxWk+bdOA5g+04ZKYiry8AgAL8YuA=
Date: Sun, 31 Mar 2019 13:09:46 +0000
Message-ID:
<VI1PR05MB4224DE370723C0ADC4B01342B6540@VI1PR05MB4224.eurprd05.prod.outlook.com>
References: <1553177917-43297-1-git-send-email-dekelp@mellanox.com>
<1553177917-43297-2-git-send-email-dekelp@mellanox.com>
<20190329135850.GH4889@6wind.com>
In-Reply-To: <20190329135850.GH4889@6wind.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: spf=none (sender IP is )
smtp.mailfrom=dekelp@mellanox.com;
x-originating-ip: [193.47.165.251]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: df4ce010-0c3f-4869-b827-08d6b5da2671
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0; PCL:0;
RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020);
SRVR:VI1PR05MB3502;
x-ms-traffictypediagnostic: VI1PR05MB3502:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <VI1PR05MB3502AFAA6CDCEF0668562F71B6540@VI1PR05MB3502.eurprd05.prod.outlook.com>
x-forefront-prvs: 0993689CD1
x-forefront-antispam-report: SFV:NSPM;
SFS:(10009020)(39860400002)(366004)(346002)(396003)(136003)(376002)(189003)(199004)(13464003)(26005)(81166006)(5660300002)(9686003)(107886003)(66066001)(4326008)(86362001)(71190400001)(76176011)(55016002)(476003)(102836004)(305945005)(6436002)(53936002)(54906003)(7736002)(316002)(3846002)(25786009)(105586002)(6246003)(11346002)(256004)(14454004)(6506007)(6116002)(97736004)(81156014)(8936002)(6916009)(229853002)(52536014)(74316002)(68736007)(99286004)(33656002)(186003)(486006)(478600001)(71200400001)(446003)(8676002)(7696005)(106356001)(2906002)(53546011);
DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR05MB3502;
H:VI1PR05MB4224.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en;
PTR:InfoNoRecords; MX:1; A:1;
received-spf: None (protection.outlook.com: mellanox.com does not designate
permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: yT4XX0FJmgGWBpDOYPPOWteaaiBPhwFiWy9EmLvIcF+hdc1UTnjedPLXYfzxYzpgi4K7oVWBHg6+5TWomrSuvF5d+GlvuwR1QoituuraRJmLIGxQCQ+tTRZchzhBe+SMs5QXfOb+TgEQmGH6yX+CAFdk3nEWMWT6ucmpOKy5bcngN3Z9zEwhfLKOWXKpY47qf6z9jNlhxNwwgRCu8EiLi/ti3IobjGfML6rWtlJfW1lpnNfFDVvAZYWWJzI5yOIC1t6TvP4Cms++RhXhu0FiZiRUBdrWle4/wHVkpmtI6IRBvj11GAhfbaDfPl58sBYgG4jWiuQpGyAdMyb+n3b6JS0YpE4DwBE5brAEAQzijGqiWcc2e+Yp3MIHGu+i+zZfxRMuhgVjMD7ndIUl5gQ31THIZ6SwM5FBHmCwzhHUFxM=
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: df4ce010-0c3f-4869-b827-08d6b5da2671
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Mar 2019 13:09:47.0187 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR05MB3502
Subject: Re: [dpdk-dev] [PATCH 1/3] ethdev: 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: <20190331130946.7jLXFSHGu4QFOXK4LeG6EPGg6BI5thXOUyvA9GM_6do@z>
Thanks, PSB.
> -----Original Message-----
> From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Sent: Friday, March 29, 2019 4:59 PM
> To: Dekel Peled <dekelp@mellanox.com>
> Cc: wenzhuo.lu@intel.com; jingjing.wu@intel.com;
> bernard.iremonger@intel.com; Yongseok Koh <yskoh@mellanox.com>;
> Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org; Ori Kam
> <orika@mellanox.com>
> Subject: Re: [PATCH 1/3] ethdev: add actions to modify TCP header fields
>=20
> Hi Derek,
It's Dekel, not Derek.
>=20
> It's been a while since I last reviewed something, sorry it took so long.
> Some comments below.
>=20
> On Thu, Mar 21, 2019 at 04:18:35PM +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.
>=20
> Out of curiosity, are these upcoming/existing OpenFlow actions? If so you
> should consider prefixing them with "OF_", otherwise leave them as is.
Not related to OpenFlow.
>=20
> > Original work by Xiaoyu Min.
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> > doc/guides/prog_guide/rte_flow.rst | 72
> ++++++++++++++++++++++++++++++++++++++
> > lib/librte_ethdev/rte_flow.c | 8 +++++
> > lib/librte_ethdev/rte_flow.h | 60
> +++++++++++++++++++++++++++++++
> > 3 files changed, 140 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 0203f4f..bdb817a 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2345,6 +2345,78 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
> error will be returned.
> > | ``mac_addr`` | MAC address |
> > +--------------+---------------+
> >
> > +Action: ``INC_TCP_SEQ``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Increase sequence number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> I know this is already present for various SET* actions, but please do no=
t tie
> actions to the presence of specific pattern items at the API level. You c=
an
> describe that the lack of a TCP header in matched *traffic* results in
> unspecified behavior though.
I accept, will change it in all relevant locations.
>=20
> Then PMD documentation can describe that the lack of a TCP pattern item
> with this action results in RTE_FLOW_ERROR_TYPE_ACTION as a PMD-specific
> constraint.
>=20
> > +
> > +.. _table_rte_flow_action_inc_tcp_seq:
> > +
> > +.. table:: INC_TCP_SEQ
> > +
> > + +-----------+--------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D+
> > + | ``value`` | Value to increase TCP sequence number by |
> > + +-----------+--------------------------------------------+
>=20
> Nit: unnecessary empty space after "by".
Will remove.
>=20
> > +
> > +Action: ``DEC_TCP_SEQ``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Decrease sequence number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> Same comment as above.
>=20
> > +
> > +.. _table_rte_flow_action_dec_tcp_seq:
> > +
> > +.. table:: DEC_TCP_SEQ
> > +
> > + +-----------+--------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D+
> > + | ``value`` | Value to decrease TCP sequence number by |
> > + +-----------+--------------------------------------------+
> > +
> > +Action: ``INC_TCP_ACK``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Increase acknowledgment number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> Ditto.
>=20
> > +
> > +.. _table_rte_flow_action_inc_tcp_ack:
> > +
> > +.. table:: INC_TCP_ACK
> > +
> > + +-----------+--------------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D=3D+
> > + | ``value`` | Value to increase TCP acknowledgment number by |
> > + +-----------+--------------------------------------------------+
> > +
> > +Action: ``DEC_TCP_ACK``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Decrease acknowledgment number in the outermost TCP header.
> > +
> > +It must be used with a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern
> item.
> > +Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>=20
> Ditto.
>=20
> > +
> > +.. _table_rte_flow_action_dec_tcp_ack:
> > +
> > +.. table:: DEC_TCP_ACK
> > +
> > + +-----------+--------------------------------------------------+
> > + | Field | Value |
> > +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D=3D+
> > + | ``value`` | Value to decrease TCP acknowledgment number by |
> > + +-----------+--------------------------------------------------+
> > +
> > Negative types
> > ~~~~~~~~~~~~~~
> >
> > diff --git a/lib/librte_ethdev/rte_flow.c
> > b/lib/librte_ethdev/rte_flow.c index 3277be1..589d0b9 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -143,6 +143,14 @@ struct rte_flow_desc_data {
> > MK_FLOW_ACTION(SET_TTL, sizeof(struct rte_flow_action_set_ttl)),
> > MK_FLOW_ACTION(SET_MAC_SRC, sizeof(struct
> rte_flow_action_set_mac)),
> > MK_FLOW_ACTION(SET_MAC_DST, sizeof(struct
> rte_flow_action_set_mac)),
> > + MK_FLOW_ACTION(INC_TCP_SEQ,
> > + sizeof(struct rte_flow_action_modify_tcp_seq)),
> > + MK_FLOW_ACTION(DEC_TCP_SEQ,
> > + sizeof(struct rte_flow_action_modify_tcp_seq)),
> > + MK_FLOW_ACTION(INC_TCP_ACK,
> > + sizeof(struct rte_flow_action_modify_tcp_ack)),
> > + MK_FLOW_ACTION(DEC_TCP_ACK,
> > + sizeof(struct rte_flow_action_modify_tcp_ack)),
> > };
> >
> > static int
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index c0fe879..74cd03e 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -1651,6 +1651,46 @@ enum rte_flow_action_type {
> > * See struct rte_flow_action_set_mac.
> > */
> > RTE_FLOW_ACTION_TYPE_SET_MAC_DST,
> > +
> > + /**
> > + * Increase sequence number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
>=20
> Ditto.
>=20
> > + *
> > + * See struct rte_flow_action_modify_tcp_seq
> > + */
> > + RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ,
> > +
> > + /**
> > + * Decrease sequence number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
>=20
> Ditto.
>=20
> > + *
> > + * See struct rte_flow_action_modify_tcp_seq
> > + */
> > + RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ,
> > +
> > + /**
> > + * Increase acknowledgment number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
> > + *
>=20
> Ditto.
>=20
> > + * See struct rte_flow_action_modify_tcp_ack
> > + */
> > + RTE_FLOW_ACTION_TYPE_INC_TCP_ACK,
> > +
> > + /**
> > + * Decrease acknowledgment number in the outermost TCP header.
> > + *
> > + * If flow pattern does not define a valid
> RTE_FLOW_ITEM_TYPE_TCP,
> > + * the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
> > + *
>=20
> Ditto.
>=20
> > + * See struct rte_flow_action_modify_tcp_ack
> > + */
> > + RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK,
> > };
> >
> > /**
> > @@ -2122,6 +2162,26 @@ struct rte_flow_action_set_mac {
> > uint8_t mac_addr[ETHER_ADDR_LEN];
> > };
> >
> > +/**
>=20
> Experimental tag is missing.
Will add it here and elsewhere.
>=20
> > + * RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> > + *
> > + * Increase/Decrease outermost TCP's sequence number
>=20
> Suggestion:
>=20
> Increase/decrease outermost TCP sequence number.
Accepted, will change here and elsewhere.
>=20
> > + */
> > +struct rte_flow_action_modify_tcp_seq {
> > + rte_be32_t value;
>=20
> Field documentation is mandatory, e.g.:
>=20
> rte_be32_t value; /**< Value to add/subtract. */
Will add it.
>=20
> Beside, I'm not sure this value should be big endian since it's not store=
d as is
> in the TCP header; it's used by the host system to compute a new value.
It isn't used by the host, but sent to NIC as part of action.
>=20
> Also what about having another field to specify what needs to be done wit=
h
> this value (e.g. add/sub/set - in which case big endian makes sense) to
> reduce the number of new actions? Something like:
>=20
> struct rte_flow_action_mod_tcp_seq {
> enum rte_flow_action_mod_tcp_seq_op {
> RTE_FLOW_ACTION_MOD_TCP_SEQ_OP_ADD,
> RTE_FLOW_ACTION_MOD_TCP_SEQ_OP_SUB,
> RTE_FLOW_ACTION_MOD_TCP_SEQ_OP_SET,
> } op; /**< Operation to perform. */
> rte_be32_t value; /**< Value to use with operation. */ };
>=20
> > +};
> > +
> > +/**
>=20
> Experimental tag also missing.
>=20
> > + * RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> > + *
> > + * Increase/Decrease TCP's acknowledgment number.
>=20
> Suggestion:
>=20
> Increase/decrease outermost TCP acknowledgment number.
Accepted, will change.
>=20
> > + */
> > +struct rte_flow_action_modify_tcp_ack {
> > + rte_be32_t value;
>=20
> Field documentation also missing.
>=20
> > +};
> > +
> > /*
> > * Definition of a single action.
> > *
> > --
> > 1.8.3.1
> >
>=20
> --
> Adrien Mazarguil
> 6WIND