Skip to contentSkip to conversation

Basic Multicast Optimizations

B.A.T.M.A.N. Superseded Linus Lüssing 4 participants 10 messages Jun 14, 2013 – Jun 26, 2013 Kbuild

v6 of 14 Superseded Latest is v14 →

lore
Share & tools
lore

Linus Lüssing

author
#0 lore raw

This is the sixth revision of the basic multicast optimization patches.

It simplifies and speeds-up the new batadv_tt_global_hash_count():

* We now use batadv_tt_global_hash_find() instead of our custom look-up.
* A counter for the orig_list was added, so that we won't need to recount with every multicast packet.

Cheers, Linus

author
#1 lore raw

PATCH v6 1/3 batman-adv: Multicast Listener Announcements via Translation Table

With this patch a node which has no bridge interface on top of its soft interface announces its local multicast listeners via the translation table.

Signed-off-by Linus Lüssing <[email protected]>
---
 Makefile               |    2 +
 Makefile.kbuild        |    1 +
 compat.h               |   20 ++++-
 gen-compat-autoconf.sh |    1 +
 main.c                 |    6 ++
 main.h                 |    1 +
 multicast.c            |  210 ++++++++++++++++++++++++++++++++++++++++++++++++
 multicast.h            |   43 ++++++++++
 soft-interface.c       |    3 +
 sysfs.c                |    6 ++
 translation-table.c    |   22 ++++-
 types.h                |   12 +++
 12 files changed, 322 insertions(+), 5 deletions(-)
 create mode 100644 multicast.c
 create mode 100644 multicast.h
Makefile +2 -0
index 407cdc4..d7c6fa6 100644
@@ -27,6 +27,8 @@ export CONFIG_BATMAN_ADV_BLA=y
 export CONFIG_BATMAN_ADV_DAT=y
 # B.A.T.M.A.N network coding (catwoman):
 export CONFIG_BATMAN_ADV_NC=n
+# B.A.T.M.A.N. multicast optimizations:
+export CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS=y
 
 PWD:=$(shell pwd)
 KERNELPATH ?= /lib/modules/$(shell uname -r)/build
Makefile.kbuild +1 -0
index 4f4aabb..f154c5b 100644
@@ -38,3 +38,4 @@ batman-adv-y += send.o
 batman-adv-y += soft-interface.o
 batman-adv-y += sysfs.o
 batman-adv-y += translation-table.o
+batman-adv-$(CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS) += multicast.o
compat.h +19 -1
index dbf1926..ddc1b29 100644
@@ -103,13 +103,31 @@ static inline struct sk_buff *netdev_alloc_skb_ip_align(struct net_device *dev,
 
 #define pr_warn pr_warning
 
+#undef  netdev_for_each_mc_addr
+#define netdev_for_each_mc_addr(mclist, dev) \
+	for (mclist = (struct bat_dev_addr_list *)dev->mc_list; mclist; \
+	     mclist = (struct bat_dev_addr_list *)mclist->next)
+
+/* Note, that this breaks the usage of the normal 'struct netdev_hw_addr'
+ * for kernels < 2.6.35 in batman-adv!
+ */
+#define netdev_hw_addr batadv_dev_addr_list
+struct batadv_dev_addr_list {
+	struct dev_addr_list *next;
+	u8  addr[MAX_ADDR_LEN];
+	u8  da_addrlen;
+	u8  da_synced;
+	int da_users;
+	int da_gusers;
+};
+
 #endif /* < KERNEL_VERSION(2, 6, 35) */
 
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 36)
 
 #define __rcu
-#define IFF_BRIDGE_PORT  0 || (hard_iface->net_dev->br_port ? 1 : 0)
+#define IFF_BRIDGE_PORT  0 || (soft_iface->br_port ? 1 : 0)
 
 struct kernel_param_ops {
 	/* Returns 0, or -errno.  arg is in kp->arg. */
gen-compat-autoconf.sh +1 -0
index 78573e4..b56ac29 100755
@@ -39,6 +39,7 @@ gen_config() {
 gen_config 'CONFIG_BATMAN_ADV_DEBUG' ${CONFIG_BATMAN_ADV_DEBUG:="n"} >> "${TMP}"
 gen_config 'CONFIG_BATMAN_ADV_BLA' ${CONFIG_BATMAN_ADV_BLA:="y"} >> "${TMP}"
 gen_config 'CONFIG_BATMAN_ADV_DAT' ${CONFIG_BATMAN_ADV_DAT:="y"} >> "${TMP}"
+gen_config 'CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS' ${CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS:="y"} >> "${TMP}"
 gen_config 'CONFIG_BATMAN_ADV_NC' ${CONFIG_BATMAN_ADV_NC:="n"} >> "${TMP}"
 
 # only regenerate compat-autoconf.h when config was changed
main.c +6 -0
index f2f1137..f5aed17 100644
@@ -33,6 +33,7 @@
 #include "gateway_client.h"
 #include "bridge_loop_avoidance.h"
 #include "distributed-arp-table.h"
+#include "multicast.h"
 #include "gateway_common.h"
 #include "hash.h"
 #include "bat_algo.h"

@@ -116,6 +117,9 @@ int batadv_mesh_init(struct net_device *soft_iface)
 	INIT_LIST_HEAD(&bat_priv->tt.changes_list);
 	INIT_LIST_HEAD(&bat_priv->tt.req_list);
 	INIT_LIST_HEAD(&bat_priv->tt.roam_list);
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+	INIT_LIST_HEAD(&bat_priv->mcast.mla_list);
+#endif
 	INIT_HLIST_HEAD(&bat_priv->tvlv.container_list);
 	INIT_HLIST_HEAD(&bat_priv->tvlv.handler_list);
 

@@ -169,6 +173,8 @@ void batadv_mesh_free(struct net_device *soft_iface)
 	batadv_dat_free(bat_priv);
 	batadv_bla_free(bat_priv);
 
+	batadv_mcast_free(bat_priv);
+
 	/* Free the TT and the originator tables only after having terminated
 	 * all the other depending components which may use these structures for
 	 * their purposes.
main.h +1 -0
index cfed2a3..ede23d5 100644
@@ -164,6 +164,7 @@ enum batadv_uev_type {
 #include <linux/percpu.h>
 #include <linux/slab.h>
 #include <net/sock.h>		/* struct sock */
+#include <net/addrconf.h>	/* ipv6 address stuff */
 #include <net/rtnetlink.h>
 #include <linux/jiffies.h>
 #include <linux/seq_file.h>
multicast.c new file +210 -0

210 lines not on this page · raw message

multicast.h new file +43 -0

43 lines not on this page · raw message

soft-interface.c +3 -0
index d897194..cf759fc 100644
@@ -471,6 +471,9 @@ static int batadv_softif_init_late(struct net_device *dev)
 #ifdef CONFIG_BATMAN_ADV_DAT
 	atomic_set(&bat_priv->distributed_arp_table, 1);
 #endif
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+	atomic_set(&bat_priv->mcast_group_awareness, 1);
+#endif
 	atomic_set(&bat_priv->ap_isolation, 0);
 	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
 	atomic_set(&bat_priv->gw_sel_class, 20);
sysfs.c +6 -0
index b70ae52..cc4e7e5 100644
@@ -369,6 +369,9 @@ 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,
 		   batadv_store_gw_bwidth);
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+BATADV_ATTR_SIF_BOOL(mcast_group_awareness, S_IRUGO | S_IWUSR, NULL);
+#endif
 #ifdef CONFIG_BATMAN_ADV_DEBUG
 BATADV_ATTR_SIF_UINT(log_level, S_IRUGO | S_IWUSR, 0, BATADV_DBG_ALL, NULL);
 #endif

@@ -386,6 +389,9 @@ static struct batadv_attribute *batadv_mesh_attrs[] = {
 #ifdef CONFIG_BATMAN_ADV_DAT
 	&batadv_attr_distributed_arp_table,
 #endif
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+	&batadv_attr_mcast_group_awareness,
+#endif
 	&batadv_attr_fragmentation,
 	&batadv_attr_ap_isolation,
 	&batadv_attr_routing_algo,
translation-table.c +18 -4
index 8404eef..18a2051 100644
@@ -26,6 +26,7 @@
 #include "originator.h"
 #include "routing.h"
 #include "bridge_loop_avoidance.h"
+#include "multicast.h"
 
 #include <linux/crc32c.h>
 

@@ -351,14 +352,16 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 {
 	struct batadv_priv *bat_priv = netdev_priv(soft_iface);
 	struct batadv_tt_local_entry *tt_local;
-	struct batadv_tt_global_entry *tt_global;
+	struct batadv_tt_global_entry *tt_global = NULL;
 	struct hlist_head *head;
 	struct batadv_tt_orig_list_entry *orig_entry;
 	int hash_added;
 	bool roamed_back = false;
 
 	tt_local = batadv_tt_local_hash_find(bat_priv, addr, vid);
-	tt_global = batadv_tt_global_hash_find(bat_priv, addr, vid);
+
+	if (!is_multicast_ether_addr(addr))
+		tt_global = batadv_tt_global_hash_find(bat_priv, addr, vid);
 
 	if (tt_local) {
 		tt_local->last_seen = jiffies;

@@ -412,8 +415,11 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 	tt_local->last_seen = jiffies;
 	tt_local->common.added_at = tt_local->last_seen;
 
-	/* the batman interface mac address should never be purged */
-	if (batadv_compare_eth(addr, soft_iface->dev_addr))
+	/* the batman interface mac and multicast addresses should never be
+	 * purged
+	 */
+	if (batadv_compare_eth(addr, soft_iface->dev_addr) ||
+	    is_multicast_ether_addr(addr))
 		tt_local->common.flags |= BATADV_TT_CLIENT_NOPURGE;
 
 	hash_added = batadv_hash_add(bat_priv->tt.local_hash, batadv_compare_tt,

@@ -1007,6 +1013,11 @@ add_orig_entry:
 	ret = true;
 
 out_remove:
+	/* Do not remove multicast addresses from the local hash on
+	 * global additions
+	 */
+	if (is_multicast_ether_addr(tt_addr))
+		goto out;
 
 	/* remove address from local hash if present */
 	local_flags = batadv_tt_local_remove(bat_priv, tt_addr, vid,

@@ -2521,6 +2532,9 @@ void batadv_tt_local_commit_changes(struct batadv_priv *bat_priv)
 {
 	uint16_t changed_num = 0;
 
+	/* Update multicast addresses in local translation table */
+	batadv_mcast_mla_tt_update(bat_priv);
+
 	if (atomic_read(&bat_priv->tt.local_changes) < 1) {
 		if (!batadv_atomic_dec_not_zero(&bat_priv->tt.ogm_append_cnt))
 			batadv_tt_tvlv_container_update(bat_priv);
types.h +12 -0
index 20a1bef..8d57426 100644
@@ -501,6 +501,12 @@ struct batadv_priv_dat {
 };
 #endif
 
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+struct batadv_priv_mcast {
+	struct list_head mla_list;
+};
+#endif
+
 /**
  * struct batadv_priv_nc - per mesh interface network coding private data
  * @work: work queue callback item for cleanup

@@ -591,6 +597,9 @@ struct batadv_priv {
 #ifdef CONFIG_BATMAN_ADV_DAT
 	atomic_t distributed_arp_table;
 #endif
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+	atomic_t mcast_group_awareness;
+#endif
 	atomic_t gw_mode;
 	atomic_t gw_sel_class;
 	atomic_t orig_interval;

@@ -625,6 +634,9 @@ struct batadv_priv {
 #ifdef CONFIG_BATMAN_ADV_DAT
 	struct batadv_priv_dat dat;
 #endif
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+	struct batadv_priv_mcast mcast;
+#endif
 #ifdef CONFIG_BATMAN_ADV_NC
 	atomic_t network_coding;
 	struct batadv_priv_nc nc;
Signature · 1 line
1.7.10.4
#2 lore raw

Re: [B.A.T.M.A.N.] [PATCHv6 1/3] batman-adv: Multicast Listener Announcements via Translation Table

Quoted 28 lines · On Saturday, June 15, 2013 01:50:07 Linus Lüssing wrote:

With this patch a node which has no bridge interface on top of its soft interface announces its local multicast listeners via the translation table.

Signed-off-by Linus Lüssing <[email protected]>
---
 Makefile               |    2 +
 Makefile.kbuild        |    1 +
 compat.h               |   20 ++++-
 gen-compat-autoconf.sh |    1 +
 main.c                 |    6 ++
 main.h                 |    1 +
 multicast.c            |  210
++++++++++++++++++++++++++++++++++++++++++++++++ multicast.h            | 
 43 ++++++++++
 soft-interface.c       |    3 +
 sysfs.c                |    6 ++
 translation-table.c    |   22 ++++-
 types.h                |   12 +++
 12 files changed, 322 insertions(+), 5 deletions(-)
 create mode 100644 multicast.c
 create mode 100644 multicast.h
Makefile +0 -0
index 407cdc4..d7c6fa6 100644

Makefile export CONFIG_BATMAN_ADV_BLA=y · line 31

Quoted 5 lines
Makefile +2 -0
@@ -27,6 +27,8 @@ export CONFIG_BATMAN_ADV_BLA=y
 export CONFIG_BATMAN_ADV_DAT=y
 # B.A.T.M.A.N network coding (catwoman):
 export CONFIG_BATMAN_ADV_NC=n
+# B.A.T.M.A.N. multicast optimizations:
+export CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS=y

Can we please find a shorter define ? How about "CONFIG_BATMAN_ADV_MCAST" ? That would be in line with the rest.

Makefile export CONFIG_BATMAN_ADV_BLA=y

Quoted 4 lines
Makefile +4 -0
+struct batadv_hw_addr {
+	struct list_head	list;
+	unsigned char addr[ETH_ALEN];
+};

All struct defines should go into types.h or packet.h if they are sent over the wire.

Quoted 33 lines
Makefile +33 -0
+/**
+ * batadv_mcast_mla_tt_update - update the own MLAs
+ * @bat_priv: the bat priv with all the soft interface information
+ *
+ * Update the own multicast listener announcements in the translation
+ * table. Also take care of registering or unregistering the multicast
+ * tvlv depending on whether the user activated or deactivated
+ * multicast optimizations.
+ */
+void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
+{
+	struct net_device *soft_iface = bat_priv->soft_iface;
+	struct list_head mcast_list;
+	int ret;
+	static bool enabled;
+
+	INIT_LIST_HEAD(&mcast_list);
+
+	/* Avoid attaching MLAs, if multicast optimization is disabled
+	 * or there is a bridge on top of our soft interface (TODO)
+	 */
+	if (!atomic_read(&bat_priv->mcast_group_awareness) ||
+	    bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT) {
+		if (enabled)
+			enabled = false;
+
+		goto update;
+	}
+
+	if (!enabled)
+		enabled = true;
+
+	ret = batadv_mcast_mla_local_collect(soft_iface, &mcast_list);

Makefile

Quoted 10 lines
Makefile +10 -0
+	if (ret < 0)
+		goto out;
+
+update:
+	batadv_mcast_mla_tt_clean(bat_priv, &mcast_list);
+	batadv_mcast_mla_tt_add(bat_priv, &mcast_list);
+
+out:
+	batadv_mcast_mla_collect_free(&mcast_list);
+}

You can use the LIST_HEAD static initializer.
The "enabled" variable does not make any sense.

How about we change some function names for the sake of clarity ? Here my suggestions:

* batadv_mcast_mla_local_collect() => batadv_mcast_mla_softif_retrieve() or
batadv_mcast_mla_softif_get()
* batadv_mcast_mla_tt_clean() => batadv_mcast_mla_tt_retract()
* batadv_mcast_mla_collect_free() => batadv_mcast_mla_list_free()

Makefile

Quoted 5 lines
Makefile +5 -0
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+struct batadv_priv_mcast {
+	struct list_head mla_list;
+};
+#endif

I don't see a good reason to use a double linked list or is there one ?

Cheers,
Marek

author
#3 lore raw

Re: [B.A.T.M.A.N.] [PATCHv6 1/3] batman-adv: Multicast Listener Announcements via Translation Table

Hi Marec,

Quoted 24 lines · Marek Lindner
Quoted 23 lines · On Saturday, June 15, 2013 01:50:07 Linus Lüssing wrote:

With this patch a node which has no bridge interface on top of its soft interface announces its local multicast listeners via the translation table.

Signed-off-by Linus Lüssing <[email protected]>
---
 Makefile               |    2 +
 Makefile.kbuild        |    1 +
 compat.h               |   20 ++++-
 gen-compat-autoconf.sh |    1 +
 main.c                 |    6 ++
 main.h                 |    1 +
 multicast.c            |  210
++++++++++++++++++++++++++++++++++++++++++++++++ multicast.h            | 
 43 ++++++++++
 soft-interface.c       |    3 +
 sysfs.c                |    6 ++
 translation-table.c    |   22 ++++-
 types.h                |   12 +++
 12 files changed, 322 insertions(+), 5 deletions(-)
 create mode 100644 multicast.c
 create mode 100644 multicast.h
Quoted 13 lines

Makefile export CONFIG_BATMAN_ADV_BLA=y · line 31

Quoted 10 lines
Makefile +2 -0
index 407cdc4..d7c6fa6 100644
@@ -27,6 +27,8 @@ export CONFIG_BATMAN_ADV_BLA=y
 export CONFIG_BATMAN_ADV_DAT=y
 # B.A.T.M.A.N network coding (catwoman):
 export CONFIG_BATMAN_ADV_NC=n
+# B.A.T.M.A.N. multicast optimizations:
+export CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS=y

Can we please find a shorter define ? How about "CONFIG_BATMAN_ADV_MCAST" ? That would be in line with the rest.

Hm, yes I also don't like the length of that so much. But I had dismissed "CONFIG_BATMAN_ADV_MCAST" so far because I thought it might seem as if batman-adv were not able to handle multicast at all without that option.

What about CONFIG_BATMAN_ADV_MCAST_OPT (hm, that could suggest 'optional', too...) or CONFIG_BATMAN_ADV_MCO?

.oO(also don't sound as nice as the other CONFIG_ exports...)

Ok, maybe CONFIG_BATMAN_ADV_MCAST is still the best option. I'll change it to that.

Quoted 9 lines

Makefile export CONFIG_BATMAN_ADV_BLA=y

Quoted 6 lines
Makefile +4 -0
+struct batadv_hw_addr {
+	struct list_head	list;
+	unsigned char addr[ETH_ALEN];
+};

All struct defines should go into types.h or packet.h if they are sent over the wire.

This struct isn't sent over the wire, it's just for local book keeping. I was thinking about naming it 'struct batadv_mcast_hw_addr' but didn't do that because it is so generic and thought maybe because of that it might be reusable in the future.

Hm, maybe it's better to rename it to 'struct batadv_mcast_hw_addr' anyways and leave it in that place to avoid bloating types.h/packet.h with "unimportant" structs and defines?

Quoted 32 lines
Quoted 32 lines
Makefile +30 -0
+/**
+ * batadv_mcast_mla_tt_update - update the own MLAs
+ * @bat_priv: the bat priv with all the soft interface information
+ *
+ * Update the own multicast listener announcements in the translation
+ * table. Also take care of registering or unregistering the multicast
+ * tvlv depending on whether the user activated or deactivated
+ * multicast optimizations.
+ */
+void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
+{
+	struct net_device *soft_iface = bat_priv->soft_iface;
+	struct list_head mcast_list;
+	int ret;
+	static bool enabled;
+
+	INIT_LIST_HEAD(&mcast_list);
+
+	/* Avoid attaching MLAs, if multicast optimization is disabled
+	 * or there is a bridge on top of our soft interface (TODO)
+	 */
+	if (!atomic_read(&bat_priv->mcast_group_awareness) ||
+	    bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT) {
+		if (enabled)
+			enabled = false;
+
+		goto update;
+	}
+
+	if (!enabled)
Quoted 8 lines

Makefile

Quoted 8 lines
Makefile +8 -0
+		enabled = true;
+
+	ret = batadv_mcast_mla_local_collect(soft_iface, &mcast_list);
+	if (ret < 0)
+		goto out;
+
+update:
+	batadv_mcast_mla_tt_clean(bat_priv, &mcast_list);

a> > + batadv_mcast_mla_tt_add(bat_priv, &mcast_list);

Quoted 6 lines

Makefile

Quoted 4 lines
Makefile +4 -0
+
+out:
+	batadv_mcast_mla_collect_free(&mcast_list);
+}

You can use the LIST_HEAD static initializer.

Ah, indeed, will change that.

Quoted 1 line

The "enabled" variable does not make any sense.

Woops, indeed, makes more sense in [PATCH 2/3], I'll move it there.

Quoted 6 lines

How about we change some function names for the sake of clarity ? Here my suggestions:

* batadv_mcast_mla_local_collect() => batadv_mcast_mla_softif_retrieve() or
batadv_mcast_mla_softif_get()

Sounds good, I like the latter very much (I was using the terms local, bridge, global - but yes, our own bridge is also kind of local, so softif is better :) ).

Quoted 1 line

* batadv_mcast_mla_tt_clean() => batadv_mcast_mla_tt_retract()

Hm, I think I'd find that a little confusing when reading a batadv_mcast_mla_tt_retract(bat_priv, mcast_list) in the code somewhere because I'd think that the things provided by mcast_list were going to get retracted. batadv_mcast_mla_tt_clean(bat_priv, mcast_list) would feel more like dusting the table while the given argument provides the list of valuable items on the table which you are taking special care of, which you don't want to accidentally knock over :) (in my opinion).

Quoted 1 line

* batadv_mcast_mla_collect_free() => batadv_mcast_mla_list_free()

Sounds good!

Quoted 9 lines

Makefile

Quoted 7 lines
Makefile +5 -0
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+struct batadv_priv_mcast {
+	struct list_head mla_list;
+};
+#endif

I don't see a good reason to use a double linked list or is there one ?

No, you're right, currently we don't need a double linked list head. I'll change it to an HLIST.

Thanks for all the feedback so far!

Quoted 3 lines

Cheers,
Marek

Cheers, Linus

#4 lore raw

Re: [B.A.T.M.A.N.] [PATCHv6 1/3] batman-adv: Multicast Listener Announcements via Translation Table

Quoted 17 lines · Linus Lüssing
Quoted 11 lines

Makefile export CONFIG_BATMAN_ADV_BLA=y · line 31

Quoted 8 lines
Makefile +2 -0
@@ -27,6 +27,8 @@ export CONFIG_BATMAN_ADV_BLA=y
 export CONFIG_BATMAN_ADV_DAT=y
 # B.A.T.M.A.N network coding (catwoman):
 export CONFIG_BATMAN_ADV_NC=n
+# B.A.T.M.A.N. multicast optimizations:
+export CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS=y

Can we please find a shorter define ? How about "CONFIG_BATMAN_ADV_MCAST" ? That would be in line with the rest.

Hm, yes I also don't like the length of that so much. But I had dismissed "CONFIG_BATMAN_ADV_MCAST" so far because I thought it might seem as if batman-adv were not able to handle multicast at all without that option.

Quoted 7 lines

What about CONFIG_BATMAN_ADV_MCAST_OPT (hm, that could suggest 'optional', too...) or CONFIG_BATMAN_ADV_MCO?

.oO(also don't sound as nice as the other CONFIG_ exports...)

Ok, maybe CONFIG_BATMAN_ADV_MCAST is still the best option. I'll change it to that.

I agree with using the CONFIG_BATMAN_ADV_MCAST too.

Quoted 11 lines
Quoted 10 lines

Makefile export CONFIG_BATMAN_ADV_BLA=y

Quoted 7 lines
Makefile +4 -0
+struct batadv_hw_addr {
+	struct list_head	list;
+	unsigned char addr[ETH_ALEN];
+};

All struct defines should go into types.h or packet.h if they are sent over the wire.

Quoted 8 lines

This struct isn't sent over the wire, it's just for local book keeping. I was thinking about naming it 'struct batadv_mcast_hw_addr' but didn't do that because it is so generic and thought maybe because of that it might be reusable in the future.

Hm, maybe it's better to rename it to 'struct batadv_mcast_hw_addr' anyways and leave it in that place to avoid bloating types.h/packet.h with "unimportant" structs and defines?

I think this struct should go into type.h too. Many structs in there are used by a single component only, but it is better to keep them into the same file. And yes, renaming to batadv_mcast_hw_addr is probably better (given that it is used by the mcast component only).

Cheers,

Signature · 4 lines
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara
author
#5 lore raw

Re: [B.A.T.M.A.N.] [PATCHv6 1/3] batman-adv: Multicast Listener Announcements via Translation Table

Quoted 2 lines · Antonio Quartulli
Quoted 2 lines
Quoted 2 lines

All struct defines should go into types.h or packet.h if they are sent over the wire.

I just noticed that I had misread this sentence as: "All struct defines which are sent over the wire go into types.h or packet.h"

Quoted 6 lines
Quoted 6 lines

This struct isn't sent over the wire, it's just for local book keeping. I was thinking about naming it 'struct batadv_mcast_hw_addr' but didn't do that because it is so generic and thought maybe because of that it might be reusable in the future.

Quoted 8 lines
Quoted 3 lines

Hm, maybe it's better to rename it to 'struct batadv_mcast_hw_addr' anyways and leave it in that place to avoid bloating types.h/packet.h with "unimportant" structs and defines?

I think this struct should go into type.h too. Many structs in there are used by a single component only, but it is better to keep them into the same file. And yes, renaming to batadv_mcast_hw_addr is probably better (given that it is used by the mcast component only).

And now this makes sense :). I'm going to move it into types.h (and will keep the name as batadv_hw_addr).

Quoted 8 lines

Cheers,

Signature · 4 lines
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara
author
#6 lore raw

PATCH v6 2/3 batman-adv: Announce new capability via multicast TVLV

In reply to Linus Lüssing

If the soft interface of a node is not part of a bridge then a node announces a new multicast TVLV: The according flag (BATADV_MCAST_LISTENER_ANNOUNCEMENT) signalizes that this node is announcing all of its multicast listeners via the translation table infrastructure. More precisely, all multicast listeners of scope greater than link-local for IPv4 and of scope greater
or equal to link-local for IPv6.

Signed-off-by Linus Lüssing <[email protected]>
---
 main.c           |    4 ++++
 multicast.c      |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 multicast.h      |   14 +++++++++++
 originator.c     |    6 +++++
 packet.h         |    7 ++++++
 soft-interface.c |    1 +
 types.h          |    4 ++++
 7 files changed, 102 insertions(+), 2 deletions(-)
main.c +4 -0
index f5aed17..3b92ba2 100644
@@ -146,6 +146,10 @@ int batadv_mesh_init(struct net_device *soft_iface)
 	if (ret < 0)
 		goto err;
 
+	ret = batadv_mcast_init(bat_priv);
+	if (ret < 0)
+		goto err;
+
 	ret = batadv_gw_init(bat_priv);
 	if (ret < 0)
 		goto err;
multicast.c +66 -2

96 lines not on this page · raw message

multicast.h +14 -0
index 3b68e3b..aa58e4b 100644
@@ -24,8 +24,12 @@
 
 void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv);
 
+int batadv_mcast_init(struct batadv_priv *bat_priv);
+
 void batadv_mcast_free(struct batadv_priv *bat_priv);
 
+void batadv_mcast_purge_orig(struct batadv_orig_node *orig_node);
+
 #else
 
 static inline void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)

@@ -33,11 +37,21 @@ static inline void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
 	return;
 }
 
+static inline int batadv_mcast_init(struct batadv_priv *bat_priv)
+{
+	return 0;
+}
+
 static inline void batadv_mcast_free(struct batadv_priv *bat_priv)
 {
 	return;
 }
 
+static inline void batadv_mcast_purge_orig(struct batadv_orig_node *orig_node)
+{
+	return;
+}
+
 #endif /* CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS */
 
 #endif /* _NET_BATMAN_ADV_MULTICAST_H_ */
originator.c +6 -0
index a591dc5..5e827fa 100644
@@ -29,6 +29,7 @@
 #include "bridge_loop_avoidance.h"
 #include "network-coding.h"
 #include "fragmentation.h"
+#include "multicast.h"
 
 /* hash class keys */
 static struct lock_class_key batadv_orig_hash_lock_class_key;

@@ -143,6 +144,8 @@ static void batadv_orig_node_free_rcu(struct rcu_head *rcu)
 
 	spin_unlock_bh(&orig_node->neigh_list_lock);
 
+	batadv_mcast_purge_orig(orig_node);
+
 	/* Free nc_nodes */
 	batadv_nc_purge_orig(orig_node->bat_priv, orig_node, NULL);
 

@@ -258,6 +261,9 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
 	reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
 	orig_node->bcast_seqno_reset = reset_time;
 	orig_node->batman_seqno_reset = reset_time;
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+	orig_node->mcast_flags = BATADV_MCAST_LISTENER_ANNOUNCEMENT;
+#endif
 
 	atomic_set(&orig_node->bond_candidates, 0);
packet.h +7 -0
index 08e175a..1090977 100644
@@ -91,6 +91,11 @@ enum batadv_icmp_packettype {
 	BATADV_PARAMETER_PROBLEM       = 12,
 };
 
+/* multicast capabilities */
+enum batadv_mcast_flags {
+	BATADV_MCAST_LISTENER_ANNOUNCEMENT = BIT(0),
+};
+
 /* tt data subtypes */
 #define BATADV_TT_DATA_TYPE_MASK 0x0F
 

@@ -145,6 +150,7 @@ enum batadv_bla_claimframe {
  * @BATADV_TVLV_NC: network coding tvlv
  * @BATADV_TVLV_TT: translation table tvlv
  * @BATADV_TVLV_ROAM: roaming advertisement tvlv
+ * @BATADV_TVLV_MCAST: multicast capability tvlv
  */
 enum batadv_tvlv_type {
 	BATADV_TVLV_GW		= 0x01,

@@ -152,6 +158,7 @@ enum batadv_tvlv_type {
 	BATADV_TVLV_NC		= 0x03,
 	BATADV_TVLV_TT		= 0x04,
 	BATADV_TVLV_ROAM	= 0x05,
+	BATADV_TVLV_MCAST	= 0x06,
 };
 
 /* the destination hardware field in the ARP frame is used to
soft-interface.c +1 -0
index cf759fc..90d06c5 100644
@@ -473,6 +473,7 @@ static int batadv_softif_init_late(struct net_device *dev)
 #endif
 #ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
 	atomic_set(&bat_priv->mcast_group_awareness, 1);
+	atomic_set(&bat_priv->mcast.num_no_mla, 0);
 #endif
 	atomic_set(&bat_priv->ap_isolation, 0);
 	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
types.h +4 -0
index 8d57426..12d3ba7 100644
@@ -163,6 +163,9 @@ struct batadv_orig_node {
 	unsigned long last_seen;
 	unsigned long bcast_seqno_reset;
 	unsigned long batman_seqno_reset;
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+	uint8_t mcast_flags;
+#endif
 	uint8_t capabilities;
 	atomic_t last_ttvn;
 	uint32_t tt_crc;

@@ -504,6 +507,7 @@ struct batadv_priv_dat {
 #ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
 struct batadv_priv_mcast {
 	struct list_head mla_list;
+	atomic_t num_no_mla;
 };
 #endif
Signature · 1 line
1.7.10.4
author
#7 lore raw

PATCH v6 3/3 batman-adv: Modified forwarding behaviour for multicast packets

In reply to Linus Lüssing

With this patch a multicast packet is not always simply flooded anymore, the bevahiour for the following cases is changed to reduce unnecessary overhead:

If all nodes within the horizon of a certain node have signalized multicast listener announcement capability (BATADV_MCAST_LISTENER_ANNOUNCEMENT) then an IPv6 multicast packet with a destination of IPv6 link-local scope coming from the upstream of this node...

* ...is dropped if there is no according multicast listener in the translation table,
* ...is forwarded via unicast if there is a single node with interested multicast listeners
* ...and otherwise still gets flooded.

Signed-off-by Linus Lüssing <[email protected]>
---
 distributed-arp-table.c |    2 +-
 multicast.c             |   56 ++++++++++++++++++++++++++++++++++
 multicast.h             |   23 ++++++++++++++
 send.c                  |    5 ++--
 send.h                  |    9 +++---
 soft-interface.c        |   18 +++++++++--
 translation-table.c     |   76 +++++++++++++++++++++++++++++++++++++----------
 translation-table.h     |    2 ++
 types.h                 |    3 +-
 9 files changed, 168 insertions(+), 26 deletions(-)
distributed-arp-table.c +1 -1
index f2543c2..87248bb 100644
@@ -1030,7 +1030,7 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
 						    BATADV_P_DAT_CACHE_REPLY,
 						    vid);
 	else
-		err = batadv_send_skb_unicast(bat_priv, skb_new, vid);
+		err = batadv_send_skb_unicast(bat_priv, skb_new, vid, false);
 
 	if (!err) {
 		batadv_inc_counter(bat_priv, BATADV_CNT_DAT_CACHED_REPLY_TX);
multicast.c +56 -0

68 lines not on this page · raw message

multicast.h +23 -0
index aa58e4b..7b291d5 100644
@@ -20,10 +20,27 @@
 #ifndef _NET_BATMAN_ADV_MULTICAST_H_
 #define _NET_BATMAN_ADV_MULTICAST_H_
 
+/**
+ * batadv_forw_mode - the way a packet should be forwarded as
+ * @BATADV_FORW_ALL: forward the packet to all nodes
+ *  (currently via classic flooding)
+ * @BATADV_FORW_SINGLE: forward the packet to a single node
+ *  (currently via the BATMAN_IV unicast routing protocol)
+ * @BATADV_FORW_NONE: don't forward, drop it
+ */
+enum batadv_forw_mode {
+	BATADV_FORW_ALL,
+	BATADV_FORW_SINGLE,
+	BATADV_FORW_NONE,
+};
+
 #ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
 
 void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv);
 
+enum batadv_forw_mode
+batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv);
+
 int batadv_mcast_init(struct batadv_priv *bat_priv);
 
 void batadv_mcast_free(struct batadv_priv *bat_priv);

@@ -37,6 +54,12 @@ static inline void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
 	return;
 }
 
+static inline enum batadv_forw_mode
+batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
+{
+	return BATADV_FORW_ALL;
+}
+
 static inline int batadv_mcast_init(struct batadv_priv *bat_priv)
 {
 	return 0;
send.c +3 -2
index b631355..68f65f1 100644
@@ -242,13 +242,14 @@ out:
  * @packet_subtype: the unicast 4addr packet subtype (only relevant for unicast
  *  4addr packets)
  * @vid: the vid to be used to search the translation table
+ * @forw_to_gw: whether to forward to a gateway (or via TT otherwise)
  *
  * Returns 1 in case of error or 0 otherwise.
  */
 int batadv_send_skb_generic_unicast(struct batadv_priv *bat_priv,
 				    struct sk_buff *skb, int packet_type,
 				    int packet_subtype,
-				    unsigned short vid)
+				    unsigned short vid, bool forw_to_gw)
 {
 	struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
 	struct batadv_unicast_packet *unicast_packet;

@@ -256,7 +257,7 @@ int batadv_send_skb_generic_unicast(struct batadv_priv *bat_priv,
 	int ret = NET_RX_DROP;
 
 	/* get routing information */
-	if (is_multicast_ether_addr(ethhdr->h_dest))
+	if (forw_to_gw)
 		orig_node = batadv_gw_get_selected_orig(bat_priv);
 	else
 		/* check for tt host - increases orig_node refcount.
send.h +5 -4
index c030cb7..642f9fc 100644
@@ -41,22 +41,23 @@ bool batadv_send_skb_prepare_unicast_4addr(struct batadv_priv *bat_priv,
 int batadv_send_skb_generic_unicast(struct batadv_priv *bat_priv,
 				    struct sk_buff *skb, int packet_type,
 				    int packet_subtype,
-				    unsigned short vid);
+				    unsigned short vid, bool forw_to_gw);
 
 /**
  * batadv_send_unicast_skb - send the skb encapsulated in a unicast packet
  * @bat_priv: the bat priv with all the soft interface information
  * @skb: the payload to send
  * @vid: the vid to be used to search the translation table
+ * @forw_to_gw: whether to forward to a gateway (or via TT otherwise)
  *
  * Returns 1 in case of error or 0 otherwise.
  */
 static inline int batadv_send_skb_unicast(struct batadv_priv *bat_priv,
 					  struct sk_buff *skb,
-					  unsigned short vid)
+					  unsigned short vid, bool forw_to_gw)
 {
 	return batadv_send_skb_generic_unicast(bat_priv, skb, BATADV_UNICAST,
-					       0, vid);
+					       0, vid, forw_to_gw);
 }
 
 /**

@@ -76,7 +77,7 @@ static inline int batadv_send_skb_unicast_4addr(struct batadv_priv *bat_priv,
 {
 	return batadv_send_skb_generic_unicast(bat_priv, skb,
 					       BATADV_UNICAST_4ADDR,
-					       packet_subtype, vid);
+					       packet_subtype, vid, false);
 }
 
 #endif /* _NET_BATMAN_ADV_SEND_H_ */
soft-interface.c +16 -2
index 90d06c5..d91526f 100644
@@ -34,6 +34,7 @@
 #include <linux/ethtool.h>
 #include <linux/etherdevice.h>
 #include <linux/if_vlan.h>
+#include "multicast.h"
 #include "bridge_loop_avoidance.h"
 #include "network-coding.h"
 

@@ -169,6 +170,8 @@ static int batadv_interface_tx(struct sk_buff *skb,
 	bool do_bcast = false;
 	unsigned short vid;
 	uint32_t seqno;
+	enum batadv_forw_mode mode;
+	bool forw_to_gw = false;
 
 	if (atomic_read(&bat_priv->mesh_state) != BATADV_MESH_ACTIVE)
 		goto dropped;

@@ -226,13 +229,24 @@ static int batadv_interface_tx(struct sk_buff *skb,
 			 * via unicast to their gateway
 			 */
 			ret = batadv_gw_is_dhcp_target(skb, &header_len);
-			if (ret)
+			if (ret) {
 				do_bcast = false;
+				forw_to_gw = true;
+			}
 			break;
 		case BATADV_GW_MODE_OFF:
 		default:
 			break;
 		}
+
+		if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
+			mode = batadv_mcast_forw_mode(skb, bat_priv);
+			if (mode == BATADV_FORW_NONE)
+				goto dropped;
+
+			if (mode == BATADV_FORW_SINGLE)
+				do_bcast = false;
+		}
 	}
 
 	/* ethernet packet should be broadcasted */

@@ -289,7 +303,7 @@ static int batadv_interface_tx(struct sk_buff *skb,
 
 		batadv_dat_snoop_outgoing_arp_reply(bat_priv, skb);
 
-		ret = batadv_send_skb_unicast(bat_priv, skb, vid);
+		ret = batadv_send_skb_unicast(bat_priv, skb, vid, forw_to_gw);
 		if (ret != 0)
 			goto dropped_freed;
 	}
translation-table.c +60 -16

135 lines not on this page · raw message

translation-table.h +2 -0
index 1d9506d..3916ca8 100644
@@ -31,6 +31,8 @@ int batadv_tt_global_seq_print_text(struct seq_file *seq, void *offset);
 void batadv_tt_global_del_orig(struct batadv_priv *bat_priv,
 			       struct batadv_orig_node *orig_node,
 			       const char *message);
+int batadv_tt_global_hash_count(struct batadv_priv *bat_priv,
+				const uint8_t *addr, unsigned short vid);
 struct batadv_orig_node *batadv_transtable_search(struct batadv_priv *bat_priv,
 						  const uint8_t *src,
 						  const uint8_t *addr,
types.h +2 -1
index 12d3ba7..f0f4a5c 100644
@@ -769,7 +769,8 @@ struct batadv_tt_local_entry {
 struct batadv_tt_global_entry {
 	struct batadv_tt_common_entry common;
 	struct hlist_head orig_list;
-	spinlock_t list_lock;	/* protects orig_list */
+	unsigned int orig_list_count;
+	spinlock_t list_lock;	/* protects orig_list and orig_list_count */
 	unsigned long roam_at;
 };
Signature · 1 line
1.7.10.4
#8 lore raw

Re: [B.A.T.M.A.N.] [PATCHv6 3/3] batman-adv: Modified forwarding behaviour for multicast packets

translation-table.c batadv_tt_global_del_orig_list(struct batadv_tt_global_entry *tt_global_entry) · line 1232

Quoted 9 lines · Linus Lüssing

static void

translation-table.c +4 -4
-batadv_tt_global_del_orig_entry(struct batadv_priv *bat_priv,
-				struct batadv_tt_global_entry *tt_global_entry,
-				struct batadv_orig_node *orig_node,
-				const char *message)
+batadv_tt_global_del_orig_node(struct batadv_priv *bat_priv,
+			       struct batadv_tt_global_entry *tt_global_entry,
+			       struct batadv_orig_node *orig_node,
+			       const char *message)

*cough*cough*kerneldoc*cough* :)

For all the remaining TT bits you have my ACK.

Cheers,

Signature · 4 lines
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara
#9 lore raw
In reply to Linus Lüssing

Looks good, Tested/Acked-by: Simon Wunderlich <[email protected]>

Thanks,
Simon

Quoted 9 lines · Linus Lüssing

This is the sixth revision of the basic multicast optimization patches.

It simplifies and speeds-up the new batadv_tt_global_hash_count():

* We now use batadv_tt_global_hash_find() instead of our custom look-up.
* A counter for the orig_list was added, so that we won't need to recount with every multicast packet.

Cheers, Linus