[B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Make number of (re)broadcasts configurable via sysfs
Depending on the scenario, people might want to adjust the number of (re)broadcast of data packets - usually higher values in sparse or lower values in dense networks.
Signed-off-by: Linus Lüssing linus.luessing@web.de --- send.c | 4 +++- soft-interface.c | 1 + sysfs-class-net-mesh | 8 ++++++++ sysfs.c | 2 ++ types.h | 1 + 5 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/send.c b/send.c index 0a0bb45..4a73c37 100644 --- a/send.c +++ b/send.c @@ -237,12 +237,14 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) struct sk_buff *skb1; struct net_device *soft_iface; struct batadv_priv *bat_priv; + int num_bcasts;
delayed_work = container_of(work, struct delayed_work, work); forw_packet = container_of(delayed_work, struct batadv_forw_packet, delayed_work); soft_iface = forw_packet->if_incoming->soft_iface; bat_priv = netdev_priv(soft_iface); + num_bcasts = atomic_read(&bat_priv->num_bcasts);
spin_lock_bh(&bat_priv->forw_bcast_list_lock); hlist_del(&forw_packet->list); @@ -271,7 +273,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) forw_packet->num_packets++;
/* if we still have some more bcasts to send */ - if (forw_packet->num_packets < 3) { + if (forw_packet->num_packets < num_bcasts) { _batadv_add_bcast_packet_to_list(bat_priv, forw_packet, msecs_to_jiffies(5)); return; diff --git a/soft-interface.c b/soft-interface.c index 403b8c4..53771a4 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -464,6 +464,7 @@ static int batadv_softif_init_late(struct net_device *dev) atomic_set(&bat_priv->gw_bandwidth, 41); atomic_set(&bat_priv->orig_interval, 1000); atomic_set(&bat_priv->hop_penalty, 30); + atomic_set(&bat_priv->num_bcasts, 3); #ifdef CONFIG_BATMAN_ADV_DEBUG atomic_set(&bat_priv->log_level, 0); #endif diff --git a/sysfs-class-net-mesh b/sysfs-class-net-mesh index bdcd8b4..2058cad 100644 --- a/sysfs-class-net-mesh +++ b/sysfs-class-net-mesh @@ -75,6 +75,14 @@ Description: to send fewer wifi packets but still the same content) is enabled or not.
+What: /sys/class/net/<mesh_iface>/mesh/num_bcasts +Date: Mar 2013 +Contact: Linus Lüssing linus.luessing@web.de +Description: + Defines the number of broadcasts used to forward + a multicast (including broadcast) payload frame on an + interface. + What: /sys/class/net/<mesh_iface>/mesh/orig_interval Date: May 2010 Contact: Marek Lindner lindner_marek@yahoo.de diff --git a/sysfs.c b/sysfs.c index 15a22ef..d166b87 100644 --- a/sysfs.c +++ b/sysfs.c @@ -435,6 +435,7 @@ BATADV_ATTR_SIF_UINT(orig_interval, S_IRUGO | S_IWUSR, 2 * BATADV_JITTER, INT_MAX, NULL); BATADV_ATTR_SIF_UINT(hop_penalty, S_IRUGO | S_IWUSR, 0, BATADV_TQ_MAX_VALUE, NULL); +BATADV_ATTR_SIF_UINT(num_bcasts, S_IRUGO | S_IWUSR, 1, INT_MAX, NULL); BATADV_ATTR_SIF_UINT(gw_sel_class, S_IRUGO | S_IWUSR, 1, BATADV_TQ_MAX_VALUE, batadv_post_gw_deselect); static BATADV_ATTR(gw_bandwidth, S_IRUGO | S_IWUSR, batadv_show_gw_bwidth, @@ -462,6 +463,7 @@ static struct batadv_attribute *batadv_mesh_attrs[] = { &batadv_attr_gw_mode, &batadv_attr_orig_interval, &batadv_attr_hop_penalty, + &batadv_attr_num_bcasts, &batadv_attr_gw_sel_class, &batadv_attr_gw_bandwidth, #ifdef CONFIG_BATMAN_ADV_DEBUG diff --git a/types.h b/types.h index aba8364..41bfa0b 100644 --- a/types.h +++ b/types.h @@ -555,6 +555,7 @@ struct batadv_priv { atomic_t gw_bandwidth; atomic_t orig_interval; atomic_t hop_penalty; + atomic_t num_bcasts; #ifdef CONFIG_BATMAN_ADV_DEBUG atomic_t log_level; #endif
From: Matthias Schiffer mschiffer@universe-factory.net
In heterogenous networks, setting the number of broadcasts to differing values on different interfaces can be beneficial.
E.g., on wireless interfaces with high packet loss a higher number of broadcasts may be necessary, whereas on low-bandwidth interfaces with relatively high reliablily (such as VPN links over slow internet lines) sending only a single packet makes more sense to preserve bandwidth.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- hard-interface.c | 1 + send.c | 14 +++++++++-- sysfs-class-net-batman-adv | 10 ++++++++ sysfs.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ types.h | 1 + 5 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/hard-interface.c b/hard-interface.c index fd99e42..6cd53d0 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -511,6 +511,7 @@ 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; + atomic_set(&hard_iface->num_bcasts, 0); INIT_LIST_HEAD(&hard_iface->list); INIT_WORK(&hard_iface->cleanup_work, batadv_hardif_remove_interface_finish); diff --git a/send.c b/send.c index 4a73c37..9a66076 100644 --- a/send.c +++ b/send.c @@ -237,7 +237,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) struct sk_buff *skb1; struct net_device *soft_iface; struct batadv_priv *bat_priv; - int num_bcasts; + int num_bcasts, if_num_bcasts, max_num_bcasts = 0;
delayed_work = container_of(work, struct delayed_work, work); forw_packet = container_of(delayed_work, struct batadv_forw_packet, @@ -262,6 +262,16 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) if (hard_iface->soft_iface != soft_iface) continue;
+ if_num_bcasts = atomic_read(&hard_iface->num_bcasts); + if (if_num_bcasts == 0) + if_num_bcasts = num_bcasts; + + if (forw_packet->num_packets >= if_num_bcasts) + continue; + + if (if_num_bcasts > max_num_bcasts) + max_num_bcasts = if_num_bcasts; + /* send a copy of the saved skb */ skb1 = skb_clone(forw_packet->skb, GFP_ATOMIC); if (skb1) @@ -273,7 +283,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) forw_packet->num_packets++;
/* if we still have some more bcasts to send */ - if (forw_packet->num_packets < num_bcasts) { + if (forw_packet->num_packets < max_num_bcasts) { _batadv_add_bcast_packet_to_list(bat_priv, forw_packet, msecs_to_jiffies(5)); return; diff --git a/sysfs-class-net-batman-adv b/sysfs-class-net-batman-adv index bdc0070..eb07a4d 100644 --- a/sysfs-class-net-batman-adv +++ b/sysfs-class-net-batman-adv @@ -1,4 +1,14 @@
+What: /sys/class/net/<iface>/batman-adv/iface_num_bcasts +Date: Mar 2013 +Contact: Matthias Schiffer mschiffer@universe-factory.net +Description: + Defines the number of broadcasts used to forward + a multicast (including broadcast) payload frame on a + single interface. When the value is 'default', the + num_bcasts value from the associated batman mesh + interface is used. + What: /sys/class/net/<iface>/batman-adv/iface_status Date: May 2010 Contact: Marek Lindner lindner_marek@yahoo.de diff --git a/sysfs.c b/sysfs.c index d166b87..152f96b 100644 --- a/sysfs.c +++ b/sysfs.c @@ -644,13 +644,69 @@ static ssize_t batadv_show_iface_status(struct kobject *kobj, return length; }
+static ssize_t batadv_show_iface_num_bcasts(struct kobject *kobj, + struct attribute *attr, char *buff) +{ + struct net_device *net_dev = batadv_kobj_to_netdev(kobj); + struct batadv_hard_iface *hard_iface; + ssize_t length; + int num_bcasts; + + hard_iface = batadv_hardif_get_by_netdev(net_dev); + if (!hard_iface) + return 0; + + num_bcasts = atomic_read(&hard_iface->num_bcasts); + if (num_bcasts) + length = sprintf(buff, "%i\n", num_bcasts); + else + length = sprintf(buff, "default\n"); + + batadv_hardif_free_ref(hard_iface); + + return length; +} + +static ssize_t batadv_store_iface_num_bcasts(struct kobject *kobj, + struct attribute *attr, char *buff, + size_t count) +{ + struct net_device *net_dev = batadv_kobj_to_netdev(kobj); + struct batadv_hard_iface *hard_iface; + int ret; + + hard_iface = batadv_hardif_get_by_netdev(net_dev); + if (!hard_iface) + return count; + + if (buff[count - 1] == '\n') + buff[count - 1] = '\0'; + + if (strncmp(buff, "default", 8) == 0) { + atomic_set(&hard_iface->num_bcasts, 0); + ret = count; + } else { + ret = batadv_store_uint_attr(buff, count, net_dev, "num_bcasts", + 0, INT_MAX, + &hard_iface->num_bcasts); + } + + batadv_hardif_free_ref(hard_iface); + + return ret; +} + + static BATADV_ATTR(mesh_iface, S_IRUGO | S_IWUSR, batadv_show_mesh_iface, batadv_store_mesh_iface); static BATADV_ATTR(iface_status, S_IRUGO, batadv_show_iface_status, NULL); +static BATADV_ATTR(iface_num_bcasts, S_IRUGO | S_IWUSR, + batadv_show_iface_num_bcasts, batadv_store_iface_num_bcasts);
static struct batadv_attribute *batadv_batman_attrs[] = { &batadv_attr_mesh_iface, &batadv_attr_iface_status, + &batadv_attr_iface_num_bcasts, NULL, };
diff --git a/types.h b/types.h index 41bfa0b..acfcb90 100644 --- a/types.h +++ b/types.h @@ -76,6 +76,7 @@ struct batadv_hard_iface { char if_status; struct net_device *net_dev; atomic_t frag_seqno; + atomic_t num_bcasts; struct kobject *hardif_obj; atomic_t refcount; struct packet_type batman_adv_ptype;
On Saturday, March 09, 2013 01:38:56 Linus Lüssing wrote:
From: Matthias Schiffer mschiffer@universe-factory.net
In heterogenous networks, setting the number of broadcasts to differing values on different interfaces can be beneficial.
E.g., on wireless interfaces with high packet loss a higher number of broadcasts may be necessary, whereas on low-bandwidth interfaces with relatively high reliablily (such as VPN links over slow internet lines) sending only a single packet makes more sense to preserve bandwidth.
In general, I like the idea but the approach isn't the best. Can't we automate these settings instead of adding hundreds of little knobs nobody will understand ? Why not detecting wifi interfaces as such and configure the broadcast value accordingly ? The same goes for ethernet / vpns ?
Cheers, Marek
On 03/08/2013 08:28 PM, Marek Lindner wrote:
On Saturday, March 09, 2013 01:38:56 Linus Lüssing wrote:
From: Matthias Schiffer mschiffer@universe-factory.net
In heterogenous networks, setting the number of broadcasts to differing values on different interfaces can be beneficial.
E.g., on wireless interfaces with high packet loss a higher number of broadcasts may be necessary, whereas on low-bandwidth interfaces with relatively high reliablily (such as VPN links over slow internet lines) sending only a single packet makes more sense to preserve bandwidth.
In general, I like the idea but the approach isn't the best. Can't we automate these settings instead of adding hundreds of little knobs nobody will understand ? Why not detecting wifi interfaces as such and configure the broadcast value accordingly ? The same goes for ethernet / vpns ?
While is makes sense to find some sensible defaults for such values, in my opinion everything should be as configurable as possible. I don't think it would be a problem to have some dozens knobs more if a normal user almost never has to touch them. On the other hand, for testing, debugging and development purposes, it can save a lot of time not having to recompile the kernel for such changes all the time.
Also, for VPN connections, automatic detection of the link type isn't generally possible, as we'd have to know which medium the VPN is going over...
Regards, Matthias
Cheers, Marek
On Saturday, March 09, 2013 03:42:34 Matthias Schiffer wrote:
In general, I like the idea but the approach isn't the best. Can't we automate these settings instead of adding hundreds of little knobs nobody will understand ? Why not detecting wifi interfaces as such and configure the broadcast value accordingly ? The same goes for ethernet / vpns ?
While is makes sense to find some sensible defaults for such values, in my opinion everything should be as configurable as possible. I don't think it would be a problem to have some dozens knobs more if a normal user almost never has to touch them. On the other hand, for testing, debugging and development purposes, it can save a lot of time not having to recompile the kernel for such changes all the time.
Knobs only used for debugging and development should never go into sysfs. User space APIs are created for life time. Once it is there and we submit it to the kernel the API has to stay and has to be supported forever.
Also, for VPN connections, automatic detection of the link type isn't generally possible, as we'd have to know which medium the VPN is going over...
You are running batman-adv over VPN links on top of wifi interfaces ?
Cheers, Marek
On 03/08/2013 09:06 PM, Marek Lindner wrote:
On Saturday, March 09, 2013 03:42:34 Matthias Schiffer wrote:
In general, I like the idea but the approach isn't the best. Can't we automate these settings instead of adding hundreds of little knobs nobody will understand ? Why not detecting wifi interfaces as such and configure the broadcast value accordingly ? The same goes for ethernet / vpns ?
While is makes sense to find some sensible defaults for such values, in my opinion everything should be as configurable as possible. I don't think it would be a problem to have some dozens knobs more if a normal user almost never has to touch them. On the other hand, for testing, debugging and development purposes, it can save a lot of time not having to recompile the kernel for such changes all the time.
Knobs only used for debugging and development should never go into sysfs. User space APIs are created for life time. Once it is there and we submit it to the kernel the API has to stay and has to be supported forever.
You have a point there... While I still think making the broadcast count adjustable makes sense, I'd also be happy to get a patch accepted that just sets the broadcast count depending on the interface type.
My current plan is:
* WLAN devices: 3 broadcasts * Everything else: 1 broadcast
Any additional types that should be considered?
Also, for VPN connections, automatic detection of the link type isn't generally possible, as we'd have to know which medium the VPN is going over...
You are running batman-adv over VPN links on top of wifi interfaces ?
Not yet, but extrapolating from the setups I've already done, it's probably just a matter of time until I am ;)
Matthias
Cheers, Marek
On Saturday, March 09, 2013 05:50:31 Matthias Schiffer wrote:
You have a point there... While I still think making the broadcast count adjustable makes sense, I'd also be happy to get a patch accepted that just sets the broadcast count depending on the interface type.
My current plan is:
- WLAN devices: 3 broadcasts
- Everything else: 1 broadcast
Any additional types that should be considered?
Sounds good to me.
Cheers, Marek
Gesendet: Freitag, 08. März 2013 um 20:28 Uhr Von: "Marek Lindner" lindner_marek@yahoo.de An: "The list for a Better Approach To Mobile Ad-hoc Networking" b.a.t.m.a.n@lists.open-mesh.org Betreff: Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif
On Saturday, March 09, 2013 01:38:56 Linus Lüssing wrote:
From: Matthias Schiffer mschiffer@universe-factory.net
In heterogenous networks, setting the number of broadcasts to differing values on different interfaces can be beneficial.
E.g., on wireless interfaces with high packet loss a higher number of broadcasts may be necessary, whereas on low-bandwidth interfaces with relatively high reliablily (such as VPN links over slow internet lines) sending only a single packet makes more sense to preserve bandwidth.
In general, I like the idea but the approach isn't the best. Can't we automate these settings instead of adding hundreds of little knobs nobody will understand ? Why not detecting wifi interfaces as such and configure the broadcast value accordingly ? The same goes for ethernet / vpns ?
I like the idea of automating things, especially if the automated mechanism isn't too complex and therefore I like the idea of Matthias latest patch. And it helps us just as well for our setup.
Nevertheless I'm still wondering, whether a manually configurable number of broadcast might be of general interest, not only for debugging and not only for obscure scenarios.
Two things I could think of: A scenario where people might use a higher multicast rate, where maybe a slightly higher rebroadcast number might become necessary. Or for wifi longshots, which could have a very changing, whether dependant link quality (while generalizing a dynamic number of broadcast isn't that easy, it's very easy to write a script to check the current link-quality (or even the local whether station :) ) towards the target which adjusts the number of rebroadcasts from this upper level.)
What do the others think? Or are such scenarios still too obscure to justify the addition of such a sysctl parameters? Has anyone on this ML maybe had a setup where s/he thought such a sysctl parameter could be useful?
Cheers, Linus
Cheers, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
"Linus Lüssing" -
Linus Lüssing
-
Marek Lindner -
Matthias Schiffer