From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00068.outbound.protection.outlook.com [40.107.0.68]) by dpdk.org (Postfix) with ESMTP id 6D4741B1C5 for ; Wed, 20 Dec 2017 17:19:45 +0100 (CET) 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; bh=QYY0FUVXjzEywUKvxRL2TBYEgr/84FX4Eh1bgUVfHik=; b=q4aI/VEF356b6r21EL5sdPEN8JEjZOMsqDZcX10ve8LWJmlbM/R7yGX3Z3QlhQoTntw2imQYKeY7I57HjuMSSyxtJ6qAB6fF/ZReRQHMXHd9qHBQSesmJgfD/Ti3keqsMrob+zvpy9XBxOa0sqTurE+HI7e5bt6mhRAfo/BHiYo= Received: from HE1PR0501MB2235.eurprd05.prod.outlook.com (10.168.33.150) by HE1PR0501MB2748.eurprd05.prod.outlook.com (10.172.125.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.282.5; Wed, 20 Dec 2017 16:19:42 +0000 Received: from HE1PR0501MB2235.eurprd05.prod.outlook.com ([fe80::1590:8128:d324:1a4f]) by HE1PR0501MB2235.eurprd05.prod.outlook.com ([fe80::1590:8128:d324:1a4f%17]) with mapi id 15.20.0323.018; Wed, 20 Dec 2017 16:19:42 +0000 From: Boris Pismenny To: =?iso-8859-1?Q?N=E9lio_Laranjeiro?= , "Anoob Joseph" , Sergio Gonzalez Monroy , Radu Nicolau , Aviad Yehezkel , Liran Liss CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v3 2/2] examples/ipsec-secgw: add target queues in flow actions Thread-Index: AQHTc0bMrrSePSz8tEuq0+Dca0YdjKM/uDwAgAAFeYCAAAlbAIABDSQAgAA4QICAABq9gIAAFR2AgAAQm4CAAA8LAIALFNjw Date: Wed, 20 Dec 2017 16:19:42 +0000 Message-ID: References: <5d3fdd0c05d5f8afd3f8e38ca03eaf25187d5c98.1513000931.git.nelio.laranjeiro@6wind.com> <5777791b-3dd6-f746-aa37-d572c108f042@caviumnetworks.com> <20171212134456.4x3uaus2poovddlf@laranjeiro-vm.dev.6wind.com> <20171212143800.ggdtdfnbknttr45g@laranjeiro-vm.dev.6wind.com> <047cadcf-13dc-5368-4ad5-a27ff25c42f8@caviumnetworks.com> <20171213100237.uvpbg2qewhxqgaln@laranjeiro-vm.dev.6wind.com> <817bec1f-4bff-ebdc-07b4-f8f24ec2084a@caviumnetworks.com> <20171213125353.2zyllxk7pwkncm76@laranjeiro-vm.dev.6wind.com> <577d100a-d021-1219-89e3-721e02b77b90@caviumnetworks.com> <20171213144710.tyqvcvpkrymzqriv@laranjeiro-vm.dev.6wind.com> In-Reply-To: <20171213144710.tyqvcvpkrymzqriv@laranjeiro-vm.dev.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=borisp@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; HE1PR0501MB2748; 6:Gy7QEjryfHi9fD9dc0SWRPjTGcbisVmVizRY5tF9iVFLNrKXWFL0ms7V1JLUtyFGCVik97mkhqzywlwBX240viVcjZDpNN8fe43kWqb86+ppa1aRugQCFVyh/0QMdcRZNHhNvk4nqinGrW7lxGJOLvemsDg76mr5eXxgDC380EpiRYXXyW5p1xcTii3xtomiPRqbtKHlAamk5DQ/uQYU3BzGJ27VAz3VHqn8KG1bWPZMEkJdS3PRq4MAya4ETeYRY6QCUDLcZtpc7DeO5BmLNAfABchi49MKV9fJTSlxNp3N3m2c3GH0E4FMrlAgsBmSo2R4TS92ekq0Q8FqN16+FDhJeScnFnOQc3wjmVhUBjE=; 5:OViGZImaVb5tcackaBMbbpZ0hSvakbzD00XiHJMc0naGDejEjmFyXfhpCvneWMtzAzWPIFf0dkIdtRiNeYf6XpSUWvBquNzRN2vt/69yqYcphBvJAjKgdIur4WRRBTkad9NBlOBV1Cw7Dx8uR3PAEGiZpwiW8hHaZgiIrgaDdgk=; 24:jSt2cFQBXZsmk00LTljxAjYvefLG6z6KvMW+iQtLYT08FP94gOtnqupLTyGPJ6C4quMdvI0Qgiz67IYRaw/gxBrMlgq0+YDQ84z6g+fCiZY=; 7:vIX6Y44jMBZ1nfuAkMlLOrErDK1TkYnnnJlHNM71KRX6RDvuoJMFH9xIf2NvkCoPqnoC8HyZp6Gls6BNahr4BO5AVstf04/kyndVjR9sgTwvwAxAgiBrPoF7RsAX8YZGiLIv1krIO/zF5vSMS2P6hvpzwzS+u4D5kzvuRHOsoDWIOd3W6CG+U778KYEilR4GSrIeJP4OpMfL3FcBR5+qgEdJLVa0zCX3dv3iJ+qXYrwO73ejlrXvi46FrF1UpTcN x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: de960e04-7e0b-4755-455e-08d547c579ff x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(5600026)(4604075)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(48565401081)(2017052603307)(7153060); SRVR:HE1PR0501MB2748; x-ms-traffictypediagnostic: HE1PR0501MB2748: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(278428928389397)(192374486261705)(17755550239193); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040470)(2401047)(5005006)(8121501046)(10201501046)(3002001)(93006095)(93001095)(3231023)(6055026)(6041268)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123562045)(20161123558120)(20161123560045)(6072148)(201708071742011); SRVR:HE1PR0501MB2748; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:HE1PR0501MB2748; x-forefront-prvs: 0527DFA348 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(39380400002)(396003)(366004)(346002)(376002)(52314003)(24454002)(199004)(189003)(5660300001)(3660700001)(6246003)(3280700002)(2900100001)(86362001)(478600001)(8936002)(6436002)(53936002)(55016002)(59450400001)(9686003)(7696005)(7736002)(2906002)(8676002)(81166006)(81156014)(6506007)(3846002)(53546011)(102836003)(6116002)(25786009)(74316002)(4326008)(99286004)(305945005)(97736004)(229853002)(6636002)(76176011)(2950100002)(14454004)(68736007)(106356001)(93886005)(110136005)(33656002)(316002)(5250100002)(105586002)(66066001); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR0501MB2748; H:HE1PR0501MB2235.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: de960e04-7e0b-4755-455e-08d547c579ff X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Dec 2017 16:19:42.1663 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0501MB2748 Subject: Re: [dpdk-dev] [PATCH v3 2/2] examples/ipsec-secgw: add target queues in flow actions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Dec 2017 16:19:45 -0000 Hi, > On Wed, Dec 13, 2017 at 07:14:19PM, Nelio Laranjeiro wrote: >=20 > Hi, >=20 > On Wed, Dec 13, 2017 at 07:23:19PM +0530, Anoob Joseph wrote: > > Hi Nelio, > > > > > > On 12/13/2017 06:23 PM, Nelio Laranjeiro wrote: > > > Hi Anoob, > > > > > > On Wed, Dec 13, 2017 at 05:08:19PM +0530, Anoob Joseph wrote: > > > > Hi Nelio, > > > > > > > > > > > > On 12/13/2017 03:32 PM, Nelio Laranjeiro wrote: > > > > > Hi Anoob, > > > > > > > > > > On Wed, Dec 13, 2017 at 12:11:18PM +0530, Anoob Joseph wrote: > > > > > > Hi Nelio, > > > > > > > > > > > > > > > > > > On 12/12/2017 08:08 PM, Nelio Laranjeiro wrote: > > > > > > > Hi Anoob, > > > > > > > > > > > > > > On Tue, Dec 12, 2017 at 07:34:31PM +0530, Anoob Joseph wrote: > > > > > > > > Hi Nelio, > > > > > > > > > > > > > > > > > > > > > > > > On 12/12/2017 07:14 PM, Nelio Laranjeiro wrote: > > > > > > > > > Hi Anoob, > > > > > > > > > > > > > > > > > > On Tue, Dec 12, 2017 at 06:13:08PM +0530, Anoob Joseph > wrote: > > > > > > > > > > Hi Nelio, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 12/11/2017 07:34 PM, Nelio Laranjeiro wrote: > > > > > > > > > > > Mellanox INNOVA NIC needs to have final target queue > > > > > > > > > > > actions to perform inline crypto. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Nelio Laranjeiro > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > Changes in v3: > > > > > > > > > > > > > > > > > > > > > > * removed PASSTHRU test for ingress. > > > > > > > > > > > * removed check on configured queues for the qu= eue > action. > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > > > > > > > > > > > > * Test the rule by PASSTHRU/RSS/QUEUE and apply= the > first one validated. > > > > > > > > > > > --- > > > > > > > > > > > examples/ipsec-secgw/ipsec.c | 57 > ++++++++++++++++++++++++++++++++++++++++++-- > > > > > > > > > > > examples/ipsec-secgw/ipsec.h | 2 +- > > > > > > > > > > > 2 files changed, 56 insertions(+), 3 > > > > > > > > > > > deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/examples/ipsec-secgw/ipsec.c > > > > > > > > > > > b/examples/ipsec-secgw/ipsec.c index > > > > > > > > > > > 17bd7620d..1b8b251c8 100644 > > > > > > > > > > > --- a/examples/ipsec-secgw/ipsec.c > > > > > > > > > > > +++ b/examples/ipsec-secgw/ipsec.c > > > > > > > > > > > @@ -142,6 +142,7 @@ create_session(struct ipsec_ctx > *ipsec_ctx, struct ipsec_sa *sa) > > > > > > > > > > > > rte_eth_dev_get_sec_ctx( > > > > > > > > > > > sa- > >portid); > > > > > > > > > > > const struct rte_security_capability > > > > > > > > > > > *sec_cap; > > > > > > > > > > > + int ret =3D 0; > > > > > > > > > > > sa->sec_session =3D > rte_security_session_create(ctx, > > > > > > > > > > > &sess_conf, > ipsec_ctx->session_pool); @@ > > > > > > > > > > > -201,15 +202,67 @@ create_session(struct ipsec_ctx > *ipsec_ctx, struct ipsec_sa *sa) > > > > > > > > > > > sa->action[0].type =3D > RTE_FLOW_ACTION_TYPE_SECURITY; > > > > > > > > > > > sa->action[0].conf =3D sa->sec_session; > > > > > > > > > > > - sa->action[1].type =3D > RTE_FLOW_ACTION_TYPE_END; > > > > > > > > > > > - > > > > > > > > > > > sa->attr.egress =3D (sa->direction =3D=3D > > > > > > > > > > > > RTE_SECURITY_IPSEC_SA_DIR_EGRESS); > > > > > > > > > > > sa->attr.ingress =3D (sa->direction =3D=3D > > > > > > > > > > > > RTE_SECURITY_IPSEC_SA_DIR_INGRESS); > > > > > > > > > > > + if (sa->attr.ingress) { > > > > > > > > > > > + uint8_t rss_key[40]; > > > > > > > > > > > + struct rte_eth_rss_conf > rss_conf =3D { > > > > > > > > > > > + .rss_key =3D rss_key, > > > > > > > > > > > + .rss_key_len =3D 40, > > > > > > > > > > > + }; > > > > > > > > > > > + struct rte_eth_dev > *eth_dev; > > > > > > > > > > > + union { > > > > > > > > > > > + struct > rte_flow_action_rss rss; > > > > > > > > > > > + struct { > > > > > > > > > > > + const struct > rte_eth_rss_conf *rss_conf; > > > > > > > > > > > + uint16_t num; > > > > > > > > > > > + uint16_t > queue[RTE_MAX_QUEUES_PER_PORT]; > > > > > > > > > > > + } local; > > > > > > > > > > > + } action_rss; > > > > > > > > > > > + unsigned int i; > > > > > > > > > > > + unsigned int j; > > > > > > > > > > > + > > > > > > > > > > > + sa->action[2].type =3D > RTE_FLOW_ACTION_TYPE_END; > > > > > > > > > > > + /* Try RSS. */ > > > > > > > > > > > + sa->action[1].type =3D > RTE_FLOW_ACTION_TYPE_RSS; > > > > > > > > > > > + sa->action[1].conf =3D > &action_rss; > > > > > > > > > > > + eth_dev =3D ctx->device; > > > > > > > > > > > + > rte_eth_dev_rss_hash_conf_get(sa->portid, > > > > > > > > > > > + > &rss_conf); > > > > > > > > > > > + for (i =3D 0, j =3D 0; > > > > > > > > > > > + i < eth_dev->data- > >nb_rx_queues; ++i) > > > > > > > > > > > + if (eth_dev->data- > >rx_queues[i]) > > > > > > > > > > > + > action_rss.local.queue[j++] =3D i; > > > > > > > > > > > + action_rss.local.num =3D j; > > > > > > > > > > > + action_rss.local.rss_conf =3D > &rss_conf; > > > > > > > > > > > + ret =3D rte_flow_validate(sa- > >portid, &sa->attr, > > > > > > > > > > > + sa- > >pattern, sa->action, > > > > > > > > > > > + &err); > > > > > > > > > > > + if (!ret) > > > > > > > > > > > + goto flow_create; > > > > > > > > > > > + /* Try Queue. */ > > > > > > > > > > > + sa->action[1].type =3D > RTE_FLOW_ACTION_TYPE_QUEUE; > > > > > > > > > > > + sa->action[1].conf =3D > > > > > > > > > > > + &(struct > rte_flow_action_queue){ > > > > > > > > > > > + .index =3D 0, > > > > > > > > > > > + }; > > > > > > > > > > > + ret =3D rte_flow_validate(sa- > >portid, &sa->attr, > > > > > > > > > > > + sa- > >pattern, sa->action, > > > > > > > > > > > + &err); > > > > > > > > > > > + if (ret) > > > > > > > > > > > + goto > flow_create_failure; > > > > > > > > > > > + } else { > > > > > > > > > > > + sa->action[1].type =3D > > > > > > > > > > > + > RTE_FLOW_ACTION_TYPE_PASSTHRU; > > > > > > > > > > > + sa->action[2].type =3D > RTE_FLOW_ACTION_TYPE_END; > > > > > > > > > > We would need flow validate here also. And, for > > > > > > > > > > egress, the application will be able to set metadata > > > > > > > > > > (set_pkt_metadata API) per packet. So flow may not be > > > > > > > > > > required for such cases. But if the flow create fails, > > > > > > > > > > the session create would also fail. It might be better = if we > check whether the PMD would need metadata (part of the sec_cap- > >ol_flags). > > > > > > > > > Seems what you are describing is outside of this scope > > > > > > > > > which is only related to correctly implement the generic > > > > > > > > > flow API with terminal actions. > > > > > > > > Since SECURITY+PASSTHRU won't be terminal, this code > > > > > > > > segment might be misleading. > > > > > > > Well, I don't mind adding an extra verification even if the > > > > > > > create should fail if the validate fails, as there is no > > > > > > > other option it is just like adding another if statement > > > > > > > considering the validate() cannot guarantee the flow will > > > > > > > be created(), other errors like ENOMEM are still possible in = the > creation stage. > > > > > > Good point. I was thinking of a scenario when flow for egress > > > > > > itself would be optional. > > > > > > > > > I'll suggest to add it in another patch. > > > > > > > > > > > > > > > > > > Anyway, the flow validate is useful in the ingress to > > > > > > > > > select the best behavior RSS/Queue, if the flow validate > > > > > > > > > may fail, the flow create should also fail for the same r= easons. > > > > > > > > > > > > > > > > > > > If the driver doesn't need metadata and the flow > > > > > > > > > > create fails, then the create session should fail. Any > thoughts? > > > > > > > > > How the create_session() can fail without having all the > > > > > > > > > informations (pattern, metadata, ...) the application wan= ts to > offload? > > > > > > > > Is flow mandatory for the egress traffic? My understanding = is, > it's not. > > > > > > > > "set_pkt_metadata" API gives application the ability to do > > > > > > > > the lookup and pass the info along with the packet. In > > > > > > > > such cases, flow creation is not necessary. > > > > > > > Some NIC need to apply a flow rule for Egress and they don't > > > > > > > need metadata for the packet. > > > > > > Understood. In that case, what I proposed could be a separate > > > > > > patch. The ingress path is proper with this patch, but we can > > > > > > keep egress open for improvements. > > > > > What do you mean with "keep egrees open for improvements"? > > > > In egress side, this set of flow actions won't be having any > > > > terminating action. And addition of PASSTHRU won't be required, as = it > will be implicit. > > > Flow API does not define any behavior on Egress. We have to define i= t. > > Understood. > > > > > > > Creating flow for egress would allow hardware to perform the SA > > > > lookup. But we cannot remove the lookup in application, as it's > > > > the SA which has the information whether the packet need to be > > > > completely offloaded. Once this lookup is done, this information > > > > could be communicated to hardware using the set_pkt_metadata. > > > > > > > > This will eliminate the second lookup in the hardware. So the flow > > > > could be optional. The current patch assumes flow is mandatory for > > > > egress as well. > > > > For Cavium hardware, egress side flow is not required and we will > > > > be using "set_pkt_metadata" API. May be Radu can give his thoughts > on this. > > > Got it, what is missing here is a verification on the sa->ol_flags > > > and only use the rte_flow for RTE_SECURITY_TX_HW_TRAILER_OFFLOAD > as > > > other NICs are using the RTE_SECURITY_TX_OLOAD_NEED_MDATA. > > Precisely. >=20 > I'll make the changes discussed here, splitting this patch into ingress/E= gress > and use the of_flags. >=20 > > > Do you know why such difference is not hidden by the library? It > > > won't help application which will need to have different > > > configuration path depending on the NIC capabilities. > > I can only speculate the reasons. I think, application will have to > > know the NIC capabilities as the usage of rte_flow would be a one time > > configuration while using set_pkt_metadata would be per packet API > > call. If library is to hide this, it would incur unwanted checks in the= data > path. >=20 > Why not embedding all the configuration part necessary for all PMD (as th= is > example is making though this modifications) inside rte_security library = and > in et_dev add a new Tx burst API with another array containing the metada= ta > for the each packet. >=20 > PMD who needs such metadata have it along with the packet they are > processing, others can ignore it. >=20 > From an application point of view, this become transparent and friendly, = one > function to set the offload request, one function to send packets and > another one in Rx for the symmetry if necessary. >=20 The call to rte_flow is necessary both on Tx and Rx. The reason for using setting the flow on Tx is to provide the packet header= fields which are required to identify the SA. This use enables extensibility. For = instance, we could describe ESP over VXLAN simply be providing the appropriate rte_fl= ow configuration. If the flow creation fails, then this is not supported. A similar use-case is the use of VLANs with ESP traffic. I understand that it is possible to use the metadata to provide application= s with all information. But, a user is better served by an API that indicates an error= when the control operation is executed (e.g. rte_flow) and not during the data-path = (e.g. metadata). Currently, the SPI is provided in the rte_security_session_conf. I think th= is is a mistake. The rte_security session is not using this field and it should be provided = only via rte_flow. This field should be provided only when protocol offload is used. Moreover, according to the IPsec RFC, the SPI can be used to identify the S= A in conjunction with the destination IP and the source IP. Both are provided in rte_flow. If you= decide to use only the information from rte_security_session_conf to identify the SA, then you are= limiting yourself. Best, Boris.