netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).