diff mbox series

[net-next,1/2] netdev-genl: do not use rtnl in netdev_nl_napi_get_doit()

Message ID 20241009232728.107604-2-edumazet@google.com (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series net: do not rely on rtnl in netdev_nl_napi_get_xxx() | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 43 this patch: 43
netdev/build_tools success Errors and warnings before: 0 (+1) this patch: 0 (+1)
netdev/cc_maintainers warning 1 maintainers not CCed: amritha.nambiar@intel.com
netdev/build_clang success Errors and warnings before: 83 this patch: 83
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4142 this patch: 4142
netdev/checkpatch warning CHECK: Alignment should match open parenthesis
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 101 this patch: 101
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-10-11--15-00 (tests: 776)

Commit Message

Eric Dumazet Oct. 9, 2024, 11:27 p.m. UTC 
With upcoming per netns RTNL, rtnl use in netdev_nl_napi_get_doit()
is a bit problematic.

Prepare netdev_nl_napi_fill_one() to not rely on RTNL:

1) netif_napi_set_irq() uses WRITE_ONCE(napi->irq, ...)

2) napi_kthread_create() uses WRITE_ONCE(napi->thread, ...)

3) Add napi->thread_pid_nr to avoid race in netdev_nl_napi_fill_one
   and __netif_napi_del()

4) netdev_nl_napi_fill_one() uses corresponding READ_ONCE()

5) netdev_nl_napi_get_doit() can use RCU instead of RTNL

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/netdevice.h |  3 ++-
 net/core/dev.c            | 21 ++++++++++++---------
 net/core/netdev-genl.c    | 21 +++++++++++----------
 3 files changed, 25 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3baf8e539b6f33caaf83961c4cf619b799e5e41d..64a5e4927901740db8dbc255ed19faca96820333 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -377,6 +377,7 @@  struct napi_struct {
 	struct list_head	dev_list;
 	struct hlist_node	napi_hash_node;
 	int			irq;
+	pid_t			thread_pid_nr;
 };
 
 enum {
@@ -2618,7 +2619,7 @@  void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
 
 static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
 {
-	napi->irq = irq;
+	WRITE_ONCE(napi->irq, irq);
 }
 
 /* Default NAPI poll() weight
diff --git a/net/core/dev.c b/net/core/dev.c
index ea5fbcd133ae4c743545945def00790ec74e2bb6..77c39a95e74df2485777bc008a507bdcc4e75a00 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1423,21 +1423,23 @@  static int napi_threaded_poll(void *data);
 
 static int napi_kthread_create(struct napi_struct *n)
 {
-	int err = 0;
+	struct task_struct *thread;
 
 	/* Create and wake up the kthread once to put it in
 	 * TASK_INTERRUPTIBLE mode to avoid the blocked task
 	 * warning and work with loadavg.
 	 */
-	n->thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
+	thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
 				n->dev->name, n->napi_id);
-	if (IS_ERR(n->thread)) {
-		err = PTR_ERR(n->thread);
+	if (IS_ERR(thread)) {
+		int err = PTR_ERR(thread);
+
 		pr_err("kthread_run failed with err %d\n", err);
-		n->thread = NULL;
+		return err;
 	}
-
-	return err;
+	WRITE_ONCE(n->thread, thread);
+	WRITE_ONCE(n->thread_pid_nr, task_pid_nr(thread));
+	return 0;
 }
 
 static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack)
@@ -6668,6 +6670,7 @@  void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
 	set_bit(NAPI_STATE_SCHED, &napi->state);
 	set_bit(NAPI_STATE_NPSVC, &napi->state);
 	list_add_rcu(&napi->dev_list, &dev->napi_list);
+	netif_napi_set_irq(napi, -1);
 	napi_hash_add(napi);
 	napi_get_frags_check(napi);
 	/* Create kthread for this napi if dev->threaded is set.
@@ -6676,7 +6679,6 @@  void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
 	 */
 	if (dev->threaded && napi_kthread_create(napi))
 		dev->threaded = false;
-	netif_napi_set_irq(napi, -1);
 }
 EXPORT_SYMBOL(netif_napi_add_weight);
 
@@ -6753,7 +6755,8 @@  void __netif_napi_del(struct napi_struct *napi)
 
 	if (napi->thread) {
 		kthread_stop(napi->thread);
-		napi->thread = NULL;
+		WRITE_ONCE(napi->thread, NULL);
+		WRITE_ONCE(napi->thread_pid_nr, 0);
 	}
 }
 EXPORT_SYMBOL(__netif_napi_del);
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 1cb954f2d39e8248bffd854cdf27eceb25293425..0dcfe3527c122884c5713e56d5e27d4e638d936f 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -163,10 +163,11 @@  netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
 {
 	void *hdr;
 	pid_t pid;
+	int irq;
 
 	if (WARN_ON_ONCE(!napi->dev))
 		return -EINVAL;
-	if (!(napi->dev->flags & IFF_UP))
+	if (!(READ_ONCE(napi->dev->flags) & IFF_UP))
 		return 0;
 
 	hdr = genlmsg_iput(rsp, info);
@@ -177,17 +178,17 @@  netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
 	    nla_put_u32(rsp, NETDEV_A_NAPI_ID, napi->napi_id))
 		goto nla_put_failure;
 
-	if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex))
+	if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX,
+			READ_ONCE(napi->dev->ifindex)))
 		goto nla_put_failure;
 
-	if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
+	irq = READ_ONCE(napi->irq);
+	if (irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, irq))
 		goto nla_put_failure;
 
-	if (napi->thread) {
-		pid = task_pid_nr(napi->thread);
-		if (nla_put_u32(rsp, NETDEV_A_NAPI_PID, pid))
-			goto nla_put_failure;
-	}
+	pid = READ_ONCE(napi->thread_pid_nr);
+	if (pid && nla_put_u32(rsp, NETDEV_A_NAPI_PID, pid))
+		goto nla_put_failure;
 
 	genlmsg_end(rsp, hdr);
 
@@ -214,7 +215,7 @@  int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
 	if (!rsp)
 		return -ENOMEM;
 
-	rtnl_lock();
+	rcu_read_lock();
 
 	napi = napi_by_id(napi_id);
 	if (napi) {
@@ -224,7 +225,7 @@  int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
 		err = -ENOENT;
 	}
 
-	rtnl_unlock();
+	rcu_read_unlock();
 
 	if (err)
 		goto err_free_msg;