About bug:
During init of clsact/ingress, it links qdisc's cpu_bstats,cpu_qstats
with mini qdisc. TCQ_F_CPUSTATS is set in qdisc->flags during init and
this flag is checked after init to allocate memory for stats.

Hence mini qdisc points to null per-cpu-stats. The problem isn't caught
while updating stats via mini qdisc because per_cpu_ptr(NULL, cpu_num)
or this_cpu_ptr(NULL) gives a valid pointer.

About fix:
Currently stats memory is allocated at two places.
- in qdisc_alloc() if TCQ_F_CPUSTATS is set in Qdisc_ops->static_flags
- in qdisc_create() if TCQ_F_CPUSTATS is set in Qdisc->flags

Qdisc_ops->static_flags is propagated to Qdisc->flags. So to fix this bug,
we set TCQ_F_CPUSTATS in static flags and additional condition to avoid
allocation after init.

Fixes: 46209401f8f6 ("net: core: introduce mini_Qdisc and eliminate usage of 
tp->q for clsact fastpath")
Signed-off-by: Prashant Bhole <bhole_prashant...@lab.ntt.co.jp>
---
 net/sched/sch_api.c     | 3 ++-
 net/sched/sch_ingress.c | 6 ++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 8a04c36e579f..de99a5e80944 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1094,7 +1094,8 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
                        goto err_out5;
        }
 
-       if (qdisc_is_percpu_stats(sch)) {
+       if (!(ops->static_flags & TCQ_F_CPUSTATS) &&
+           qdisc_is_percpu_stats(sch)) {
                sch->cpu_bstats =
                        netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
                if (!sch->cpu_bstats)
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index 7ca2be20dd6f..0a3fba46dfd3 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -82,8 +82,6 @@ static int ingress_init(struct Qdisc *sch, struct nlattr *opt,
        if (err)
                return err;
 
-       sch->flags |= TCQ_F_CPUSTATS;
-
        return 0;
 }
 
@@ -127,6 +125,7 @@ static struct Qdisc_ops ingress_qdisc_ops __read_mostly = {
        .destroy        =       ingress_destroy,
        .dump           =       ingress_dump,
        .owner          =       THIS_MODULE,
+       .static_flags   =       TCQ_F_CPUSTATS,
 };
 
 struct clsact_sched_data {
@@ -202,8 +201,6 @@ static int clsact_init(struct Qdisc *sch, struct nlattr 
*opt,
        if (err)
                return err;
 
-       sch->flags |= TCQ_F_CPUSTATS;
-
        return 0;
 }
 
@@ -235,6 +232,7 @@ static struct Qdisc_ops clsact_qdisc_ops __read_mostly = {
        .destroy        =       clsact_destroy,
        .dump           =       ingress_dump,
        .owner          =       THIS_MODULE,
+       .static_flags   =       TCQ_F_CPUSTATS,
 };
 
 static int __init ingress_module_init(void)
-- 
2.13.6


Reply via email to