On Tue, 21 Jan 2025 18:07:40 +0100
Serhii Iliushyk <sil-...@napatech.com> wrote:
> From: Danylo Vodopianov <dvo-...@napatech.com>
>
> CI found couple coverity problems which were fixed in this commit.
>
> CID: 448965 Error handling issues (CHECKED_RETURN).
>
> Thread return code check was added.
>
> Coverity issue: 448965
> Fixes: a1ba8c473f5c ("net/ntnic: add statistics poll")
>
> Signed-off-by: Danylo Vodopianov <dvo-...@napatech.com>
> ---
> drivers/net/ntnic/ntnic_ethdev.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ntnic/ntnic_ethdev.c
> b/drivers/net/ntnic/ntnic_ethdev.c
> index 2a2643a106..620d023a71 100644
> --- a/drivers/net/ntnic/ntnic_ethdev.c
> +++ b/drivers/net/ntnic/ntnic_ethdev.c
> @@ -2516,8 +2516,11 @@ static int init_shutdown(void)
> NT_LOG(DBG, NTNIC, "Starting shutdown handler");
> kill_pmd = 0;
> previous_handler = signal(SIGINT, signal_handler_func_int);
> - THREAD_CREATE(&shutdown_tid, shutdown_thread, NULL);
> -
> + int ret = THREAD_CREATE(&shutdown_tid, shutdown_thread, NULL);
> + if (ret != 0) {
> + NT_LOG(ERR, NTNIC, "Failed to create shutdown thread, error
> code: %d", ret);
> + return -1;
> + }
> /*
> * 1 time calculation of 1 sec stat update rtc cycles to prevent stat
> poll
> * flooding by OVS from multiple virtual port threads - no need to be
> precise
> --
The whole thread, SIGINT stuff is a big mistake.
You are making all sorts of assumptions about application environment
such as SIGINT won't be used by other things, drivers creating internal threads
is a bad idea.
Also access to the flag kill_pmd is not being done in a thread safe manner.
What is it really trying to do?
IMHO, The driver should have never been merged with this stuff.
