net/sched: don't use dynamic lockdep keys with clsact/ingress/noqueue

Currently we are registering one dynamic lockdep key for each allocated
qdisc, to avoid false deadlock reports when mirred (or TC eBPF) redirects
packets to another device while the root lock is acquired [1].
Since dynamic keys are a limited resource, we can save them at least for
qdiscs that are not meant to acquire the root lock in the traffic path,
or to carry traffic at all, like:

 - clsact
 - ingress
 - noqueue

Don't register dynamic keys for the above schedulers, so that we hit
MAX_LOCKDEP_KEYS later in our tests.

[1] https://github.com/multipath-tcp/mptcp_net-next/issues/451

Changes in v2:
 - change ordering of spin_lock_init() vs. lockdep_register_key()
   (Jakub Kicinski)

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Link: https://patch.msgid.link/94448f7fa7c4f52d2ce416a4895ec87d456d7417.1770220576.git.dcaratti@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index e703c507d0daa97ae7c3bf131e322b1eafcc5664..18a419cd9d949b3d29813521deb990dce51b2a11 100644 (file)
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -308,4 +308,28 @@ static inline unsigned int qdisc_peek_len(struct Qdisc *sch)
        return len;
 }
 
+static inline void qdisc_lock_init(struct Qdisc *sch,
+                                  const struct Qdisc_ops *ops)
+{
+       spin_lock_init(&sch->q.lock);
+
+       /* Skip dynamic keys if nesting is not possible */
+       if (ops->static_flags & TCQ_F_INGRESS ||
+           ops == &noqueue_qdisc_ops)
+               return;
+
+       lockdep_register_key(&sch->root_lock_key);
+       lockdep_set_class(&sch->q.lock, &sch->root_lock_key);
+}
+
+static inline void qdisc_lock_uninit(struct Qdisc *sch,
+                                    const struct Qdisc_ops *ops)
+{
+       if (ops->static_flags & TCQ_F_INGRESS ||
+           ops == &noqueue_qdisc_ops)
+               return;
+
+       lockdep_unregister_key(&sch->root_lock_key);
+}
+
 #endif

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f56b18c8aebf73ea1547fb7e8eac331413cbe88d..443c116e866361d647a583c62e7555bf51d03c71 100644 (file)
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1353,7 +1353,7 @@ err_out4:
                ops->destroy(sch);
        qdisc_put_stab(rtnl_dereference(sch->stab));
 err_out3:
-       lockdep_unregister_key(&sch->root_lock_key);
+       qdisc_lock_uninit(sch, ops);
        netdev_put(dev, &sch->dev_tracker);
        qdisc_free(sch);
 err_out2:

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 852e603c17551ee719bf1c561848d5ef0699ab5d..98ffe64de51f4358d20a3780d7a010911736a61a 100644 (file)
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -955,9 +955,7 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
        __skb_queue_head_init(&sch->gso_skb);
        __skb_queue_head_init(&sch->skb_bad_txq);
        gnet_stats_basic_sync_init(&sch->bstats);
-       lockdep_register_key(&sch->root_lock_key);
-       spin_lock_init(&sch->q.lock);
-       lockdep_set_class(&sch->q.lock, &sch->root_lock_key);
+       qdisc_lock_init(sch, ops);
 
        if (ops->static_flags & TCQ_F_CPUSTATS) {
                sch->cpu_bstats =
@@ -987,7 +985,7 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
 
        return sch;
 errout1:
-       lockdep_unregister_key(&sch->root_lock_key);
+       qdisc_lock_uninit(sch, ops);
        kfree(sch);
 errout:
        return ERR_PTR(err);
@@ -1076,7 +1074,7 @@ static void __qdisc_destroy(struct Qdisc *qdisc)
        if (ops->destroy)
                ops->destroy(qdisc);
 
-       lockdep_unregister_key(&qdisc->root_lock_key);
+       qdisc_lock_uninit(qdisc, ops);
        bpf_module_put(ops, ops->owner);
        netdev_put(dev, &qdisc->dev_tracker);