[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241009232728.107604-2-edumazet@google.com>
Date: Wed, 9 Oct 2024 23:27:26 +0000
From: Eric Dumazet <edumazet@...gle.com>
To: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: Kuniyuki Iwashima <kuniyu@...zon.com>, netdev@...r.kernel.org, eric.dumazet@...il.com,
Eric Dumazet <edumazet@...gle.com>
Subject: [PATCH net-next 1/2] netdev-genl: do not use rtnl in netdev_nl_napi_get_doit()
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@...gle.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 --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;
--
2.47.0.rc0.187.ge670bccf7e-goog
Powered by blists - more mailing lists