[1/2] eal/windows: Add needed calls to detect vdev PMD
Checks
Commit Message
From: Tal Shnaiderman <talshn@mellanox.com>
Add needed function calls in rte_eal_init to detect vdev PMD.
eal_option_device_parse()
rte_service_init()
rte_bus_probe()
Signed-off-by: Tal Shnaiderman <talshn@mellanox.com>
---
lib/librte_eal/common/meson.build | 1 +
lib/librte_eal/windows/eal.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
Comments
Windows team, I am expecting at least 2 reviews on this patch.
24/06/2020 16:56, talshn@mellanox.com:
> From: Tal Shnaiderman <talshn@mellanox.com>
>
> Add needed function calls in rte_eal_init to detect vdev PMD.
>
> eal_option_device_parse()
> rte_service_init()
> rte_bus_probe()
>
> Signed-off-by: Tal Shnaiderman <talshn@mellanox.com>
> ---
> lib/librte_eal/common/meson.build | 1 +
> lib/librte_eal/windows/eal.c | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
> index dc9b1d2feb..a38b66a2d0 100644
> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> @@ -30,6 +30,7 @@ if is_windows
> 'malloc_heap.c',
> 'rte_malloc.c',
> 'eal_common_timer.c',
> + 'rte_service.c',
> )
> subdir_done()
> endif
> diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
> index d8cfe5cc4d..8e89560aba 100644
> --- a/lib/librte_eal/windows/eal.c
> +++ b/lib/librte_eal/windows/eal.c
> @@ -270,6 +270,11 @@ rte_eal_init(int argc, char **argv)
> if (fctret < 0)
> exit(1);
>
> + if (eal_option_device_parse()) {
> + rte_errno = ENODEV;
> + return -1;
> + }
> +
> /* Prevent creation of shared memory files. */
> if (internal_conf->in_memory == 0) {
> RTE_LOG(WARNING, EAL, "Multi-process support is requested, "
> @@ -359,6 +364,20 @@ rte_eal_init(int argc, char **argv)
> rte_panic("Cannot create thread\n");
> }
>
> + /* initialize services so vdevs register service during bus_probe. */
> + if (rte_service_init()) {
> + rte_eal_init_alert("rte_service_init() failed");
> + rte_errno = ENOEXEC;
> + return -1;
> + }
> +
> + /* Probe all the buses and devices/drivers on them */
> + if (rte_bus_probe()) {
> + rte_eal_init_alert("Cannot probe devices");
> + rte_errno = ENOTSUP;
> + return -1;
> + }
> +
> /*
> * Launch a dummy function on all slave lcores, so that master lcore
> * knows they are all ready when this function returns.
On Wed, Jun 24, 2020 at 05:56:20PM +0300, talshn@mellanox.com wrote:
> From: Tal Shnaiderman <talshn@mellanox.com>
>
> Add needed function calls in rte_eal_init to detect vdev PMD.
>
> eal_option_device_parse()
> rte_service_init()
> rte_bus_probe()
>
> Signed-off-by: Tal Shnaiderman <talshn@mellanox.com>
> ---
> lib/librte_eal/common/meson.build | 1 +
> lib/librte_eal/windows/eal.c | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
Implicit declaration warning:
../lib/librte_eal/windows/eal.c:371:6: warning: implicit declaration of function 'rte_service_init' is invalid in C99 [-Wimplicit-function-declaration]
if (rte_service_init()) {
^
1 warning generated.
Adding "#include <rte_service_component.h>" in windows/eal.c should solve it.
Getting some linker error:
Creating library drivers\librte_bus_vdev.dll.a and object drivers\librte_bus_vdev.dll.exp
bus_vdev_vdev.c.obj : error LNK2019: unresolved external symbol rte_log_register_type_and_pick_level referenced in function __vdev_logtype_bus
drivers\librte_bus_vdev-0.200.3.dll : fatal error LNK1120: 1 unresolved externals
Probably just a missing export for "rte_log_register_type_and_pick_level" in rte_eal_exports.def?
After adding the missing include and export, compilation and linking are successful.
I see some errors when running the app though:
EAL: error allocating rte services array
EAL: FATAL: rte_service_init() failed
EAL: rte_service_init() failed
PANIC in main():
Cannot init EAL
6: [<unknown> (RtlUserThreadStart+0x21)[0x7FFACF283460]]
5: [<unknown> (BaseThreadInitThunk+0x14)[0x7FFACE7A6DF0]]
4: [<missing_symbols>]
3: [<missing_symbols>]
2: [<missing_symbols>]
1: [<missing_symbols>]
Thank you for the comments, you're right, either master has changed since I sent those patches or I missed out on some exports/includes, anyhow I'll fix and resend v2.
The rte_panic you're seeing is unrelated to the new code (BTW, you should build a debug version if you want to see rte_panic's backtrace), it is a result of your setup not being configured with the memory management requirement of "Lock pages" (see [1]).
Dmitry, It looks like we got to this stage since hugepage_claim_privilege() cannot actually detect that "Lock pages" isn't granted to the current user, as a result we fail on the first usage of a memory management call [in this case rte_calloc()] without indication to the reason.
Is it possible to add an actual check that the current user is in the list of grantees?
Alternatively, It would be great to have this privilege added programmatically, I tried the MSDN example in [2] but it didn't work for me while testing, maybe Microsoft team can check if there is a way to do it?
[1] https://doc.dpdk.org/guides/windows_gsg/run_apps.html
[2] https://docs.microsoft.com/en-us/windows/win32/memory/creating-a-file-mapping-using-large-pages?redirectedfrom=MSDN
> -----Original Message-----
> From: Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>
> Subject: Re: [PATCH 1/2] eal/windows: Add needed calls to detect vdev PMD
>
> On Wed, Jun 24, 2020 at 05:56:20PM +0300, talshn@mellanox.com wrote:
> > From: Tal Shnaiderman <talshn@mellanox.com>
> >
> > Add needed function calls in rte_eal_init to detect vdev PMD.
> >
> > eal_option_device_parse()
> > rte_service_init()
> > rte_bus_probe()
> >
> > Signed-off-by: Tal Shnaiderman <talshn@mellanox.com>
> > ---
> > lib/librte_eal/common/meson.build | 1 +
> > lib/librte_eal/windows/eal.c | 19 +++++++++++++++++++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/lib/librte_eal/common/meson.build
> > b/lib/librte_eal/common/meson.build
>
> Implicit declaration warning:
> ../lib/librte_eal/windows/eal.c:371:6: warning: implicit declaration of function
> 'rte_service_init' is invalid in C99 [-Wimplicit-function-declaration]
> if (rte_service_init()) {
> ^
> 1 warning generated.
>
> Adding "#include <rte_service_component.h>" in windows/eal.c should
> solve it.
>
> Getting some linker error:
> Creating library drivers\librte_bus_vdev.dll.a and object
> drivers\librte_bus_vdev.dll.exp bus_vdev_vdev.c.obj : error LNK2019:
> unresolved external symbol rte_log_register_type_and_pick_level
> referenced in function __vdev_logtype_bus drivers\librte_bus_vdev-
> 0.200.3.dll : fatal error LNK1120: 1 unresolved externals
>
> Probably just a missing export for "rte_log_register_type_and_pick_level" in
> rte_eal_exports.def?
>
> After adding the missing include and export, compilation and linking are
> successful.
> I see some errors when running the app though:
>
> EAL: error allocating rte services array
> EAL: FATAL: rte_service_init() failed
> EAL: rte_service_init() failed
> PANIC in main():
> Cannot init EAL
> 6: [<unknown> (RtlUserThreadStart+0x21)[0x7FFACF283460]]
> 5: [<unknown> (BaseThreadInitThunk+0x14)[0x7FFACE7A6DF0]]
> 4: [<missing_symbols>]
> 3: [<missing_symbols>]
> 2: [<missing_symbols>]
> 1: [<missing_symbols>]
On Tue, 7 Jul 2020 08:04:00 +0000, Tal Shnaiderman wrote:
> Dmitry, It looks like we got to this stage since hugepage_claim_privilege() cannot actually detect that "Lock pages" isn't granted to the current user, as a result we fail on the first usage of a memory management call [in this case rte_calloc()] without indication to the reason.
>
> Is it possible to add an actual check that the current user is in the list of grantees?
Thanks, I'll look into it.
> Alternatively, It would be great to have this privilege added programmatically, I tried the MSDN example in [2] but it didn't work for me while testing, maybe Microsoft team can check if there is a way to do it?
I don't think it's a good idea from security perspective if an application
grants its user new privileges implicitly. Process with SeLockMemory
privilege can affect overall system performance and stability.
On 7/7/2020 1:39 AM, Dmitry Kozlyuk wrote:
> On Tue, 7 Jul 2020 08:04:00 +0000, Tal Shnaiderman wrote:
>> Dmitry, It looks like we got to this stage since hugepage_claim_privilege() cannot actually detect that "Lock pages" isn't granted to the current user, as a result we fail on the first usage of a memory management call [in this case rte_calloc()] without indication to the reason.
>>
>> Is it possible to add an actual check that the current user is in the list of grantees?
> Thanks, I'll look into it.
>
>> Alternatively, It would be great to have this privilege added programmatically, I tried the MSDN example in [2] but it didn't work for me while testing, maybe Microsoft team can check if there is a way to do it?
> I don't think it's a good idea from security perspective if an application
> grants its user new privileges implicitly. Process with SeLockMemory
> privilege can affect overall system performance and stability.
I agree. This is something we forbid, when we do security reviews for
our other products here inside Intel.
Best to have the user explicitly acquire this privilege.
ranjit m.
@@ -30,6 +30,7 @@ if is_windows
'malloc_heap.c',
'rte_malloc.c',
'eal_common_timer.c',
+ 'rte_service.c',
)
subdir_done()
endif
@@ -270,6 +270,11 @@ rte_eal_init(int argc, char **argv)
if (fctret < 0)
exit(1);
+ if (eal_option_device_parse()) {
+ rte_errno = ENODEV;
+ return -1;
+ }
+
/* Prevent creation of shared memory files. */
if (internal_conf->in_memory == 0) {
RTE_LOG(WARNING, EAL, "Multi-process support is requested, "
@@ -359,6 +364,20 @@ rte_eal_init(int argc, char **argv)
rte_panic("Cannot create thread\n");
}
+ /* initialize services so vdevs register service during bus_probe. */
+ if (rte_service_init()) {
+ rte_eal_init_alert("rte_service_init() failed");
+ rte_errno = ENOEXEC;
+ return -1;
+ }
+
+ /* Probe all the buses and devices/drivers on them */
+ if (rte_bus_probe()) {
+ rte_eal_init_alert("Cannot probe devices");
+ rte_errno = ENOTSUP;
+ return -1;
+ }
+
/*
* Launch a dummy function on all slave lcores, so that master lcore
* knows they are all ready when this function returns.