> > 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.
|