DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] net/enic: check notify set return value during init
@ 2026-07-15 10:31 Alexey Simakov
  2026-07-16  0:40 ` Hyong Youb Kim (hyonkim)
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Simakov @ 2026-07-15 10:31 UTC (permalink / raw)
  To: John Daley, Hyong Youb Kim, Neil Horman, Sujith Sankar,
	David Marchand
  Cc: dev, stable, Alexey Simakov

The return value of vnic_dev_notify_set() is silently ignored in
enic_dev_init(), so a memory allocation failure or hardware command
error goes unnoticed and the driver continues with uninitialized
notification state.

Check the return value and propagate the error to abort probe when
notification setup fails.

Fixes: fefed3d1e62c ("enic: new driver")
Cc: stable@dpdk.org

Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
---
v2 changes: validate return code of vnic_dev_notify_set()
in driver init section
v1 link:
https://patches.dpdk.org/project/dpdk/patch/20260707112014.82821-1-bigalex934@gmail.com/

 drivers/net/enic/enic_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 2696fa77d4..f46f429135 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1887,7 +1887,9 @@ static int enic_dev_init(struct enic *enic)
 	LIST_INIT(&enic->flows);
 
 	/* set up link status checking */
-	vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
+	err = vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
+	if (err)
+		return err;
 
 	enic->overlay_offload = false;
 	/*
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH v2] net/enic: check notify set return value during init
  2026-07-15 10:31 [PATCH v2] net/enic: check notify set return value during init Alexey Simakov
@ 2026-07-16  0:40 ` Hyong Youb Kim (hyonkim)
  2026-07-17  9:22   ` Alexey Simakov
  0 siblings, 1 reply; 6+ messages in thread
From: Hyong Youb Kim (hyonkim) @ 2026-07-16  0:40 UTC (permalink / raw)
  To: Alexey Simakov, John Daley (johndale), Neil Horman, Sujith Sankar,
	David Marchand
  Cc: dev@dpdk.org, stable@dpdk.org



> -----Original Message-----
> From: Alexey Simakov <bigalex934@gmail.com>
> Sent: Wednesday, July 15, 2026 7:31 PM
> To: John Daley (johndale) <johndale@cisco.com>; Hyong Youb Kim (hyonkim)
> <hyonkim@cisco.com>; Neil Horman <nhorman@tuxdriver.com>; Sujith Sankar
> <ssujith@cisco.com>; David Marchand <david.marchand@redhat.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Alexey Simakov <bigalex934@gmail.com>
> Subject: [PATCH v2] net/enic: check notify set return value during init
> 
> The return value of vnic_dev_notify_set() is silently ignored in
> enic_dev_init(), so a memory allocation failure or hardware command
> error goes unnoticed and the driver continues with uninitialized
> notification state.
> 
> Check the return value and propagate the error to abort probe when
> notification setup fails.
> 
> Fixes: fefed3d1e62c ("enic: new driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
> ---
> v2 changes: validate return code of vnic_dev_notify_set()
> in driver init section
> v1 link:
> https://patches.dpdk.org/project/dpdk/patch/20260707112014.82821-1-
> bigalex934@gmail.com/
> 
>  drivers/net/enic/enic_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 2696fa77d4..f46f429135 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -1887,7 +1887,9 @@ static int enic_dev_init(struct enic *enic)
>  	LIST_INIT(&enic->flows);
> 
>  	/* set up link status checking */
> -	vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
> +	err = vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
> +	if (err)
> +		return err;
> 

Could you add an error message, like the following? Looks like all error returns in this function log a message.

dev_err(enic, "failed to enable notify buffer\n");

Thanks.
-Hyong

>  	enic->overlay_offload = false;
>  	/*
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] net/enic: check notify set return value during init
  2026-07-16  0:40 ` Hyong Youb Kim (hyonkim)
@ 2026-07-17  9:22   ` Alexey Simakov
  2026-07-19  3:04     ` Hyong Youb Kim (hyonkim)
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Simakov @ 2026-07-17  9:22 UTC (permalink / raw)
  To: Hyong Youb Kim (hyonkim)
  Cc: John Daley (johndale), Neil Horman, Sujith Sankar, David Marchand,
	dev@dpdk.org, stable@dpdk.org

Sure, also dpdk ai review higlightet potential problems in memory
management

For example:

enic->cq = rte_zmalloc("enic_vnic_cq", sizeof(struct vnic_cq) *
			       enic->conf_cq_count, 8);
enic->intr = rte_zmalloc("enic_vnic_intr", sizeof(struct vnic_intr) *
			enic->conf_intr_count, 8);
enic->rq = rte_zmalloc("enic_vnic_rq", sizeof(struct vnic_rq) *
			enic->conf_rq_count, 8);
enic->wq = rte_zmalloc("enic_vnic_wq", sizeof(struct vnic_wq) *
			enic->conf_wq_count, 8);

if (enic->conf_cq_count > 0 && enic->cq == NULL) {
	dev_err(enic, "failed to allocate vnic_cq, aborting.\n");
	return -1;
}
if (enic->conf_intr_count > 0 && enic->intr == NULL) {
	dev_err(enic, "failed to allocate vnic_intr, aborting.\n");
	return -1;
}

Lets imagine that enic->cq is allocated successfully, after that
when we trying to allocate enic->intr we are failing, so in this
case we leaking in second if statement, this related to all error
handling paths in driver, since as far as I understand resource
free happening only in enic_dev_deinit(...) function.

So my question is it problem? If yes, should I address this issue
in scope of my patch or better to make it in another?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v2] net/enic: check notify set return value during init
  2026-07-17  9:22   ` Alexey Simakov
@ 2026-07-19  3:04     ` Hyong Youb Kim (hyonkim)
  2026-07-20  9:33       ` [PATCH v3] " Alexey Simakov
  0 siblings, 1 reply; 6+ messages in thread
From: Hyong Youb Kim (hyonkim) @ 2026-07-19  3:04 UTC (permalink / raw)
  To: Alexey Simakov
  Cc: John Daley (johndale), Neil Horman, Sujith Sankar, David Marchand,
	dev@dpdk.org, stable@dpdk.org



> -----Original Message-----
> From: Alexey Simakov <bigalex934@gmail.com>
> Sent: Friday, July 17, 2026 6:23 PM
> To: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>
> Cc: John Daley (johndale) <johndale@cisco.com>; Neil Horman
> <nhorman@tuxdriver.com>; Sujith Sankar <ssujith@cisco.com>; David
> Marchand <david.marchand@redhat.com>; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH v2] net/enic: check notify set return value during init
> 
> Sure, also dpdk ai review higlightet potential problems in memory
> management
> 
> For example:
> 
> enic->cq = rte_zmalloc("enic_vnic_cq", sizeof(struct vnic_cq) *
> 			       enic->conf_cq_count, 8);
> enic->intr = rte_zmalloc("enic_vnic_intr", sizeof(struct vnic_intr) *
> 			enic->conf_intr_count, 8);
> enic->rq = rte_zmalloc("enic_vnic_rq", sizeof(struct vnic_rq) *
> 			enic->conf_rq_count, 8);
> enic->wq = rte_zmalloc("enic_vnic_wq", sizeof(struct vnic_wq) *
> 			enic->conf_wq_count, 8);
> 
> if (enic->conf_cq_count > 0 && enic->cq == NULL) {
> 	dev_err(enic, "failed to allocate vnic_cq, aborting.\n");
> 	return -1;
> }
> if (enic->conf_intr_count > 0 && enic->intr == NULL) {
> 	dev_err(enic, "failed to allocate vnic_intr, aborting.\n");
> 	return -1;
> }
> 
> Lets imagine that enic->cq is allocated successfully, after that
> when we trying to allocate enic->intr we are failing, so in this
> case we leaking in second if statement, this related to all error
> handling paths in driver, since as far as I understand resource
> free happening only in enic_dev_deinit(...) function.
> 
> So my question is it problem? If yes, should I address this issue
> in scope of my patch or better to make it in another?

Why don't you create one cleanup patch that addresses both issues,
since they are in the same function?

Thanks.
-Hyong

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3] net/enic: check notify set return value during init
  2026-07-19  3:04     ` Hyong Youb Kim (hyonkim)
@ 2026-07-20  9:33       ` Alexey Simakov
  2026-07-20 22:56         ` Hyong Youb Kim (hyonkim)
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Simakov @ 2026-07-20  9:33 UTC (permalink / raw)
  To: hyonkim; +Cc: david.marchand, dev, johndale, ssujith, stable, Alexey Simakov

The return value of vnic_dev_notify_set() is silently ignored in
enic_dev_init(), so a memory allocation failure or hardware command
error goes unnoticed and the driver continues with uninitialized
notification state.

Check the return value and propagate the error to abort probe when
notification setup fails.

Fixes: fefed3d1e62c ("enic: new driver")
Cc: stable@dpdk.org

Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
---

v3 changes: add log message

v2 link: https://patches.dpdk.org/project/dpdk/patch/20260715103105.39417-1-bigalex934@gmail.com/
v2 changes: validate return code of vnic_dev_notify_set() in driver init section

v1 link: https://patches.dpdk.org/project/dpdk/patch/20260707112014.82821-1-bigalex934@gmail.com/

 drivers/net/enic/enic_main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 2696fa77d4..2a1a65d8da 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1887,7 +1887,11 @@ static int enic_dev_init(struct enic *enic)
 	LIST_INIT(&enic->flows);
 
 	/* set up link status checking */
-	vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
+	err = vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
+	if (err) {
+		dev_err(enic, "failed to enable notify buffer\n");
+		return err;
+	}
 
 	enic->overlay_offload = false;
 	/*
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH v3] net/enic: check notify set return value during init
  2026-07-20  9:33       ` [PATCH v3] " Alexey Simakov
@ 2026-07-20 22:56         ` Hyong Youb Kim (hyonkim)
  0 siblings, 0 replies; 6+ messages in thread
From: Hyong Youb Kim (hyonkim) @ 2026-07-20 22:56 UTC (permalink / raw)
  To: Alexey Simakov
  Cc: david.marchand@redhat.com, dev@dpdk.org, John Daley (johndale),
	ssujith@cisco.com, stable@dpdk.org



> -----Original Message-----
> From: Alexey Simakov <bigalex934@gmail.com>
> Sent: Monday, July 20, 2026 6:34 PM
> To: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>
> Cc: david.marchand@redhat.com; dev@dpdk.org; John Daley (johndale)
> <johndale@cisco.com>; ssujith@cisco.com; stable@dpdk.org; Alexey Simakov
> <bigalex934@gmail.com>
> Subject: [PATCH v3] net/enic: check notify set return value during init
> 
> The return value of vnic_dev_notify_set() is silently ignored in
> enic_dev_init(), so a memory allocation failure or hardware command
> error goes unnoticed and the driver continues with uninitialized
> notification state.
> 
> Check the return value and propagate the error to abort probe when
> notification setup fails.
> 
> Fixes: fefed3d1e62c ("enic: new driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
> ---
> 
> v3 changes: add log message
> 
> v2 link: https://patches.dpdk.org/project/dpdk/patch/20260715103105.39417-
> 1-bigalex934@gmail.com/
> v2 changes: validate return code of vnic_dev_notify_set() in driver init section
> 
> v1 link: https://patches.dpdk.org/project/dpdk/patch/20260707112014.82821-
> 1-bigalex934@gmail.com/
> 
>  drivers/net/enic/enic_main.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 2696fa77d4..2a1a65d8da 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -1887,7 +1887,11 @@ static int enic_dev_init(struct enic *enic)
>  	LIST_INIT(&enic->flows);
> 
>  	/* set up link status checking */
> -	vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
> +	err = vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
> +	if (err) {
> +		dev_err(enic, "failed to enable notify buffer\n");
> +		return err;
> +	}
> 

Acked-by: Hyong Youb Kim <hyonkim@cisco.com>

Thanks.
-Hyong

>  	enic->overlay_offload = false;
>  	/*
> --
> 2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-20 22:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 10:31 [PATCH v2] net/enic: check notify set return value during init Alexey Simakov
2026-07-16  0:40 ` Hyong Youb Kim (hyonkim)
2026-07-17  9:22   ` Alexey Simakov
2026-07-19  3:04     ` Hyong Youb Kim (hyonkim)
2026-07-20  9:33       ` [PATCH v3] " Alexey Simakov
2026-07-20 22:56         ` Hyong Youb Kim (hyonkim)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox