On Wed, 2004-09-29 at 08:48, Thomas Graf wrote:
> * jamal <1096426464.1045.133.camel@xxxxxxxxxxxxxxxx> 2004-09-28 22:54
> > Lets do it on gnet stats though so we can make it more accessible.
> > I think your granularity maybe too thin: bytes,packets, drops
> > may need to be in the same TLV.
>
> I partially agree as long as no structs visible to userspace will
> ever change after introduction.
>
Note, this comes as a lesson from the current TC_STAT maintainance
headache. We messed up there but didnt have the hindsight we do now.
Adding a new user space visible entry becomes a matter of intrioducing
a new TLV.
> I'd really like to avoid maintaining different structs for collection
> and transfer which must be kept in sync.
As a general principle i agree with you - this is why tc_stat got us in
trouble to begin with. The side effect is the cost of TLV 32 bit header
to transport a 32 bit or worse 8 bit.
Putting entries into semantical groups will help reduce the overhead but
should only be done when makes sense to do so.
ex i see someone counting packets will also count bytes and maybe count
drops. Essentially if things dont make sense grouping, then dont group.
Later we figure it was a mistake, sure add a new TLV.
The problem with tc_stat is it was too fat.
> Having the application do its own struct and fill it based on
> if (tb[TCA_STATS_*]) conditions avoids all possible compatibility
> issues. It would be possible for the application to choose between
> the original and a _64 variant if we ever introduce them.
Well, same principle applies to XSTAT; but thats app specific.
> So this would be my approach:
>
> Leave gnet_stats as-is and extend it as needed but hide it from
> userspace.
Its too fat ;-> I dont see why an action which doesnt queue need to
have access to queue stats.
> Extend gen_copy_stats to add a nested TLV for every stat in
> the structure. We could even introduce a bitmap to allow the
> struct gnet_stats user to define which statistics are defined
> and which aren't. This way we could avoid the mess with pfifo
> and bfifo both using backlog but with a different unit in mind
> (packets/bytes).
>
I think we should kill gnet_stat in its current form. The lessons of
tc_stat still haunt me.
> > do you have cycles to run with that patch?
>
> Yes, it is related to my work so it's not a problem.
Thanks. We can talk offline then.
cheers,
jamal
|