[B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Make number of (re)broadcasts configurable via sysfs

[B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Make number of (re)broadcasts configurable via sysfs

[Linus Lüssing]

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
-- 
1.7.10.4

[B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif

[Linus Lüssing]

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;
-- 
1.7.10.4

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif

[Marek Lindner]

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

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif

[Matthias Schiffer]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

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
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif

[Marek Lindner]

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

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif

[Matthias Schiffer]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

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
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif

[Marek Lindner]

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

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: make number of broadcasts configurable per hardif

["Linus Lüssing"]

> 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
>