From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0F0DAA00C2; Thu, 23 Apr 2020 11:09:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DEF401D562; Thu, 23 Apr 2020 11:09:56 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by dpdk.org (Postfix) with ESMTP id 77FB01C294; Thu, 23 Apr 2020 11:09:55 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 03N95h9k031206; Thu, 23 Apr 2020 02:09:54 -0700 Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-0016f401.pphosted.com with ESMTP id 30jd016pnr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 23 Apr 2020 02:09:54 -0700 Received: from m0045851.ppops.net (m0045851.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 03N969MQ031354; Thu, 23 Apr 2020 02:09:54 -0700 Received: from sc-exch02.marvell.com ([199.233.58.182]) by mx0b-0016f401.pphosted.com with ESMTP id 30jd016pnp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Thu, 23 Apr 2020 02:09:54 -0700 Received: from SC-EXCH01.marvell.com (10.93.176.81) by SC-EXCH02.marvell.com (10.93.176.82) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 23 Apr 2020 02:09:52 -0700 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.106) by SC-EXCH01.marvell.com (10.93.176.81) with Microsoft SMTP Server (TLS) id 15.0.1497.2 via Frontend Transport; Thu, 23 Apr 2020 02:09:52 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=e24Y0hlq2TFohmtt9cShKB26IIQAZS7B3jtS5dmrC2uWOguYX+kU2eOGNs/DoOYy6Ng09DYEYEPmFYsjqY1Hlr+iCCQg8udUkcAkl1Iy8KEevUNZOkheqKsYuYAKAM9HX3O4a/U6qk/PYKUJvG9Vp/Qfpud27H0jue7vNz1zFjV7Tm9ZBHHsoVBCYPk5UfAGMHESVQp+/KfV5js2BdkpDWt512gSYdrrMc6x8pgdqs46sGSqVRs3UtYvrcgBw+mbKOhMYPfiqgmnsJoXkOmq25It0QHRKgv/IroaNXAA8ee5bXlY7p1nGV4V9s2J4hI4nWvwPZa1Ebx66mIDzw94oA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=j2hjX7mJEsA2tEqV5zbYvRGVzakea62PUJ6/NoCi/r4=; b=mBLE1qrMPVi/W8MnLtTxoQwFbQDODBTUoT2MgdJNwMVc6m6TgS4jIQUBbIf6iov4gTzxP1dMRNfr6WL+XPPpqYPF6fgYUUA8wIO7uvkUWNlVcLPiKimPtgUThgvLehbfeBTxiq/MuC3nsBRWRMdMnOKwrF/S4+P54+RFkLGUsEwgxHKwXkl93SDG5eU49nHZal80wyRIUKPO5PvzhvbY3QJiAaVyOI8KApQO7sGBlrl1s7buZgWCzHDQ9I7BGnTxVRJM9FHAzu9hv/xtDMjrrXJyaYiZ9Iv6Rlsit/OCVJwKesSUru8SuKYOzoskcAEEItspUPzInSsYbL2EA8sopA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=j2hjX7mJEsA2tEqV5zbYvRGVzakea62PUJ6/NoCi/r4=; b=sdtPxAGNGk2IRe25v/nsQkKP26/ePpIAmeEgVRGs+9uRyyDv/hIolmpG38iwOH9Igc36PPc5+stxNgVj0T87mWKhoe6fV5ZsR/6hHgxrNJ2lzUnpbf7aqSXGmQYOJUPBjFbACbamqfaM/xUtwy0iBhw8OprHU7h1+M5DkXF3pA0= Received: from MN2PR18MB2877.namprd18.prod.outlook.com (2603:10b6:208:3b::26) by MN2PR18MB2797.namprd18.prod.outlook.com (2603:10b6:208:a0::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2937.13; Thu, 23 Apr 2020 09:09:51 +0000 Received: from MN2PR18MB2877.namprd18.prod.outlook.com ([fe80::648f:e7fa:f95e:191b]) by MN2PR18MB2877.namprd18.prod.outlook.com ([fe80::648f:e7fa:f95e:191b%2]) with mapi id 15.20.2921.030; Thu, 23 Apr 2020 09:09:51 +0000 From: Anoob Joseph To: "Ananyev, Konstantin" , "dev@dpdk.org" , Lukasz Wojciechowski CC: "akhil.goyal@nxp.com" , "Doherty, Declan" , "techboard@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] security: fix crash at accessing non-implemented ops Thread-Index: AQHWGQET5ssCrfrdCkGeU0EkO7BVz6iGFf8AgABAugCAABAnMA== Date: Thu, 23 Apr 2020 09:09:51 +0000 Message-ID: References: <20200422235158.24497-1-konstantin.ananyev@intel.com> In-Reply-To: Accept-Language: en-IN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [27.34.246.85] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 4b8858c3-a2a3-4a0c-39c9-08d7e7661476 x-ms-traffictypediagnostic: MN2PR18MB2797: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:2276; x-forefront-prvs: 03827AF76E x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR18MB2877.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(366004)(396003)(39860400002)(376002)(346002)(136003)(19627235002)(64756008)(66446008)(66556008)(66476007)(76116006)(66946007)(966005)(9686003)(2906002)(8936002)(15650500001)(81156014)(55016002)(478600001)(54906003)(316002)(7696005)(5660300002)(4326008)(86362001)(33656002)(71200400001)(52536014)(53546011)(186003)(6506007)(26005)(110136005); DIR:OUT; SFP:1101; received-spf: None (protection.outlook.com: marvell.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: EqHp56+6FzydsqPnRzjMi7uee7LXMehlW4nwnk0/vNEER+aVDCuqh14H+yPfeqyQ31q0ojZA06yyhE2R47lFoduUd/evGSw16xQvwH8lj/IL9jpy82pzdrmv2pO4EztG2CIrh2CIvNhO1CuHitrG4YcIojq20FZ4al7Xktl72G+m8AZpRamTpTIbn0lqaHUDorlZLxGrYpUx435xr9zQIk5jSnZnfHmIA0sK2bCrf/TerH3YKnqHDXzjVMXbKfnIsaN4/vFgCFCh0UMKm4qACBtt4HNUBAQrZzTDDQTrTAHBWpflI9cl1xPzjw5rfWINitaD5bjZQx+9P4Eej3j/vhTBJXOANhjmZaxMUPSMyPaMhmcGHzOuCc2at62QTiNhbAGKXdk3IR0iYnw61LOrSE388tsecuBjw7nd3zmBrvczUb7SuTTvyR+JHgh7CuFnboqABbmmFsLxutPos8UrHPGILCMHq1t0XU7taIuFFY/Pev2Ts7v3FY7DSjYVSvy8QH0foQ6O0PfDgfT6NPyd7w== x-ms-exchange-antispam-messagedata: L98T1GGMSh9/GLslQQl8hMkq7S57QDSC01IFTo8p3D+SAOApN53CE9j+4k7eY7MIhCruZVkvNAlR9nd//pVh6TxG3maqkIE7dqKDvli13ov/q0vt8KTUQJ6D/JonBYEWpGW9UU0H4ozF9NPm1Y5vbA== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 4b8858c3-a2a3-4a0c-39c9-08d7e7661476 X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Apr 2020 09:09:51.1047 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: PtzDAXZ92J8skQ4LrsWITOr26rG+tRJ3KcYo/sPRWkGTa/EqY2xnJxi7ZMSD3z2m9CbLMBwgClkbPbSrrfV1bw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR18MB2797 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138, 18.0.676 definitions=2020-04-23_07:2020-04-22, 2020-04-23 signatures=0 Subject: Re: [dpdk-dev] [PATCH] security: fix crash at accessing non-implemented ops 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Konstantin, Please see inline. Thanks, Anoob > -----Original Message----- > From: Ananyev, Konstantin > Sent: Thursday, April 23, 2020 1:24 PM > To: Anoob Joseph ; dev@dpdk.org > Cc: akhil.goyal@nxp.com; Doherty, Declan ; > techboard@dpdk.org > Subject: [EXT] RE: [dpdk-dev] [PATCH] security: fix crash at accessing no= n- > implemented ops >=20 > External Email >=20 > ---------------------------------------------------------------------- > > > > Hi Konstantin, > > > > These are data path ops and so it will be better if we can avoid such c= hecks in > the datapath. The same is done in ethdev also. >=20 > AFAIK, get_userdata is an *optional* dev-ops function that can be used b= y data- > path. > So far there was no strict requirement for the rte_security PMDs to *alwa= ys* > implement it. [Anoob] I don't think DPDK categorizes dev-ops as *optional* and *always*. = If yes, can you point me? My understanding is, all ops are optional. For example, I could implement a= crypto PMD which is doing packet delivery only via event device (using cry= pto adapter). So dequeue op will not be implemented in that case and DPDK s= pec allows it.=20 =20 > So what you guys did is a silent change of public API behaviour. [Anoob] I believe Lukasz had submitted 3 or 4 revisions and it was all in t= he ML. RTE_DEBUG was suggested by Thomas I guess. =20 > As result ixgbe, (and probably some others rte_security PMDs) stopped wor= king > properly. [Anoob] set_pkt_metadata() is the only one of interest to IXGBE. And I beli= eve the function is implemented as well. So what exactly is the concern? =20 > I don't see any point in these changes, but if you'd like to do that, at = least our > usual procedure has to be followed: > 1. Send and RFC to get an agreement with rte_security PMDs maintainers (o= ne > release ahead) 2. send a deprecation note (one release ahead) 3. change t= he > behaviour of the public API 4. update release notes >=20 > AFAIK 1), 2), 4) wasn't done. > So I think right now we need to revert original behaviour. >=20 > > > > https://urldefense.proofpoint.com/v2/url?u=3Dhttp-3A__code.dpdk.org_dpd= k > > _v20.02_source_lib_librte-5Fethdev_rte-5Fethdev.h-23L4372&d=3DDwIFAg&c= =3Dn > > KjWec2b6R0mOyPaz7xtfQ&r=3DjPfB8rwwviRSxyLWs2n6B- > WYLn1v9SyTMrT5EQqh2TU&m=3D > > 6ObfSanVVuHOsiqVlWxXsFWi- > 2XNp76HCOX0vbUfma4&s=3DjDVyDDEILmgY1Yb9ZBswBVbn > > 8FpZuQI5ukH_osmtUiI&e=3D > > > > Datapath functions in cryptodev (enqueue/dequeue) doesn't even have suc= h > checks. > > https://urldefense.proofpoint.com/v2/url?u=3Dhttp-3A__code.dpdk.org_dpd= k > > _v20.02_source_lib_librte-5Fcryptodev_rte-5Fcryptodev.h-23L962&d=3DDwIF= A > > g&c=3DnKjWec2b6R0mOyPaz7xtfQ&r=3DjPfB8rwwviRSxyLWs2n6B- > WYLn1v9SyTMrT5EQqh2 > > TU&m=3D6ObfSanVVuHOsiqVlWxXsFWi- > 2XNp76HCOX0vbUfma4&s=3DLEWQOKs0r2Im_zL95VI > > df4kQ2Pu0iRHV9Co2J1gsNBE&e=3D >=20 > That's a different story: > rx_burst/tx_burst, enqueue/dequeue are mandatory dev-ops functions that h= ave > to be implemented by each ethdev/cryptodev API. [Anoob] I couldn't find any reference stating that way. If you can point me= , I can update that to include datapath ops required for inline protocol pr= ocessing. >=20 > > > > > > Thanks, > > Anoob > > > > > -----Original Message----- > > > From: dev On Behalf Of Konstantin Ananyev > > > Sent: Thursday, April 23, 2020 5:22 AM > > > To: dev@dpdk.org > > > Cc: akhil.goyal@nxp.com; declan.doherty@intel.com; Konstantin > > > Ananyev > > > Subject: [dpdk-dev] [PATCH] security: fix crash at accessing > > > non-implemented ops > > > > > > Valid checks for optional function pointers inside dev-ops were > > > disabled by undefined macro. > > > > > > Fixes: b6ee98547847 ("security: fix verification of parameters") > > > > > > Signed-off-by: Konstantin Ananyev > > > --- > > > lib/librte_security/rte_security.c | 4 ---- > > > 1 file changed, 4 deletions(-) > > > > > > diff --git a/lib/librte_security/rte_security.c > > > b/lib/librte_security/rte_security.c > > > index d475b0977..b65430ce2 100644 > > > --- a/lib/librte_security/rte_security.c > > > +++ b/lib/librte_security/rte_security.c > > > @@ -107,11 +107,9 @@ rte_security_set_pkt_metadata(struct > > > rte_security_ctx *instance, > > > struct rte_security_session *sess, > > > struct rte_mbuf *m, void *params) { -#ifdef > RTE_DEBUG > > > RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, set_pkt_metadata, - > > > EINVAL, > > > -ENOTSUP); > > > RTE_PTR_OR_ERR_RET(sess, -EINVAL); -#endif > > > return instance->ops->set_pkt_metadata(instance->device, > > > sess, m, params); > > > } > > > @@ -121,9 +119,7 @@ rte_security_get_userdata(struct > > > rte_security_ctx *instance, uint64_t md) { > > > void *userdata =3D NULL; > > > > > > -#ifdef RTE_DEBUG > > > RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, get_userdata, NULL, > > > NULL); -#endif > > > if (instance->ops->get_userdata(instance->device, md, &userdata)) > > > return NULL; > > > > > > -- > > > 2.17.1