Re: [B.A.T.M.A.N.] [RFC 1/2] batman-adv: Add interface to keep stats

[Marek Lindner <lindner_marek@yahoo.de>]

On Tuesday, April 10, 2012 13:33:11 Martin Hundebøll wrote:
> On 04/04/2012 10:07 AM, Marek Lindner wrote:
> > On Monday, April 02, 2012 14:48:15 Martin Hundebøll wrote:
> >> @@ -265,6 +266,18 @@ static int vis_data_open(struct inode *inode,
> >> struct file *file) return single_open(file, vis_seq_print_text,
> >> net_dev);
> >> 
> >>   }
> >> 
> >> +static int bat_stats_open(struct inode *inode, struct file *file)
> >> +{
> >> +	struct net_device *net_dev = (struct net_device *)inode->i_private;
> >> +	return single_open(file, bat_stats_show, net_dev);
> >> +}
> > 
> > Did you consider using bat_priv->stats or ethtool->get_ethtool_stats() ?
> 
> Wouldn't that require patching netdevice.h? And in that case, is there any
> chance to get batman-adv specific counters into the generel netdev stats
> structure?

No, I did not mean we should patch netdevice.h. The kernel has traffic 
counters we might be able to re-use for our purpose. Especially 
get_ethtool_stats() allows to export custom counters to userspace.
The kernel developers don't like duplicating existing infrastructure. At least 
we should know what the kernel offers and come up with a reasonable 
explanation why they don't help us.


> >> +static int bat_stats_clear_open(struct inode *inode, struct file *file)
> >> +{
> >> +	struct net_device *net_dev = (struct net_device *)inode->i_private;
> >> +	return single_open(file, bat_stats_clear, net_dev);
> >> +}
> > 
> > How about writing 0 to the stats file resets the counters, thus avoiding
> > a second file ?
> 
> That is possible, but then I would have to write the debugfs-handling
> myself, instead of using the nice macro defined in bat_debugfs.c. It's a
> tradeoff between code size and number of files in debugfs.

Alternatively, you could extend the existing macro to also set a file handler 
for writes instead of assuming only reads. :-)


> > If you add an ifdef around the definition here would be no need to malloc
> > bat_stats separately, right ?
> 
> Do we prefer IFDEFs in the definition over mallocs? (I guess so by your
> comment). If thats the case, then no problem with me.

The types.h is full of ifdefs already. It helps to keep the structs smaller 
when functionality is not compiled into the module.

Regards,
Marek