Hi there,
thanks for all the feedback for the changes I wanted to make to the sysfs code base. I tried to address bugs and suggestions. The macros are now a bit smaller than in the beginning, the missing static has been added (thanks Andrew and Sven).
Due to Marecs comment on the issues with changing sysfs file names in the kernel (which I was not aware of, thanks for explaining) and comments about old sysfs filenames being more descriptive, I changed the bat_priv names instead now. I don't really see a good reason yet why they should not be equal.
The hop_penalty and num_bcasts patches will follow later. There've been some discussions about what the limit and rules for exposing defines in sysfs should be.
Of course, having a good set of default values and not that many options configurable during runtime at all or most parameters just being adjusted automatically, makes the routing protocol itself much more stable and reliable, and less complex and easy to use for the user. However, certain assessable scenarios might benefit from certain parameters being tweakable during runtime, making optimizations and tests with different values quick and easy without having to rebuild and install the kernel module or even firmware images.
Simon's idea so far was, to introduce a debug/ folder in sysfs which is hidden by default and could be enabled with a KConfig option, so that tinkerers still have a chance for their needs for tweaking parameters. num_bcasts would probably be one of the first candidates as a debug-parameter.
If I forgot any very important points from the discussions so far, please don't hesitate to add them here.
And as always, any comments and suggestions highly appreciated :).
Cheers, Linus
Both sysfs entries and variable names shall be as descriptive as possible while not exceeding a certain length. This patch renames bat_priv atomics to be equally descriptive with their according sysfs entries.
Unifying sysfs and bat_priv atomic names also makes it easier to find each others pendant.
The reduced ("type"-)information which was previously indicated with a _enabled for booleans got substituted by a comment in bat_priv.
This patch has also been done in regards for the future BAT_ATTR_* macros (they only need one name argument instead of a file and variable name).
Signed-off-by: Linus Lüssing linus.luessing@web.de --- aggregation.c | 4 ++-- bat_sysfs.c | 24 ++++++++++++------------ hard-interface.c | 6 +++--- routing.c | 4 ++-- soft-interface.c | 6 +++--- types.h | 16 ++++++++-------- unicast.c | 2 +- 7 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/aggregation.c b/aggregation.c index 46b9c2b..05a75a6 100644 --- a/aggregation.c +++ b/aggregation.c @@ -200,7 +200,7 @@ void add_bat_packet_to_list(struct bat_priv *bat_priv, /* find position for the packet in the forward queue */ spin_lock_irqsave(&bat_priv->forw_bat_list_lock, flags); /* own packets are not to be aggregated */ - if ((atomic_read(&bat_priv->aggregation_enabled)) && (!own_packet)) { + if ((atomic_read(&bat_priv->aggregated_ogms)) && (!own_packet)) { hlist_for_each_entry(forw_packet_pos, tmp_node, &bat_priv->forw_bat_list, list) { if (can_aggregate_with(batman_packet, @@ -227,7 +227,7 @@ void add_bat_packet_to_list(struct bat_priv *bat_priv, * later on */ if ((!own_packet) && - (atomic_read(&bat_priv->aggregation_enabled))) + (atomic_read(&bat_priv->aggregated_ogms))) send_time += msecs_to_jiffies(MAX_AGGREGATION_MS);
new_aggregated_packet(packet_buff, packet_len, diff --git a/bat_sysfs.c b/bat_sysfs.c index 3f551f3..a11aa46 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -44,7 +44,7 @@ static ssize_t show_aggr_ogms(struct kobject *kobj, struct attribute *attr, { struct device *dev = to_dev(kobj->parent); struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int aggr_status = atomic_read(&bat_priv->aggregation_enabled); + int aggr_status = atomic_read(&bat_priv->aggregated_ogms);
return sprintf(buff, "%s\n", aggr_status == 0 ? "disabled" : "enabled"); @@ -76,15 +76,15 @@ static ssize_t store_aggr_ogms(struct kobject *kobj, struct attribute *attr, return -EINVAL; }
- if (atomic_read(&bat_priv->aggregation_enabled) == aggr_tmp) + if (atomic_read(&bat_priv->aggregated_ogms) == aggr_tmp) return count;
bat_info(net_dev, "Changing aggregation from: %s to: %s\n", - atomic_read(&bat_priv->aggregation_enabled) == 1 ? + atomic_read(&bat_priv->aggregated_ogms) == 1 ? "enabled" : "disabled", aggr_tmp == 1 ? "enabled" : "disabled");
- atomic_set(&bat_priv->aggregation_enabled, (unsigned)aggr_tmp); + atomic_set(&bat_priv->aggregated_ogms, (unsigned)aggr_tmp); return count; }
@@ -93,7 +93,7 @@ static ssize_t show_bond(struct kobject *kobj, struct attribute *attr, { struct device *dev = to_dev(kobj->parent); struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int bond_status = atomic_read(&bat_priv->bonding_enabled); + int bond_status = atomic_read(&bat_priv->bonding);
return sprintf(buff, "%s\n", bond_status == 0 ? "disabled" : "enabled"); @@ -125,15 +125,15 @@ static ssize_t store_bond(struct kobject *kobj, struct attribute *attr, return -EINVAL; }
- if (atomic_read(&bat_priv->bonding_enabled) == bonding_enabled_tmp) + if (atomic_read(&bat_priv->bonding) == bonding_enabled_tmp) return count;
bat_info(net_dev, "Changing bonding from: %s to: %s\n", - atomic_read(&bat_priv->bonding_enabled) == 1 ? + atomic_read(&bat_priv->bonding) == 1 ? "enabled" : "disabled", bonding_enabled_tmp == 1 ? "enabled" : "disabled");
- atomic_set(&bat_priv->bonding_enabled, (unsigned)bonding_enabled_tmp); + atomic_set(&bat_priv->bonding, (unsigned)bonding_enabled_tmp); return count; }
@@ -142,7 +142,7 @@ static ssize_t show_frag(struct kobject *kobj, struct attribute *attr, { struct device *dev = to_dev(kobj->parent); struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int frag_status = atomic_read(&bat_priv->frag_enabled); + int frag_status = atomic_read(&bat_priv->fragmentation);
return sprintf(buff, "%s\n", frag_status == 0 ? "disabled" : "enabled"); @@ -174,15 +174,15 @@ static ssize_t store_frag(struct kobject *kobj, struct attribute *attr, return -EINVAL; }
- if (atomic_read(&bat_priv->frag_enabled) == frag_enabled_tmp) + if (atomic_read(&bat_priv->fragmentation) == frag_enabled_tmp) return count;
bat_info(net_dev, "Changing fragmentation from: %s to: %s\n", - atomic_read(&bat_priv->frag_enabled) == 1 ? + atomic_read(&bat_priv->fragmentation) == 1 ? "enabled" : "disabled", frag_enabled_tmp == 1 ? "enabled" : "disabled");
- atomic_set(&bat_priv->frag_enabled, (unsigned)frag_enabled_tmp); + atomic_set(&bat_priv->fragmentation, (unsigned)frag_enabled_tmp); update_min_mtu(net_dev); return count; } diff --git a/hard-interface.c b/hard-interface.c index afc2c3d..3c59209 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -208,7 +208,7 @@ int hardif_min_mtu(struct net_device *soft_iface) * (have MTU > 1500 + BAT_HEADER_LEN) */ int min_mtu = ETH_DATA_LEN;
- if (atomic_read(&bat_priv->frag_enabled)) + if (atomic_read(&bat_priv->fragmentation)) goto out;
rcu_read_lock(); @@ -332,7 +332,7 @@ int hardif_enable_interface(struct batman_if *batman_if, char *iface_name) bat_info(batman_if->soft_iface, "Adding interface: %s\n", batman_if->net_dev->name);
- if (atomic_read(&bat_priv->frag_enabled) && batman_if->net_dev->mtu < + if (atomic_read(&bat_priv->fragmentation) && batman_if->net_dev->mtu < ETH_DATA_LEN + BAT_HEADER_LEN) bat_info(batman_if->soft_iface, "The MTU of interface %s is too small (%i) to handle " @@ -343,7 +343,7 @@ int hardif_enable_interface(struct batman_if *batman_if, char *iface_name) batman_if->net_dev->name, batman_if->net_dev->mtu, ETH_DATA_LEN + BAT_HEADER_LEN);
- if (!atomic_read(&bat_priv->frag_enabled) && batman_if->net_dev->mtu < + if (!atomic_read(&bat_priv->fragmentation) && batman_if->net_dev->mtu < ETH_DATA_LEN + BAT_HEADER_LEN) bat_info(batman_if->soft_iface, "The MTU of interface %s is too small (%i) to handle " diff --git a/routing.c b/routing.c index 1377f01..cc8b77f 100644 --- a/routing.c +++ b/routing.c @@ -1035,7 +1035,7 @@ struct neigh_node *find_router(struct orig_node *orig_node, return orig_node->router;
bat_priv = netdev_priv(recv_if->soft_iface); - bonding_enabled = atomic_read(&bat_priv->bonding_enabled); + bonding_enabled = atomic_read(&bat_priv->bonding);
if (!bonding_enabled) return orig_node->router; @@ -1184,7 +1184,7 @@ int route_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if, unicast_packet = (struct unicast_packet *)skb->data;
if (unicast_packet->packet_type == BAT_UNICAST && - atomic_read(&bat_priv->frag_enabled) && + atomic_read(&bat_priv->fragmentation) && skb->len > batman_if->net_dev->mtu) return frag_send_skb(skb, bat_priv, batman_if, dstaddr); diff --git a/soft-interface.c b/soft-interface.c index 683ec5f..7a899b5 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -587,14 +587,14 @@ struct net_device *softif_create(char *name)
bat_priv = netdev_priv(soft_iface);
- atomic_set(&bat_priv->aggregation_enabled, 1); - atomic_set(&bat_priv->bonding_enabled, 0); + atomic_set(&bat_priv->aggregated_ogms, 1); + atomic_set(&bat_priv->bonding, 0); atomic_set(&bat_priv->vis_mode, VIS_TYPE_CLIENT_UPDATE); atomic_set(&bat_priv->gw_mode, GW_MODE_OFF); atomic_set(&bat_priv->gw_class, 0); atomic_set(&bat_priv->orig_interval, 1000); atomic_set(&bat_priv->log_level, 0); - atomic_set(&bat_priv->frag_enabled, 1); + atomic_set(&bat_priv->fragmentation, 1); atomic_set(&bat_priv->bcast_queue_left, BCAST_QUEUE_LEN); atomic_set(&bat_priv->batman_queue_left, BATMAN_QUEUE_LEN);
diff --git a/types.h b/types.h index dec2791..44b3c07 100644 --- a/types.h +++ b/types.h @@ -124,14 +124,14 @@ struct neigh_node { struct bat_priv { atomic_t mesh_state; struct net_device_stats stats; - atomic_t aggregation_enabled; - atomic_t bonding_enabled; - atomic_t frag_enabled; - atomic_t vis_mode; - atomic_t gw_mode; - atomic_t gw_class; - atomic_t orig_interval; - atomic_t log_level; + atomic_t aggregated_ogms; /* boolean */ + atomic_t bonding; /* boolean */ + atomic_t fragmentation; /* boolean */ + atomic_t vis_mode; /* VIS_TYPE_* */ + atomic_t gw_mode; /* GW_MODE_* */ + atomic_t gw_class; /* uint */ + atomic_t orig_interval; /* uint */ + atomic_t log_level; /* uint */ atomic_t bcast_seqno; atomic_t bcast_queue_left; atomic_t batman_queue_left; diff --git a/unicast.c b/unicast.c index bca69f6..6ceaf87 100644 --- a/unicast.c +++ b/unicast.c @@ -322,7 +322,7 @@ int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv) /* copy the destination for faster routing */ memcpy(unicast_packet->dest, orig_node->orig, ETH_ALEN);
- if (atomic_read(&bat_priv->frag_enabled) && + if (atomic_read(&bat_priv->fragmentation) && data_len + sizeof(struct unicast_packet) > batman_if->net_dev->mtu) { /* send frag skb decreases ttl */
Sysfs configuration options that just took a boolean value (enable(d)/disable(d)/0/1) and integer setting basically all had the same structure.
To avoid even more copy and pasting in the future and to make introducing new configuration parameters for batman-adv simpler, more generic wrapper functions are being introduced with this commit. They can deal with boolean and unsigned integer parameters, storing them in the specified atomic_t variables.
Signed-off-by: Linus Lüssing linus.luessing@web.de --- bat_sysfs.c | 224 +++++++++++++++++++++++----------------------------------- 1 files changed, 89 insertions(+), 135 deletions(-)
diff --git a/bat_sysfs.c b/bat_sysfs.c index a11aa46..6a60561 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -39,6 +39,81 @@ struct bat_attribute bat_attr_##_name = { \ .store = _store, \ };
+static int store_bool_attr(char *buff, size_t count, + struct net_device *net_dev, + char *attr_name, atomic_t *attr) +{ + int enabled = -1; + + if (buff[count - 1] == '\n') + buff[count - 1] = '\0'; + + if ((strncmp(buff, "1", 2) == 0) || + (strncmp(buff, "enable", 7) == 0) || + (strncmp(buff, "enabled", 8) == 0)) + enabled = 1; + + if ((strncmp(buff, "0", 2) == 0) || + (strncmp(buff, "disable", 8) == 0) || + (strncmp(buff, "disabled", 9) == 0)) + enabled = 0; + + if (enabled < 0) { + bat_info(net_dev, + "%s: Invalid parameter received: %s\n", + attr_name, buff); + return -EINVAL; + } + + if (atomic_read(attr) == enabled) + return count; + + bat_info(net_dev, "%s: Changing from: %s to: %s\n", attr_name, + atomic_read(attr) == 1 ? "enabled" : "disabled", + enabled == 1 ? "enabled" : "disabled"); + + atomic_set(attr, (unsigned)enabled); + return count; +} + +static int store_uint_attr(char *buff, size_t count, + struct net_device *net_dev, char *attr_name, + unsigned int min, unsigned int max, atomic_t *attr) +{ + unsigned long uint_val; + int ret; + + ret = strict_strtoul(buff, 10, &uint_val); + if (ret) { + bat_info(net_dev, + "%s: Invalid parameter received: %s\n", + attr_name, buff); + return -EINVAL; + } + + if (uint_val < min) { + bat_info(net_dev, "%s: Value is too small: %lu min: %u\n", + attr_name, uint_val, min); + return -EINVAL; + } + + if (uint_val > max) { + bat_info(net_dev, "%s: Value is too big: %lu max: %u\n", + attr_name, uint_val, max); + return -EINVAL; + } + + if (atomic_read(attr) == uint_val) + return count; + + bat_info(net_dev, "%s: Changing from: %i to: %lu\n", + attr_name, atomic_read(attr), uint_val); + + atomic_set(attr, uint_val); + return count; +} + + static ssize_t show_aggr_ogms(struct kobject *kobj, struct attribute *attr, char *buff) { @@ -56,36 +131,9 @@ static ssize_t store_aggr_ogms(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct bat_priv *bat_priv = netdev_priv(net_dev); - int aggr_tmp = -1; - - if (((count == 2) && (buff[0] == '1')) || - (strncmp(buff, "enable", 6) == 0)) - aggr_tmp = 1; - - if (((count == 2) && (buff[0] == '0')) || - (strncmp(buff, "disable", 7) == 0)) - aggr_tmp = 0; - - if (aggr_tmp < 0) { - if (buff[count - 1] == '\n') - buff[count - 1] = '\0'; - - bat_info(net_dev, - "Invalid parameter for 'aggregate OGM' setting" - "received: %s\n", buff); - return -EINVAL; - } - - if (atomic_read(&bat_priv->aggregated_ogms) == aggr_tmp) - return count; - - bat_info(net_dev, "Changing aggregation from: %s to: %s\n", - atomic_read(&bat_priv->aggregated_ogms) == 1 ? - "enabled" : "disabled", aggr_tmp == 1 ? "enabled" : - "disabled");
- atomic_set(&bat_priv->aggregated_ogms, (unsigned)aggr_tmp); - return count; + return store_bool_attr(buff, count, net_dev, (char *)attr->name, + &bat_priv->aggregated_ogms); }
static ssize_t show_bond(struct kobject *kobj, struct attribute *attr, @@ -105,36 +153,9 @@ static ssize_t store_bond(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct bat_priv *bat_priv = netdev_priv(net_dev); - int bonding_enabled_tmp = -1; - - if (((count == 2) && (buff[0] == '1')) || - (strncmp(buff, "enable", 6) == 0)) - bonding_enabled_tmp = 1;
- if (((count == 2) && (buff[0] == '0')) || - (strncmp(buff, "disable", 7) == 0)) - bonding_enabled_tmp = 0; - - if (bonding_enabled_tmp < 0) { - if (buff[count - 1] == '\n') - buff[count - 1] = '\0'; - - bat_err(net_dev, - "Invalid parameter for 'bonding' setting received: " - "%s\n", buff); - return -EINVAL; - } - - if (atomic_read(&bat_priv->bonding) == bonding_enabled_tmp) - return count; - - bat_info(net_dev, "Changing bonding from: %s to: %s\n", - atomic_read(&bat_priv->bonding) == 1 ? - "enabled" : "disabled", - bonding_enabled_tmp == 1 ? "enabled" : "disabled"); - - atomic_set(&bat_priv->bonding, (unsigned)bonding_enabled_tmp); - return count; + return store_bool_attr(buff, count, net_dev, (char *)attr->name, + &bat_priv->bonding); }
static ssize_t show_frag(struct kobject *kobj, struct attribute *attr, @@ -154,37 +175,14 @@ static ssize_t store_frag(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct bat_priv *bat_priv = netdev_priv(net_dev); - int frag_enabled_tmp = -1; - - if (((count == 2) && (buff[0] == '1')) || - (strncmp(buff, "enable", 6) == 0)) - frag_enabled_tmp = 1; - - if (((count == 2) && (buff[0] == '0')) || - (strncmp(buff, "disable", 7) == 0)) - frag_enabled_tmp = 0; - - if (frag_enabled_tmp < 0) { - if (buff[count - 1] == '\n') - buff[count - 1] = '\0'; - - bat_err(net_dev, - "Invalid parameter for 'fragmentation' setting on mesh" - "received: %s\n", buff); - return -EINVAL; - } - - if (atomic_read(&bat_priv->fragmentation) == frag_enabled_tmp) - return count; + int ret;
- bat_info(net_dev, "Changing fragmentation from: %s to: %s\n", - atomic_read(&bat_priv->fragmentation) == 1 ? - "enabled" : "disabled", - frag_enabled_tmp == 1 ? "enabled" : "disabled"); + ret = store_bool_attr(buff, count, net_dev, (char *)attr->name, + &bat_priv->fragmentation); + if (ret) + update_min_mtu(net_dev);
- atomic_set(&bat_priv->fragmentation, (unsigned)frag_enabled_tmp); - update_min_mtu(net_dev); - return count; + return ret; }
static ssize_t show_vis_mode(struct kobject *kobj, struct attribute *attr, @@ -300,31 +298,9 @@ static ssize_t store_orig_interval(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct bat_priv *bat_priv = netdev_priv(net_dev); - unsigned long orig_interval_tmp; - int ret;
- ret = strict_strtoul(buff, 10, &orig_interval_tmp); - if (ret) { - bat_info(net_dev, "Invalid parameter for 'orig_interval' " - "setting received: %s\n", buff); - return -EINVAL; - } - - if (orig_interval_tmp < JITTER * 2) { - bat_info(net_dev, "New originator interval too small: %li " - "(min: %i)\n", orig_interval_tmp, JITTER * 2); - return -EINVAL; - } - - if (atomic_read(&bat_priv->orig_interval) == orig_interval_tmp) - return count; - - bat_info(net_dev, "Changing originator interval from: %i to: %li\n", - atomic_read(&bat_priv->orig_interval), - orig_interval_tmp); - - atomic_set(&bat_priv->orig_interval, orig_interval_tmp); - return count; + return store_uint_attr(buff, count, net_dev, (char *)attr->name, + 2 * JITTER, UINT_MAX, &bat_priv->orig_interval); }
#ifdef CONFIG_BATMAN_ADV_DEBUG @@ -344,31 +320,9 @@ static ssize_t store_log_level(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct bat_priv *bat_priv = netdev_priv(net_dev); - unsigned long log_level_tmp; - int ret; - - ret = strict_strtoul(buff, 10, &log_level_tmp); - if (ret) { - bat_info(net_dev, "Invalid parameter for 'log_level' " - "setting received: %s\n", buff); - return -EINVAL; - } - - if (log_level_tmp > 3) { - bat_info(net_dev, "New log level too big: %li " - "(max: %i)\n", log_level_tmp, 3); - return -EINVAL; - } - - if (atomic_read(&bat_priv->log_level) == log_level_tmp) - return count; - - bat_info(net_dev, "Changing log level from: %i to: %li\n", - atomic_read(&bat_priv->log_level), - log_level_tmp);
- atomic_set(&bat_priv->log_level, (unsigned)log_level_tmp); - return count; + return store_uint_attr(buff, count, net_dev, (char *)attr->name, + 0, 3, &bat_priv->log_level); } #endif
This makes exposing bat_priv's atomic_ts storing boolean or uint values possible with only two additional lines of code in the future. It also further reduces the amount of duplicate code in bat_sysfs.c.
Signed-off-by: Linus Lüssing linus.luessing@web.de --- bat_sysfs.c | 205 ++++++++++++++++++++++++++--------------------------------- 1 files changed, 89 insertions(+), 116 deletions(-)
diff --git a/bat_sysfs.c b/bat_sysfs.c index 6a60561..0d58e9c 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -31,6 +31,7 @@
#define to_dev(obj) container_of(obj, struct device, kobj)
+/* Use this, if you have customized show and store functions */ #define BAT_ATTR(_name, _mode, _show, _store) \ struct bat_attribute bat_attr_##_name = { \ .attr = {.name = __stringify(_name), \ @@ -39,6 +40,65 @@ struct bat_attribute bat_attr_##_name = { \ .store = _store, \ };
+#define BAT_ATTR_STORE_BOOL(_name, _post_func) \ +ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff, size_t count) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return __store_bool_attr(buff, count, _post_func, attr, \ + &bat_priv->_name, net_dev); \ +} + +#define BAT_ATTR_SHOW_BOOL(_name) \ +ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return sprintf(buff, "%s\n", \ + atomic_read(&bat_priv->_name) == 0 ? \ + "disabled" : "enabled"); \ +} \ + +/* Use this, if you are going to turn a [name] in bat_priv on or off */ +#define BAT_ATTR_BOOL(_name, _mode, _post_func) \ + static BAT_ATTR_STORE_BOOL(_name, _post_func) \ + static BAT_ATTR_SHOW_BOOL(_name) \ + static BAT_ATTR(_name, _mode, show_##_name, store_##_name) + + +#define BAT_ATTR_STORE_UINT(_name, _min, _max, _post_func) \ +ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff, size_t count) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return __store_uint_attr(buff, count, _min, _max, _post_func, \ + attr, &bat_priv->_name, net_dev); \ +} + +#define BAT_ATTR_SHOW_UINT(_name) \ +ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return sprintf(buff, "%i\n", atomic_read(&bat_priv->_name)); \ +} \ + +/* Use this, if you are going to set [name] in bat_priv to unsigned integer + * values only */ +#define BAT_ATTR_UINT(_name, _mode, _min, _max, _post_func) \ + static BAT_ATTR_STORE_UINT(_name, _min, _max, _post_func) \ + static BAT_ATTR_SHOW_UINT(_name) \ + static BAT_ATTR(_name, _mode, show_##_name, store_##_name) + + static int store_bool_attr(char *buff, size_t count, struct net_device *net_dev, char *attr_name, atomic_t *attr) @@ -76,6 +136,21 @@ static int store_bool_attr(char *buff, size_t count, return count; }
+inline static ssize_t __store_bool_attr(char *buff, size_t count, + void (*post_func)(struct net_device *), + struct attribute *attr, + atomic_t *attr_store, struct net_device *net_dev) +{ + int ret; + + ret = store_bool_attr(buff, count, net_dev, (char *)attr->name, + attr_store); + if (post_func && ret) + post_func(net_dev); + + return ret; +} + static int store_uint_attr(char *buff, size_t count, struct net_device *net_dev, char *attr_name, unsigned int min, unsigned int max, atomic_t *attr) @@ -113,74 +188,18 @@ static int store_uint_attr(char *buff, size_t count, return count; }
- -static ssize_t show_aggr_ogms(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int aggr_status = atomic_read(&bat_priv->aggregated_ogms); - - return sprintf(buff, "%s\n", - aggr_status == 0 ? "disabled" : "enabled"); -} - -static ssize_t store_aggr_ogms(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_bool_attr(buff, count, net_dev, (char *)attr->name, - &bat_priv->aggregated_ogms); -} - -static ssize_t show_bond(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int bond_status = atomic_read(&bat_priv->bonding); - - return sprintf(buff, "%s\n", - bond_status == 0 ? "disabled" : "enabled"); -} - -static ssize_t store_bond(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_bool_attr(buff, count, net_dev, (char *)attr->name, - &bat_priv->bonding); -} - -static ssize_t show_frag(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int frag_status = atomic_read(&bat_priv->fragmentation); - - return sprintf(buff, "%s\n", - frag_status == 0 ? "disabled" : "enabled"); -} - -static ssize_t store_frag(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) +inline static ssize_t __store_uint_attr(char *buff, size_t count, + int min, int max, + void (*post_func)(struct net_device *), + struct attribute *attr, + atomic_t *attr_store, struct net_device *net_dev) { - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); int ret;
- ret = store_bool_attr(buff, count, net_dev, (char *)attr->name, - &bat_priv->fragmentation); - if (ret) - update_min_mtu(net_dev); + ret = store_uint_attr(buff, count, net_dev, (char *)attr->name, + min, max, attr_store); + if (post_func && ret) + post_func(net_dev);
return ret; } @@ -282,60 +301,14 @@ static ssize_t store_gw_mode(struct kobject *kobj, struct attribute *attr, return gw_mode_set(net_dev, buff, count); }
-static ssize_t show_orig_interval(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - - return sprintf(buff, "%i\n", - atomic_read(&bat_priv->orig_interval)); -} - -static ssize_t store_orig_interval(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_uint_attr(buff, count, net_dev, (char *)attr->name, - 2 * JITTER, UINT_MAX, &bat_priv->orig_interval); -} - -#ifdef CONFIG_BATMAN_ADV_DEBUG -static ssize_t show_log_level(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int log_level = atomic_read(&bat_priv->log_level); - - return sprintf(buff, "%d\n", log_level); -} - -static ssize_t store_log_level(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_uint_attr(buff, count, net_dev, (char *)attr->name, - 0, 3, &bat_priv->log_level); -} -#endif - -static BAT_ATTR(aggregated_ogms, S_IRUGO | S_IWUSR, - show_aggr_ogms, store_aggr_ogms); -static BAT_ATTR(bonding, S_IRUGO | S_IWUSR, show_bond, store_bond); -static BAT_ATTR(fragmentation, S_IRUGO | S_IWUSR, show_frag, store_frag); +BAT_ATTR_BOOL(aggregated_ogms, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_BOOL(bonding, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_BOOL(fragmentation, S_IRUGO | S_IWUSR, update_min_mtu); static BAT_ATTR(vis_mode, S_IRUGO | S_IWUSR, show_vis_mode, store_vis_mode); static BAT_ATTR(gw_mode, S_IRUGO | S_IWUSR, show_gw_mode, store_gw_mode); -static BAT_ATTR(orig_interval, S_IRUGO | S_IWUSR, - show_orig_interval, store_orig_interval); +BAT_ATTR_UINT(orig_interval, S_IRUGO | S_IWUSR, 2 * JITTER, INT_MAX, NULL); #ifdef CONFIG_BATMAN_ADV_DEBUG -static BAT_ATTR(log_level, S_IRUGO | S_IWUSR, show_log_level, store_log_level); +BAT_ATTR_UINT(log_level, S_IRUGO | S_IWUSR, 0, 3, NULL); #endif
static struct bat_attribute *mesh_attrs[] = {
Linus Lüssing wrote:
This makes exposing bat_priv's atomic_ts storing boolean or uint values possible with only two additional lines of code in the future. It also further reduces the amount of duplicate code in bat_sysfs.c.
Signed-off-by: Linus Lüssing linus.luessing@web.de
Please fix the checkpatch problems first:
WARNING: please, no space before tabs #47: FILE: batman-adv/bat_sysfs.c:55: +ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, ^I$
WARNING: storage class should be at the beginning of the declaration #101: FILE: batman-adv/bat_sysfs.c:139: +inline static ssize_t __store_bool_attr(char *buff, size_t count,
ERROR: inline keyword should sit between storage class and type #101: FILE: batman-adv/bat_sysfs.c:139: +inline static ssize_t __store_bool_attr(char *buff, size_t count,
WARNING: storage class should be at the beginning of the declaration #126: FILE: batman-adv/bat_sysfs.c:191: +inline static ssize_t __store_uint_attr(char *buff, size_t count,
ERROR: inline keyword should sit between storage class and type #126: FILE: batman-adv/bat_sysfs.c:191: +inline static ssize_t __store_uint_attr(char *buff, size_t count,
total: 2 errors, 3 warnings, 0 checks, 241 lines checked
0003-batman-adv-Introduce-generic-BAT_ATTR_-macros.patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.
This makes exposing bat_priv's atomic_ts storing boolean or uint values possible with only two additional lines of code in the future. It also further reduces the amount of duplicate code in bat_sysfs.c.
Signed-off-by: Linus Lüssing linus.luessing@web.de --- bat_sysfs.c | 205 ++++++++++++++++++++++++++--------------------------------- 1 files changed, 89 insertions(+), 116 deletions(-)
diff --git a/bat_sysfs.c b/bat_sysfs.c index 6a60561..7efe1dd 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -31,6 +31,7 @@
#define to_dev(obj) container_of(obj, struct device, kobj)
+/* Use this, if you have customized show and store functions */ #define BAT_ATTR(_name, _mode, _show, _store) \ struct bat_attribute bat_attr_##_name = { \ .attr = {.name = __stringify(_name), \ @@ -39,6 +40,65 @@ struct bat_attribute bat_attr_##_name = { \ .store = _store, \ };
+#define BAT_ATTR_STORE_BOOL(_name, _post_func) \ +ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff, size_t count) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return __store_bool_attr(buff, count, _post_func, attr, \ + &bat_priv->_name, net_dev); \ +} + +#define BAT_ATTR_SHOW_BOOL(_name) \ +ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return sprintf(buff, "%s\n", \ + atomic_read(&bat_priv->_name) == 0 ? \ + "disabled" : "enabled"); \ +} \ + +/* Use this, if you are going to turn a [name] in bat_priv on or off */ +#define BAT_ATTR_BOOL(_name, _mode, _post_func) \ + static BAT_ATTR_STORE_BOOL(_name, _post_func) \ + static BAT_ATTR_SHOW_BOOL(_name) \ + static BAT_ATTR(_name, _mode, show_##_name, store_##_name) + + +#define BAT_ATTR_STORE_UINT(_name, _min, _max, _post_func) \ +ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff, size_t count) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return __store_uint_attr(buff, count, _min, _max, _post_func, \ + attr, &bat_priv->_name, net_dev); \ +} + +#define BAT_ATTR_SHOW_UINT(_name) \ +ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff) \ +{ \ + struct device *dev = to_dev(kobj->parent); \ + struct net_device *net_dev = to_net_dev(dev); \ + struct bat_priv *bat_priv = netdev_priv(net_dev); \ + return sprintf(buff, "%i\n", atomic_read(&bat_priv->_name)); \ +} \ + +/* Use this, if you are going to set [name] in bat_priv to unsigned integer + * values only */ +#define BAT_ATTR_UINT(_name, _mode, _min, _max, _post_func) \ + static BAT_ATTR_STORE_UINT(_name, _min, _max, _post_func) \ + static BAT_ATTR_SHOW_UINT(_name) \ + static BAT_ATTR(_name, _mode, show_##_name, store_##_name) + + static int store_bool_attr(char *buff, size_t count, struct net_device *net_dev, char *attr_name, atomic_t *attr) @@ -76,6 +136,21 @@ static int store_bool_attr(char *buff, size_t count, return count; }
+static inline ssize_t __store_bool_attr(char *buff, size_t count, + void (*post_func)(struct net_device *), + struct attribute *attr, + atomic_t *attr_store, struct net_device *net_dev) +{ + int ret; + + ret = store_bool_attr(buff, count, net_dev, (char *)attr->name, + attr_store); + if (post_func && ret) + post_func(net_dev); + + return ret; +} + static int store_uint_attr(char *buff, size_t count, struct net_device *net_dev, char *attr_name, unsigned int min, unsigned int max, atomic_t *attr) @@ -113,74 +188,18 @@ static int store_uint_attr(char *buff, size_t count, return count; }
- -static ssize_t show_aggr_ogms(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int aggr_status = atomic_read(&bat_priv->aggregated_ogms); - - return sprintf(buff, "%s\n", - aggr_status == 0 ? "disabled" : "enabled"); -} - -static ssize_t store_aggr_ogms(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_bool_attr(buff, count, net_dev, (char *)attr->name, - &bat_priv->aggregated_ogms); -} - -static ssize_t show_bond(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int bond_status = atomic_read(&bat_priv->bonding); - - return sprintf(buff, "%s\n", - bond_status == 0 ? "disabled" : "enabled"); -} - -static ssize_t store_bond(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_bool_attr(buff, count, net_dev, (char *)attr->name, - &bat_priv->bonding); -} - -static ssize_t show_frag(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int frag_status = atomic_read(&bat_priv->fragmentation); - - return sprintf(buff, "%s\n", - frag_status == 0 ? "disabled" : "enabled"); -} - -static ssize_t store_frag(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) +static inline ssize_t __store_uint_attr(char *buff, size_t count, + int min, int max, + void (*post_func)(struct net_device *), + struct attribute *attr, + atomic_t *attr_store, struct net_device *net_dev) { - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); int ret;
- ret = store_bool_attr(buff, count, net_dev, (char *)attr->name, - &bat_priv->fragmentation); - if (ret) - update_min_mtu(net_dev); + ret = store_uint_attr(buff, count, net_dev, (char *)attr->name, + min, max, attr_store); + if (post_func && ret) + post_func(net_dev);
return ret; } @@ -282,60 +301,14 @@ static ssize_t store_gw_mode(struct kobject *kobj, struct attribute *attr, return gw_mode_set(net_dev, buff, count); }
-static ssize_t show_orig_interval(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - - return sprintf(buff, "%i\n", - atomic_read(&bat_priv->orig_interval)); -} - -static ssize_t store_orig_interval(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_uint_attr(buff, count, net_dev, (char *)attr->name, - 2 * JITTER, UINT_MAX, &bat_priv->orig_interval); -} - -#ifdef CONFIG_BATMAN_ADV_DEBUG -static ssize_t show_log_level(struct kobject *kobj, struct attribute *attr, - char *buff) -{ - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int log_level = atomic_read(&bat_priv->log_level); - - return sprintf(buff, "%d\n", log_level); -} - -static ssize_t store_log_level(struct kobject *kobj, struct attribute *attr, - char *buff, size_t count) -{ - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - struct bat_priv *bat_priv = netdev_priv(net_dev); - - return store_uint_attr(buff, count, net_dev, (char *)attr->name, - 0, 3, &bat_priv->log_level); -} -#endif - -static BAT_ATTR(aggregated_ogms, S_IRUGO | S_IWUSR, - show_aggr_ogms, store_aggr_ogms); -static BAT_ATTR(bonding, S_IRUGO | S_IWUSR, show_bond, store_bond); -static BAT_ATTR(fragmentation, S_IRUGO | S_IWUSR, show_frag, store_frag); +BAT_ATTR_BOOL(aggregated_ogms, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_BOOL(bonding, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_BOOL(fragmentation, S_IRUGO | S_IWUSR, update_min_mtu); static BAT_ATTR(vis_mode, S_IRUGO | S_IWUSR, show_vis_mode, store_vis_mode); static BAT_ATTR(gw_mode, S_IRUGO | S_IWUSR, show_gw_mode, store_gw_mode); -static BAT_ATTR(orig_interval, S_IRUGO | S_IWUSR, - show_orig_interval, store_orig_interval); +BAT_ATTR_UINT(orig_interval, S_IRUGO | S_IWUSR, 2 * JITTER, INT_MAX, NULL); #ifdef CONFIG_BATMAN_ADV_DEBUG -static BAT_ATTR(log_level, S_IRUGO | S_IWUSR, show_log_level, store_log_level); +BAT_ATTR_UINT(log_level, S_IRUGO | S_IWUSR, 0, 3, NULL); #endif
static struct bat_attribute *mesh_attrs[] = {
We actually do not need an extra struct device variable, therefore replacing them with defines that directly get the bat_priv or net_device. This further reduces the code size in bat_sysfs.c and especially shortens some macros.
Signed-off-by: Linus Lüssing linus.luessing@web.de --- bat_sysfs.c | 40 ++++++++++++++-------------------------- 1 files changed, 14 insertions(+), 26 deletions(-)
diff --git a/bat_sysfs.c b/bat_sysfs.c index 0d58e9c..c20af1f 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -29,7 +29,9 @@ #include "vis.h" #include "compat.h"
-#define to_dev(obj) container_of(obj, struct device, kobj) +#define to_dev(obj) container_of(obj, struct device, kobj) +#define kobj_to_netdev(obj) to_net_dev(to_dev(obj->parent)) +#define kobj_to_batpriv(obj) netdev_priv(kobj_to_netdev(obj))
/* Use this, if you have customized show and store functions */ #define BAT_ATTR(_name, _mode, _show, _store) \ @@ -44,8 +46,7 @@ struct bat_attribute bat_attr_##_name = { \ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ char *buff, size_t count) \ { \ - struct device *dev = to_dev(kobj->parent); \ - struct net_device *net_dev = to_net_dev(dev); \ + struct net_device *net_dev = kobj_to_netdev(kobj); \ struct bat_priv *bat_priv = netdev_priv(net_dev); \ return __store_bool_attr(buff, count, _post_func, attr, \ &bat_priv->_name, net_dev); \ @@ -55,9 +56,7 @@ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ char *buff) \ { \ - struct device *dev = to_dev(kobj->parent); \ - struct net_device *net_dev = to_net_dev(dev); \ - struct bat_priv *bat_priv = netdev_priv(net_dev); \ + struct bat_priv *bat_priv = kobj_to_batpriv(kobj); \ return sprintf(buff, "%s\n", \ atomic_read(&bat_priv->_name) == 0 ? \ "disabled" : "enabled"); \ @@ -74,8 +73,7 @@ ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ char *buff, size_t count) \ { \ - struct device *dev = to_dev(kobj->parent); \ - struct net_device *net_dev = to_net_dev(dev); \ + struct net_device *net_dev = kobj_to_netdev(kobj); \ struct bat_priv *bat_priv = netdev_priv(net_dev); \ return __store_uint_attr(buff, count, _min, _max, _post_func, \ attr, &bat_priv->_name, net_dev); \ @@ -85,9 +83,7 @@ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ char *buff) \ { \ - struct device *dev = to_dev(kobj->parent); \ - struct net_device *net_dev = to_net_dev(dev); \ - struct bat_priv *bat_priv = netdev_priv(net_dev); \ + struct bat_priv *bat_priv = kobj_to_batpriv(kobj); \ return sprintf(buff, "%i\n", atomic_read(&bat_priv->_name)); \ } \
@@ -207,8 +203,7 @@ inline static ssize_t __store_uint_attr(char *buff, size_t count, static ssize_t show_vis_mode(struct kobject *kobj, struct attribute *attr, char *buff) { - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); + struct bat_priv *bat_priv = kobj_to_batpriv(kobj); int vis_mode = atomic_read(&bat_priv->vis_mode);
return sprintf(buff, "%s\n", @@ -219,8 +214,7 @@ static ssize_t show_vis_mode(struct kobject *kobj, struct attribute *attr, static ssize_t store_vis_mode(struct kobject *kobj, struct attribute *attr, char *buff, size_t count) { - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); + struct net_device *net_dev = kobj_to_netdev(kobj); struct bat_priv *bat_priv = netdev_priv(net_dev); unsigned long val; int ret, vis_mode_tmp = -1; @@ -261,8 +255,7 @@ static ssize_t store_vis_mode(struct kobject *kobj, struct attribute *attr, static ssize_t show_gw_mode(struct kobject *kobj, struct attribute *attr, char *buff) { - struct device *dev = to_dev(kobj->parent); - struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); + struct bat_priv *bat_priv = kobj_to_batpriv(kobj); int down, up, bytes_written; int gw_mode = atomic_read(&bat_priv->gw_mode); int gw_class = atomic_read(&bat_priv->gw_class); @@ -295,9 +288,7 @@ static ssize_t show_gw_mode(struct kobject *kobj, struct attribute *attr, static ssize_t store_gw_mode(struct kobject *kobj, struct attribute *attr, char *buff, size_t count) { - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); - + struct net_device *net_dev = kobj_to_netdev(kobj); return gw_mode_set(net_dev, buff, count); }
@@ -377,8 +368,7 @@ void sysfs_del_meshif(struct net_device *dev) static ssize_t show_mesh_iface(struct kobject *kobj, struct attribute *attr, char *buff) { - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); + struct net_device *net_dev = kobj_to_netdev(kobj); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); ssize_t length;
@@ -396,8 +386,7 @@ static ssize_t show_mesh_iface(struct kobject *kobj, struct attribute *attr, static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, char *buff, size_t count) { - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); + struct net_device *net_dev = kobj_to_netdev(kobj); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); int status_tmp = -1; int ret; @@ -450,8 +439,7 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, static ssize_t show_iface_status(struct kobject *kobj, struct attribute *attr, char *buff) { - struct device *dev = to_dev(kobj->parent); - struct net_device *net_dev = to_net_dev(dev); + struct net_device *net_dev = kobj_to_netdev(kobj); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); ssize_t length;
When having a mixed topology of both very mobile and rather static nodes, you are usually best advised to set the originator interval on all nodes to a level best suited for the most mobile node.
However, if most of the nodes are rather static, this can create a lot of undesired overhead as a trade-off then. If setting the interval too low on the static nodes, a mobile node might be chosen as a router for too long, not switching away from it fast enough because of its mobility and the low frequency of ogms of static nodes.
Exposing the hop_penalty is especially useful for the stated scenario: A static node can keep the default originator interval, a mobile node can select a quicker one resulting in faster route updates towards this mobile node. Additionally, such a mobile node could select a higher hop penalty (or even set it to 255 to disable acting as a router for other nodes) to make it less desirable, letting other nodes avoid selecting this mobile node as a router.
Signed-off-by: Linus Lüssing linus.luessing@ascom.ch --- bat_sysfs.c | 2 ++ main.h | 2 -- send.c | 7 ++++--- soft-interface.c | 1 + types.h | 1 + 5 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/bat_sysfs.c b/bat_sysfs.c index c20af1f..4e5ba79 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -298,6 +298,7 @@ BAT_ATTR_BOOL(fragmentation, S_IRUGO | S_IWUSR, update_min_mtu); static BAT_ATTR(vis_mode, S_IRUGO | S_IWUSR, show_vis_mode, store_vis_mode); static BAT_ATTR(gw_mode, S_IRUGO | S_IWUSR, show_gw_mode, store_gw_mode); BAT_ATTR_UINT(orig_interval, S_IRUGO | S_IWUSR, 2 * JITTER, INT_MAX, NULL); +BAT_ATTR_UINT(hop_penalty, S_IRUGO | S_IWUSR, 0, TQ_MAX_VALUE, NULL); #ifdef CONFIG_BATMAN_ADV_DEBUG BAT_ATTR_UINT(log_level, S_IRUGO | S_IWUSR, 0, 3, NULL); #endif @@ -309,6 +310,7 @@ static struct bat_attribute *mesh_attrs[] = { &bat_attr_vis_mode, &bat_attr_gw_mode, &bat_attr_orig_interval, + &bat_attr_hop_penalty, #ifdef CONFIG_BATMAN_ADV_DEBUG &bat_attr_log_level, #endif diff --git a/main.h b/main.h index cc42eb4..de3c9b9 100644 --- a/main.h +++ b/main.h @@ -52,8 +52,6 @@ #define TQ_LOCAL_BIDRECT_RECV_MINIMUM 1 #define TQ_TOTAL_BIDRECT_LIMIT 1
-#define TQ_HOP_PENALTY 10 - #define NUM_WORDS (TQ_LOCAL_WINDOW_SIZE / WORD_BIT_SIZE)
#define PACKBUFF_SIZE 2000 diff --git a/send.c b/send.c index 180e18b..943437b 100644 --- a/send.c +++ b/send.c @@ -35,9 +35,10 @@ static void send_outstanding_bcast_packet(struct work_struct *work);
/* apply hop penalty for a normal link */ -static uint8_t hop_penalty(const uint8_t tq) +static uint8_t hop_penalty(const uint8_t tq, struct bat_priv *bat_priv) { - return (tq * (TQ_MAX_VALUE - TQ_HOP_PENALTY)) / (TQ_MAX_VALUE); + int hop_penalty = atomic_read(&bat_priv->hop_penalty); + return (tq * (TQ_MAX_VALUE - hop_penalty)) / (TQ_MAX_VALUE); }
/* when do we schedule our own packet to be sent */ @@ -339,7 +340,7 @@ void schedule_forward_packet(struct orig_node *orig_node, }
/* apply hop penalty */ - batman_packet->tq = hop_penalty(batman_packet->tq); + batman_packet->tq = hop_penalty(batman_packet->tq, bat_priv);
bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: tq_orig: %i, tq_avg: %i, " diff --git a/soft-interface.c b/soft-interface.c index 7a899b5..69c283c 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -593,6 +593,7 @@ struct net_device *softif_create(char *name) atomic_set(&bat_priv->gw_mode, GW_MODE_OFF); atomic_set(&bat_priv->gw_class, 0); atomic_set(&bat_priv->orig_interval, 1000); + atomic_set(&bat_priv->hop_penalty, 10); atomic_set(&bat_priv->log_level, 0); atomic_set(&bat_priv->fragmentation, 1); atomic_set(&bat_priv->bcast_queue_left, BCAST_QUEUE_LEN); diff --git a/types.h b/types.h index 44b3c07..299769e 100644 --- a/types.h +++ b/types.h @@ -131,6 +131,7 @@ struct bat_priv { atomic_t gw_mode; /* GW_MODE_* */ atomic_t gw_class; /* uint */ atomic_t orig_interval; /* uint */ + atomic_t hop_penalty; /* uint */ atomic_t log_level; /* uint */ atomic_t bcast_seqno; atomic_t bcast_queue_left;
On Wednesday 13 October 2010 13:33:09 Linus Lüssing wrote:
Exposing the hop_penalty is especially useful for the stated scenario: A static node can keep the default originator interval, a mobile node can select a quicker one resulting in faster route updates towards this mobile node. Additionally, such a mobile node could select a higher hop penalty (or even set it to 255 to disable acting as a router for other nodes) to make it less desirable, letting other nodes avoid selecting this mobile node as a router.
Could you please complete this patch by providing the proper sysfs API documentation. For a reference simply check the sysfs-class-net-mesh file which is now part of the repository.
Thanks, Marek
Signed-off-by: Linus Lüssing linus.luessing@ascom.ch --- sysfs-class-net-mesh | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/sysfs-class-net-mesh b/sysfs-class-net-mesh index 5aa1912..f3f9070 100644 --- a/sysfs-class-net-mesh +++ b/sysfs-class-net-mesh @@ -21,6 +21,18 @@ Description: Defines the interval in milliseconds in which batman sends its protocol messages.
+What: /sys/class/net/<mesh_iface>/mesh/hop_penalty +Date: Oct 2010 +Contact: Linus Lüssing linus.luessing@web.de +Description: + Defines the penalty which will be applied to an + originator message's tq-field on every hop. In a + mixed topology of static and mobile nodes, it might + be desired to increase this value on the mobile ones + instead of increasing the originator interval + everywhere - or even set it to its maximum to disable + forwarding on a node. + What: /sys/class/net/<mesh_iface>/mesh/vis_mode Date: May 2010 Contact: Marek Lindner lindner_marek@yahoo.de
On Monday 18 October 2010 12:01:59 Linus Lüssing wrote:
Defines the penalty which will be applied to anoriginator message's tq-field on every hop. In amixed topology of static and mobile nodes, it mightbe desired to increase this value on the mobile onesinstead of increasing the originator intervaleverywhere - or even set it to its maximum to disableforwarding on a node.
Sorry, but I don't quite understand the connection between hop penalty and orig interval that you are trying to make here. The orig interval regulates the interval in which the network is flooded with topology information whereas the hop penalty influences the likelihood of being chosen as a "hop" towards a destination.
Regards, Marek
Signed-off-by: Linus Lüssing linus.luessing@ascom.ch --- sysfs-class-net-mesh | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/sysfs-class-net-mesh b/sysfs-class-net-mesh index 5aa1912..2c03e76 100644 --- a/sysfs-class-net-mesh +++ b/sysfs-class-net-mesh @@ -21,6 +21,13 @@ Description: Defines the interval in milliseconds in which batman sends its protocol messages.
+What: /sys/class/net/<mesh_iface>/mesh/hop_penalty +Date: Oct 2010 +Contact: Linus Lüssing linus.luessing@web.de +Description: + Defines the penalty which will be applied to an + originator message's tq-field on every hop. + What: /sys/class/net/<mesh_iface>/mesh/vis_mode Date: May 2010 Contact: Marek Lindner lindner_marek@yahoo.de
On Monday 18 October 2010 23:44:13 Linus Lüssing wrote:
+Description:
Defines the penalty which will be applied to anoriginator message's tq-field on every hop.
Applied in revision 1845.
Thanks, Marek
On Wednesday 13 October 2010 13:33:09 Linus Lüssing wrote:
Exposing the hop_penalty is especially useful for the stated scenario: A static node can keep the default originator interval, a mobile node can select a quicker one resulting in faster route updates towards this mobile node. Additionally, such a mobile node could select a higher hop penalty (or even set it to 255 to disable acting as a router for other nodes) to make it less desirable, letting other nodes avoid selecting this mobile node as a router.
Applied in revision 1843.
Thanks, Marek
On Wednesday 13 October 2010 00:22:01 Linus Lüssing wrote:
Simon's idea so far was, to introduce a debug/ folder in sysfs which is hidden by default and could be enabled with a KConfig option, so that tinkerers still have a chance for their needs for tweaking parameters. num_bcasts would probably be one of the first candidates as a debug-parameter.
I think that is a good compromise. However, the kernel folks might not like the idea of having such "volatile" options within the /sys file system or does anyone know existing code behaving in a similar fashion ? How about making this debug folder available inside our debugfs entry ?
Cheers, Marek
On Wednesday 13 October 2010 00:22:01 Linus Lüssing wrote:
thanks for all the feedback for the changes I wanted to make to the sysfs code base. I tried to address bugs and suggestions. The macros are now a bit smaller than in the beginning, the missing static has been added (thanks Andrew and Sven).
Due to Marecs comment on the issues with changing sysfs file names in the kernel (which I was not aware of, thanks for explaining) and comments about old sysfs filenames being more descriptive, I changed the bat_priv names instead now. I don't really see a good reason yet why they should not be equal.
Patches 1-4 have been applied (revision 1833-1836).
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Linus Lüssing -
Linus Lüssing -
Marek Lindner -
Sven Eckelmann