[B.A.T.M.A.N.] Basic Multicast Optimizations

[B.A.T.M.A.N.] Basic Multicast Optimizations

[Linus Lüssing]

This is the second revision of the basic multicast optimization patches.
It includes one functional fix and many style fixes, thanks to Antonios
feedback.

Three things were not included from these suggestions:

* The default mcast TVLV flags were left as it is not as easy to
switch. (see answer/explanation to [PATCH 2/3])
* for_each_pmc_rcu() was not moved to a commen net header file,
only a TODO was added. Will do that after this code might have
landed in net.


Additionally the limitation of 255 multicast listeners per originator
was removed (it was more a relic of the pre-TT announcement mechanism
and such limitations should be taken care of by the current TT
infrastructure instead).

Cheers, Linus

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

[Linus Lüssing]

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 <linus.luessing@web.de>
---
 Makefile               |    2 +
 Makefile.kbuild        |    1 +
 compat.c               |   21 +++++
 compat.h               |   18 ++++
 gen-compat-autoconf.sh |    1 +
 main.c                 |    6 ++
 main.h                 |    9 ++
 multicast.c            |  213 ++++++++++++++++++++++++++++++++++++++++++++++++
 multicast.h            |   43 ++++++++++
 soft-interface.c       |    3 +
 sysfs.c                |    6 ++
 translation-table.c    |   22 ++++-
 types.h                |   12 +++
 13 files changed, 353 insertions(+), 4 deletions(-)
 create mode 100644 multicast.c
 create mode 100644 multicast.h

diff --git a/Makefile b/Makefile
index 407cdc4..d7c6fa6 100644
--- a/Makefile
+++ b/Makefile
@@ -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
diff --git a/Makefile.kbuild b/Makefile.kbuild
index 8ddbfe6..d3efe3a 100644
--- a/Makefile.kbuild
+++ b/Makefile.kbuild
@@ -38,3 +38,4 @@ batman-adv-y += soft-interface.o
 batman-adv-y += sysfs.o
 batman-adv-y += translation-table.o
 batman-adv-y += unicast.o
+batman-adv-$(CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS) += multicast.o
diff --git a/compat.c b/compat.c
index 1f3a39d..27df197 100644
--- a/compat.c
+++ b/compat.c
@@ -25,6 +25,27 @@
 #include <linux/version.h>
 #include "main.h"
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 38)
+
+void batadv_for_each_pmc_rcu_init(struct in_device *in_dev,
+				  struct ip_mc_list **pmc)
+{
+	read_lock(&in_dev->mc_list_lock);
+	*pmc = in_dev->mc_list;
+}
+
+bool batadv_for_each_pmc_rcu_check(struct in_device *in_dev,
+				   struct ip_mc_list *pmc)
+{
+	if (pmc == NULL)
+		read_unlock(&in_dev->mc_list_lock);
+
+	return (pmc != NULL);
+}
+
+#endif /* < KERNEL_VERSION(2, 6, 38) */
+
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3, 0, 0)
 
 void batadv_free_rcu_gw_node(struct rcu_head *rcu)
diff --git a/compat.h b/compat.h
index 8bd4f62..de1556a 100644
--- a/compat.h
+++ b/compat.h
@@ -152,6 +152,24 @@ static inline int batadv_param_set_copystring(const char *val,
 
 #endif /* < KERNEL_VERSION(2, 6, 37) */
 
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 38)
+
+void batadv_for_each_pmc_rcu_init(struct in_device *in_dev,
+				  struct ip_mc_list **pmc);
+bool batadv_for_each_pmc_rcu_check(struct in_device *in_dev,
+				   struct ip_mc_list *pmc);
+
+#undef for_each_pmc_rcu
+#define for_each_pmc_rcu(in_dev, pmc)				\
+	for (batadv_for_each_pmc_rcu_init(in_dev, &pmc);	\
+	     batadv_for_each_pmc_rcu_check(in_dev, pmc);	\
+	     pmc = NULL)					\
+		for (; pmc != NULL; pmc = pmc->next)
+
+#endif /* < KERNEL_VERSION(2, 6, 38) */
+
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39)
 
 #define kstrtoul strict_strtoul
diff --git a/gen-compat-autoconf.sh b/gen-compat-autoconf.sh
index 78573e4..b56ac29 100755
--- a/gen-compat-autoconf.sh
+++ b/gen-compat-autoconf.sh
@@ -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
diff --git a/main.c b/main.c
index af89c09..1c82c18 100644
--- a/main.c
+++ b/main.c
@@ -33,6 +33,7 @@
 #include "bridge_loop_avoidance.h"
 #include "distributed-arp-table.h"
 #include "unicast.h"
+#include "multicast.h"
 #include "gateway_common.h"
 #include "hash.h"
 #include "bat_algo.h"
@@ -115,6 +116,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);
 
@@ -168,6 +172,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.
diff --git a/main.h b/main.h
index 795345f..c865a9c 100644
--- a/main.h
+++ b/main.h
@@ -141,6 +141,14 @@ enum batadv_uev_type {
 /* Append 'batman-adv: ' before kernel messages */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+/* identical to the one used in net/ipv4/igmp.c
+ * TODO: remove redundancy, put in some header file
+ */
+#define for_each_pmc_rcu(in_dev, pmc)				\
+	for (pmc = rcu_dereference(in_dev->mc_list);		\
+	     pmc != NULL;					\
+	     pmc = rcu_dereference(pmc->next_rcu))
+
 /* Kernel headers */
 
 #include <linux/mutex.h>	/* mutex */
@@ -155,6 +163,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>
diff --git a/multicast.c b/multicast.c
new file mode 100644
index 0000000..56a1128
--- /dev/null
+++ b/multicast.c
@@ -0,0 +1,213 @@
+/* Copyright (C) 2013 B.A.T.M.A.N. contributors:
+ *
+ * Linus Lüssing
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA
+ */
+
+#include "main.h"
+#include "originator.h"
+#include "hard-interface.h"
+#include "translation-table.h"
+
+/**
+ * batadv_mcast_mla_local_collect - collect local multicast listeners
+ * @dev: the device to collect multicast addresses from
+ * @mcast_list: a list to put found addresses into
+ *
+ * Collect multicast addresses of the local multicast listeners
+ * on the given interface, dev, in the given mcast_list.
+ *
+ * Return -ENOMEM on memory allocation error or the number of
+ * items added to the mcast_list otherwise.
+ */
+static int batadv_mcast_mla_local_collect(struct net_device *dev,
+					  struct list_head *mcast_list)
+{
+	struct netdev_hw_addr *mc_list_entry, *new;
+	int ret = 0;
+
+	netif_addr_lock_bh(dev);
+	netdev_for_each_mc_addr(mc_list_entry, dev) {
+		new = kmalloc(sizeof(*new), GFP_ATOMIC);
+		if (!new) {
+			ret = -ENOMEM;
+			break;
+		}
+
+		memcpy(&new->addr, &mc_list_entry->addr, ETH_ALEN);
+		list_add(&new->list, mcast_list);
+		ret++;
+	}
+	netif_addr_unlock_bh(dev);
+
+	return ret;
+}
+
+/**
+ * batadv_mcast_mla_is_duplicate - check whether an address is in a list
+ * @mcast_addr: the multicast address to check
+ * @mcast_list: the list with multicast addresses to search in
+ *
+ * Return true if the given address is already in the given list.
+ * Otherwise returns false.
+ */
+static bool batadv_mcast_mla_is_duplicate(uint8_t *mcast_addr,
+					  struct list_head *mcast_list)
+{
+	struct netdev_hw_addr *mcast_entry;
+
+	list_for_each_entry(mcast_entry, mcast_list, list)
+		if (batadv_compare_eth(mcast_entry->addr, mcast_addr))
+			return true;
+
+	return false;
+}
+
+/**
+ * batadv_mcast_mla_collect_free - free a list of multicast addresses
+ * @mcast_list: the list to free
+ *
+ * Remove and free all items in the given mcast_list.
+ */
+void batadv_mcast_mla_collect_free(struct list_head *mcast_list)
+{
+	struct netdev_hw_addr *mcast_entry, *tmp;
+
+	list_for_each_entry_safe(mcast_entry, tmp, mcast_list, list) {
+		list_del(&mcast_entry->list);
+		kfree(mcast_entry);
+	}
+}
+
+/**
+ * batadv_mcast_mla_tt_clean - clean up multicast listener announcements
+ * @bat_priv: the bat priv with all the soft interface information
+ * @mcast_list: a list of addresses which should _not_ be removed
+ *
+ * Retract the announcement of any multicast listener from the
+ * translation table except the ones listed in the given mcast_list.
+ *
+ * If mcast_list is NULL then all are retracted.
+ */
+static void batadv_mcast_mla_tt_clean(struct batadv_priv *bat_priv,
+				      struct list_head *mcast_list)
+{
+	struct netdev_hw_addr *mcast_entry, *tmp;
+
+	list_for_each_entry_safe(mcast_entry, tmp, &bat_priv->mcast.mla_list,
+				 list) {
+		if (mcast_list &&
+		    batadv_mcast_mla_is_duplicate(mcast_entry->addr,
+						  mcast_list))
+			continue;
+
+		batadv_tt_local_remove(bat_priv, mcast_entry->addr,
+				       "mcast TT outdated", false);
+
+		list_del(&mcast_entry->list);
+		kfree(mcast_entry);
+	}
+}
+
+/**
+ * batadv_mcast_mla_tt_add - add multicast listener announcements
+ * @bat_priv: the bat priv with all the soft interface information
+ * @mcast_list: a list of addresses which are going to get added
+ *
+ * Add multicast listener announcements from the given mcast_list to the
+ * translation table if they have not been added yet.
+ */
+static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv,
+				    struct list_head *mcast_list)
+{
+	struct netdev_hw_addr *mcast_entry, *tmp;
+
+	if (!mcast_list)
+		return;
+
+	list_for_each_entry_safe(mcast_entry, tmp, mcast_list, list) {
+		if (batadv_mcast_mla_is_duplicate(mcast_entry->addr,
+						  &bat_priv->mcast.mla_list))
+			continue;
+
+		batadv_tt_local_add(bat_priv->soft_iface, mcast_entry->addr,
+				    BATADV_NULL_IFINDEX);
+		list_move_tail(&mcast_entry->list, &bat_priv->mcast.mla_list);
+	}
+}
+
+/**
+ * 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 batadv_hard_iface *primary_if;
+	struct net_device *soft_iface;
+	struct list_head mcast_list;
+	int ret;
+	static bool enabled;
+
+	INIT_LIST_HEAD(&mcast_list);
+
+	primary_if = batadv_primary_if_get_selected(bat_priv);
+	if (!primary_if)
+		goto out;
+
+	soft_iface = primary_if->soft_iface;
+
+	/* 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);
+	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:
+	if (primary_if)
+		batadv_hardif_free_ref(primary_if);
+
+	batadv_mcast_mla_collect_free(&mcast_list);
+}
+
+/**
+ * batadv_mcast_free - free the multicast optimizations structures
+ * @bat_priv: the bat priv with all the soft interface information
+ */
+void batadv_mcast_free(struct batadv_priv *bat_priv)
+{
+	batadv_mcast_mla_tt_clean(bat_priv, NULL);
+}
diff --git a/multicast.h b/multicast.h
new file mode 100644
index 0000000..3b68e3b
--- /dev/null
+++ b/multicast.h
@@ -0,0 +1,43 @@
+/* Copyright (C) 2013 B.A.T.M.A.N. contributors:
+ *
+ * Linus Lüssing
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA
+ */
+
+#ifndef _NET_BATMAN_ADV_MULTICAST_H_
+#define _NET_BATMAN_ADV_MULTICAST_H_
+
+#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
+
+void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv);
+
+void batadv_mcast_free(struct batadv_priv *bat_priv);
+
+#else
+
+static inline void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
+{
+	return;
+}
+
+static inline void batadv_mcast_free(struct batadv_priv *bat_priv)
+{
+	return;
+}
+
+#endif /* CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS */
+
+#endif /* _NET_BATMAN_ADV_MULTICAST_H_ */
diff --git a/soft-interface.c b/soft-interface.c
index a6ab9e4..b97dfa2 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -457,6 +457,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);
diff --git a/sysfs.c b/sysfs.c
index cbbb77b..76390d0 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -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,
diff --git a/translation-table.c b/translation-table.c
index df6b5cd..be5cfe1 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -26,6 +26,7 @@
 #include "originator.h"
 #include "routing.h"
 #include "bridge_loop_avoidance.h"
+#include "multicast.h"
 
 #include <linux/crc32c.h>
 
@@ -274,14 +275,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);
-	tt_global = batadv_tt_global_hash_find(bat_priv, addr);
+
+	if (!is_multicast_ether_addr(addr))
+		tt_global = batadv_tt_global_hash_find(bat_priv, addr);
 
 	if (tt_local) {
 		tt_local->last_seen = jiffies;
@@ -332,8 +335,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,
@@ -919,6 +925,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,
@@ -2346,6 +2357,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);
diff --git a/types.h b/types.h
index c84f5cc..5d73a75 100644
--- a/types.h
+++ b/types.h
@@ -473,6 +473,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
@@ -561,6 +567,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;
@@ -595,6 +604,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;
-- 
1.7.10.4

[B.A.T.M.A.N.] [PATCHv2 2/3] batman-adv: Announce new capability via multicast TVLV

[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 <linus.luessing@web.de>
---
 main.c           |    4 ++++
 multicast.c      |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 multicast.h      |    7 ++++++
 originator.c     |    3 +++
 packet.h         |    7 ++++++
 soft-interface.c |    1 +
 types.h          |    4 ++++
 7 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/main.c b/main.c
index 1c82c18..6ab5b2d 100644
--- a/main.c
+++ b/main.c
@@ -145,6 +145,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;
diff --git a/multicast.c b/multicast.c
index 56a1128..36e4c59 100644
--- a/multicast.c
+++ b/multicast.c
@@ -165,6 +165,7 @@ void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
 	struct list_head mcast_list;
 	int ret;
 	static bool enabled;
+	uint8_t mcast_flags;
 
 	INIT_LIST_HEAD(&mcast_list);
 
@@ -179,14 +180,22 @@ void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
 	 */
 	if (!atomic_read(&bat_priv->mcast_group_awareness) ||
 	    bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT) {
-		if (enabled)
+		if (enabled) {
+			batadv_tvlv_container_unregister(bat_priv,
+							 BATADV_TVLV_MCAST, 1);
 			enabled = false;
+		}
 
 		goto update;
 	}
 
-	if (!enabled)
+	if (!enabled) {
+		mcast_flags = BATADV_MCAST_LISTENER_ANNOUNCEMENT;
+		batadv_tvlv_container_register(bat_priv, BATADV_TVLV_MCAST, 1,
+					       &mcast_flags,
+					       sizeof(mcast_flags));
 		enabled = true;
+	}
 
 	ret = batadv_mcast_mla_local_collect(soft_iface, &mcast_list);
 	if (ret < 0)
@@ -204,10 +213,59 @@ out:
 }
 
 /**
+ * batadv_mcast_tvlv_ogm_handler_v1 - process incoming multicast tvlv container
+ * @bat_priv: the bat priv with all the soft interface information
+ * @orig: the orig_node of the ogm
+ * @flags: flags indicating the tvlv state (see batadv_tvlv_handler_flags)
+ * @tvlv_value: tvlv buffer containing the multicast data
+ * @tvlv_value_len: tvlv buffer length
+ */
+static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
+					     struct batadv_orig_node *orig,
+					     uint8_t flags,
+					     void *tvlv_value,
+					     uint16_t tvlv_value_len)
+{
+	uint8_t mcast_flags = BATADV_NO_FLAGS;
+
+	/* only fetch the tvlv value if the handler wasn't called via the
+	 * CIFNOTFND flag and if there is data to fetch
+	 */
+	if (!(flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND) &&
+	    (tvlv_value) && (tvlv_value_len == sizeof(mcast_flags)))
+		mcast_flags = *(uint8_t *)tvlv_value;
+
+	if (!(mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT) &&
+	    orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT) {
+		atomic_inc(&bat_priv->mcast.num_non_aware);
+	} else if (mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT &&
+		   !(orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT)) {
+		atomic_dec(&bat_priv->mcast.num_non_aware);
+	}
+
+	orig->mcast_flags = mcast_flags;
+}
+
+/**
+ * batadv_mcast_init - initialize the multicast optimizations structures
+ * @bat_priv: the bat priv with all the soft interface information
+ */
+int batadv_mcast_init(struct batadv_priv *bat_priv)
+{
+	batadv_tvlv_handler_register(bat_priv, batadv_mcast_tvlv_ogm_handler_v1,
+				     NULL, BATADV_TVLV_MCAST, 1,
+				     BATADV_TVLV_HANDLER_OGM_CIFNOTFND);
+	return 0;
+}
+
+/**
  * batadv_mcast_free - free the multicast optimizations structures
  * @bat_priv: the bat priv with all the soft interface information
  */
 void batadv_mcast_free(struct batadv_priv *bat_priv)
 {
+	batadv_tvlv_container_unregister(bat_priv, BATADV_TVLV_MCAST, 1);
+	batadv_tvlv_handler_unregister(bat_priv, BATADV_TVLV_MCAST, 1);
+
 	batadv_mcast_mla_tt_clean(bat_priv, NULL);
 }
diff --git a/multicast.h b/multicast.h
index 3b68e3b..9955a18 100644
--- a/multicast.h
+++ b/multicast.h
@@ -24,6 +24,8 @@
 
 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);
 
 #else
@@ -33,6 +35,11 @@ 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;
diff --git a/originator.c b/originator.c
index 5d53d2f..acc0c2d 100644
--- a/originator.c
+++ b/originator.c
@@ -257,6 +257,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);
 
diff --git a/packet.h b/packet.h
index 7d45890..c89df30 100644
--- a/packet.h
+++ b/packet.h
@@ -97,6 +97,11 @@ enum batadv_unicast_frag_flags {
 	BATADV_UNI_FRAG_LARGETAIL = BIT(1),
 };
 
+/* multicast capabilities */
+enum batadv_mcast_flags {
+	BATADV_MCAST_LISTENER_ANNOUNCEMENT = BIT(0),
+};
+
 /* tt data subtypes */
 #define BATADV_TT_DATA_TYPE_MASK 0x0F
 
@@ -143,6 +148,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,
@@ -150,6 +156,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
diff --git a/soft-interface.c b/soft-interface.c
index b97dfa2..8bdd649 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -459,6 +459,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_non_aware, 0);
 #endif
 	atomic_set(&bat_priv->ap_isolation, 0);
 	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
diff --git a/types.h b/types.h
index 5d73a75..c4e5545 100644
--- a/types.h
+++ b/types.h
@@ -146,6 +146,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;
@@ -476,6 +479,7 @@ struct batadv_priv_dat {
 #ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS
 struct batadv_priv_mcast {
 	struct list_head mla_list;
+	atomic_t num_non_aware;
 };
 #endif
 
-- 
1.7.10.4

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

[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 <linus.luessing@web.de>
---
 multicast.c         |   43 +++++++++++++++++++++++++++++++++
 multicast.h         |    8 +++++++
 soft-interface.c    |   10 ++++++++
 translation-table.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 translation-table.h |    1 +
 5 files changed, 128 insertions(+)

diff --git a/multicast.c b/multicast.c
index 36e4c59..bd55c8f 100644
--- a/multicast.c
+++ b/multicast.c
@@ -213,6 +213,49 @@ out:
 }
 
 /**
+ * batadv_mcast_flood - check on how to forward a multicast packet
+ * @skb: The multicast packet to check
+ * @bat_priv: the bat priv with all the soft interface information
+ *
+ * Return 1 if the packet should be flooded, 0 if it should be forwarded
+ * via unicast or -1 if it should be drooped.
+ */
+int batadv_mcast_flood(struct sk_buff *skb, struct batadv_priv *bat_priv)
+{
+	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
+	struct ipv6hdr *ip6hdr;
+	int count, ret = 1;
+
+	if (atomic_read(&bat_priv->mcast_group_awareness) &&
+	    !atomic_read(&bat_priv->mcast.num_non_aware) &&
+	    ntohs(ethhdr->h_proto) == ETH_P_IPV6) {
+		if (!pskb_may_pull(skb, sizeof(*ethhdr) + sizeof(*ip6hdr))) {
+			ret = -1;
+			goto out;
+		}
+
+		ip6hdr = ipv6_hdr(skb);
+
+		/* TODO: Implement Multicast Router Discovery, then add
+		 * scope >= IPV6_ADDR_SCOPE_LINKLOCAL, too
+		 */
+		if (IPV6_ADDR_MC_SCOPE(&ip6hdr->daddr) !=
+		    IPV6_ADDR_SCOPE_LINKLOCAL)
+			goto out;
+
+		count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest);
+
+		if (!count)
+			ret = -1;
+		else if (count == 1)
+			ret = 0;
+	}
+
+out:
+	return ret;
+}
+
+/**
  * batadv_mcast_tvlv_ogm_handler_v1 - process incoming multicast tvlv container
  * @bat_priv: the bat priv with all the soft interface information
  * @orig: the orig_node of the ogm
diff --git a/multicast.h b/multicast.h
index 9955a18..0c2baad 100644
--- a/multicast.h
+++ b/multicast.h
@@ -24,6 +24,8 @@
 
 void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv);
 
+int batadv_mcast_flood(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);
@@ -35,6 +37,12 @@ static inline void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
 	return;
 }
 
+static inline int batadv_mcast_flood(struct sk_buff *skb,
+				     struct batadv_priv *bat_priv)
+{
+	return 1;
+}
+
 static inline int batadv_mcast_init(struct batadv_priv *bat_priv)
 {
 	return 0;
diff --git a/soft-interface.c b/soft-interface.c
index 8bdd649..83e4679 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -36,6 +36,7 @@
 #include <linux/if_vlan.h>
 #include <linux/if_ether.h>
 #include "unicast.h"
+#include "multicast.h"
 #include "bridge_loop_avoidance.h"
 #include "network-coding.h"
 
@@ -222,6 +223,15 @@ static int batadv_interface_tx(struct sk_buff *skb,
 		}
 	}
 
+	if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
+		ret = batadv_mcast_flood(skb, bat_priv);
+		if (ret < 0)
+			goto dropped;
+
+		if (!ret)
+			do_bcast = false;
+	}
+
 	/* ethernet packet should be broadcasted */
 	if (do_bcast) {
 		primary_if = batadv_primary_if_get_selected(bat_priv);
diff --git a/translation-table.c b/translation-table.c
index be5cfe1..680c5c1 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -83,6 +83,72 @@ batadv_tt_hash_find(struct batadv_hashtable *hash, const void *data)
 	return tt_common_entry_tmp;
 }
 
+/**
+ * batadv_tt_orig_entries_count - count the number of originators
+ * @head: a list of originators
+ *
+ * Return the number of originator entries in the given list.
+ *
+ * The caller needs to hold the rcu_read_lock().
+ */
+static int batadv_tt_orig_entries_count(struct hlist_head *head)
+{
+	struct batadv_tt_orig_list_entry *orig_entry;
+	int count = 0;
+
+	hlist_for_each_entry_rcu(orig_entry, head, list) {
+		if (!atomic_read(&orig_entry->refcount))
+			continue;
+
+		count++;
+	}
+
+	return count;
+}
+
+/**
+ * batadv_tt_global_hash_count - count the number of orig entries
+ * @hash: hash table containing the tt entries
+ * @data: the data to count entries for
+ *
+ * Return the number of originators advertising the given address/data
+ * (excluding ourself).
+ */
+int batadv_tt_global_hash_count(struct batadv_priv *bat_priv, const void *data)
+{
+	struct hlist_head *head, *orig_list;
+	struct batadv_tt_common_entry *tt_common_entry;
+	struct batadv_tt_global_entry *tt_global_entry;
+	uint32_t index;
+	int count = 0;
+
+	if (!bat_priv->tt.global_hash)
+		goto out;
+
+	index = batadv_choose_orig(data, bat_priv->tt.global_hash->size);
+	head = &bat_priv->tt.global_hash->table[index];
+
+	rcu_read_lock();
+	hlist_for_each_entry_rcu(tt_common_entry, head, hash_entry) {
+		if (!batadv_compare_eth(tt_common_entry, data))
+			continue;
+
+		if (!atomic_read(&tt_common_entry->refcount))
+			continue;
+
+		tt_global_entry = container_of(tt_common_entry,
+					       struct batadv_tt_global_entry,
+					       common);
+		orig_list = &tt_global_entry->orig_list;
+		count = batadv_tt_orig_entries_count(orig_list);
+		break;
+	}
+	rcu_read_unlock();
+
+out:
+	return count;
+}
+
 static struct batadv_tt_local_entry *
 batadv_tt_local_hash_find(struct batadv_priv *bat_priv, const void *data)
 {
diff --git a/translation-table.h b/translation-table.h
index 015d8b9..5986c57 100644
--- a/translation-table.h
+++ b/translation-table.h
@@ -31,6 +31,7 @@ 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 void *data);
 struct batadv_orig_node *batadv_transtable_search(struct batadv_priv *bat_priv,
 						  const uint8_t *src,
 						  const uint8_t *addr);
-- 
1.7.10.4

Re: [B.A.T.M.A.N.] Basic Multicast Optimizations

[Linus Lüssing]

On Fri, May 24, 2013 at 10:02:25AM +0200, Linus Lüssing wrote:
> This is the second revision of the basic multicast optimization patches.
> It includes one functional fix and many style fixes, thanks to Antonios
> feedback.
> 
> Three things were not included from these suggestions:
> 
> * The default mcast TVLV flags were left as it is not as easy to
> switch. (see answer/explanation to [PATCH 2/3])
> * for_each_pmc_rcu() was not moved to a commen net header file,
> only a TODO was added. Will do that after this code might have
> landed in net.
* VLAN support was not added yet as this will be some more work. Any
vlan frames with a multicast destination will still get flooded
for now so at least these patches shouldn't introduce any
regressions for those. However I have to admit that I didn't think
of VLANs yet and Marek suggested to check how much extra work it
might create later compared to adding that right from the start.
I'll have a look at that.

> 
> 
> Additionally the limitation of 255 multicast listeners per originator
> was removed (it was more a relic of the pre-TT announcement mechanism
> and such limitations should be taken care of by the current TT
> infrastructure instead).
> 
> Cheers, Linus

Re: [B.A.T.M.A.N.] Basic Multicast Optimizations

[Antonio Quartulli]

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

On Fri, May 24, 2013 at 11:00:50AM +0200, Linus Lüssing wrote:
> On Fri, May 24, 2013 at 10:02:25AM +0200, Linus Lüssing wrote:
> > This is the second revision of the basic multicast optimization patches.
> > It includes one functional fix and many style fixes, thanks to Antonios
> > feedback.
> > 
> > Three things were not included from these suggestions:
> > 
> > * The default mcast TVLV flags were left as it is not as easy to
> > switch. (see answer/explanation to [PATCH 2/3])
> > * for_each_pmc_rcu() was not moved to a commen net header file,
> > only a TODO was added. Will do that after this code might have
> > landed in net.
> * VLAN support was not added yet as this will be some more work. Any
> vlan frames with a multicast destination will still get flooded
> for now so at least these patches shouldn't introduce any
> regressions for those. However I have to admit that I didn't think
> of VLANs yet and Marek suggested to check how much extra work it
> might create later compared to adding that right from the start.
> I'll have a look at that.

I think (but I am not sure) that the TT-VLAN feature that I just sent to the ml
(I have to send v3..) could probably help in this direction, because a TT entry
is not defined by its MAC address only anymore, but by the couple {MAC, vid}.

Since your code uses the TT to store the multicast addresses, I think that VLAN
support can be added "easily"[tm] after the TT-VLAN code will be in.


Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] Basic Multicast Optimizations

[Marek Lindner]

Hi,

On Friday, May 24, 2013 16:02:25 Linus Lüssing wrote:
> * for_each_pmc_rcu() was not moved to a commen net header file,
> only a TODO was added. Will do that after this code might have
> landed in net.

we discussed this further and concluded that we can't simply make that 
decision. The order in which patches are going to be merged into net has to be 
decided by David Miller. So, please prepare a patch destined for net, send it 
to netdev (cc the batman ml if you like) to explain what we are planning to do 
and ask for advice.

Cheers,
Marek

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

[Simon Wunderlich]

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

On Fri, May 24, 2013 at 10:02:26AM +0200, 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 <linus.luessing@web.de>
> ---
>  Makefile               |    2 +
>  Makefile.kbuild        |    1 +
>  compat.c               |   21 +++++
>  compat.h               |   18 ++++
>  gen-compat-autoconf.sh |    1 +
>  main.c                 |    6 ++
>  main.h                 |    9 ++
>  multicast.c            |  213 ++++++++++++++++++++++++++++++++++++++++++++++++
>  multicast.h            |   43 ++++++++++
>  soft-interface.c       |    3 +
>  sysfs.c                |    6 ++
>  translation-table.c    |   22 ++++-
>  types.h                |   12 +++
>  13 files changed, 353 insertions(+), 4 deletions(-)
>  create mode 100644 multicast.c
>  create mode 100644 multicast.h
> 
> diff --git a/Makefile b/Makefile
> index 407cdc4..d7c6fa6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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
> diff --git a/Makefile.kbuild b/Makefile.kbuild
> index 8ddbfe6..d3efe3a 100644
> --- a/Makefile.kbuild
> +++ b/Makefile.kbuild
> @@ -38,3 +38,4 @@ batman-adv-y += soft-interface.o
>  batman-adv-y += sysfs.o
>  batman-adv-y += translation-table.o
>  batman-adv-y += unicast.o
> +batman-adv-$(CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS) += multicast.o
> diff --git a/compat.c b/compat.c
> index 1f3a39d..27df197 100644
> --- a/compat.c
> +++ b/compat.c
> @@ -25,6 +25,27 @@
>  #include <linux/version.h>
>  #include "main.h"
>  
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 38)
> +
> +void batadv_for_each_pmc_rcu_init(struct in_device *in_dev,
> +				  struct ip_mc_list **pmc)
> +{
> +	read_lock(&in_dev->mc_list_lock);
> +	*pmc = in_dev->mc_list;
> +}
> +
> +bool batadv_for_each_pmc_rcu_check(struct in_device *in_dev,
> +				   struct ip_mc_list *pmc)
> +{
> +	if (pmc == NULL)
> +		read_unlock(&in_dev->mc_list_lock);
> +
> +	return (pmc != NULL);
> +}
> +
> +#endif /* < KERNEL_VERSION(2, 6, 38) */


I guess there is some more compat code required here.

For kernel 2.6.32 I get

compat.h:159:14: warning: ‘struct ip_mc_list’ declared inside parameter list [enabled by default]
compat.h:159:14: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
compat.h:159:14: warning: ‘struct in_device’ declared inside parameter list [enabled by default]
compat.h:161:15: warning: ‘struct ip_mc_list’ declared inside parameter list [enabled by default]
compat.h:161:15: warning: ‘struct in_device’ declared inside parameter list [enabled by default]

Am I missing config options? These structs are defined in linux/igmp.h though ...

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [B.A.T.M.A.N.] [PATCHv2 2/3] batman-adv: Announce new capability via multicast TVLV

[Simon Wunderlich]

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

On Fri, May 24, 2013 at 10:02:27AM +0200, Linus Lüssing wrote:
> 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 <linus.luessing@web.de>
> ---
>  main.c           |    4 ++++
>  multicast.c      |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  multicast.h      |    7 ++++++
>  originator.c     |    3 +++
>  packet.h         |    7 ++++++
>  soft-interface.c |    1 +
>  types.h          |    4 ++++
>  7 files changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/main.c b/main.c
> index 1c82c18..6ab5b2d 100644
> --- a/main.c
> +++ b/main.c
> @@ -145,6 +145,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;
> diff --git a/multicast.c b/multicast.c
> index 56a1128..36e4c59 100644
> --- a/multicast.c
> +++ b/multicast.c
> @@ -204,10 +213,59 @@ out:
>  }
>  
>  /**
> + * batadv_mcast_tvlv_ogm_handler_v1 - process incoming multicast tvlv container
> + * @bat_priv: the bat priv with all the soft interface information
> + * @orig: the orig_node of the ogm
> + * @flags: flags indicating the tvlv state (see batadv_tvlv_handler_flags)
> + * @tvlv_value: tvlv buffer containing the multicast data
> + * @tvlv_value_len: tvlv buffer length
> + */
> +static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
> +					     struct batadv_orig_node *orig,
> +					     uint8_t flags,
> +					     void *tvlv_value,
> +					     uint16_t tvlv_value_len)
> +{
> +	uint8_t mcast_flags = BATADV_NO_FLAGS;
> +
> +	/* only fetch the tvlv value if the handler wasn't called via the
> +	 * CIFNOTFND flag and if there is data to fetch
> +	 */
> +	if (!(flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND) &&
> +	    (tvlv_value) && (tvlv_value_len == sizeof(mcast_flags)))
> +		mcast_flags = *(uint8_t *)tvlv_value;
> +
> +	if (!(mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT) &&
> +	    orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT) {
> +		atomic_inc(&bat_priv->mcast.num_non_aware);
> +	} else if (mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT &&
> +		   !(orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT)) {
> +		atomic_dec(&bat_priv->mcast.num_non_aware);
> +	}

What happens if the orig_node is removed/times out? I can't see where num_non_aware
is updated in this case.

We do have a work struct touch every orig every second anyway (batadv_purge_orig), maybe
put counting multicast aware routers there? Then you don't have to care about
synchronizing stuff ...

> +
> +	orig->mcast_flags = mcast_flags;
> +}
> +
> +/**
> + * batadv_mcast_init - initialize the multicast optimizations structures
> + * @bat_priv: the bat priv with all the soft interface information
> + */
> +int batadv_mcast_init(struct batadv_priv *bat_priv)
> +{
> +	batadv_tvlv_handler_register(bat_priv, batadv_mcast_tvlv_ogm_handler_v1,
> +				     NULL, BATADV_TVLV_MCAST, 1,
> +				     BATADV_TVLV_HANDLER_OGM_CIFNOTFND);
> +	return 0;
> +}
> +
> +/**
>   * batadv_mcast_free - free the multicast optimizations structures
>   * @bat_priv: the bat priv with all the soft interface information
>   */
>  void batadv_mcast_free(struct batadv_priv *bat_priv)
>  {
> +	batadv_tvlv_container_unregister(bat_priv, BATADV_TVLV_MCAST, 1);
> +	batadv_tvlv_handler_unregister(bat_priv, BATADV_TVLV_MCAST, 1);
> +
>  	batadv_mcast_mla_tt_clean(bat_priv, NULL);
>  }

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Simon Wunderlich]

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

Only a few style suggestions here, nothing critical and you can ignore
them if you don't like them. :)

On Fri, May 24, 2013 at 10:02:28AM +0200, Linus Lüssing wrote:
> 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 <linus.luessing@web.de>
> ---
>  multicast.c         |   43 +++++++++++++++++++++++++++++++++
>  multicast.h         |    8 +++++++
>  soft-interface.c    |   10 ++++++++
>  translation-table.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  translation-table.h |    1 +
>  5 files changed, 128 insertions(+)
> 
> diff --git a/multicast.c b/multicast.c
> index 36e4c59..bd55c8f 100644
> --- a/multicast.c
> +++ b/multicast.c
> @@ -213,6 +213,49 @@ out:
>  }
>  
>  /**
> + * batadv_mcast_flood - check on how to forward a multicast packet
> + * @skb: The multicast packet to check
> + * @bat_priv: the bat priv with all the soft interface information
> + *
> + * Return 1 if the packet should be flooded, 0 if it should be forwarded
> + * via unicast or -1 if it should be drooped.
> + */
> +int batadv_mcast_flood(struct sk_buff *skb, struct batadv_priv *bat_priv)
> +{
> +	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
> +	struct ipv6hdr *ip6hdr;
> +	int count, ret = 1;
> +
> +	if (atomic_read(&bat_priv->mcast_group_awareness) &&
> +	    !atomic_read(&bat_priv->mcast.num_non_aware) &&
> +	    ntohs(ethhdr->h_proto) == ETH_P_IPV6) {

You can safe an indendation below if you return -1 immediately here
if the statement above is false. Also multiple statements might be better
for readability and later changes, e.g.

if (!atomic_read(&bat_priv->mcast_group_awareness))
	return 1;

if (atomic_read(&bat_priv->mcast.num_non_aware))
	return 1;

if (ntohs(ethhdr->h_proto) != ETH_P_IPV6)
	return 1;

> +		if (!pskb_may_pull(skb, sizeof(*ethhdr) + sizeof(*ip6hdr))) {
> +			ret = -1;
> +			goto out;
> +		}

You could directly return -1 here, the out label is not needed
(as we don't unlock/free anything here).

I don't quite understand why you return -1, maybe the packet could still
be forwarded even if it could not be pulled?

> +
> +		ip6hdr = ipv6_hdr(skb);
> +
> +		/* TODO: Implement Multicast Router Discovery, then add
> +		 * scope >= IPV6_ADDR_SCOPE_LINKLOCAL, too
> +		 */
> +		if (IPV6_ADDR_MC_SCOPE(&ip6hdr->daddr) !=
> +		    IPV6_ADDR_SCOPE_LINKLOCAL)
> +			goto out;
> +
> +		count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest);
> +
> +		if (!count)
> +			ret = -1;
> +		else if (count == 1)
> +			ret = 0;
> +	}

You could use a switch statement here instead for readability, e.g.:

switch (count) {
case 0: 
	return -1;
case 1:
	return 0;
default:
	return 1;
}
> +
> +out:
> +	return ret;
> +}
> +
> +/**
>   * batadv_mcast_tvlv_ogm_handler_v1 - process incoming multicast tvlv container
>   * @bat_priv: the bat priv with all the soft interface information
>   * @orig: the orig_node of the ogm
> diff --git a/soft-interface.c b/soft-interface.c
> index 8bdd649..83e4679 100644
> --- a/soft-interface.c
> +++ b/soft-interface.c
> @@ -36,6 +36,7 @@
>  #include <linux/if_vlan.h>
>  #include <linux/if_ether.h>
>  #include "unicast.h"
> +#include "multicast.h"
>  #include "bridge_loop_avoidance.h"
>  #include "network-coding.h"
>  
> @@ -222,6 +223,15 @@ static int batadv_interface_tx(struct sk_buff *skb,
>  		}
>  	}
>  
> +	if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {

I'd suggest to put this inside the "is_multicast_etheraddr()" above to make more clear
that this handles multicast packets. I was a little confused by the
do_bcast && !is_broadcast_ether_addr() first, but that might just be me.

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Marek Lindner]

On Tuesday, May 28, 2013 23:40:54 Simon Wunderlich wrote:
> You can safe an indendation below if you return -1 immediately here
> if the statement above is false. Also multiple statements might be better
> for readability and later changes, e.g.
> 
> if (!atomic_read(&bat_priv->mcast_group_awareness))
>         return 1;
> 
> if (atomic_read(&bat_priv->mcast.num_non_aware))
>         return 1;
> 
> if (ntohs(ethhdr->h_proto) != ETH_P_IPV6)
>         return 1;

A quick hint from my end: The kernel maintainer don't like code that returns 
"random" values like 1/0/-1. Either use bool or meaningful defines instead of 
raw numbers.

Cheers,
Marek

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

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 12:39:33AM +0800, Marek Lindner wrote:
> On Tuesday, May 28, 2013 23:40:54 Simon Wunderlich wrote:
> > You can safe an indendation below if you return -1 immediately here
> > if the statement above is false. Also multiple statements might be better
> > for readability and later changes, e.g.
> > 
> > if (!atomic_read(&bat_priv->mcast_group_awareness))
> >         return 1;
> > 
> > if (atomic_read(&bat_priv->mcast.num_non_aware))
> >         return 1;
> > 
> > if (ntohs(ethhdr->h_proto) != ETH_P_IPV6)
> >         return 1;
> 
> A quick hint from my end: The kernel maintainer don't like code that returns 
> "random" values like 1/0/-1. Either use bool or meaningful defines instead of 
> raw numbers.

Very good suggestion.
I think you can define an enum for this. It will make the code much easier to
understand.

Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]