diff mbox series

[RFC,v3,19/19] batman-adv: Trigger genl notification on sysfs config change

Message ID 20181207135846.6152-20-sven@narfation.org (mailing list archive)
State RFC, archived
Delegated to: Simon Wunderlich
Headers
Series batman-adv: netlink restructuring, part 2 |

Commit Message

Sven Eckelmann Dec. 7, 2018, 1:58 p.m. UTC 
  The generic netlink code is expected to trigger notification messages when
configuration might have been changed. But the configuration of batman-adv
is most of the time still done using sysfs. So the sysfs interface should
also trigger the corresponding netlink messages via the "config" multicast
group.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/netlink.c | 10 +++----
 net/batman-adv/netlink.h |  6 ++++
 net/batman-adv/sysfs.c   | 63 +++++++++++++++++++++++++++++++++-------
 3 files changed, 63 insertions(+), 16 deletions(-)

Comments

Linus Lüssing Jan. 4, 2019, 2:29 a.m. UTC | #1 
On Fri, Dec 07, 2018 at 02:58:46PM +0100, Sven Eckelmann wrote:
> The generic netlink code is expected to trigger notification messages when
> configuration might have been changed. But the configuration of batman-adv
> is most of the time still done using sysfs. So the sysfs interface should
> also trigger the corresponding netlink messages via the "config" multicast
> group.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---

I'm wondering, before it was quite easy to add new sysfs
attributes with just a few lines thanks to macros. Now we
need to add code in four more places in netlink.c whenever we introduce a
new configuration option.

Would it be possible (and maybe even reduce code size?) if the
sysfs and netlink parts were wrapped into some (macro) functions and/or
structs, to still be able to add options from a central place and with
only a few lines?

Or would that become too ugly? Or not worth the effort for the few
options we have right now?
Sven Eckelmann Jan. 4, 2019, 7:58 a.m. UTC | #2 
On Friday, 4 January 2019 03.29.16 CET Linus Lüssing wrote:
[...]
> I'm wondering, before it was quite easy to add new sysfs
> attributes with just a few lines thanks to macros. Now we
> need to add code in four more places in netlink.c whenever we introduce a
> new configuration option.
> 
> Would it be possible (and maybe even reduce code size?) if the
> sysfs and netlink parts were wrapped into some (macro) functions and/or
> structs, to still be able to add options from a central place and with
> only a few lines?
> 
> Or would that become too ugly? Or not worth the effort for the few
> options we have right now?

You only need a couple of lines to add netlink things. And we don't want sysfs 
anymore. So no need to add ugly hacks to add both netlink+sysfs with a "couple 
of lines".

Kind regards,
	Sven
Linus Lüssing Jan. 5, 2019, 3:03 p.m. UTC | #3 
On Fri, Jan 04, 2019 at 08:58:32AM +0100, Sven Eckelmann wrote:
> On Friday, 4 January 2019 03.29.16 CET Linus Lüssing wrote:
> [...]
> > I'm wondering, before it was quite easy to add new sysfs
> > attributes with just a few lines thanks to macros. Now we
> > need to add code in four more places in netlink.c whenever we introduce a
> > new configuration option.
> > 
> > Would it be possible (and maybe even reduce code size?) if the
> > sysfs and netlink parts were wrapped into some (macro) functions and/or
> > structs, to still be able to add options from a central place and with
> > only a few lines?
> > 
> > Or would that become too ugly? Or not worth the effort for the few
> > options we have right now?
> 
> You only need a couple of lines to add netlink things. And we don't want sysfs 
> anymore. So no need to add ugly hacks to add both netlink+sysfs with a "couple 
> of lines".
> 
> Kind regards,
> 	Sven

Ok
diff mbox series

Patch

diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index 4065d871..37552736 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -383,7 +383,7 @@  static int batadv_netlink_mesh_put(struct sk_buff *msg,
  *
  * Return: 0 on success, < 0 on error
  */
-static int batadv_netlink_notify_mesh(struct batadv_priv *bat_priv)
+int batadv_netlink_notify_mesh(struct batadv_priv *bat_priv)
 {
 	struct sk_buff *msg;
 	int ret;
@@ -850,8 +850,8 @@  static int batadv_netlink_hardif_put(struct sk_buff *msg,
  *
  * Return: 0 on success, < 0 on error
  */
-static int batadv_netlink_notify_hardif(struct batadv_priv *bat_priv,
-					struct batadv_hard_iface *hard_iface)
+int batadv_netlink_notify_hardif(struct batadv_priv *bat_priv,
+				 struct batadv_hard_iface *hard_iface)
 {
 	struct sk_buff *msg;
 	int ret;
@@ -1055,8 +1055,8 @@  static int batadv_netlink_vlan_put(struct sk_buff *msg,
  *
  * Return: 0 on success, < 0 on error
  */
-static int batadv_netlink_notify_vlan(struct batadv_priv *bat_priv,
-				      struct batadv_softif_vlan *vlan)
+int batadv_netlink_notify_vlan(struct batadv_priv *bat_priv,
+			       struct batadv_softif_vlan *vlan)
 {
 	struct sk_buff *msg;
 	int ret;
diff --git a/net/batman-adv/netlink.h b/net/batman-adv/netlink.h
index 571d9a5a..8d17ce0e 100644
--- a/net/batman-adv/netlink.h
+++ b/net/batman-adv/netlink.h
@@ -34,6 +34,12 @@  int batadv_netlink_tpmeter_notify(struct batadv_priv *bat_priv, const u8 *dst,
 				  u8 result, u32 test_time, u64 total_bytes,
 				  u32 cookie);
 
+int batadv_netlink_notify_mesh(struct batadv_priv *bat_priv);
+int batadv_netlink_notify_hardif(struct batadv_priv *bat_priv,
+				 struct batadv_hard_iface *hard_iface);
+int batadv_netlink_notify_vlan(struct batadv_priv *bat_priv,
+			       struct batadv_softif_vlan *vlan);
+
 extern struct genl_family batadv_netlink_family;
 
 #endif /* _NET_BATMAN_ADV_NETLINK_H_ */
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 344e5787..7c67fe29 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -48,6 +48,7 @@ 
 #include "gateway_common.h"
 #include "hard-interface.h"
 #include "log.h"
+#include "netlink.h"
 #include "network-coding.h"
 #include "soft-interface.h"
 
@@ -154,9 +155,14 @@  ssize_t batadv_store_##_name(struct kobject *kobj,			\
 {									\
 	struct net_device *net_dev = batadv_kobj_to_netdev(kobj);	\
 	struct batadv_priv *bat_priv = netdev_priv(net_dev);		\
+	ssize_t length;							\
+									\
+	length = __batadv_store_bool_attr(buff, count, _post_func, attr,\
+					  &bat_priv->_name, net_dev);	\
 									\
-	return __batadv_store_bool_attr(buff, count, _post_func, attr,	\
-					&bat_priv->_name, net_dev);	\
+	batadv_netlink_notify_mesh(bat_priv);				\
+									\
+	return length;							\
 }
 
 #define BATADV_ATTR_SIF_SHOW_BOOL(_name)				\
@@ -186,11 +192,16 @@  ssize_t batadv_store_##_name(struct kobject *kobj,			\
 {									\
 	struct net_device *net_dev = batadv_kobj_to_netdev(kobj);	\
 	struct batadv_priv *bat_priv = netdev_priv(net_dev);		\
+	ssize_t length;							\
 									\
-	return __batadv_store_uint_attr(buff, count, _min, _max,	\
-					_post_func, attr,		\
-					&bat_priv->_var, net_dev,	\
-					NULL);	\
+	length = __batadv_store_uint_attr(buff, count, _min, _max,	\
+					  _post_func, attr,		\
+					  &bat_priv->_var, net_dev,	\
+					  NULL);			\
+									\
+	batadv_netlink_notify_mesh(bat_priv);				\
+									\
+	return length;							\
 }
 
 #define BATADV_ATTR_SIF_SHOW_UINT(_name, _var)				\
@@ -223,6 +234,11 @@  ssize_t batadv_store_vlan_##_name(struct kobject *kobj,			\
 					      attr, &vlan->_name,	\
 					      bat_priv->soft_iface);	\
 									\
+	if (vlan->vid)							\
+		batadv_netlink_notify_vlan(bat_priv, vlan);		\
+	else								\
+		batadv_netlink_notify_mesh(bat_priv);			\
+									\
 	batadv_softif_vlan_put(vlan);					\
 	return res;							\
 }
@@ -256,6 +272,7 @@  ssize_t batadv_store_##_name(struct kobject *kobj,			\
 {									\
 	struct net_device *net_dev = batadv_kobj_to_netdev(kobj);	\
 	struct batadv_hard_iface *hard_iface;				\
+	struct batadv_priv *bat_priv;					\
 	ssize_t length;							\
 									\
 	hard_iface = batadv_hardif_get_by_netdev(net_dev);		\
@@ -268,6 +285,11 @@  ssize_t batadv_store_##_name(struct kobject *kobj,			\
 					  hard_iface->soft_iface,	\
 					  net_dev);			\
 									\
+	if (hard_iface->soft_iface) {					\
+		bat_priv = netdev_priv(hard_iface->soft_iface);		\
+		batadv_netlink_notify_hardif(bat_priv, hard_iface);	\
+	}								\
+									\
 	batadv_hardif_put(hard_iface);				\
 	return length;							\
 }
@@ -537,6 +559,9 @@  static ssize_t batadv_store_gw_mode(struct kobject *kobj,
 	batadv_gw_check_client_stop(bat_priv);
 	atomic_set(&bat_priv->gw.mode, (unsigned int)gw_mode_tmp);
 	batadv_gw_tvlv_container_update(bat_priv);
+
+	batadv_netlink_notify_mesh(bat_priv);
+
 	return count;
 }
 
@@ -563,6 +588,7 @@  static ssize_t batadv_store_gw_sel_class(struct kobject *kobj,
 					 size_t count)
 {
 	struct batadv_priv *bat_priv = batadv_kobj_to_batpriv(kobj);
+	ssize_t length;
 
 	/* setting the GW selection class is allowed only if the routing
 	 * algorithm in use implements the GW API
@@ -578,10 +604,14 @@  static ssize_t batadv_store_gw_sel_class(struct kobject *kobj,
 		return bat_priv->algo_ops->gw.store_sel_class(bat_priv, buff,
 							      count);
 
-	return __batadv_store_uint_attr(buff, count, 1, BATADV_TQ_MAX_VALUE,
-					batadv_post_gw_reselect, attr,
-					&bat_priv->gw.sel_class,
-					bat_priv->soft_iface, NULL);
+	length = __batadv_store_uint_attr(buff, count, 1, BATADV_TQ_MAX_VALUE,
+					  batadv_post_gw_reselect, attr,
+					  &bat_priv->gw.sel_class,
+					  bat_priv->soft_iface, NULL);
+
+	batadv_netlink_notify_mesh(bat_priv);
+
+	return length;
 }
 
 static ssize_t batadv_show_gw_bwidth(struct kobject *kobj,
@@ -601,12 +631,18 @@  static ssize_t batadv_store_gw_bwidth(struct kobject *kobj,
 				      struct attribute *attr, char *buff,
 				      size_t count)
 {
+	struct batadv_priv *bat_priv = batadv_kobj_to_batpriv(kobj);
 	struct net_device *net_dev = batadv_kobj_to_netdev(kobj);
+	ssize_t length;
 
 	if (buff[count - 1] == '\n')
 		buff[count - 1] = '\0';
 
-	return batadv_gw_bandwidth_set(net_dev, buff, count);
+	length = batadv_gw_bandwidth_set(net_dev, buff, count);
+
+	batadv_netlink_notify_mesh(bat_priv);
+
+	return length;
 }
 
 /**
@@ -674,6 +710,8 @@  static ssize_t batadv_store_isolation_mark(struct kobject *kobj,
 		    "New skb mark for extended isolation: %#.8x/%#.8x\n",
 		    bat_priv->isolation_mark, bat_priv->isolation_mark_mask);
 
+	batadv_netlink_notify_mesh(bat_priv);
+
 	return count;
 }
 
@@ -1078,6 +1116,7 @@  static ssize_t batadv_store_throughput_override(struct kobject *kobj,
 						struct attribute *attr,
 						char *buff, size_t count)
 {
+	struct batadv_priv *bat_priv = batadv_kobj_to_batpriv(kobj);
 	struct net_device *net_dev = batadv_kobj_to_netdev(kobj);
 	struct batadv_hard_iface *hard_iface;
 	u32 tp_override;
@@ -1108,6 +1147,8 @@  static ssize_t batadv_store_throughput_override(struct kobject *kobj,
 
 	atomic_set(&hard_iface->bat_v.throughput_override, tp_override);
 
+	batadv_netlink_notify_hardif(bat_priv, hard_iface);
+
 out:
 	batadv_hardif_put(hard_iface);
 	return count;