netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Graf <tgraf@suug.ch>
To: jamal <hadi@cyberus.ca>
Cc: "David S. Miller" <davem@davemloft.net>, netdev@oss.sgi.com
Subject: Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
Date: Fri, 29 Oct 2004 14:53:56 +0200	[thread overview]
Message-ID: <20041029125356.GM12289@postel.suug.ch> (raw)
In-Reply-To: <1099053306.1025.95.camel@jzny.localdomain>

> > Regarding the generic statistics for actions:
> > 
> > I thought about dumping the most common stats basic,queue,rate_est
> > in tcf_action_copy_stats and let the action module dump additional
> > stats in its get_stats implementation. 
> 
> I think the action code seems clean to me as is.
> Here are my thoughts:
> Most of them just need queue stats (actually at the moment all of them)
> so in tca_gen, struct tc_stats stats needs replacement to make sure they
> only have a queue stat because it is generic.

What about the rate estimator and basic stats in pkt_act.h? tca_gen
requires at least basic,queue and rate_est for me to compile.

> All actions call
> qdisc_copy_stats calls in their stats dumpers. Replace that call with
> the magic you have in the new stats dumping scheme.

That's a lot of duplicated code which could be avoided easly. It's
only a cosmetic thing but I don't see any reason for duplicating code :->

So this would be my way of doing it:

tcf_action_copy_stats {
  start_copy()
  copy_basic_stats()
  copy_queue_stats()
  copy_rate_est()
  call get_stats() and let the action module dump additional stats
  finish_copy
}

I'd also suggest to make a new TLV type TCA_ACT_STATS and not reuse
TCA_STATS.

Another issue... do we want the compatibility stuff and provide the
old tc_stats? I'd say no but it's your call.

> PS:- I hope this is after the bk snapshot with current cleanups you have
> is tested?

Sure. Always planning ahead.

  reply	other threads:[~2004-10-29 12:53 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-10-29  0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
2004-10-29  0:22 ` [PATCH 1/6] PKT_SCHED: Add generic classifier routines Thomas Graf
2004-10-29 10:23   ` [RESEND " Thomas Graf
2004-10-29  0:23 ` [PATCH 2/6] cls_fw: Cleanup fw_classify Thomas Graf
2004-10-29  0:24 ` [PATCH 3/6] cls_fw: Use generic routines to configure action/policer Thomas Graf
2004-10-29  0:24 ` [PATCH 4/6] cls_fw: Use generic routines to dump action/policer Thomas Graf
2004-10-29  0:25 ` [PATCH 5/6] cls_fw: Whitespace/ifdef fixes Thomas Graf
2004-10-29  0:26 ` [PATCH 6/6] PKT_SCHED: break is not enough to stop walking Thomas Graf
2004-10-29 10:26 ` [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT Thomas Graf
2004-10-29 11:39   ` jamal
2004-10-29 11:53     ` Thomas Graf
2004-10-29 12:35       ` jamal
2004-10-29 12:53         ` Thomas Graf [this message]
2004-10-29 13:12           ` jamal
2004-10-29 14:24             ` Thomas Graf
2004-10-29 14:53               ` jamal
2004-11-02  0:39   ` David S. Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20041029125356.GM12289@postel.suug.ch \
    --to=tgraf@suug.ch \
    --cc=davem@davemloft.net \
    --cc=hadi@cyberus.ca \
    --cc=netdev@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).