Thread (17 messages) flat view 17 messages, 3 authors, 2004-11-02

Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT

From: Thomas Graf <tgraf@suug.ch>
Date: 2004-10-29 12:53:56

quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help