diff mbox

[v3,7/8] batman-adv: add debugfs structure for information per interface

Message ID 1384366492-27310-8-git-send-email-sw@simonwunderlich.de (mailing list archive)
State Superseded, archived
Headers

Commit Message

Simon Wunderlich Nov. 13, 2013, 6:14 p.m. UTC 
  From: Simon Wunderlich <simon@open-mesh.com>

To show information per interface, add a debugfs hardif structure
similar to the system in sysfs. A folder "$debugfs/batman_adv/hardif"
will be created and will contain all hard interfaces. Files are not
yet added.

Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
---
Changes to PATCHv2:
 * add macro to add hardif debug infos
---
 debugfs.c        |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 debugfs.h        |    3 +++
 hard-interface.c |    9 +++++++
 types.h          |    2 ++
 4 files changed, 92 insertions(+)

Comments

Marek Lindner Nov. 21, 2013, 1:38 a.m. UTC | #1 
On Wednesday 13 November 2013 19:14:51 Simon Wunderlich wrote:
>  void batadv_debugfs_init(void)
>  {
>         struct batadv_debuginfo **bat_debug;
> @@ -378,6 +395,11 @@ void batadv_debugfs_init(void)
>         if (!batadv_debugfs)
>                 goto err;
>  
> +       batadv_hardif_debugfs =
> debugfs_create_dir(BATADV_DEBUGFS_HARDIF_SUBDIR,
> +                                                  batadv_debugfs);
> +       if (batadv_hardif_debugfs == ERR_PTR(-ENODEV))
> +               goto err;
> +
>         for (bat_debug = batadv_general_debuginfos; *bat_debug; ++bat_debug)
> { file = debugfs_create_file(((*bat_debug)->attr).name, S_IFREG |
> ((*bat_debug)->attr).mode,

I see no specific reason to create a 'hardif' subdirectory. We developers may 
know what the term relates to but it is reasonable to assume our users do not. 
How about adding the interface directory (e.g. wlan0) right next to the batX 
directory ? For clarity we could also move the interface directory into the 
corresponding batX folder. In case multiple batX interfaces it might makes 
things more obvious.

Cheers,
Marek
Simon Wunderlich Nov. 21, 2013, 1:22 p.m. UTC | #2 
Hi Marek,

> On Wednesday 13 November 2013 19:14:51 Simon Wunderlich wrote:
> >  void batadv_debugfs_init(void)
> >  {
> >  
> >         struct batadv_debuginfo **bat_debug;
> > 
> > @@ -378,6 +395,11 @@ void batadv_debugfs_init(void)
> > 
> >         if (!batadv_debugfs)
> >         
> >                 goto err;
> > 
> > +       batadv_hardif_debugfs =
> > debugfs_create_dir(BATADV_DEBUGFS_HARDIF_SUBDIR,
> > +                                                  batadv_debugfs);
> > +       if (batadv_hardif_debugfs == ERR_PTR(-ENODEV))
> > +               goto err;
> > +
> > 
> >         for (bat_debug = batadv_general_debuginfos; *bat_debug;
> >         ++bat_debug)
> > 
> > { file = debugfs_create_file(((*bat_debug)->attr).name, S_IFREG |
> > ((*bat_debug)->attr).mode,
> 
> I see no specific reason to create a 'hardif' subdirectory. We developers
> may know what the term relates to but it is reasonable to assume our users
> do not. How about adding the interface directory (e.g. wlan0) right next
> to the batX directory ? 

thanks for the suggestion, I've done that in the reviewed patch v4 (there is 
also a v5 fixing another style issue antonio pointed out).

> For clarity we could also move the interface
> directory into the corresponding batX folder. In case multiple batX
> interfaces it might makes things more obvious.

That's a little harder to do as the folder creation is done when the interface 
is registered to batman-adv, just as for sysfs. Although not impossible, as 
discussed on IRC, I'd like to keep it at the proposal from above. This will 
also keep the folders in the same flat hierarchy as we have in /sys/class/net/* 
to manage batman devices. I think it's good to keep it at this consistent 
scheme.

Thanks,
    Simon
diff mbox

Patch

diff --git a/debugfs.c b/debugfs.c
index 367ea86..97a3f34 100644
--- a/debugfs.c
+++ b/debugfs.c
@@ -29,6 +29,7 @@ 
 #include "network-coding.h"
 
 static struct dentry *batadv_debugfs;
+static struct dentry *batadv_hardif_debugfs;
 
 #ifdef CONFIG_BATMAN_ADV_DEBUG
 #define BATADV_LOG_BUFF_MASK (batadv_log_buff_len - 1)
@@ -366,6 +367,22 @@  static struct batadv_debuginfo *batadv_mesh_debuginfos[] = {
 	NULL,
 };
 
+#define BATADV_HARDIF_DEBUGINFO(_name, _mode, _open)		\
+struct batadv_debuginfo batadv_hardif_debuginfo_##_name = {	\
+	.attr = { .name = __stringify(_name),			\
+		  .mode = _mode, },				\
+	.fops = { .owner = THIS_MODULE,				\
+		  .open = _open,				\
+		  .read	= seq_read,				\
+		  .llseek = seq_lseek,				\
+		  .release = single_release,			\
+		}						\
+};
+
+static struct batadv_debuginfo *batadv_hardif_debuginfos[] = {
+	NULL,
+};
+
 void batadv_debugfs_init(void)
 {
 	struct batadv_debuginfo **bat_debug;
@@ -378,6 +395,11 @@  void batadv_debugfs_init(void)
 	if (!batadv_debugfs)
 		goto err;
 
+	batadv_hardif_debugfs = debugfs_create_dir(BATADV_DEBUGFS_HARDIF_SUBDIR,
+						   batadv_debugfs);
+	if (batadv_hardif_debugfs == ERR_PTR(-ENODEV))
+		goto err;
+
 	for (bat_debug = batadv_general_debuginfos; *bat_debug; ++bat_debug) {
 		file = debugfs_create_file(((*bat_debug)->attr).name,
 					   S_IFREG | ((*bat_debug)->attr).mode,
@@ -393,12 +415,68 @@  void batadv_debugfs_init(void)
 	return;
 err:
 	debugfs_remove_recursive(batadv_debugfs);
+	batadv_debugfs = NULL;
+	batadv_hardif_debugfs = NULL;
 }
 
 void batadv_debugfs_destroy(void)
 {
 	debugfs_remove_recursive(batadv_debugfs);
 	batadv_debugfs = NULL;
+	batadv_hardif_debugfs = NULL;
+}
+
+/**
+ * batadv_debugfs_add_hardif - creates the base directory for a hard interface
+ *  in debugfs.
+ * @hard_iface: hard interface which should be added.
+ */
+int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface)
+{
+	struct batadv_debuginfo **bat_debug;
+	struct dentry *file;
+
+	if (!batadv_debugfs)
+		goto out;
+
+	hard_iface->debug_dir = debugfs_create_dir(hard_iface->net_dev->name,
+						   batadv_hardif_debugfs);
+	if (!hard_iface->debug_dir)
+		goto out;
+
+	for (bat_debug = batadv_hardif_debuginfos; *bat_debug; ++bat_debug) {
+		file = debugfs_create_file(((*bat_debug)->attr).name,
+					   S_IFREG | ((*bat_debug)->attr).mode,
+					   hard_iface->debug_dir,
+					   hard_iface->net_dev,
+					   &(*bat_debug)->fops);
+		if (!file)
+			goto rem_attr;
+	}
+
+	return 0;
+rem_attr:
+	debugfs_remove_recursive(hard_iface->debug_dir);
+	hard_iface->debug_dir = NULL;
+out:
+#ifdef CONFIG_DEBUG_FS
+	return -ENOMEM;
+#else
+	return 0;
+#endif /* CONFIG_DEBUG_FS */
+}
+
+/**
+ * batadv_debugfs_del_hardif - delete the base directory for a hard interface
+ *  in debugfs.
+ * @hard_iface: hard interface which is deleted.
+ */
+void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface)
+{
+	if (batadv_hardif_debugfs) {
+		debugfs_remove_recursive(hard_iface->debug_dir);
+		hard_iface->debug_dir = NULL;
+	}
 }
 
 int batadv_debugfs_add_meshif(struct net_device *dev)
diff --git a/debugfs.h b/debugfs.h
index 169ab7b..489b567 100644
--- a/debugfs.h
+++ b/debugfs.h
@@ -16,10 +16,13 @@ 
 #define _NET_BATMAN_ADV_DEBUGFS_H_
 
 #define BATADV_DEBUGFS_SUBDIR "batman_adv"
+#define BATADV_DEBUGFS_HARDIF_SUBDIR "hardif"
 
 void batadv_debugfs_init(void);
 void batadv_debugfs_destroy(void);
 int batadv_debugfs_add_meshif(struct net_device *dev);
 void batadv_debugfs_del_meshif(struct net_device *dev);
+int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface);
+void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface);
 
 #endif /* _NET_BATMAN_ADV_DEBUGFS_H_ */
diff --git a/hard-interface.c b/hard-interface.c
index 42db38e..3873175 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -20,6 +20,7 @@ 
 #include "translation-table.h"
 #include "routing.h"
 #include "sysfs.h"
+#include "debugfs.h"
 #include "originator.h"
 #include "hash.h"
 #include "bridge_loop_avoidance.h"
@@ -536,6 +537,7 @@  static void batadv_hardif_remove_interface_finish(struct work_struct *work)
 	hard_iface = container_of(work, struct batadv_hard_iface,
 				  cleanup_work);
 
+	batadv_debugfs_del_hardif(hard_iface);
 	batadv_sysfs_del_hardif(&hard_iface->hardif_obj);
 	batadv_hardif_free_ref(hard_iface);
 }
@@ -566,6 +568,11 @@  batadv_hardif_add_interface(struct net_device *net_dev)
 	hard_iface->net_dev = net_dev;
 	hard_iface->soft_iface = NULL;
 	hard_iface->if_status = BATADV_IF_NOT_IN_USE;
+
+	ret = batadv_debugfs_add_hardif(hard_iface);
+	if (ret)
+		goto free_sysfs;
+
 	INIT_LIST_HEAD(&hard_iface->list);
 	INIT_WORK(&hard_iface->cleanup_work,
 		  batadv_hardif_remove_interface_finish);
@@ -582,6 +589,8 @@  batadv_hardif_add_interface(struct net_device *net_dev)
 
 	return hard_iface;
 
+free_sysfs:
+	batadv_sysfs_del_hardif(&hard_iface->hardif_obj);
 free_if:
 	kfree(hard_iface);
 release_dev:
diff --git a/types.h b/types.h
index 245a547..0e5d238 100644
--- a/types.h
+++ b/types.h
@@ -81,6 +81,7 @@  struct batadv_hard_iface_bat_iv {
  * @rcu: struct used for freeing in an RCU-safe manner
  * @bat_iv: BATMAN IV specific per hard interface data
  * @cleanup_work: work queue callback item for hard interface deinit
+ * @debug_dir: dentry for nc subdir in batman-adv directory in debugfs
  */
 struct batadv_hard_iface {
 	struct list_head list;
@@ -95,6 +96,7 @@  struct batadv_hard_iface {
 	struct rcu_head rcu;
 	struct batadv_hard_iface_bat_iv bat_iv;
 	struct work_struct cleanup_work;
+	struct dentry *debug_dir;
 };
 
 /**