patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
To: Anoob Joseph <anoobj@marvell.com>, Thomas Monjalon <thomas@monjalon.net>
Cc: Akhil Goyal <akhil.goyal@nxp.com>,
	Declan Doherty <declan.doherty@intel.com>,
	Aviad Yehezkel <aviadye@mellanox.com>,
	Boris Pismenny <borisp@mellanox.com>,
	Radu Nicolau <radu.nicolau@intel.com>,
	Anoob Joseph <anoob.joseph@caviumnetworks.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [EXT] Re: [dpdk-dev] [PATCH v2 01/13] security: fix verification of parameters
Date: Wed, 8 Apr 2020 17:49:32 +0200	[thread overview]
Message-ID: <827f9660-dd8e-8440-c8b0-d34064ffdffe@partner.samsung.com> (raw)
In-Reply-To: <MN2PR18MB2877F5455CF2C6C51B0AAE4BDFC00@MN2PR18MB2877.namprd18.prod.outlook.com>

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 = '<unknown 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.

Best regards
Lukasz


BTW
I got still some warnings from linters when processing Fixes flags. I 
used the fixline alias recommended on dpdk build manual, but the 
automated linter does not find the commits I mention there. What have I 
done wrong?


W dniu 08.04.2020 o 16:44, Anoob Joseph pisze:
> Hi Thomas,
>
>> ----------------------------------------------------------------------
>> 08/04/2020 15:02, Anoob Joseph:
>>> Hi Thomas,
>>>
>>>> 08/04/2020 05:13, Lukasz Wojciechowski:
>>>>> This patch adds verification of the parameters to the ret_security
>>>>> API functions. All required parameters are checked if they are not NULL.
>>>> [...]
>>>>> --- a/config/common_base
>>>>> +++ b/config/common_base
>>>>>   CONFIG_RTE_LIBRTE_SECURITY=y
>>>>> +CONFIG_RTE_LIBRTE_SECURITY_DEBUG=n
>>>> Is it a leftover?
>>>>
>>> [Anoob]  It is similar to 'RTE_LIBRTE_ETHDEV_DEBUG' for usage in
>>> datapath. Like in,
>>> https://protect2.fireeye.com/url?k=ebdcc0dd-b6100959-ebdd4b92-0cc47aa8f5ba-33f3beec7b92faf2&q=1&u=https%3A%2F%2Furldefense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttp-3A__code.dpdk.org_dpdk
>>> _latest_source_lib_librte-5Fethdev_rte-5Fethdev.h-23L4378&d=DwICAg&c=n
>>> KjWec2b6R0mOyPaz7xtfQ&r=jPfB8rwwviRSxyLWs2n6B-
>> WYLn1v9SyTMrT5EQqh2TU&m=
>> STCBgRhcnCb9M6MWQL9CUszLwy2r0NJ_3m93_D5UX3g&s=HVsD0LKZ2Q6UCW
>> BSRvbw9beD
>>> 7OtuQyWPrRrx9eofnz8&e=
>> 1/ I don't see it used in this patch
> [Anoob] Following snippet uses.
>
> +#ifdef RTE_LIBRTE_SECURITY_DEBUG
> +	RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, set_pkt_metadata, -EINVAL,
> +			-ENOTSUP);
> +	RTE_PTR_OR_ERR_RET(sess, -EINVAL);
> +#endif
>
>> 2/ Adding makefile-only option is weird
>> 3/ Adding new compile-time options is discouraged
> [Anoob] This is only introduced for data path APIs. And the same approach is followed in eth dev as well.
>
> Thanks,
> Anoob
>
-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


  reply	other threads:[~2020-04-08 15:49 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200312151654.7218-1-l.wojciechow@partner.samsung.com>
     [not found] ` <20200408031351.4288-1-l.wojciechow@partner.samsung.com>
     [not found]   ` <CGME20200408031447eucas1p1376332353faa0d217e7be8c32271405f@eucas1p1.samsung.com>
2020-04-08  3:13     ` [dpdk-stable] " Lukasz Wojciechowski
2020-04-08 12:54       ` Thomas Monjalon
2020-04-08 13:02         ` [dpdk-stable] [dpdk-dev] " Anoob Joseph
2020-04-08 13:26           ` Thomas Monjalon
2020-04-08 14:44             ` [dpdk-stable] [EXT] " Anoob Joseph
2020-04-08 15:49               ` Lukasz Wojciechowski [this message]
2020-04-08 17:51                 ` Thomas Monjalon
2020-04-09 10:14                   ` Bruce Richardson
2020-04-09 10:54                     ` Thomas Monjalon
2020-04-09 11:13                       ` Bruce Richardson
2020-04-09 14:07                         ` Lukasz Wojciechowski
2020-04-09 14:21                           ` Lukasz Wojciechowski
2020-04-09 15:22                             ` Thomas Monjalon
2020-04-09 16:10                               ` Lukasz Wojciechowski
2020-04-10  8:45                                 ` Bruce Richardson
     [not found]   ` <CGME20200408031448eucas1p2b36997fc73f5b5e2aadb6e4bb965063b@eucas1p2.samsung.com>
2020-04-08  3:13     ` [dpdk-stable] [PATCH v2 02/13] security: fix return types in documentation Lukasz Wojciechowski
     [not found]   ` <CGME20200408031448eucas1p2d6df7ff419bb093606a2f9115297f45a@eucas1p2.samsung.com>
2020-04-08  3:13     ` [dpdk-stable] [PATCH v2 03/13] security: fix session counter Lukasz Wojciechowski
     [not found]   ` <CGME20200408031449eucas1p1ca89719463cbaf29e9f7c81beaec88c2@eucas1p1.samsung.com>
2020-04-08  3:13     ` [dpdk-stable] [PATCH v2 04/13] app/test: fix macro definition Lukasz Wojciechowski
2020-04-08 12:53       ` Thomas Monjalon
2020-04-08 16:15         ` Lukasz Wojciechowski
2020-04-08 17:47           ` Thomas Monjalon
2020-04-09 14:10             ` Lukasz Wojciechowski
     [not found]   ` <20200409172502.1693-1-l.wojciechow@partner.samsung.com>
     [not found]     ` <CGME20200409172529eucas1p1f02aaf66052f45ac75ba9e9f63ef1c3a@eucas1p1.samsung.com>
2020-04-09 17:24       ` [dpdk-stable] [PATCH v3 01/13] security: fix verification of parameters Lukasz Wojciechowski
2020-04-13 15:42         ` [dpdk-stable] [dpdk-dev] " Anoob Joseph
     [not found]     ` <CGME20200409172530eucas1p27297a83a9d7508e3f8a8f88850cbe37c@eucas1p2.samsung.com>
2020-04-09 17:24       ` [dpdk-stable] [PATCH v3 02/13] security: fix return types in documentation Lukasz Wojciechowski
2020-04-13 15:43         ` [dpdk-stable] [dpdk-dev] " Anoob Joseph
     [not found]     ` <CGME20200409172531eucas1p1c3ec21532e5e232ff2d68d56f096e71c@eucas1p1.samsung.com>
2020-04-09 17:24       ` [dpdk-stable] [PATCH v3 03/13] security: fix session counter Lukasz Wojciechowski
2020-04-13 15:48         ` [dpdk-stable] [dpdk-dev] " Anoob Joseph
     [not found]     ` <CGME20200409172532eucas1p285bc6767be1d62a0098d177a7757169f@eucas1p2.samsung.com>
2020-04-09 17:24       ` [dpdk-stable] [PATCH v3 04/13] app/test: remove macro definition Lukasz Wojciechowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=827f9660-dd8e-8440-c8b0-d34064ffdffe@partner.samsung.com \
    --to=l.wojciechow@partner.samsung.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoob.joseph@caviumnetworks.com \
    --cc=anoobj@marvell.com \
    --cc=aviadye@mellanox.com \
    --cc=borisp@mellanox.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=radu.nicolau@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).