| Message ID | 20210609103326.278782-4-toke@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Clean up and document RCU-based object protection for XDP_REDIRECT | expand |
On Wed, Jun 09, 2021 at 12:33:12PM +0200, Toke Høiland-Jørgensen wrote: > Some of the XDP helpers (in particular, xdp_do_redirect()) will get a > struct net_device reference using dev_get_by_index_rcu(). These are called > from a NAPI poll context, which means the RCU reference liveness is ensured > by local_bh_disable(). Add rcu_read_lock_bh_held() as a condition to the > RCU list traversal in dev_get_by_index_rcu() so lockdep understands that > the dereferences are safe from *both* an rcu_read_lock() *and* with > local_bh_disable(). > > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > net/core/dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index febb23708184..a499c5ffe4a5 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1002,7 +1002,7 @@ struct net_device *dev_get_by_index_rcu(struct net *net, int ifindex) > struct net_device *dev; > struct hlist_head *head = dev_index_hash(net, ifindex); > > - hlist_for_each_entry_rcu(dev, head, index_hlist) > + hlist_for_each_entry_rcu(dev, head, index_hlist, rcu_read_lock_bh_held()) Is it needed? hlist_for_each_entry_rcu() checks for rcu_read_lock_any_held(). Did lockdep complain?
Martin KaFai Lau <kafai@fb.com> writes: > On Wed, Jun 09, 2021 at 12:33:12PM +0200, Toke Høiland-Jørgensen wrote: >> Some of the XDP helpers (in particular, xdp_do_redirect()) will get a >> struct net_device reference using dev_get_by_index_rcu(). These are called >> from a NAPI poll context, which means the RCU reference liveness is ensured >> by local_bh_disable(). Add rcu_read_lock_bh_held() as a condition to the >> RCU list traversal in dev_get_by_index_rcu() so lockdep understands that >> the dereferences are safe from *both* an rcu_read_lock() *and* with >> local_bh_disable(). >> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- >> net/core/dev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/core/dev.c b/net/core/dev.c >> index febb23708184..a499c5ffe4a5 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -1002,7 +1002,7 @@ struct net_device *dev_get_by_index_rcu(struct net *net, int ifindex) >> struct net_device *dev; >> struct hlist_head *head = dev_index_hash(net, ifindex); >> >> - hlist_for_each_entry_rcu(dev, head, index_hlist) >> + hlist_for_each_entry_rcu(dev, head, index_hlist, rcu_read_lock_bh_held()) > Is it needed? hlist_for_each_entry_rcu() checks for > rcu_read_lock_any_held(). Did lockdep complain? Ah, yes, I think you're right. I totally missed that rcu_read_lock_any_held() includes a '!preemptible()' check at the end. I'll drop this patch, then! -Toke
diff --git a/net/core/dev.c b/net/core/dev.c index febb23708184..a499c5ffe4a5 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1002,7 +1002,7 @@ struct net_device *dev_get_by_index_rcu(struct net *net, int ifindex) struct net_device *dev; struct hlist_head *head = dev_index_hash(net, ifindex); - hlist_for_each_entry_rcu(dev, head, index_hlist) + hlist_for_each_entry_rcu(dev, head, index_hlist, rcu_read_lock_bh_held()) if (dev->ifindex == ifindex) return dev;
Some of the XDP helpers (in particular, xdp_do_redirect()) will get a struct net_device reference using dev_get_by_index_rcu(). These are called from a NAPI poll context, which means the RCU reference liveness is ensured by local_bh_disable(). Add rcu_read_lock_bh_held() as a condition to the RCU list traversal in dev_get_by_index_rcu() so lockdep understands that the dereferences are safe from *both* an rcu_read_lock() *and* with local_bh_disable(). Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- net/core/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)