netdev - Re: [PATCH] net_sched: accurate bytes/packets stats/rates

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Mon, 17 Jan 2011 08:17:49 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Changli Gao <xiaosuo@...il.com>
Cc:	Jarek Poplawski <jarkao2@...il.com>,
	Stephen Hemminger <shemminger@...tta.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Patrick McHardy <kaber@...sh.net>, jamal <hadi@...erus.ca>
Subject: Re: [PATCH] net_sched: accurate bytes/packets stats/rates

Le lundi 17 janvier 2011 à 08:09 +0800, Changli Gao a écrit :
> On Mon, Jan 17, 2011 at 6:35 AM, Jarek Poplawski <jarkao2@...il.com> wrote:
> > On Fri, Jan 14, 2011 at 11:03:42AM -0800, Stephen Hemminger wrote:
> >> From Eric Dumazet <eric.dumazet@...il.com>
> >>
> >> In commit 44b8288308ac9d (net_sched: pfifo_head_drop problem), we fixed
> >> a problem with pfifo_head drops that incorrectly decreased
> >> sch->bstats.bytes and sch->bstats.packets
> >>
> >> Several qdiscs (CHOKe, SFQ, pfifo_head, ...) are able to drop a
> >> previously enqueued packet, and bstats cannot be changed, so
> >> bstats/rates are not accurate (over estimated)
> >>
> >> This patch changes the qdisc_bstats updates to be done at dequeue() time
> >> instead of enqueue() time. bstats counters no longer account for dropped
> >> frames, and rates are more correct, since enqueue() bursts dont have
> >> effect on dequeue() rate.
> >>
> >> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
> >> Acked-by: Stephen Hemminger <shemminger@...tta.com>
> >>
> >> CC: Patrick McHardy <kaber@...sh.net>
> >> CC: Jarek Poplawski <jarkao2@...il.com>
> >> CC: jamal <hadi@...erus.ca>
> > ...
> >> --- a/net/sched/sch_drr.c     2011-01-14 09:19:00.830857886 -0800
> >> +++ b/net/sched/sch_drr.c     2011-01-14 09:28:20.398631228 -0800
> >> @@ -376,7 +376,6 @@ static int drr_enqueue(struct sk_buff *s
> >>       }
> >>
> >>       bstats_update(&cl->bstats, skb);
> >
> > Why leave leaf classes with different stats?
> >
> 
> HTB also has the same problem, and I have fixed in my own product, but
> the patch was rejected.
> 
> http://patchwork.ozlabs.org/patch/6227/
> 

Hmm, considering qdisc stats are not used in kernel (only updated and
reported to tc users) it seems to me counting arrival instead of
departure rates is mostly useless for the user, if drops are ignored.

(I am not speaking of direct drops, when we try to enqueue() this skb,
but later ones, when another skb is enqueued and we drop a previously
enqueued skb)

User really wants to see the effective departure rate, to check its
qdisc parameters in respect with kernel ones (HZ=100/1000, HIGH res
timers off/on, ...)

Arrival rates are of litle use. However, it might be good to have a
second "bstats" only for dropped packets/bytes, or extend bstats in a
compatible way (maybe adding fields to the end of structure)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html