On 04/10/2012 01:05 PM, Marek Lindner wrote:
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.
Sounds like get_ethtool_stats() could do the job - I will look into that.
+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. :-)
Definitely an option :)
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.
As long as we agree on ifdefs. They weren't there until BLAII and DAT added them, but I don't have a problem with'em. Let's see how long this patch-set will live, as I expect to use the get_ethtool_stats(), if possible.