[dpdk-dev] [EXT] Re: [PATCH v2 01/13] security: fix verification of parameters
Lukasz Wojciechowski
l.wojciechow at partner.samsung.com
Wed Apr 8 17:49:32 CEST 2020
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 at partner.samsung.com
More information about the dev
mailing list