Skip to contentSkip to conversation

eal: fix device unregister for event registered with device_name NULL

PATCH Applied [email protected] 3 participants 3 messages Jul 18, 2024 – Oct 10, 2024

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 eal: fix device unregister for event registered with device_name NULL

From: Malcolm Bumgardner <[email protected]>

In the device event unregister code, it unconditionally remove all callbacks which are registered with device_name set to NULL. This results in many callbacks uncorrectly removed.

Fix this by only removing callbacks with matching cb_fn and cb_arg.

Signed-off-by Malcolm Bumgardner <[email protected]>
Fixes a753e53d517b ("eal: add device event monitor framework")
Cc [email protected]
Signed-off-by Long Li <[email protected]>
---
 lib/eal/common/eal_common_dev.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
lib/eal/common/eal_common_dev.c +7 -6
index a99252b02f..70aa04dcd9 100644
@@ -550,16 +550,17 @@ rte_dev_event_callback_unregister(const char *device_name,
 		next = TAILQ_NEXT(event_cb, next);
 
 		if (device_name != NULL && event_cb->dev_name != NULL) {
-			if (!strcmp(event_cb->dev_name, device_name)) {
-				if (event_cb->cb_fn != cb_fn ||
-				    (cb_arg != (void *)-1 &&
-				    event_cb->cb_arg != cb_arg))
-					continue;
-			}
+			if (strcmp(event_cb->dev_name, device_name))
+				continue;
 		} else if (device_name != NULL) {
 			continue;
 		}
 
+		/* Remove only matching callback with arg */
+		if (event_cb->cb_fn != cb_fn ||
+		    (cb_arg != (void *)-1 && event_cb->cb_arg != cb_arg))
+			continue;
+
 		/*
 		 * if this callback is not executing right now,
 		 * then remove it.
Signature · 1 line
2.43.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 8 lines · On Thu, 18 Jul 2024 12:37:28 -0700 [email protected] wrote:

From: Malcolm Bumgardner <[email protected]>

In the device event unregister code, it unconditionally remove all callbacks which are registered with device_name set to NULL. This results in many callbacks uncorrectly removed.

Fix this by only removing callbacks with matching cb_fn and cb_arg.

Quoted 8 lines
Signed-off-by Malcolm Bumgardner <[email protected]>
Fixes a753e53d517b ("eal: add device event monitor framework")
Cc [email protected]
Signed-off-by Long Li <[email protected]>
---
 lib/eal/common/eal_common_dev.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

This patch solves the problem in the netvsc driver.
The netvsc driver registers a device event callback to discover when the VF is added/removed. When the netvsc driver is stoppped, it unregisters this callback. The bug is that the rte_dev code would end up removing all callbacks, including the callback for other netvsc devices.

Perhaps a bugzilla entry or more direct example would have gotten more attention on this.

Reviewed-by Stephen Hemminger <[email protected]>
Reviewed-by Stephen Hemminger on v1 1/1 Applied
#2 lore raw
In reply to [email protected]
Quoted 7 lines · On Thu, Jul 18, 2024 at 9:37 PM <[email protected]> wrote:

From: Malcolm Bumgardner <[email protected]>

In the device event unregister code, it unconditionally remove all callbacks which are registered with device_name set to NULL. This results in many callbacks uncorrectly removed.

Quoted 7 lines

Fix this by only removing callbacks with matching cb_fn and cb_arg.

Fixes a753e53d517b ("eal: add device event monitor framework")
Cc [email protected]
Signed-off-by Malcolm Bumgardner <[email protected]>
Signed-off-by Long Li <[email protected]>
Reviewed-by Stephen Hemminger <[email protected]>

Applied, thanks.

Signature · 1 line
David Marchand