From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cong Wang Subject: Re: [PATCH net-next v3 4/5] net/tc: introduce TC_ACT_REINJECT. Date: Thu, 26 Jul 2018 16:25:46 -0700 Message-ID: References: <3c20787be0fd5d64728ffed46ae0a7dff10d7e05.1532437050.git.pabeni@redhat.com> <6a9fd3c8f861c203c7e12a2a2e477796c5e093d5.camel@redhat.com> <20180725170859.GC20383@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Marcelo Ricardo Leitner , Paolo Abeni , Linux Kernel Network Developers , Jiri Pirko , Daniel Borkmann , Eyal Birger , David Miller To: Jamal Hadi Salim Return-path: Received: from mail-pg1-f195.google.com ([209.85.215.195]:43684 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730610AbeG0ApA (ORCPT ); Thu, 26 Jul 2018 20:45:00 -0400 Received: by mail-pg1-f195.google.com with SMTP id v13-v6so2061269pgr.10 for ; Thu, 26 Jul 2018 16:25:58 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Jul 26, 2018 at 5:52 AM Jamal Hadi Salim wrote: > > On 25/07/18 01:09 PM, Marcelo Ricardo Leitner wrote: > > On Wed, Jul 25, 2018 at 09:48:16AM -0700, Cong Wang wrote: > >> On Wed, Jul 25, 2018 at 5:27 AM Jamal Hadi Salim wrote: > >>> > >>> Those changes were there from the beginning (above patch did > >>> not introduce them). > >>> IIRC, the reason was to distinguish between policy intended > >>> drops and drops because of errors. > >> > >> There must be a limit for "overlimit" to make sense. There is > >> no limit in mirred action's context, probably there is only > >> such a limit in act_police. So, all rest should not touch overlimit. > > > > +1 > > > > I agree we should at least record drop count(unrelated patch though). > we should keep overlimit (for no other reason other than this > has been around for at least 15 years). > > On why "overlimit"? It is just a name for a counter that is useless > for most actions (but was still being transfered to user space). > It is the closest counter to counting "this failed because of > runtime errors" as opposed to "user asked us to drop this". > > Probably a good alternative is to make a very small stats v3 structure > (we have migrated stats structures before) and extend for > each action/classifier/qdisc to add its extra counters using XSTATS. Agreed. > > Note: > If you are _mirroring_ packets - incrementing the drop counter is > misleading because the packet is not dropped by the system. > i.e the qdisc will not record it as dropped; it should for > redirect policy. It is useful to be able to tell > them apart when you are collecting analytics just for actions. Sounds like we just need another counter rather than re-using overlimit or drops. > (if youve worked on a massive amount of machines you'll appreciate > being able to debug by looking at counters that reduce ambiguity). > Yes, this is how I found out the overlimit of htb qdisc is inaccurate or misleading, instead the one in htb class is accurate, see: commit 3c75f6ee139d464351f8ab77a042dd3635769d98 Author: Eric Dumazet Date: Mon Sep 18 12:36:22 2017 -0700 net_sched: sch_htb: add per class overlimits counter