From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 11DE06CA2 for ; Fri, 8 Jul 2016 16:00:23 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 08 Jul 2016 07:00:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,330,1464678000"; d="scan'208";a="1003207373" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by fmsmga001.fm.intel.com with ESMTP; 08 Jul 2016 07:00:23 -0700 Received: from irsmsx111.ger.corp.intel.com (10.108.20.4) by irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 8 Jul 2016 15:00:20 +0100 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.125]) by irsmsx111.ger.corp.intel.com ([169.254.2.182]) with mapi id 14.03.0248.002; Fri, 8 Jul 2016 15:00:20 +0100 From: "De Lara Guarch, Pablo" To: Neil Horman CC: "dev@dpdk.org" , "Richardson, Bruce" , Thomas Monjalon , Stephen Hemminger , Panu Matilainen Thread-Topic: [PATCH] crypto: normalize cryptodev pmd names with macros Thread-Index: AQHR2GXEj+5/tTpl0kO53K+daAZYH6AOPPlwgAAm7QCAACh8kA== Date: Fri, 8 Jul 2016 14:00:20 +0000 Message-ID: References: <1467905863-27038-1-git-send-email-nhorman@tuxdriver.com> <20160708121744.GA14917@hmsreliant.think-freely.org> In-Reply-To: <20160708121744.GA14917@hmsreliant.think-freely.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTI1YzNkZTUtMjVkYi00N2FiLWE1MGMtMWFhNjU3OWE1NmY0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkNUeEJyTXZkNHFjRkl2OTltTjRHWk1wNmRHZnJQRURkZm9ZRzFXVXZzQlk9In0= x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jul 2016 14:00:24 -0000 > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Friday, July 08, 2016 1:18 PM > To: De Lara Guarch, Pablo > Cc: dev@dpdk.org; Richardson, Bruce; Thomas Monjalon; Stephen > Hemminger; Panu Matilainen > Subject: Re: [PATCH] crypto: normalize cryptodev pmd names with macros >=20 > On Fri, Jul 08, 2016 at 09:09:10AM +0000, De Lara Guarch, Pablo wrote: > > Hi Neil, > > > > > -----Original Message----- > > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > Sent: Thursday, July 07, 2016 4:38 PM > > > To: dev@dpdk.org > > > Cc: Neil Horman; Richardson, Bruce; Thomas Monjalon; De Lara Guarch, > > > Pablo; Stephen Hemminger; Panu Matilainen > > > Subject: [PATCH] crypto: normalize cryptodev pmd names with macros > > > > > > Recently reported, the introduction of pmd information exports led to= a > > > breakage of cryptodev unit tests because the test infrastructure reli= es on > the > > > cryptodev names being available in macros. This patch fixes the pmd > naming > > > to > > > use the macro names. Note that the macro names were already pre- > > > stringified, > > > which won't work as the PMD_REGISTER_DRIVER macro requires the > name in > > > both a > > > processing token and stringified form. As such the names are defined= now > as > > > tokens, and converted where needed to stringified form on demand usin= g > > > RTE_STR. > > > > > > Tested using the null and aesni_mb crypto pmds, as I don't have hardw= are > for > > > any > > > other device. Also not build tested on snow3g or kasumi pmd because > > > building > > > those requires external libraries that appear to not be open source > licensed. > > > > > > Signed-off-by: Neil Horman > > > CC: Bruce Richardson > > > CC: Thomas Monjalon > > > CC: "De Lara Guarch, Pablo" > > > CC: Stephen Hemminger > > > CC: Panu Matilainen > > > --- > > > app/test/test_cryptodev.c | 20 ++++++++++--= -------- > > > app/test/test_cryptodev_perf.c | 18 +++++++++---= ------ > > > drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 +++---- > > > drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h | 6 +++--- > > > drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 7 +++---- > > > drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 2 +- > > > drivers/crypto/kasumi/rte_kasumi_pmd.c | 5 ++--- > > > drivers/crypto/null/null_crypto_pmd.c | 7 +++---- > > > drivers/crypto/null/null_crypto_pmd_private.h | 6 +++--- > > > drivers/crypto/qat/rte_qat_cryptodev.c | 4 ++-- > > > drivers/crypto/snow3g/rte_snow3g_pmd.c | 4 ++-- > > > lib/librte_cryptodev/rte_cryptodev.h | 12 ++++++------ > > > 12 files changed, 47 insertions(+), 51 deletions(-) > > > > Thanks for this patch. I tested snow3g and kasumi, and they don't compi= le. > > I have a fix for that, so I can send a v2 of this patch if it is OK for= you. > > >=20 > I suppose thats fine, sure, though I'm really not comfortable with an ope= n > source project requiring what appears to be non-open source components > (though I > can't really tell what the snow3g or kasumi license is). Regardless, wha= ts the > compilation error? drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function 'snow3g_pmd_qp_crea= te_processed_ops_ring': drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: error: 'cryptodev_snow3= g_pmd' undeclared (first use in this function) SNOW3G_LOG_ERR("Unable to reuse existing ring %s" = = ^ =20 dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: note: each u= ndeclared identifier is reported only once for each function it appears in dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function 'snow3g_= pmd_session_configure': dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:296:117: error: 'cryp= todev_snow3g_pmd' undeclared (first use in this function) SNOW3G_LOG_ERR("invalid session struct"); ... The solution is adding RTE_STR in SNOW3G_LOG_ERR. ... >=20 >=20 > > Also, we should make these changes in the other PMDs as well, right? > > I mean, avoid setting the name of the driver directly in the structure = and go > back to the original name. > > I can do that as well, if you want (maybe a separate patch, as this one= is > only related to crypto). > > > I think thats kind of two questions: >=20 > 1) Should we remove the static setting of the name in the pmd_driver > structure > in favor of doing it in the registration macro? >=20 > 2) Should we be consistent in the name conversion (from the setting in th= e > structure instance definition to the string in the macro parameter)? >=20 > The answer to (1) is yes, though having it in both places is harmless, si= nce the > former will just get overridden. We should definately remove the static > setting, to avoid confusion, but theres not any functional rush to do so. Will do that in a separate patch. >=20 > The answer to (2) is yes, but I think thats already done. I don't think = we > deviated in too many places (if any), as the strings for all the net devi= ces > were directly set (i.e. not through macros), and I just transferred them. Some driver names have changed (like eth_pcap to pcap). I can revert that to the original name and we can rename them in the next r= elease, after a deprecation notice, since this is breaking the API. >=20 > Neil >=20 > > Thanks, > > Pablo > >