From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 01A5B7F1C for ; Fri, 8 Jul 2016 16:15:40 +0200 (CEST) Received: from cpe-2606-a000-111b-40ed-7aac-c0ff-fec2-933b.dyn6.twc.com ([2606:a000:111b:40ed:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1bLWYK-0005ev-KV; Fri, 08 Jul 2016 10:14:55 -0400 Date: Fri, 8 Jul 2016 10:14:37 -0400 From: Neil Horman To: "De Lara Guarch, Pablo" Cc: "dev@dpdk.org" , "Richardson, Bruce" , Thomas Monjalon , Stephen Hemminger , Panu Matilainen Message-ID: <20160708141437.GE14917@hmsreliant.think-freely.org> References: <1467905863-27038-1-git-send-email-nhorman@tuxdriver.com> <20160708121744.GA14917@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Score: -1.0 (-) X-Spam-Status: No 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:15:40 -0000 On Fri, Jul 08, 2016 at 02:00:20PM +0000, De Lara Guarch, Pablo wrote: > > > > -----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 > > > > 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 relies 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 using > > > > RTE_STR. > > > > > > > > Tested using the null and aesni_mb crypto pmds, as I don't have hardware > > 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 compile. > > > I have a fix for that, so I can send a v2 of this patch if it is OK for you. > > > > > > > I suppose thats fine, sure, though I'm really not comfortable with an open > > 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, whats the > > compilation error? > > drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function 'snow3g_pmd_qp_create_processed_ops_ring': > drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: error: 'cryptodev_snow3g_pmd' undeclared (first use in this function) > SNOW3G_LOG_ERR("Unable to reuse existing ring %s" > ^ > dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: note: each undeclared 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: 'cryptodev_snow3g_pmd' undeclared (first use in this function) > SNOW3G_LOG_ERR("invalid session struct"); > > ... > > The solution is adding RTE_STR in SNOW3G_LOG_ERR. > Ah,thanks. Its odd, I used cscope to do a find and replace for all the other instances of the RTE_STR conversion, I wonder how I missed that. Neil > ... > > > > > > > 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: > > > > 1) Should we remove the static setting of the name in the pmd_driver > > structure > > in favor of doing it in the registration macro? > > > > 2) Should we be consistent in the name conversion (from the setting in the > > structure instance definition to the string in the macro parameter)? > > > > The answer to (1) is yes, though having it in both places is harmless, since 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. > > > > > 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 devices > > 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 release, > after a deprecation notice, since this is breaking the API. > > > > > Neil > > > > > Thanks, > > > Pablo > > > >