IB/core: Add ordered workqueue for RoCE GID management

PATCH In review Leon Romanovsky 2 participants 2 messages May 30, 2017 – Jun 6, 2017 1 message not in the archive InfiniBand

lore

Share & tools

lore

Leon Romanovsky

author

2017-05-30 06:58 UTC #0 lore raw

PATCH 1/1 IB/core: Add ordered workqueue for RoCE GID management

From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/[email protected]>

Currently the RoCE GID management uses the ib_wq to do add and delete new GIDs according to the netdev events.

The ib_wq isn't an ordered workqueue and thus two work elements can be executed concurrently which will result in unexpected behavior and inconsistency of the GIDs cache content.

Example:
ifconfig eth1 11.11.11.11/16 up

This command will invoke the following netdev events in the following order:
1. NETDEV_UP
2. NETDEV_DOWN
3. NETDEV_UP

If (2) and (3) will be executed concurrently or in reverse order, instead of having a new GID with 11.11.11.11 IP, we will end up without any new GIDs.

Signed-off-by Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/roce_gid_mgmt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

drivers/infiniband/core/roce_gid_mgmt.c +9 -2

index db958d3207ef..94a9eefb3cfc 100644
@@ -42,6 +42,8 @@
 #include <rdma/ib_cache.h>
 #include <rdma/ib_addr.h>
 
+static struct workqueue_struct *gid_cache_wq;
+
 enum gid_op_type {
 	GID_DEL = 0,
 	GID_ADD

@@ -560,7 +562,7 @@ static int netdevice_queue_work(struct netdev_event_work_cmd *cmds,
 	}
 	INIT_WORK(&ndev_work->work, netdevice_event_work_handler);
 
-	queue_work(ib_wq, &ndev_work->work);
+	queue_work(gid_cache_wq, &ndev_work->work);
 
 	return NOTIFY_DONE;
 }

@@ -693,7 +695,7 @@ static int addr_event(struct notifier_block *this, unsigned long event,
 	dev_hold(ndev);
 	work->gid_attr.ndev   = ndev;
 
-	queue_work(ib_wq, &work->work);
+	queue_work(gid_cache_wq, &work->work);
 
 	return NOTIFY_DONE;
 }

@@ -740,6 +742,10 @@ static struct notifier_block nb_inet6addr = {
 
 int __init roce_gid_mgmt_init(void)
 {
+	gid_cache_wq = alloc_ordered_workqueue("gid-cache-wq", 0);
+	if (!gid_cache_wq)
+		return -ENOMEM;
+
 	register_inetaddr_notifier(&nb_inetaddr);
 	if (IS_ENABLED(CONFIG_IPV6))
 		register_inet6addr_notifier(&nb_inet6addr);

@@ -764,4 +770,5 @@ void __exit roce_gid_mgmt_cleanup(void)
 	 * ib-core is removed, all physical devices have been removed,
 	 * so no issue with remaining hardware contexts.
 	 */
+	destroy_workqueue(gid_cache_wq);
 }

--
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170530065806.6245-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Yuval Shaia

Reviewed-by

2017-06-06 07:12 UTC #2 lore raw

Quoted 69 lines · Leon Romanovsky

From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/[email protected]>
Currently the RoCE GID management uses the ib_wq to do add and delete new GIDs according to the netdev events.
The ib_wq isn't an ordered workqueue and thus two work elements can be executed concurrently which will result in unexpected behavior and inconsistency of the GIDs cache content.
Example:
ifconfig eth1 11.11.11.11/16 up
This command will invoke the following netdev events in the following order:
1. NETDEV_UP
2. NETDEV_DOWN
3. NETDEV_UP
If (2) and (3) will be executed concurrently or in reverse order, instead of having a new GID with 11.11.11.11 IP, we will end up without any new GIDs.
Signed-off-by Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/roce_gid_mgmt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/infiniband/core/roce_gid_mgmt.c +8 -2
index db958d3207ef..94a9eefb3cfc 100644
@@ -42,6 +42,8 @@
 #include <rdma/ib_cache.h>
 #include <rdma/ib_addr.h>
 
+static struct workqueue_struct *gid_cache_wq;
+
 enum gid_op_type {
 	GID_DEL = 0,
 	GID_ADD
@@ -560,7 +562,7 @@ static int netdevice_queue_work(struct netdev_event_work_cmd *cmds,
 	}
 	INIT_WORK(&ndev_work->work, netdevice_event_work_handler);
 
-	queue_work(ib_wq, &ndev_work->work);
+	queue_work(gid_cache_wq, &ndev_work->work);
 
 	return NOTIFY_DONE;
 }
@@ -693,7 +695,7 @@ static int addr_event(struct notifier_block *this, unsigned long event,
 	dev_hold(ndev);
 	work->gid_attr.ndev   = ndev;
 
-	queue_work(ib_wq, &work->work);
+	queue_work(gid_cache_wq, &work->work);
 
 	return NOTIFY_DONE;
 }
@@ -740,6 +742,10 @@ static struct notifier_block nb_inet6addr = {
 
 int __init roce_gid_mgmt_init(void)
 {
+	gid_cache_wq = alloc_ordered_workqueue("gid-cache-wq", 0);
+	if (!gid_cache_wq)
+		return -ENOMEM;
+
 	register_inetaddr_notifier(&nb_inetaddr);
 	if (IS_ENABLED(CONFIG_IPV6))
 		register_inet6addr_notifier(&nb_inet6addr);

drivers/infiniband/core/roce_gid_mgmt.c void __exit roce_gid_mgmt_cleanup(void) · line 774

Quoted 5 lines

drivers/infiniband/core/roce_gid_mgmt.c +1 -0

@@ -764,4 +770,5 @@ void __exit roce_gid_mgmt_cleanup(void)
 	 * ib-core is removed, all physical devices have been removed,
 	 * so no issue with remaining hardware contexts.
 	 */
+	destroy_workqueue(gid_cache_wq);
 }

Reviewed-by Yuval Shaia <[email protected]>

Quoted 7 lines

--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html