batman-adv: introduce 'no_rebroadcast' option
Commit Message
This patch introduces a new sysfs option named "no_rebroadcast" on
a per hard interface basis. It allows manually enabling a split-horizon
like behaviour for the layer 2 multicast payload frames, in that
incoming multicast payload frames on such a hard interface are only
being rebroadcasted on all interfaces except the incoming one instead
of being rebroadcasted on all interfaces.
Such an option should only be enabled if you are certain that these
rebroadcasts are unnecessary. This is usually the case for instance
for point-to-point wifi longshots or wired links.
This option can especially safe a significant amount of upload overhead
if the neighbourhood on a link is rather large, for instance in some
transitive, symmetric VPN configurations.
Using this option wrongly will break your mesh network, use this option
wisely and at your own risk!
Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
hard-interface.c | 2 ++
send.c | 4 +++
sysfs-class-net-batman-adv | 10 ++++++++
sysfs.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
types.h | 1 +
5 files changed, 76 insertions(+)
Comments
On Tue, Sep 24, 2013 at 03:40:33PM +0200, Linus Lüssing wrote:
>
> Using this option wrongly will break your mesh network, use this option
> wisely and at your own risk!
Exactly for this reason (As we discussed on IRC) I think this mechanism should be
reasoned a bit more. As I told you, it would be nice to have a self-activating
mechanism which is able to switch it on or off when really needed.
The common network administrator will probably not have enough knowledge to
understand when this is useful or when this is breaking things.
Moreover, with this patch you are adding a userspace API that we won't be able
to remove anymore (!) and since the feature needs some more time before being
mature I'd rather to suggest to avoid this.
Cheers,
Hi Antonio,
On Tue, Sep 24, 2013 at 03:46:50PM +0200, Antonio Quartulli wrote:
> On Tue, Sep 24, 2013 at 03:40:33PM +0200, Linus Lüssing wrote:
> >
> > Using this option wrongly will break your mesh network, use this option
> > wisely and at your own risk!
>
> Exactly for this reason (As we discussed on IRC) I think this mechanism should be
> reasoned a bit more.
Hm, my personal reason to want such a feature is included, the VPN
example. And since I know that using batman-adv over VPNs is not
uncommon, I'd love to have this feature upstream.
I believe such a feature would be more commonly used than a
non-default hop penalty for example, therefore I think it is
suitable to be considered for upstream.
> As I told you, it would be nice to have a self-activating
> mechanism which is able to switch it on or off when really needed.
I totally agree that a self-activating mechanism would be nice
indeed. Maybe even with a self-adjusting number of rebroadcasts.
But since that is all non-trivial I'm not going to work on
such an automatic feature before my queued multicast patches and
the ones to follow are upstream. I'd prefer not hopping between
multiple feature patchsets again to spare everyone a déjà vu.
So since such an automatic feature is not going to be available
upstream within the next one or two years at least, I guess,
I don't see why the easy, manual option which could be added to
a release relatively quickly, should be spared from the
interested users.
>
> The common network administrator will probably not have enough knowledge to
> understand when this is useful or when this is breaking things.
That's why I added the extra warning at the end.
>
>
> Moreover, with this patch you are adding a userspace API that we won't be able
> to remove anymore (!)
So we should have never added a hop-penalty sysfs option either?
That one could have been automized, too.
> and since the feature needs some more time before being
> mature I'd rather to suggest to avoid this.
I'm sorry, I currently fail to see in which regards this would
need a considerable amount of time to mature. To me this option
seems easy and straightforward, especially as other routing
protocols like babel for instance have such a manual option
available, too. If you have certain, specific maturity concerns,
things I might have overlooked, I'd be happy if you could share
them.
>
>
> Cheers,
>
> --
> Antonio Quartulli
Cheers, Linus
On Tuesday 24 September 2013 16:57:24 Linus Lüssing wrote:
> On Tue, Sep 24, 2013 at 03:46:50PM +0200, Antonio Quartulli wrote:
> > On Tue, Sep 24, 2013 at 03:40:33PM +0200, Linus Lüssing wrote:
> > >
> > >
> > > Using this option wrongly will break your mesh network, use this option
> > > wisely and at your own risk!
> >
> >
> >
> > Exactly for this reason (As we discussed on IRC) I think this mechanism
> > should be reasoned a bit more.
>
> Hm, my personal reason to want such a feature is included, the VPN
> example. And since I know that using batman-adv over VPNs is not
> uncommon, I'd love to have this feature upstream.
Admittedly, I haven't followed the entire discussion but what speaks against
setting the rebroadcast count define for non-wireless to 0 ? Matthias (if I am
not mistaken) provided a patch for the wireless / non-wireless distinction not
too long ago.
Seems way easier as we are looking for a quick and temporary solution ?
Cheers,
Marek
On Wed, Sep 25, 2013 at 11:03:48PM +0800, Marek Lindner wrote:
> On Tuesday 24 September 2013 16:57:24 Linus Lüssing wrote:
> > On Tue, Sep 24, 2013 at 03:46:50PM +0200, Antonio Quartulli wrote:
> > > On Tue, Sep 24, 2013 at 03:40:33PM +0200, Linus Lüssing wrote:
> > > >
> > > >
> > > > Using this option wrongly will break your mesh network, use this option
> > > > wisely and at your own risk!
> > >
> > >
> > >
> > > Exactly for this reason (As we discussed on IRC) I think this mechanism
> > > should be reasoned a bit more.
> >
> > Hm, my personal reason to want such a feature is included, the VPN
> > example. And since I know that using batman-adv over VPNs is not
> > uncommon, I'd love to have this feature upstream.
>
> Admittedly, I haven't followed the entire discussion but what speaks against
> setting the rebroadcast count define for non-wireless to 0 ? Matthias (if I am
> not mistaken) provided a patch for the wireless / non-wireless distinction not
> too long ago.
> Seems way easier as we are looking for a quick and temporary solution ?
>
Just to summarize the IRC discussion for the archive: For our case
(and about half a dozen other Freifunk communities) this wouldn't work
because we'd want to have it to 0 for some VPN interfaces but not all. (*)
The bar for introducing new configuration / tuning options is a
high one for batman-adv because ease of use is a high priority for
the batman team. Therefore things should be automized as much as
possible or configuration options should only be introduced if
there is a high demand for it. Also if it is easy to misuse such an
option and if misuse causes havoc then such patches are not very
welcome, too.
Since this option can generally be automized (with some, but
doable effort) we decided to apply this patch in our project
locally and agreed on not bringing the patch posted here upstream
now.
Cheers, Linus
(*) We have a VPN topology with a few, fully meshed master servers
and several more clients which are connected to two random master
servers. The VPN software we are using, fastd, does not provide
any mesh routing / forwarding capabilities (so it kind of behaves
like OpenVPN in bridged mode without the client-to-client option or
tinc with Mode=switch,Broadcast=no,Forwarding=off).
@@ -602,6 +602,8 @@ batadv_hardif_add_interface(struct net_device *net_dev)
/* extra reference for return */
atomic_set(&hard_iface->refcount, 2);
+ atomic_set(&hard_iface->no_rebroadcast, 0);
+
batadv_check_known_mac_addr(hard_iface->net_dev);
list_add_tail_rcu(&hard_iface->list, &batadv_hardif_list);
@@ -496,6 +496,10 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
if (forw_packet->num_packets >= hard_iface->num_bcasts)
continue;
+ if (atomic_read(&hard_iface->no_rebroadcast) &&
+ forw_packet->skb->dev == hard_iface->net_dev)
+ continue;
+
/* send a copy of the saved skb */
skb1 = skb_clone(forw_packet->skb, GFP_ATOMIC);
if (skb1)
@@ -13,3 +13,13 @@ Description:
displays the batman mesh interface this <iface>
currently is associated with.
+What: /sys/class/net/<iface>/batman-adv/no_rebroadcast
+Date: Sep 2013
+Contact: Linus Lüssing <linus.luessing@web.de>
+Description:
+ With this option set incoming multicast payload frames on
+ <iface> are not being rebroadcasted on <iface> again. This
+ option should be set on links which are known to be transitive
+ and symmetric only, for instance point-to-point wifi longshots
+ or wired links. Using this option wrongly is going to
+ break your mesh network, use at your own risk!
@@ -110,6 +110,17 @@ struct batadv_attribute batadv_attr_vlan_##_name = { \
.store = _store, \
};
+/* Use this, if you have customized show and store functions
+ * for hard interface attrs
+ */
+#define BATADV_ATTR_HIF(_name, _mode, _show, _store) \
+struct batadv_attribute batadv_attr_hif_##_name = { \
+ .attr = {.name = __stringify(_name), \
+ .mode = _mode }, \
+ .show = _show, \
+ .store = _store, \
+};
+
/* Use this, if you have customized show and store functions */
#define BATADV_ATTR(_name, _mode, _show, _store) \
struct batadv_attribute batadv_attr_##_name = { \
@@ -215,6 +226,52 @@ ssize_t batadv_show_vlan_##_name(struct kobject *kobj, \
static BATADV_ATTR_VLAN(_name, _mode, batadv_show_vlan_##_name, \
batadv_store_vlan_##_name)
+#define BATADV_ATTR_HIF_STORE_BOOL(_name, _post_func) \
+ssize_t batadv_store_hif_##_name(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; \
+ size_t res; \
+ \
+ hard_iface = batadv_hardif_get_by_netdev(net_dev); \
+ if (!hard_iface) \
+ return 0; \
+ \
+ res = __batadv_store_bool_attr(buff, count, _post_func, \
+ attr, &hard_iface->_name, \
+ hard_iface->soft_iface); \
+ batadv_hardif_free_ref(hard_iface); \
+ return res; \
+}
+
+#define BATADV_ATTR_HIF_SHOW_BOOL(_name) \
+ssize_t batadv_show_hif_##_name(struct kobject *kobj, \
+ struct attribute *attr, char *buff) \
+{ \
+ struct net_device *net_dev = batadv_kobj_to_netdev(kobj); \
+ struct batadv_hard_iface *hard_iface; \
+ size_t res; \
+ \
+ hard_iface = batadv_hardif_get_by_netdev(net_dev); \
+ if (!hard_iface) \
+ return 0; \
+ \
+ res = sprintf(buff, "%s\n", \
+ atomic_read(&hard_iface->_name) == 0 ? \
+ "disabled" : "enabled"); \
+ batadv_hardif_free_ref(hard_iface); \
+ return res; \
+}
+
+/* Use this, if you are going to turn a [name] in the vlan struct on or off */
+#define BATADV_ATTR_HIF_BOOL(_name, _mode, _post_func) \
+ static BATADV_ATTR_HIF_STORE_BOOL(_name, _post_func) \
+ static BATADV_ATTR_HIF_SHOW_BOOL(_name) \
+ static BATADV_ATTR_HIF(_name, _mode, batadv_show_hif_##_name, \
+ batadv_store_hif_##_name)
+
static int batadv_store_bool_attr(char *buff, size_t count,
struct net_device *net_dev,
const char *attr_name, atomic_t *attr)
@@ -750,10 +807,12 @@ static ssize_t batadv_show_iface_status(struct kobject *kobj,
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);
+BATADV_ATTR_HIF_BOOL(no_rebroadcast, S_IRUGO | S_IWUSR, NULL);
static struct batadv_attribute *batadv_batman_attrs[] = {
&batadv_attr_mesh_iface,
&batadv_attr_iface_status,
+ &batadv_attr_hif_no_rebroadcast,
NULL,
};
@@ -76,6 +76,7 @@ struct batadv_hard_iface {
struct rcu_head rcu;
struct batadv_hard_iface_bat_iv bat_iv;
struct work_struct cleanup_work;
+ atomic_t no_rebroadcast;
};
/**