* [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum()
@ 2024-10-09 18:44 Eric Dumazet
2024-10-09 18:44 ` [PATCH net-next 1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq Eric Dumazet
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Eric Dumazet @ 2024-10-09 18:44 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Kuniyuki Iwashima, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
This series is inspired by a syzbot report showing
rtnl contention and one thread blocked in:
7 locks held by syz-executor/10835:
#0: ffff888033390420 (sb_writers#8){.+.+}-{0:0}, at: file_start_write include/linux/fs.h:2931 [inline]
#0: ffff888033390420 (sb_writers#8){.+.+}-{0:0}, at: vfs_write+0x224/0xc90 fs/read_write.c:679
#1: ffff88806df6bc88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x1ea/0x500 fs/kernfs/file.c:325
#2: ffff888026fcf3c8 (kn->active#50){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x20e/0x500 fs/kernfs/file.c:326
#3: ffffffff8f56f848 (nsim_bus_dev_list_lock){+.+.}-{3:3}, at: new_device_store+0x1b4/0x890 drivers/net/netdevsim/bus.c:166
#4: ffff88805e0140e8 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:1014 [inline]
#4: ffff88805e0140e8 (&dev->mutex){....}-{3:3}, at: __device_attach+0x8e/0x520 drivers/base/dd.c:1005
#5: ffff88805c5fb250 (&devlink->lock_key#55){+.+.}-{3:3}, at: nsim_drv_probe+0xcb/0xb80 drivers/net/netdevsim/dev.c:1534
#6: ffffffff8fcd1748 (rtnl_mutex){+.+.}-{3:3}, at: fib_seq_sum+0x31/0x290 net/core/fib_notifier.c:46
This is not a bug fix, unless I am mistaken, thus targeting net-next.
Eric Dumazet (5):
fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq
ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq
ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq
ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq
net: do not acquire rtnl in fib_seq_sum()
include/net/fib_notifier.h | 2 +-
include/net/fib_rules.h | 2 +-
include/net/ip6_fib.h | 8 ++++----
include/net/ip_fib.h | 4 ++--
include/net/netns/ipv4.h | 2 +-
net/core/fib_notifier.c | 2 --
net/core/fib_rules.c | 14 ++++++++------
net/ipv4/fib_notifier.c | 10 +++++-----
net/ipv4/fib_rules.c | 2 +-
net/ipv4/ipmr.c | 10 ++++------
net/ipv6/fib6_notifier.c | 2 +-
net/ipv6/fib6_rules.c | 2 +-
net/ipv6/ip6_fib.c | 14 +++++++-------
net/ipv6/ip6mr.c | 10 ++++------
14 files changed, 40 insertions(+), 44 deletions(-)
--
2.47.0.rc0.187.ge670bccf7e-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
@ 2024-10-09 18:44 ` Eric Dumazet
2024-10-09 23:47 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 2/5] ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq Eric Dumazet
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Eric Dumazet @ 2024-10-09 18:44 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Kuniyuki Iwashima, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
Using RTNL to protect ops->fib_rules_seq reads seems a big hammer.
Writes are protected by RTNL.
We can use READ_ONCE() on readers.
Constify 'struct net' argument of fib_rules_seq_read()
and lookup_rules_ops().
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/fib_rules.h | 2 +-
net/core/fib_rules.c | 14 ++++++++------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index d17855c52ef926383f2585a6f31094899f1e7908..04383d90a1e38847d9d10f8fd0c4bf2ef67af713 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -176,7 +176,7 @@ int fib_default_rule_add(struct fib_rules_ops *, u32 pref, u32 table);
bool fib_rule_matchall(const struct fib_rule *rule);
int fib_rules_dump(struct net *net, struct notifier_block *nb, int family,
struct netlink_ext_ack *extack);
-unsigned int fib_rules_seq_read(struct net *net, int family);
+unsigned int fib_rules_seq_read(const struct net *net, int family);
int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack);
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 154a2681f55cc6861d418927c396bec5d840578c..82ef090c0037817f15902d8784467f21419b5af7 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -101,7 +101,8 @@ static void notify_rule_change(int event, struct fib_rule *rule,
struct fib_rules_ops *ops, struct nlmsghdr *nlh,
u32 pid);
-static struct fib_rules_ops *lookup_rules_ops(struct net *net, int family)
+static struct fib_rules_ops *lookup_rules_ops(const struct net *net,
+ int family)
{
struct fib_rules_ops *ops;
@@ -370,7 +371,9 @@ static int call_fib_rule_notifiers(struct net *net,
.rule = rule,
};
- ops->fib_rules_seq++;
+ ASSERT_RTNL();
+ /* Paired with READ_ONCE() in fib_rules_seq() */
+ WRITE_ONCE(ops->fib_rules_seq, ops->fib_rules_seq + 1);
return call_fib_notifiers(net, event_type, &info.info);
}
@@ -397,17 +400,16 @@ int fib_rules_dump(struct net *net, struct notifier_block *nb, int family,
}
EXPORT_SYMBOL_GPL(fib_rules_dump);
-unsigned int fib_rules_seq_read(struct net *net, int family)
+unsigned int fib_rules_seq_read(const struct net *net, int family)
{
unsigned int fib_rules_seq;
struct fib_rules_ops *ops;
- ASSERT_RTNL();
-
ops = lookup_rules_ops(net, family);
if (!ops)
return 0;
- fib_rules_seq = ops->fib_rules_seq;
+ /* Paired with WRITE_ONCE() in call_fib_rule_notifiers() */
+ fib_rules_seq = READ_ONCE(ops->fib_rules_seq);
rules_ops_put(ops);
return fib_rules_seq;
--
2.47.0.rc0.187.ge670bccf7e-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 2/5] ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
2024-10-09 18:44 ` [PATCH net-next 1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq Eric Dumazet
@ 2024-10-09 18:44 ` Eric Dumazet
2024-10-09 23:53 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 3/5] ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq Eric Dumazet
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Eric Dumazet @ 2024-10-09 18:44 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Kuniyuki Iwashima, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
Using RTNL to protect ops->fib_rules_seq reads seems a big hammer.
Writes are protected by RTNL.
We can use READ_ONCE() when reading it.
Constify 'struct net' argument of fib4_rules_seq_read()
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip_fib.h | 4 ++--
include/net/netns/ipv4.h | 2 +-
net/ipv4/fib_notifier.c | 8 ++++----
net/ipv4/fib_rules.c | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 967e4dc555face31a736f6108cea6b929478beba..4c2e2d1481ebff5292b9e433f56fa7289ba8e139 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -347,7 +347,7 @@ static inline int fib4_rules_dump(struct net *net, struct notifier_block *nb,
return 0;
}
-static inline unsigned int fib4_rules_seq_read(struct net *net)
+static inline unsigned int fib4_rules_seq_read(const struct net *net)
{
return 0;
}
@@ -411,7 +411,7 @@ static inline bool fib4_has_custom_rules(const struct net *net)
bool fib4_rule_default(const struct fib_rule *rule);
int fib4_rules_dump(struct net *net, struct notifier_block *nb,
struct netlink_ext_ack *extack);
-unsigned int fib4_rules_seq_read(struct net *net);
+unsigned int fib4_rules_seq_read(const struct net *net);
static inline bool fib4_rules_early_flow_dissect(struct net *net,
struct sk_buff *skb,
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 276f622f3516871c438be27bafe61c039445b335..10c0a8dc37a23e793007ee47706b402fffbd08da 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -263,7 +263,7 @@ struct netns_ipv4 {
#endif
struct fib_notifier_ops *notifier_ops;
- unsigned int fib_seq; /* protected by rtnl_mutex */
+ unsigned int fib_seq; /* writes protected by rtnl_mutex */
struct fib_notifier_ops *ipmr_notifier_ops;
unsigned int ipmr_seq; /* protected by rtnl_mutex */
diff --git a/net/ipv4/fib_notifier.c b/net/ipv4/fib_notifier.c
index 0e23ade74493ce10a3f2572c39e091f132684884..21c85c80de641112d66a28645b1fb17c7071863f 100644
--- a/net/ipv4/fib_notifier.c
+++ b/net/ipv4/fib_notifier.c
@@ -22,15 +22,15 @@ int call_fib4_notifiers(struct net *net, enum fib_event_type event_type,
ASSERT_RTNL();
info->family = AF_INET;
- net->ipv4.fib_seq++;
+ /* Paired with READ_ONCE() in fib4_seq_read() */
+ WRITE_ONCE(net->ipv4.fib_seq, net->ipv4.fib_seq + 1);
return call_fib_notifiers(net, event_type, info);
}
static unsigned int fib4_seq_read(struct net *net)
{
- ASSERT_RTNL();
-
- return net->ipv4.fib_seq + fib4_rules_seq_read(net);
+ /* Paired with WRITE_ONCE() in call_fib4_notifiers() */
+ return READ_ONCE(net->ipv4.fib_seq) + fib4_rules_seq_read(net);
}
static int fib4_dump(struct net *net, struct notifier_block *nb,
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index b07292d50ee76603f983c81a55d45abca89266e1..8325224ef07232d05b59c58011625daae847af30 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -74,7 +74,7 @@ int fib4_rules_dump(struct net *net, struct notifier_block *nb,
return fib_rules_dump(net, nb, AF_INET, extack);
}
-unsigned int fib4_rules_seq_read(struct net *net)
+unsigned int fib4_rules_seq_read(const struct net *net)
{
return fib_rules_seq_read(net, AF_INET);
}
--
2.47.0.rc0.187.ge670bccf7e-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 3/5] ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
2024-10-09 18:44 ` [PATCH net-next 1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq Eric Dumazet
2024-10-09 18:44 ` [PATCH net-next 2/5] ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq Eric Dumazet
@ 2024-10-09 18:44 ` Eric Dumazet
2024-10-09 23:55 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 4/5] ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq Eric Dumazet
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Eric Dumazet @ 2024-10-09 18:44 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Kuniyuki Iwashima, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
Using RTNL to protect ops->fib_rules_seq reads seems a big hammer.
Writes are protected by RTNL.
We can use READ_ONCE() when reading it.
Constify 'struct net' argument of fib6_tables_seq_read() and
fib6_rules_seq_read().
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip6_fib.h | 8 ++++----
net/ipv6/fib6_rules.c | 2 +-
net/ipv6/ip6_fib.c | 14 +++++++-------
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 6cb867ce4878423fbb9049e69445a6dbf8f31ba7..7c87873ae211c5fa80d34e8f3b8df0e813976390 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -394,7 +394,7 @@ struct fib6_table {
struct fib6_node tb6_root;
struct inet_peer_base tb6_peers;
unsigned int flags;
- unsigned int fib_seq;
+ unsigned int fib_seq; /* writes protected by rtnl_mutex */
struct hlist_head tb6_gc_hlist; /* GC candidates */
#define RT6_TABLE_HAS_DFLT_ROUTER BIT(0)
};
@@ -563,7 +563,7 @@ int call_fib6_notifiers(struct net *net, enum fib_event_type event_type,
int __net_init fib6_notifier_init(struct net *net);
void __net_exit fib6_notifier_exit(struct net *net);
-unsigned int fib6_tables_seq_read(struct net *net);
+unsigned int fib6_tables_seq_read(const struct net *net);
int fib6_tables_dump(struct net *net, struct notifier_block *nb,
struct netlink_ext_ack *extack);
@@ -632,7 +632,7 @@ void fib6_rules_cleanup(void);
bool fib6_rule_default(const struct fib_rule *rule);
int fib6_rules_dump(struct net *net, struct notifier_block *nb,
struct netlink_ext_ack *extack);
-unsigned int fib6_rules_seq_read(struct net *net);
+unsigned int fib6_rules_seq_read(const struct net *net);
static inline bool fib6_rules_early_flow_dissect(struct net *net,
struct sk_buff *skb,
@@ -676,7 +676,7 @@ static inline int fib6_rules_dump(struct net *net, struct notifier_block *nb,
{
return 0;
}
-static inline unsigned int fib6_rules_seq_read(struct net *net)
+static inline unsigned int fib6_rules_seq_read(const struct net *net)
{
return 0;
}
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 04a9ed5e8310f23cb7d947b732be5dd19916bf39..c85c1627cb16ed0bdfe4c6026bb0132cdd7be6b7 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -56,7 +56,7 @@ int fib6_rules_dump(struct net *net, struct notifier_block *nb,
return fib_rules_dump(net, nb, AF_INET6, extack);
}
-unsigned int fib6_rules_seq_read(struct net *net)
+unsigned int fib6_rules_seq_read(const struct net *net)
{
return fib_rules_seq_read(net, AF_INET6);
}
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index eb111d20615c6274647eeb413d0b9475aaa3ae6c..cea160b249d2d75d03c867d2298da76eb0c7114e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -345,17 +345,17 @@ static void __net_init fib6_tables_init(struct net *net)
#endif
-unsigned int fib6_tables_seq_read(struct net *net)
+unsigned int fib6_tables_seq_read(const struct net *net)
{
unsigned int h, fib_seq = 0;
rcu_read_lock();
for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
- struct hlist_head *head = &net->ipv6.fib_table_hash[h];
- struct fib6_table *tb;
+ const struct hlist_head *head = &net->ipv6.fib_table_hash[h];
+ const struct fib6_table *tb;
hlist_for_each_entry_rcu(tb, head, tb6_hlist)
- fib_seq += tb->fib_seq;
+ fib_seq += READ_ONCE(tb->fib_seq);
}
rcu_read_unlock();
@@ -400,7 +400,7 @@ int call_fib6_entry_notifiers(struct net *net,
.rt = rt,
};
- rt->fib6_table->fib_seq++;
+ WRITE_ONCE(rt->fib6_table->fib_seq, rt->fib6_table->fib_seq + 1);
return call_fib6_notifiers(net, event_type, &info.info);
}
@@ -416,7 +416,7 @@ int call_fib6_multipath_entry_notifiers(struct net *net,
.nsiblings = nsiblings,
};
- rt->fib6_table->fib_seq++;
+ WRITE_ONCE(rt->fib6_table->fib_seq, rt->fib6_table->fib_seq + 1);
return call_fib6_notifiers(net, event_type, &info.info);
}
@@ -427,7 +427,7 @@ int call_fib6_entry_notifiers_replace(struct net *net, struct fib6_info *rt)
.nsiblings = rt->fib6_nsiblings,
};
- rt->fib6_table->fib_seq++;
+ WRITE_ONCE(rt->fib6_table->fib_seq, rt->fib6_table->fib_seq + 1);
return call_fib6_notifiers(net, FIB_EVENT_ENTRY_REPLACE, &info.info);
}
--
2.47.0.rc0.187.ge670bccf7e-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 4/5] ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
` (2 preceding siblings ...)
2024-10-09 18:44 ` [PATCH net-next 3/5] ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq Eric Dumazet
@ 2024-10-09 18:44 ` Eric Dumazet
2024-10-10 0:05 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 5/5] net: do not acquire rtnl in fib_seq_sum() Eric Dumazet
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Eric Dumazet @ 2024-10-09 18:44 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Kuniyuki Iwashima, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
mr_call_vif_notifiers() and mr_call_mfc_notifiers() already
uses WRITE_ONCE() on the write side.
Using RTNL to protect the reads seems a big hammer.
Constify 'struct net' argument of ip6mr_rules_seq_read()
and ipmr_rules_seq_read().
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/ipmr.c | 8 +++-----
net/ipv6/ip6mr.c | 8 +++-----
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 089864c6a35eec146a1ba90c22d79245f8e48158..35ed0316518424c7742a93bd72d56295e1eb01aa 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -288,7 +288,7 @@ static int ipmr_rules_dump(struct net *net, struct notifier_block *nb,
return fib_rules_dump(net, nb, RTNL_FAMILY_IPMR, extack);
}
-static unsigned int ipmr_rules_seq_read(struct net *net)
+static unsigned int ipmr_rules_seq_read(const struct net *net)
{
return fib_rules_seq_read(net, RTNL_FAMILY_IPMR);
}
@@ -346,7 +346,7 @@ static int ipmr_rules_dump(struct net *net, struct notifier_block *nb,
return 0;
}
-static unsigned int ipmr_rules_seq_read(struct net *net)
+static unsigned int ipmr_rules_seq_read(const struct net *net)
{
return 0;
}
@@ -3037,9 +3037,7 @@ static const struct net_protocol pim_protocol = {
static unsigned int ipmr_seq_read(struct net *net)
{
- ASSERT_RTNL();
-
- return net->ipv4.ipmr_seq + ipmr_rules_seq_read(net);
+ return READ_ONCE(net->ipv4.ipmr_seq) + ipmr_rules_seq_read(net);
}
static int ipmr_dump(struct net *net, struct notifier_block *nb,
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 2ce4ae0d8dc3b443986d7a7b4177a057f5affaec..3f9501fd8c1ae583d4862128e8620ce6cc114d25 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -276,7 +276,7 @@ static int ip6mr_rules_dump(struct net *net, struct notifier_block *nb,
return fib_rules_dump(net, nb, RTNL_FAMILY_IP6MR, extack);
}
-static unsigned int ip6mr_rules_seq_read(struct net *net)
+static unsigned int ip6mr_rules_seq_read(const struct net *net)
{
return fib_rules_seq_read(net, RTNL_FAMILY_IP6MR);
}
@@ -335,7 +335,7 @@ static int ip6mr_rules_dump(struct net *net, struct notifier_block *nb,
return 0;
}
-static unsigned int ip6mr_rules_seq_read(struct net *net)
+static unsigned int ip6mr_rules_seq_read(const struct net *net)
{
return 0;
}
@@ -1262,9 +1262,7 @@ static int ip6mr_device_event(struct notifier_block *this,
static unsigned int ip6mr_seq_read(struct net *net)
{
- ASSERT_RTNL();
-
- return net->ipv6.ipmr_seq + ip6mr_rules_seq_read(net);
+ return READ_ONCE(net->ipv6.ipmr_seq) + ip6mr_rules_seq_read(net);
}
static int ip6mr_dump(struct net *net, struct notifier_block *nb,
--
2.47.0.rc0.187.ge670bccf7e-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 5/5] net: do not acquire rtnl in fib_seq_sum()
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
` (3 preceding siblings ...)
2024-10-09 18:44 ` [PATCH net-next 4/5] ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq Eric Dumazet
@ 2024-10-09 18:44 ` Eric Dumazet
2024-10-10 0:07 ` Kuniyuki Iwashima
2024-10-09 21:16 ` [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() David Ahern
2024-10-11 22:50 ` patchwork-bot+netdevbpf
6 siblings, 1 reply; 13+ messages in thread
From: Eric Dumazet @ 2024-10-09 18:44 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Kuniyuki Iwashima, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
After we made sure no fib_seq_read() handlers needs RTNL anymore,
we can remove RTNL from fib_seq_sum().
Note that after RTNL was dropped, fib_seq_sum() result was possibly
outdated anyway.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/fib_notifier.h | 2 +-
net/core/fib_notifier.c | 2 --
net/ipv4/fib_notifier.c | 2 +-
net/ipv4/ipmr.c | 2 +-
net/ipv6/fib6_notifier.c | 2 +-
net/ipv6/ip6mr.c | 2 +-
6 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/net/fib_notifier.h b/include/net/fib_notifier.h
index 6d59221ff05ad605dea6bf0e5f3d6b9d24237537..48aad6128feab6452c91e11ec31f9f1f238032d8 100644
--- a/include/net/fib_notifier.h
+++ b/include/net/fib_notifier.h
@@ -28,7 +28,7 @@ enum fib_event_type {
struct fib_notifier_ops {
int family;
struct list_head list;
- unsigned int (*fib_seq_read)(struct net *net);
+ unsigned int (*fib_seq_read)(const struct net *net);
int (*fib_dump)(struct net *net, struct notifier_block *nb,
struct netlink_ext_ack *extack);
struct module *owner;
diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
index fc96259807b62fd2b41c47f385d7db23a7e38186..5cdca49b1d7c1cc9097c6a9fe93fae701c54eaca 100644
--- a/net/core/fib_notifier.c
+++ b/net/core/fib_notifier.c
@@ -43,7 +43,6 @@ static unsigned int fib_seq_sum(struct net *net)
struct fib_notifier_ops *ops;
unsigned int fib_seq = 0;
- rtnl_lock();
rcu_read_lock();
list_for_each_entry_rcu(ops, &fn_net->fib_notifier_ops, list) {
if (!try_module_get(ops->owner))
@@ -52,7 +51,6 @@ static unsigned int fib_seq_sum(struct net *net)
module_put(ops->owner);
}
rcu_read_unlock();
- rtnl_unlock();
return fib_seq;
}
diff --git a/net/ipv4/fib_notifier.c b/net/ipv4/fib_notifier.c
index 21c85c80de641112d66a28645b1fb17c7071863f..b1551c26554b79b70a29c9fab7f5ba8d7c0049c5 100644
--- a/net/ipv4/fib_notifier.c
+++ b/net/ipv4/fib_notifier.c
@@ -27,7 +27,7 @@ int call_fib4_notifiers(struct net *net, enum fib_event_type event_type,
return call_fib_notifiers(net, event_type, info);
}
-static unsigned int fib4_seq_read(struct net *net)
+static unsigned int fib4_seq_read(const struct net *net)
{
/* Paired with WRITE_ONCE() in call_fib4_notifiers() */
return READ_ONCE(net->ipv4.fib_seq) + fib4_rules_seq_read(net);
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 35ed0316518424c7742a93bd72d56295e1eb01aa..7a95daeb1946ad2f9c6d00a75469e37f92dddf9c 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -3035,7 +3035,7 @@ static const struct net_protocol pim_protocol = {
};
#endif
-static unsigned int ipmr_seq_read(struct net *net)
+static unsigned int ipmr_seq_read(const struct net *net)
{
return READ_ONCE(net->ipv4.ipmr_seq) + ipmr_rules_seq_read(net);
}
diff --git a/net/ipv6/fib6_notifier.c b/net/ipv6/fib6_notifier.c
index f87ae33e1d01f4e8d55f2af435bd8eff72bd9ea6..949b72610df704b6afe455daf0d680dbe4504e87 100644
--- a/net/ipv6/fib6_notifier.c
+++ b/net/ipv6/fib6_notifier.c
@@ -22,7 +22,7 @@ int call_fib6_notifiers(struct net *net, enum fib_event_type event_type,
return call_fib_notifiers(net, event_type, info);
}
-static unsigned int fib6_seq_read(struct net *net)
+static unsigned int fib6_seq_read(const struct net *net)
{
return fib6_tables_seq_read(net) + fib6_rules_seq_read(net);
}
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 3f9501fd8c1ae583d4862128e8620ce6cc114d25..9528e17665fdb0ce4be07c76703ba74f06386370 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1260,7 +1260,7 @@ static int ip6mr_device_event(struct notifier_block *this,
return NOTIFY_DONE;
}
-static unsigned int ip6mr_seq_read(struct net *net)
+static unsigned int ip6mr_seq_read(const struct net *net)
{
return READ_ONCE(net->ipv6.ipmr_seq) + ip6mr_rules_seq_read(net);
}
--
2.47.0.rc0.187.ge670bccf7e-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum()
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
` (4 preceding siblings ...)
2024-10-09 18:44 ` [PATCH net-next 5/5] net: do not acquire rtnl in fib_seq_sum() Eric Dumazet
@ 2024-10-09 21:16 ` David Ahern
2024-10-11 22:50 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 13+ messages in thread
From: David Ahern @ 2024-10-09 21:16 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Kuniyuki Iwashima, Jiri Pirko, netdev, eric.dumazet
On 10/9/24 12:44 PM, Eric Dumazet wrote:
> This series is inspired by a syzbot report showing
> rtnl contention and one thread blocked in:
>
> 7 locks held by syz-executor/10835:
> #0: ffff888033390420 (sb_writers#8){.+.+}-{0:0}, at: file_start_write include/linux/fs.h:2931 [inline]
> #0: ffff888033390420 (sb_writers#8){.+.+}-{0:0}, at: vfs_write+0x224/0xc90 fs/read_write.c:679
> #1: ffff88806df6bc88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x1ea/0x500 fs/kernfs/file.c:325
> #2: ffff888026fcf3c8 (kn->active#50){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x20e/0x500 fs/kernfs/file.c:326
> #3: ffffffff8f56f848 (nsim_bus_dev_list_lock){+.+.}-{3:3}, at: new_device_store+0x1b4/0x890 drivers/net/netdevsim/bus.c:166
> #4: ffff88805e0140e8 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:1014 [inline]
> #4: ffff88805e0140e8 (&dev->mutex){....}-{3:3}, at: __device_attach+0x8e/0x520 drivers/base/dd.c:1005
> #5: ffff88805c5fb250 (&devlink->lock_key#55){+.+.}-{3:3}, at: nsim_drv_probe+0xcb/0xb80 drivers/net/netdevsim/dev.c:1534
> #6: ffffffff8fcd1748 (rtnl_mutex){+.+.}-{3:3}, at: fib_seq_sum+0x31/0x290 net/core/fib_notifier.c:46
>
> This is not a bug fix, unless I am mistaken, thus targeting net-next.
>
>
> Eric Dumazet (5):
> fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq
> ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq
> ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq
> ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq
> net: do not acquire rtnl in fib_seq_sum()
>
> include/net/fib_notifier.h | 2 +-
> include/net/fib_rules.h | 2 +-
> include/net/ip6_fib.h | 8 ++++----
> include/net/ip_fib.h | 4 ++--
> include/net/netns/ipv4.h | 2 +-
> net/core/fib_notifier.c | 2 --
> net/core/fib_rules.c | 14 ++++++++------
> net/ipv4/fib_notifier.c | 10 +++++-----
> net/ipv4/fib_rules.c | 2 +-
> net/ipv4/ipmr.c | 10 ++++------
> net/ipv6/fib6_notifier.c | 2 +-
> net/ipv6/fib6_rules.c | 2 +-
> net/ipv6/ip6_fib.c | 14 +++++++-------
> net/ipv6/ip6mr.c | 10 ++++------
> 14 files changed, 40 insertions(+), 44 deletions(-)
>
For the set:
Reviewed-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq
2024-10-09 18:44 ` [PATCH net-next 1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq Eric Dumazet
@ 2024-10-09 23:47 ` Kuniyuki Iwashima
0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2024-10-09 23:47 UTC (permalink / raw)
To: edumazet; +Cc: davem, dsahern, eric.dumazet, jiri, kuba, kuniyu, netdev, pabeni
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 9 Oct 2024 18:44:01 +0000
> Using RTNL to protect ops->fib_rules_seq reads seems a big hammer.
>
> Writes are protected by RTNL.
> We can use READ_ONCE() on readers.
>
> Constify 'struct net' argument of fib_rules_seq_read()
> and lookup_rules_ops().
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/5] ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq
2024-10-09 18:44 ` [PATCH net-next 2/5] ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq Eric Dumazet
@ 2024-10-09 23:53 ` Kuniyuki Iwashima
0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2024-10-09 23:53 UTC (permalink / raw)
To: edumazet; +Cc: davem, dsahern, eric.dumazet, jiri, kuba, kuniyu, netdev, pabeni
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 9 Oct 2024 18:44:02 +0000
> Using RTNL to protect ops->fib_rules_seq reads seems a big hammer.
>
> Writes are protected by RTNL.
> We can use READ_ONCE() when reading it.
>
> Constify 'struct net' argument of fib4_rules_seq_read()
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 3/5] ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq
2024-10-09 18:44 ` [PATCH net-next 3/5] ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq Eric Dumazet
@ 2024-10-09 23:55 ` Kuniyuki Iwashima
0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2024-10-09 23:55 UTC (permalink / raw)
To: edumazet; +Cc: davem, dsahern, eric.dumazet, jiri, kuba, kuniyu, netdev, pabeni
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 9 Oct 2024 18:44:03 +0000
> Using RTNL to protect ops->fib_rules_seq reads seems a big hammer.
>
> Writes are protected by RTNL.
> We can use READ_ONCE() when reading it.
>
> Constify 'struct net' argument of fib6_tables_seq_read() and
> fib6_rules_seq_read().
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 4/5] ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq
2024-10-09 18:44 ` [PATCH net-next 4/5] ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq Eric Dumazet
@ 2024-10-10 0:05 ` Kuniyuki Iwashima
0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2024-10-10 0:05 UTC (permalink / raw)
To: edumazet; +Cc: davem, dsahern, eric.dumazet, jiri, kuba, kuniyu, netdev, pabeni
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 9 Oct 2024 18:44:04 +0000
> mr_call_vif_notifiers() and mr_call_mfc_notifiers() already
> uses WRITE_ONCE() on the write side.
>
> Using RTNL to protect the reads seems a big hammer.
>
> Constify 'struct net' argument of ip6mr_rules_seq_read()
> and ipmr_rules_seq_read().
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 5/5] net: do not acquire rtnl in fib_seq_sum()
2024-10-09 18:44 ` [PATCH net-next 5/5] net: do not acquire rtnl in fib_seq_sum() Eric Dumazet
@ 2024-10-10 0:07 ` Kuniyuki Iwashima
0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2024-10-10 0:07 UTC (permalink / raw)
To: edumazet; +Cc: davem, dsahern, eric.dumazet, jiri, kuba, kuniyu, netdev, pabeni
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 9 Oct 2024 18:44:05 +0000
> After we made sure no fib_seq_read() handlers needs RTNL anymore,
> we can remove RTNL from fib_seq_sum().
>
> Note that after RTNL was dropped, fib_seq_sum() result was possibly
> outdated anyway.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum()
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
` (5 preceding siblings ...)
2024-10-09 21:16 ` [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() David Ahern
@ 2024-10-11 22:50 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-10-11 22:50 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, dsahern, kuniyu, jiri, netdev, eric.dumazet
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 9 Oct 2024 18:44:00 +0000 you wrote:
> This series is inspired by a syzbot report showing
> rtnl contention and one thread blocked in:
>
> 7 locks held by syz-executor/10835:
> #0: ffff888033390420 (sb_writers#8){.+.+}-{0:0}, at: file_start_write include/linux/fs.h:2931 [inline]
> #0: ffff888033390420 (sb_writers#8){.+.+}-{0:0}, at: vfs_write+0x224/0xc90 fs/read_write.c:679
> #1: ffff88806df6bc88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x1ea/0x500 fs/kernfs/file.c:325
> #2: ffff888026fcf3c8 (kn->active#50){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x20e/0x500 fs/kernfs/file.c:326
> #3: ffffffff8f56f848 (nsim_bus_dev_list_lock){+.+.}-{3:3}, at: new_device_store+0x1b4/0x890 drivers/net/netdevsim/bus.c:166
> #4: ffff88805e0140e8 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:1014 [inline]
> #4: ffff88805e0140e8 (&dev->mutex){....}-{3:3}, at: __device_attach+0x8e/0x520 drivers/base/dd.c:1005
> #5: ffff88805c5fb250 (&devlink->lock_key#55){+.+.}-{3:3}, at: nsim_drv_probe+0xcb/0xb80 drivers/net/netdevsim/dev.c:1534
> #6: ffffffff8fcd1748 (rtnl_mutex){+.+.}-{3:3}, at: fib_seq_sum+0x31/0x290 net/core/fib_notifier.c:46
>
> [...]
Here is the summary with links:
- [net-next,1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq
https://git.kernel.org/netdev/net-next/c/a716ff52bebf
- [net-next,2/5] ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq
https://git.kernel.org/netdev/net-next/c/16207384d292
- [net-next,3/5] ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq
https://git.kernel.org/netdev/net-next/c/e60ea4544776
- [net-next,4/5] ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq
https://git.kernel.org/netdev/net-next/c/055202b16c58
- [net-next,5/5] net: do not acquire rtnl in fib_seq_sum()
https://git.kernel.org/netdev/net-next/c/2698acd6ea47
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-10-11 22:50 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 18:44 [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() Eric Dumazet
2024-10-09 18:44 ` [PATCH net-next 1/5] fib: rules: use READ_ONCE()/WRITE_ONCE() on ops->fib_rules_seq Eric Dumazet
2024-10-09 23:47 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 2/5] ipv4: use READ_ONCE()/WRITE_ONCE() on net->ipv4.fib_seq Eric Dumazet
2024-10-09 23:53 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 3/5] ipv6: use READ_ONCE()/WRITE_ONCE() on fib6_table->fib_seq Eric Dumazet
2024-10-09 23:55 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 4/5] ipmr: use READ_ONCE() to read net->ipv[46].ipmr_seq Eric Dumazet
2024-10-10 0:05 ` Kuniyuki Iwashima
2024-10-09 18:44 ` [PATCH net-next 5/5] net: do not acquire rtnl in fib_seq_sum() Eric Dumazet
2024-10-10 0:07 ` Kuniyuki Iwashima
2024-10-09 21:16 ` [PATCH net-next 0/5] net: remove RTNL from fib_seq_sum() David Ahern
2024-10-11 22:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).