netdev - Re: [RFC PATCH 12/17] net: sched: add support for TCQ_F_NOLOCK subqueues to sch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Nov 2017 20:22:30 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Eric Dumazet <eric.dumazet@...il.com>, make0818@...il.com,
        Network Development <netdev@...r.kernel.org>,
        Jiří Pírko <jiri@...nulli.us>,
        Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [RFC PATCH 12/17] net: sched: add support for TCQ_F_NOLOCK
 subqueues to sch_mq

>  /**
>   * gnet_stats_copy_queue - copy queue statistics into statistics TLV
> diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
> index 213b586..bc59f05 100644
> --- a/net/sched/sch_mq.c
> +++ b/net/sched/sch_mq.c
> @@ -17,6 +17,7 @@
>  #include <linux/skbuff.h>
>  #include <net/netlink.h>
>  #include <net/pkt_sched.h>
> +#include <net/sch_generic.h>
>
>  struct mq_sched {
>         struct Qdisc            **qdiscs;
> @@ -103,15 +104,25 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
>         memset(&sch->qstats, 0, sizeof(sch->qstats));
>
>         for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
> +               struct gnet_stats_basic_cpu __percpu *cpu_bstats = NULL;
> +               struct gnet_stats_queue __percpu *cpu_qstats = NULL;
> +               __u32 qlen = 0;
> +
>                 qdisc = netdev_get_tx_queue(dev, ntx)->qdisc_sleeping;
>                 spin_lock_bh(qdisc_lock(qdisc));
> -               sch->q.qlen             += qdisc->q.qlen;
> -               sch->bstats.bytes       += qdisc->bstats.bytes;
> -               sch->bstats.packets     += qdisc->bstats.packets;
> -               sch->qstats.backlog     += qdisc->qstats.backlog;
> -               sch->qstats.drops       += qdisc->qstats.drops;
> -               sch->qstats.requeues    += qdisc->qstats.requeues;
> -               sch->qstats.overlimits  += qdisc->qstats.overlimits;
> +
> +               if (qdisc_is_percpu_stats(qdisc)) {
> +                       cpu_bstats = qdisc->cpu_bstats;
> +                       cpu_qstats = qdisc->cpu_qstats;
> +               }

Else, these fields are NULL. So no need for the intermediate
variables.

> +
> +               qlen = qdisc_qlen_sum(qdisc);
> +
> +               __gnet_stats_copy_basic(NULL, &sch->bstats,
> +                                       cpu_bstats, &qdisc->bstats);
> +               __gnet_stats_copy_queue(&sch->qstats,
> +                                       cpu_qstats, &qdisc->qstats, qlen);

These functions add if the percpu variant is taken, but assign otherwise,
so overwrite on each iteration of the loop.