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 23FAAA0597; Thu, 9 Apr 2020 16:21:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 09EB81C2E8; Thu, 9 Apr 2020 16:21:28 +0200 (CEST) Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id EFBD81C2B9 for ; Thu, 9 Apr 2020 16:21:26 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20200409142126euoutp01a0ea2553efb7c2e2b8b8b021e00a0962~ELKwZTpuL0194701947euoutp01E for ; Thu, 9 Apr 2020 14:21:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20200409142126euoutp01a0ea2553efb7c2e2b8b8b021e00a0962~ELKwZTpuL0194701947euoutp01E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1586442086; bh=gBULCyiHU0i03gRkYYOgnMLj9Q7Kx6yTxT2XMcMnnc8=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=L28UJccnKkXulOjYFai2cj7eNbw8jeltSuUHInKd3Qom82zdc4DhXydEG+X7wpyxU qDRI+LMR7AzgK/En6TObK3a3bl2oPIGniWiZMo3RP9advOlJFodAGzMKLTLQn7EiWf cD8kSYzgl8e6Y51VeBNbD2sFinQDBixmumjp/OU4= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20200409142126eucas1p23e42d5513bcdbccacc0afce434c9d8b4~ELKwPk3vm0563605636eucas1p2o; Thu, 9 Apr 2020 14:21:26 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 83.5E.60679.66F2F8E5; Thu, 9 Apr 2020 15:21:26 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20200409142125eucas1p27f3fff117cee93e12d47a94527148194~ELKv3O74O0564705647eucas1p2Z; Thu, 9 Apr 2020 14:21:25 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20200409142125eusmtrp105b969eb977fa2f4a73446e52b7f0207~ELKv2e15v3088330883eusmtrp1N; Thu, 9 Apr 2020 14:21:25 +0000 (GMT) X-AuditID: cbfec7f4-0e5ff7000001ed07-ff-5e8f2f66bed6 Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 45.58.07950.56F2F8E5; Thu, 9 Apr 2020 15:21:25 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20200409142124eusmtip2a6c37b2aa10a2e08e5817bd77706204a~ELKu5LkNT1517015170eusmtip2q; Thu, 9 Apr 2020 14:21:24 +0000 (GMT) From: Lukasz Wojciechowski To: Bruce Richardson , Thomas Monjalon Cc: Anoob Joseph , Akhil Goyal , Declan Doherty , Aviad Yehezkel , Boris Pismenny , Radu Nicolau , Anoob Joseph , "dev@dpdk.org" , "stable@dpdk.org" Message-ID: <91240a63-76d0-508e-3071-b1e871c74294@partner.samsung.com> Date: Thu, 9 Apr 2020 16:21:23 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrJKsWRmVeSWpSXmKPExsWy7djPc7pp+v1xBvO7eSzWn5nHaHHq9gdm i2VbtjJZfJi8hNHi2I92dosbq+wt3jxoYrF492k7k0Vbl4DFv44/7BafHpxgceD22HCin9Xj 14KlrB6L97xk8pi88CKzx7Pph5k8jt2cxu6x8d0OpgD2KC6blNSczLLUIn27BK6Mj1N3MhbM 1624tfcnewNjs2oXIyeHhICJxOlDjaxdjFwcQgIrGCXO/d7FDOF8YZS40zWFEcL5zCgxe+oH dpiWiTeeskMkljNK3G2/zgLhvGWU+HduIytIlbBArMTWuQ/YQGw2AVuJIzO/gsVFBKIkPuxv AtvBLPCYSeL1kt8sIAleATeJI8eXga1gEVCRuPn/KFiDKNCgc49usELUCEqcnPkErJ5TwF2i ad5tMJtZQF6ieetsZghbXOLWk/lMIAskBG6xS6yc8ZsV4m4XiSdzpkHZwhKvjm+B+kdG4vTk HhaIhm2MEld//2SEcPYzSlzvXQFVZS1x+N9voH84gFZoSqzfpQ9iSgg4SszfEAhh8knceCsI cQOfxKRt05khwrwSHW1CEDP0JJ72TGWE2fpn7ROWCYxKs5B8NgvJN7OQfDMLYe0CRpZVjOKp pcW56anFRnmp5XrFibnFpXnpesn5uZsYgQns9L/jX3Yw7vqTdIhRgINRiYfXgKE/Tog1say4 MvcQowQHs5IIr3dTb5wQb0piZVVqUX58UWlOavEhRmkOFiVxXuNFL2OFBNITS1KzU1MLUotg skwcnFINjBoyjpemNOr/NOT8HScqomW4pCafi+1C312upbN+vbW6GF6cuO52Hfu7OOv+VyUZ dmkCPmsSprdn/p/6N+pdmWLx4XADvh25batWlPreVFu+bXuo/grflJ5n+w7V9bB36Xssj405 kXiaWaS9LC6mrnONTcz9Rz52qqzrV2kf3p/bWuYq/TxfiaU4I9FQi7moOBEAtR8uWlwDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrFIsWRmVeSWpSXmKPExsVy+t/xe7qp+v1xBqdu61isPzOP0eLU7Q/M Fsu2bGWy+DB5CaPFsR/t7BY3VtlbvHnQxGLx7tN2Jou2LgGLfx1/2C0+PTjB4sDtseFEP6vH rwVLWT0W73nJ5DF54UVmj2fTDzN5HLs5jd1j47sdTAHsUXo2RfmlJakKGfnFJbZK0YYWRnqG lhZ6RiaWeobG5rFWRqZK+nY2Kak5mWWpRfp2CXoZH6fuZCyYr1txa+9P9gbGZtUuRk4OCQET iYk3nrJ3MXJxCAksZZR49v8gcxcjB1BCRuLDJQGIGmGJP9e62CBqXjNKvOh/zwSSEBaIlfiy fB4jiM0mYCtxZOZXVhBbRCBKYlHbWTCbWeApk8T8fVALWpglpjWeZgNJ8Aq4SRw5vowdxGYR UJG4+f8oWIMo0ND+5t2MEDWCEidnPmEBsTkF3CWa5t1mgRhqJjFv80NmCFteonnrbChbXOLW k/lMExiFZiFpn4WkZRaSlllIWhYwsqxiFEktLc5Nzy020itOzC0uzUvXS87P3cQIjNdtx35u 2cHY9S74EKMAB6MSD68BQ3+cEGtiWXFl7iFGCQ5mJRFe76beOCHelMTKqtSi/Pii0pzU4kOM pkDPTWSWEk3OB6aSvJJ4Q1NDcwtLQ3Njc2MzCyVx3g6BgzFCAumJJanZqakFqUUwfUwcnFIN jJUr7lV+1K/8r7s1ufXRzhPhu27NNt5cbqeQ8H2nh09MpVKZi5Gs4cGyIOPXN3l+749ewda9 tKpB+9z5gsUBVqsaRWpyFgorT+ybqanpdzOBa/ay35lfUoWcFk/RcmMXftl+Q+379H1bPM2n H+/+dGzBxt28yUYi21ZM+jBZV9T9gZTVxjWBzUosxRmJhlrMRcWJALt+txjtAgAA X-CMS-MailID: 20200409142125eucas1p27f3fff117cee93e12d47a94527148194 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200409111335eucas1p1997192d7da7d72562ac7787670a15916 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200409111335eucas1p1997192d7da7d72562ac7787670a15916 References: <20200312151654.7218-1-l.wojciechow@partner.samsung.com> <2333397.jE0xQCEvom@thomas> <20200409101445.GA613@bricha3-MOBL.ger.corp.intel.com> <22404743.ouqheUzb2q@thomas> <20200409111324.GC613@bricha3-MOBL.ger.corp.intel.com> Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v2 01/13] security: fix verification of parameters 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" W dniu 09.04.2020 o 16:07, Lukasz Wojciechowski pisze: > > W dniu 09.04.2020 o 13:13, Bruce Richardson pisze: >> On Thu, Apr 09, 2020 at 12:54:10PM +0200, Thomas Monjalon wrote: >>> 09/04/2020 12:14, Bruce Richardson: >>>> On Wed, Apr 08, 2020 at 07:51:35PM +0200, Thomas Monjalon wrote: >>>>> 08/04/2020 17:49, Lukasz Wojciechowski: >>>>>> Hi guys, >>>>>> >>>>>> I don't know what is the current status of "legacy" build using >>>>>> gnumakes, so I added the new DEBUG flag to config just as it was >>>>>> done in >>>>>> other libs like eventdev. >>>>>> Many guides still point config files as the one that should be >>>>>> changed >>>>>> in order to enable some features, so I thought I should add it >>>>>> there. >>>>>> >>>>>> If I understand well the official build system now is the one >>>>>> based on >>>>>> using meson and ninja, however it hasn't got anything similar to the >>>>>> gnamakefiles system, e.g. >>>>>> in the meson.build file for libraries all the libraries have build >>>>>> variable set to true and there are few ifs that check it, but as >>>>>> it's >>>>>> set to true all libraries build always. >>>>>> And each library considered there defines RTE_LIBRTE_[LIBRARY_NAME]. >>>>>> It's kind of weird. >>>>>> >>>>>>      foreach l:libraries >>>>>>      *    build = true** >>>>>>      *    reason = '' # set if build == false to >>>>>> explain why >>>>>>           ... >>>>>>      *    if not build* >>>>>>               dpdk_libs_disabled += name >>>>>>               set_variable(name.underscorify() + >>>>>> '_disable_reason', reason) >>>>>>           else >>>>>>               enabled_libs += name >>>>>>      *dpdk_conf.set('RTE_LIBRTE_' + name.to_upper(), 1)* >>>>>>           ... >>>>>> >>>>>> Have you think about reusing config files in meson configuration and >>>>>> have a single point of configuration? Of course all meson flags can >>>>>> overwrite the default config. >>>>> This is on purpose. >>>>> We are removing most of compile-time options with meson. >>>>> >>>>> I think we can use a global option for debug-specific code. >>>>> Bruce, what do you recommend? >>>>> >>>> Meson has a built-in global debug setting which could be used. >>>> However, >>>> that may be too course-grained. If that is the case there are a >>>> couple of >>>> options: >>>> >>>> 1 Each library can have it's own debug flag defined, which is set on >>>>    the commandline in CFLAGS. Can be done right now - just reuse >>>> any of the >>>>    debug variables in the existing make config files (stripping off >>>> the >>>>    CONFIG_), e.g. CFLAGS=-DRTE_MALLOC_DEBUG >>>> 2 Since that is perhaps not the most usable - though easiest to >>>> implement - >>>>    we can look to add a general debug option (or couple of options) in >>>>    meson, e.g. debug_libs=, debug_drivers=, where each option takes >>>> a list of >>>>    libs or drivers to pass the debug flags to. This will require a >>>> little >>>>    work in the meson build infrastructure, but is not that hard. >>>> The harder >>>>    part is standardizing the debug flags across all components. >>>> >>>> The advantage of #1 is that it works today and just needs some >>>> documentation for each lib/driver what it's debug flags are. The >>>> advantage >>>> of #2 is more usability, but it requires a lot more work to >>>> standardize >>>> IMHO. >>> In this case, we need a general option as the one already provided >>> by meson. >>> It means: "I am not in production, I want to see anything behaving >>> wrong >>> in the datapath." >>> "Anything" means we don't need a per-library switch. >>> And for the other needs (out of fast path), we have a new function: >>>     rte_log_can_log(mylogtype, RTE_LOG_DEBUG) >>> >> To use the general option in meson something like below is probably all >> that is needed to flag the debug build to all components: >> >> diff --git a/config/meson.build b/config/meson.build >> index 49482091d..b01cd1251 100644 >> --- a/config/meson.build >> +++ b/config/meson.build >> @@ -176,6 +176,10 @@ endif >>   # add -include rte_config to cflags >>   add_project_arguments('-include', 'rte_config.h', language: 'c') >> >> +if get_option('debug') >> +       add_project_arguments('-DDEBUG', language: 'c') >> +endif >> + > > This will conflict with DEBUG define for log level. Just to be more precise, the log level is defined as RTE_LOG_DEBUG, but in few places you can find something like: #define NTB_LOG(level, fmt, args...) \     rte_log(RTE_LOG_ ## level, ntb_logtype,    "%s(): " fmt "\n", \         __func__, ##args) and usage like this: NTB_LOG(DEBUG, "Link is not up."); > > How about adding similar define in library meson.build file? , e.g > > diff --git a/lib/librte_security/meson.build > b/lib/librte_security/meson.build > index 5679c8b5c..ee92483c5 100644 > --- a/lib/librte_security/meson.build > +++ b/lib/librte_security/meson.build > @@ -4,3 +4,7 @@ >  sources = files('rte_security.c') >  headers = files('rte_security.h', 'rte_security_driver.h') >  deps += ['mempool', 'cryptodev'] > + > +if get_option('debug') > + add_project_arguments('-DRTE_LIBRTE_SECURITY_DEBUG', language: 'c') > +endif > > >>   # enable extra warnings and disable any unwanted warnings >>   warning_flags = [ >>          # -Wall is added by meson by default, so add -Wextra only >> -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com