b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] Basic Multicast Optimizations
@ 2013-07-03 22:03 Linus Lüssing
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 1/4] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Linus Lüssing @ 2013-07-03 22:03 UTC (permalink / raw)
  To: b.a.t.m.a.n

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

It adds the style changes as suggested by Marek via eMail.

It also adds various suggestion from IRC (for instance adding kerneldoc for the
functions touched in translation-table.c, making the counter an atomic
one to be on the safe side for some "exotic" targets).


PATCHv7 4/4 is a new one and the result of a longer discussion on IRC:
Whether to add some support for some IPv4 addresses already for the cost of
a slightly larger complexity both code and documentation wise
- for a case which is uncommon for most mesh networks anyway: a mesh
with no bridges on any node involved.

So the fourth patch is sort of an RFC patch one, too (although the consense/
tendency on IRC so far was to add and support this case).

If everyone is fine with that concept, then I can also easily merge it
into patches 2/4 and 3/4 (if that is desired).

Cheers, Linus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [B.A.T.M.A.N.] [PATCHv7 1/4] batman-adv: Multicast Listener Announcements via Translation Table
  2013-07-03 22:03 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
@ 2013-07-03 22:03 ` Linus Lüssing
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 2/4] batman-adv: Announce new capability via multicast TVLV Linus Lüssing
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Linus Lüssing @ 2013-07-03 22:03 UTC (permalink / raw)
  To: b.a.t.m.a.n

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.h               |   20 ++++-
 gen-compat-autoconf.sh |    1 +
 main.c                 |    6 ++
 main.h                 |    1 +
 multicast.c            |  199 ++++++++++++++++++++++++++++++++++++++++++++++++
 multicast.h            |   43 +++++++++++
 soft-interface.c       |    3 +
 sysfs-class-net-mesh   |    9 +++
 sysfs.c                |    6 ++
 translation-table.c    |   22 +++++-
 types.h                |   22 ++++++
 13 files changed, 330 insertions(+), 5 deletions(-)
 create mode 100644 multicast.c
 create mode 100644 multicast.h

diff --git a/Makefile b/Makefile
index 407cdc4..ce9bf32 100644
--- a/net/batman-adv/Makefile
+++ b/net/batman-adv/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=y
 
 PWD:=$(shell pwd)
 KERNELPATH ?= /lib/modules/$(shell uname -r)/build
diff --git a/Makefile.kbuild b/Makefile.kbuild
index 4f4aabb..66d1cbe 100644
--- a/net/batman-adv/Makefile.kbuild
+++ b/net/batman-adv/Makefile.kbuild
@@ -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) += multicast.o
diff --git a/compat.h b/compat.h
index 457be3d..fa18c24 100644
--- a/net/batman-adv/compat.h
+++ b/net/batman-adv/compat.h
@@ -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. */
diff --git a/gen-compat-autoconf.sh b/gen-compat-autoconf.sh
index 78573e4..f625b6f 100755
--- a/net/batman-adv/gen-compat-autoconf.sh
+++ b/net/batman-adv/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' ${CONFIG_BATMAN_ADV_MCAST:="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 58cd339..d24b950 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -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"
@@ -117,6 +118,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
+	INIT_HLIST_HEAD(&bat_priv->mcast.mla_list);
+#endif
 	INIT_HLIST_HEAD(&bat_priv->tvlv.container_list);
 	INIT_HLIST_HEAD(&bat_priv->tvlv.handler_list);
 	INIT_HLIST_HEAD(&bat_priv->softif_vlan_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 cfed2a3..ede23d5 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -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>
diff --git a/multicast.c b/multicast.c
new file mode 100644
index 0000000..7ea19ab
--- /dev/null
+++ b/net/batman-adv/multicast.c
@@ -0,0 +1,199 @@
+/* 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_softif_get - get softif 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 soft 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_softif_get(struct net_device *dev,
+				       struct hlist_head *mcast_list)
+{
+	struct netdev_hw_addr *mc_list_entry;
+	struct batadv_hw_addr *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);
+		hlist_add_head(&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 hlist_head *mcast_list)
+{
+	struct batadv_hw_addr *mcast_entry;
+
+	hlist_for_each_entry(mcast_entry, mcast_list, list)
+		if (batadv_compare_eth(mcast_entry->addr, mcast_addr))
+			return true;
+
+	return false;
+}
+
+/**
+ * batadv_mcast_mla_list_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_list_free(struct hlist_head *mcast_list)
+{
+	struct batadv_hw_addr *mcast_entry;
+	struct hlist_node *tmp;
+
+	hlist_for_each_entry_safe(mcast_entry, tmp, mcast_list, list) {
+		hlist_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 hlist_head *mcast_list)
+{
+	struct batadv_hw_addr *mcast_entry;
+	struct hlist_node *tmp;
+
+	hlist_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,
+				       BATADV_NO_FLAGS,
+				       "mcast TT outdated", false);
+
+		hlist_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 hlist_head *mcast_list)
+{
+	struct batadv_hw_addr *mcast_entry;
+	struct hlist_node *tmp;
+
+	if (!mcast_list)
+		return;
+
+	hlist_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_NO_FLAGS, BATADV_NULL_IFINDEX);
+		hlist_del(&mcast_entry->list);
+		hlist_add_head(&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 net_device *soft_iface = bat_priv->soft_iface;
+	struct hlist_head mcast_list = HLIST_HEAD_INIT;
+	int ret;
+
+	/* 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->multicast_mode) ||
+	    bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT)
+		goto update;
+
+	ret = batadv_mcast_mla_softif_get(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:
+	batadv_mcast_mla_list_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..8c03487
--- /dev/null
+++ b/net/batman-adv/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
+
+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 */
+
+#endif /* _NET_BATMAN_ADV_MULTICAST_H_ */
diff --git a/soft-interface.c b/soft-interface.c
index 25662d9..f984918 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -646,6 +646,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
+	atomic_set(&bat_priv->multicast_mode, 1);
+#endif
 	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
 	atomic_set(&bat_priv->gw_sel_class, 20);
 	atomic_set(&bat_priv->gw.bandwidth_down, 100);
diff --git a/sysfs-class-net-mesh b/sysfs-class-net-mesh
index ee4a479..ac1b561 100644
--- a/net/batman-adv/sysfs-class-net-mesh
+++ b/net/batman-adv/sysfs-class-net-mesh
@@ -68,6 +68,15 @@ Description:
                 Defines the penalty which will be applied to an
                 originator message's tq-field on every hop.
 
+What:           /sys/class/net/<mesh_iface>/mesh/multicast_mode
+Date:           June 2013
+Contact:        Linus Lüssing <linus.luessing@web.de>
+Description:
+                Indicates whether multicast optimizations are enabled
+                or disabled. If set to zero then all nodes in the
+                mesh are going to use classic flooding for any
+                multicast packet with no optimizations.
+
 What:           /sys/class/net/<mesh_iface>/mesh/network_coding
 Date:           Nov 2012
 Contact:        Martin Hundeboll <martin@hundeboll.net>
diff --git a/sysfs.c b/sysfs.c
index cd60fba..0ff7291 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -460,6 +460,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
+BATADV_ATTR_SIF_BOOL(multicast_mode, 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
@@ -477,6 +480,9 @@ static struct batadv_attribute *batadv_mesh_attrs[] = {
 #ifdef CONFIG_BATMAN_ADV_DAT
 	&batadv_attr_distributed_arp_table,
 #endif
+#ifdef CONFIG_BATMAN_ADV_MCAST
+	&batadv_attr_multicast_mode,
+#endif
 	&batadv_attr_fragmentation,
 	&batadv_attr_routing_algo,
 	&batadv_attr_gw_mode,
diff --git a/translation-table.c b/translation-table.c
index 7ac5ddc..1a2fb69 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/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>
 
@@ -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,
@@ -2532,6 +2543,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 0fbf07c..810f014 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -501,6 +501,12 @@ struct batadv_priv_dat {
 };
 #endif
 
+#ifdef CONFIG_BATMAN_ADV_MCAST
+struct batadv_priv_mcast {
+	struct hlist_head mla_list;
+};
+#endif
+
 /**
  * struct batadv_priv_nc - per mesh interface network coding private data
  * @work: work queue callback item for cleanup
@@ -610,6 +616,9 @@ struct batadv_priv {
 #ifdef CONFIG_BATMAN_ADV_DAT
 	atomic_t distributed_arp_table;
 #endif
+#ifdef CONFIG_BATMAN_ADV_MCAST
+	atomic_t multicast_mode;
+#endif
 	atomic_t gw_mode;
 	atomic_t gw_sel_class;
 	atomic_t orig_interval;
@@ -646,6 +655,9 @@ struct batadv_priv {
 #ifdef CONFIG_BATMAN_ADV_DAT
 	struct batadv_priv_dat dat;
 #endif
+#ifdef CONFIG_BATMAN_ADV_MCAST
+	struct batadv_priv_mcast mcast;
+#endif
 #ifdef CONFIG_BATMAN_ADV_NC
 	atomic_t network_coding;
 	struct batadv_priv_nc nc;
@@ -971,6 +983,16 @@ struct batadv_dat_entry {
 };
 
 /**
+ * struct batadv_hw_addr - a list entry for a MAC address
+ * @list: list node for the linking of entries
+ * @addr: the MAC address of this list entry
+ */
+struct batadv_hw_addr {
+	struct hlist_node list;
+	unsigned char addr[ETH_ALEN];
+};
+
+/**
  * struct batadv_dat_candidate - candidate destination for DAT operations
  * @type: the type of the selected candidate. It can one of the following:
  *	  - BATADV_DAT_CANDIDATE_NOT_FOUND
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [B.A.T.M.A.N.] [PATCHv7 2/4] batman-adv: Announce new capability via multicast TVLV
  2013-07-03 22:03 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 1/4] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing
@ 2013-07-03 22:03 ` Linus Lüssing
  2013-07-06 10:57   ` Marek Lindner
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 3/4] batman-adv: Modified forwarding behaviour for multicast packets Linus Lüssing
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Linus Lüssing @ 2013-07-03 22:03 UTC (permalink / raw)
  To: b.a.t.m.a.n

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 +++
 main.h           |    1 +
 multicast.c      |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 multicast.h      |   14 +++++++++++
 originator.c     |    6 +++++
 packet.h         |    7 ++++++
 soft-interface.c |    1 +
 types.h          |    4 +++
 8 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/main.c b/main.c
index d24b950..ce58900 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/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/main.h b/main.h
index ede23d5..fe612d6 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -68,6 +68,7 @@
 #define BATADV_ROAMING_MAX_TIME 20000
 #define BATADV_ROAMING_MAX_COUNT 5
 
+#define BATADV_UNINIT_FLAGS -1
 #define BATADV_NO_FLAGS 0
 
 #define BATADV_NULL_IFINDEX 0 /* dummy ifindex used to avoid iface checks */
diff --git a/multicast.c b/multicast.c
index 7ea19ab..4af4bc9 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -169,13 +169,30 @@ void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv)
 	struct net_device *soft_iface = bat_priv->soft_iface;
 	struct hlist_head mcast_list = HLIST_HEAD_INIT;
 	int ret;
+	static bool enabled;
+	uint8_t mcast_flags;
 
 	/* 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->multicast_mode) ||
-	    bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT)
+	    bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT) {
+		if (enabled) {
+			batadv_tvlv_container_unregister(bat_priv,
+							 BATADV_TVLV_MCAST, 1);
+			enabled = false;
+		}
+
 		goto update;
+	}
+
+	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_softif_get(soft_iface, &mcast_list);
 	if (ret < 0)
@@ -190,10 +207,65 @@ 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 ||
+	     orig->mcast_flags & BATADV_UNINIT_FLAGS))
+		atomic_inc(&bat_priv->mcast.num_no_mla);
+	else if (mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT &&
+		   !(orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT))
+		atomic_dec(&bat_priv->mcast.num_no_mla);
+
+	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);
 }
+
+void batadv_mcast_purge_orig(struct batadv_orig_node *orig_node)
+{
+	if (!(orig_node->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT))
+		atomic_dec(&orig_node->bat_priv->mcast.num_no_mla);
+}
diff --git a/multicast.h b/multicast.h
index 8c03487..d8bb869 100644
--- a/net/batman-adv/multicast.h
+++ b/net/batman-adv/multicast.h
@@ -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 */
 
 #endif /* _NET_BATMAN_ADV_MULTICAST_H_ */
diff --git a/originator.c b/originator.c
index a591dc5..9f899cd 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -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
+	orig_node->mcast_flags = BATADV_UNINIT_FLAGS;
+#endif
 
 	atomic_set(&orig_node->bond_candidates, 0);
 
diff --git a/packet.h b/packet.h
index 08e175a..1090977 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -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
diff --git a/soft-interface.c b/soft-interface.c
index f984918..1992c42 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -648,6 +648,7 @@ static int batadv_softif_init_late(struct net_device *dev)
 #endif
 #ifdef CONFIG_BATMAN_ADV_MCAST
 	atomic_set(&bat_priv->multicast_mode, 1);
+	atomic_set(&bat_priv->mcast.num_no_mla, 0);
 #endif
 	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
 	atomic_set(&bat_priv->gw_sel_class, 20);
diff --git a/types.h b/types.h
index 810f014..ceb96b0 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -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
+	int 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
 struct batadv_priv_mcast {
 	struct hlist_head mla_list;
+	atomic_t num_no_mla;
 };
 #endif
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [B.A.T.M.A.N.] [PATCHv7 3/4] batman-adv: Modified forwarding behaviour for multicast packets
  2013-07-03 22:03 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 1/4] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 2/4] batman-adv: Announce new capability via multicast TVLV Linus Lüssing
@ 2013-07-03 22:03 ` Linus Lüssing
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 4/4] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support Linus Lüssing
  2013-07-04  5:06 ` [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
  4 siblings, 0 replies; 10+ messages in thread
From: Linus Lüssing @ 2013-07-03 22:03 UTC (permalink / raw)
  To: b.a.t.m.a.n

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         |   60 +++++++++++++++++++++++++++++++++++
 multicast.h         |   23 ++++++++++++++
 soft-interface.c    |   18 +++++++++--
 translation-table.c |   86 +++++++++++++++++++++++++++++++++++++++++----------
 translation-table.h |    2 ++
 types.h             |    2 ++
 6 files changed, 173 insertions(+), 18 deletions(-)

diff --git a/multicast.c b/multicast.c
index 4af4bc9..dee86ab 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -21,6 +21,7 @@
 #include "originator.h"
 #include "hard-interface.h"
 #include "translation-table.h"
+#include "multicast.h"
 
 /**
  * batadv_mcast_mla_softif_get - get softif multicast listeners
@@ -207,6 +208,61 @@ out:
 }
 
 /**
+ * batadv_mcast_forw_mode - 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 the forwarding mode as enum batadv_forw_mode.
+ */
+enum batadv_forw_mode
+batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
+{
+	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
+	struct ipv6hdr *ip6hdr;
+	int count;
+
+	if (!atomic_read(&bat_priv->multicast_mode))
+		return BATADV_FORW_ALL;
+
+	if (atomic_read(&bat_priv->mcast.num_no_mla))
+		return BATADV_FORW_ALL;
+
+	if (ntohs(ethhdr->h_proto) != ETH_P_IPV6)
+		return BATADV_FORW_ALL;
+
+	/* We might fail due to out-of-memory -> drop it */
+	if (!pskb_may_pull(skb, sizeof(*ethhdr) + sizeof(*ip6hdr)))
+		return BATADV_FORW_NONE;
+
+	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)
+		return BATADV_FORW_ALL;
+
+	/* We cannot snoop and therefore cannot optimize the
+	 * all-nodes-multicast address
+	 */
+	if (ipv6_addr_is_ll_all_nodes(&ip6hdr->daddr))
+		return BATADV_FORW_ALL;
+
+	count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest,
+					    BATADV_NO_FLAGS);
+
+	switch (count) {
+	case 0:
+		return BATADV_FORW_NONE;
+	case 1:
+		return BATADV_FORW_SINGLE;
+	default:
+		return BATADV_FORW_ALL;
+	}
+}
+
+/**
  * 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
@@ -264,6 +320,10 @@ void batadv_mcast_free(struct batadv_priv *bat_priv)
 	batadv_mcast_mla_tt_clean(bat_priv, NULL);
 }
 
+/**
+ * batadv_mcast_purge_orig - reset originator global mcast state modifications
+ * @orig_node: the originator which is going to get purged
+ */
 void batadv_mcast_purge_orig(struct batadv_orig_node *orig_node)
 {
 	if (!(orig_node->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT))
diff --git a/multicast.h b/multicast.h
index d8bb869..c48bb8c 100644
--- a/net/batman-adv/multicast.h
+++ b/net/batman-adv/multicast.h
@@ -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
 
 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;
diff --git a/soft-interface.c b/soft-interface.c
index 1992c42..8fad00e 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -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);
 
-		if (is_multicast_ether_addr(ethhdr->h_dest))
+		if (forw_to_gw)
 			ret = batadv_send_skb_via_gw(bat_priv, skb, vid);
 		else
 			ret = batadv_send_skb_via_tt(bat_priv, skb, vid);
diff --git a/translation-table.c b/translation-table.c
index 1a2fb69..fff0d45 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -195,6 +195,32 @@ batadv_tt_global_entry_free_ref(struct batadv_tt_global_entry *tt_global_entry)
 	}
 }
 
+/**
+ * batadv_tt_global_hash_count - count the number of orig entries
+ * @hash: hash table containing the tt entries
+ * @addr: the mac address of the client to count entries for
+ * @vid: VLAN identifier
+ *
+ * Return the number of originators advertising the given address/data
+ * (excluding ourself).
+ */
+int batadv_tt_global_hash_count(struct batadv_priv *bat_priv,
+				const uint8_t *addr, unsigned short vid)
+{
+	struct batadv_tt_global_entry *tt_global_entry;
+	int count = 0;
+
+	tt_global_entry = batadv_tt_global_hash_find(bat_priv, addr, vid);
+	if (!tt_global_entry)
+		goto out;
+
+	count = atomic_read(&tt_global_entry->orig_list_count);
+	batadv_tt_global_entry_free_ref(tt_global_entry);
+
+out:
+	return count;
+}
+
 static void batadv_tt_orig_list_entry_free_rcu(struct rcu_head *rcu)
 {
 	struct batadv_tt_orig_list_entry *orig_entry;
@@ -875,6 +901,8 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global,
 	hlist_add_head_rcu(&orig_entry->list,
 			   &tt_global->orig_list);
 	spin_unlock_bh(&tt_global->list_lock);
+	atomic_inc(&tt_global->orig_list_count);
+
 out:
 	if (orig_entry)
 		batadv_tt_orig_list_entry_free_ref(orig_entry);
@@ -944,6 +972,7 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		common->added_at = jiffies;
 
 		INIT_HLIST_HEAD(&tt_global_entry->orig_list);
+		atomic_set(&tt_global_entry->orig_list_count, 0);
 		spin_lock_init(&tt_global_entry->list_lock);
 
 		hash_added = batadv_hash_add(bat_priv->tt.global_hash,
@@ -1164,6 +1193,23 @@ out:
 	return 0;
 }
 
+/**
+ * batadv_tt_global_del_orig_entry - remove and free an orig_entry
+ * @tt_global_entry: the global entry to remove the orig_entry from
+ * @orig_entry: the orig entry to remove and free
+ *
+ * Remove an orig_entry from its list in the given tt_global_entry and
+ * free this orig_entry afterwards.
+ */
+static void
+batadv_tt_global_del_orig_entry(struct batadv_tt_global_entry *tt_global_entry,
+				struct batadv_tt_orig_list_entry *orig_entry)
+{
+	atomic_dec(&tt_global_entry->orig_list_count);
+	hlist_del_rcu(&orig_entry->list);
+	batadv_tt_orig_list_entry_free_ref(orig_entry);
+}
+
 /* deletes the orig list of a tt_global_entry */
 static void
 batadv_tt_global_del_orig_list(struct batadv_tt_global_entry *tt_global_entry)
@@ -1174,18 +1220,26 @@ batadv_tt_global_del_orig_list(struct batadv_tt_global_entry *tt_global_entry)
 
 	spin_lock_bh(&tt_global_entry->list_lock);
 	head = &tt_global_entry->orig_list;
-	hlist_for_each_entry_safe(orig_entry, safe, head, list) {
-		hlist_del_rcu(&orig_entry->list);
-		batadv_tt_orig_list_entry_free_ref(orig_entry);
-	}
+	hlist_for_each_entry_safe(orig_entry, safe, head, list)
+		batadv_tt_global_del_orig_entry(tt_global_entry, orig_entry);
 	spin_unlock_bh(&tt_global_entry->list_lock);
 }
 
+/**
+ * batadv_tt_global_del_orig_node - remove orig_node from a global tt entry
+ * @bat_priv: the bat priv with all the soft interface information
+ * @tt_global_entry: the global entry to remove the orig_node from
+ * @orig_node: the originator announcing the client
+ * @message: message to append to the log on deletion
+ *
+ * Remove the given orig_node and its according orig_entry from the given
+ * global tt entry.
+ */
 static void
-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)
 {
 	struct hlist_head *head;
 	struct hlist_node *safe;
@@ -1202,8 +1256,8 @@ batadv_tt_global_del_orig_entry(struct batadv_priv *bat_priv,
 				   orig_node->orig,
 				   tt_global_entry->common.addr,
 				   BATADV_PRINT_VID(vid), message);
-			hlist_del_rcu(&orig_entry->list);
-			batadv_tt_orig_list_entry_free_ref(orig_entry);
+			batadv_tt_global_del_orig_entry(tt_global_entry,
+							orig_entry);
 		}
 	}
 	spin_unlock_bh(&tt_global_entry->list_lock);
@@ -1245,8 +1299,8 @@ batadv_tt_global_del_roaming(struct batadv_priv *bat_priv,
 		/* there is another entry, we can simply delete this
 		 * one and can still use the other one.
 		 */
-		batadv_tt_global_del_orig_entry(bat_priv, tt_global_entry,
-						orig_node, message);
+		batadv_tt_global_del_orig_node(bat_priv, tt_global_entry,
+					       orig_node, message);
 }
 
 /**
@@ -1272,8 +1326,8 @@ static void batadv_tt_global_del(struct batadv_priv *bat_priv,
 		goto out;
 
 	if (!roaming) {
-		batadv_tt_global_del_orig_entry(bat_priv, tt_global_entry,
-						orig_node, message);
+		batadv_tt_global_del_orig_node(bat_priv, tt_global_entry,
+					       orig_node, message);
 
 		if (hlist_empty(&tt_global_entry->orig_list))
 			batadv_tt_global_free(bat_priv, tt_global_entry,
@@ -1342,8 +1396,8 @@ void batadv_tt_global_del_orig(struct batadv_priv *bat_priv,
 						 struct batadv_tt_global_entry,
 						 common);
 
-			batadv_tt_global_del_orig_entry(bat_priv, tt_global,
-							orig_node, message);
+			batadv_tt_global_del_orig_node(bat_priv, tt_global,
+						       orig_node, message);
 
 			if (hlist_empty(&tt_global->orig_list)) {
 				vid = tt_global->common.vid;
diff --git a/translation-table.h b/translation-table.h
index c6bf33c..396e7e1 100644
--- a/net/batman-adv/translation-table.h
+++ b/net/batman-adv/translation-table.h
@@ -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,
diff --git a/types.h b/types.h
index ceb96b0..7535c69 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -784,12 +784,14 @@ struct batadv_tt_local_entry {
  * struct batadv_tt_global_entry - translation table global entry data
  * @common: general translation table data
  * @orig_list: list of orig nodes announcing this non-mesh client
+ * @orig_list_count: number of items in the orig_list
  * @list_lock: lock protecting orig_list
  * @roam_at: time at which TT_GLOBAL_ROAM was set
  */
 struct batadv_tt_global_entry {
 	struct batadv_tt_common_entry common;
 	struct hlist_head orig_list;
+	atomic_t orig_list_count;
 	spinlock_t list_lock;	/* protects orig_list */
 	unsigned long roam_at;
 };
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [B.A.T.M.A.N.] [PATCHv7 4/4] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support
  2013-07-03 22:03 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
                   ` (2 preceding siblings ...)
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 3/4] batman-adv: Modified forwarding behaviour for multicast packets Linus Lüssing
@ 2013-07-03 22:03 ` Linus Lüssing
  2013-07-12  9:12   ` Simon Wunderlich
  2013-07-04  5:06 ` [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
  4 siblings, 1 reply; 10+ messages in thread
From: Linus Lüssing @ 2013-07-03 22:03 UTC (permalink / raw)
  To: b.a.t.m.a.n

With this patch a node may additionally perform the dropping or
unicasting behaviour for a link-local IPv4 and link-local-all-nodes
IPv6 multicast packet, too.

The extra counter and BATADV_MCAST_HAS_NO_BRIDGE flag is needed
because with a future bridge snooping support integration a node with a
bridge on top of its soft interface is not able to reliably detect its
multicast listeners for IPv4 link-local and the IPv6
link-local-all-nodes addresses anymore (see RFC4541, section 2.1.2.2
and section 3).

Even though this new flag and the BATADV_MCAST_LISTENER_ANNOUNCEMENT
flag are currently set and unset at the same time, so even though this
new flag does make "no difference" now, it'll ensure a seamless
integration of multicast bridge support without needing to break
compatibility later.

Also note, that even with multicast bridge support it will need only
one node with a bridge to disable optimizations for link-local IPv4
and IPv6-link-local-all-nodes multicast, resulting in flooding all
these packets again. So the 224.0.0.x address range and the ff02::1
address will never be a safe choice for multicast streaming etc. if
you do not control every node.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 main.h           |    1 +
 multicast.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++----------
 packet.h         |    1 +
 soft-interface.c |    1 +
 types.h          |    1 +
 5 files changed, 143 insertions(+), 31 deletions(-)

diff --git a/main.h b/main.h
index fe612d6..913bfd4 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -166,6 +166,7 @@ enum batadv_uev_type {
 #include <linux/slab.h>
 #include <net/sock.h>		/* struct sock */
 #include <net/addrconf.h>	/* ipv6 address stuff */
+#include <linux/ip.h>
 #include <net/rtnetlink.h>
 #include <linux/jiffies.h>
 #include <linux/seq_file.h>
diff --git a/multicast.c b/multicast.c
index dee86ab..27781cc 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -208,31 +208,61 @@ out:
 }
 
 /**
- * batadv_mcast_forw_mode - check on how to forward a multicast packet
- * @skb: The multicast packet to check
+ * batadv_mcast_forw_mode_check_ipv4 - check for optimized forwarding potential
+ * @skb: the IPv4 packet to check
  * @bat_priv: the bat priv with all the soft interface information
  *
- * Return the forwarding mode as enum batadv_forw_mode.
+ * Check whether the given IPv4 packet has the potential to
+ * be forwarded with a mode more optimal than classic flooding.
+ *
+ * If so then return 0. Otherwise -EINVAL is returned or -ENOMEM if we are
+ * out of memory.
+ */
+static int batadv_mcast_forw_mode_check_ipv4(struct sk_buff *skb,
+					     struct batadv_priv *bat_priv)
+{
+	struct iphdr *iphdr;
+
+	/* We might fail due to out-of-memory -> drop it */
+	if (!pskb_may_pull(skb, sizeof(struct ethhdr) + sizeof(*iphdr)))
+		return -ENOMEM;
+
+	iphdr = ip_hdr(skb);
+
+	/* TODO: Implement Multicast Router Discovery, then add
+	 * scope >= link local, too
+	 */
+	if (!ipv4_is_local_multicast(iphdr->daddr))
+		return -EINVAL;
+
+	/* With one bridge involved, we cannot be certain about
+	 * link-local multicast listener announcements anymore
+	 */
+	if (atomic_read(&bat_priv->mcast.num_has_bridge))
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * batadv_mcast_forw_mode_check_ipv6 - check for optimized forwarding potential
+ * @skb: the IPv6 packet to check
+ * @bat_priv: the bat priv with all the soft interface information
+ *
+ * Check whether the given IPv6 packet has the potential to
+ * be forwarded with a mode more optimal than classic flooding.
+ *
+ * If so then return 0. Otherwise -EINVAL is returned or -ENOMEM if we are
+ * out of memory.
  */
-enum batadv_forw_mode
-batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
+static int batadv_mcast_forw_mode_check_ipv6(struct sk_buff *skb,
+					     struct batadv_priv *bat_priv)
 {
-	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
 	struct ipv6hdr *ip6hdr;
-	int count;
-
-	if (!atomic_read(&bat_priv->multicast_mode))
-		return BATADV_FORW_ALL;
-
-	if (atomic_read(&bat_priv->mcast.num_no_mla))
-		return BATADV_FORW_ALL;
-
-	if (ntohs(ethhdr->h_proto) != ETH_P_IPV6)
-		return BATADV_FORW_ALL;
 
 	/* We might fail due to out-of-memory -> drop it */
-	if (!pskb_may_pull(skb, sizeof(*ethhdr) + sizeof(*ip6hdr)))
-		return BATADV_FORW_NONE;
+	if (!pskb_may_pull(skb, sizeof(struct ethhdr) + sizeof(*ip6hdr)))
+		return -ENOMEM;
 
 	ip6hdr = ipv6_hdr(skb);
 
@@ -241,18 +271,73 @@ batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
 	 */
 	if (IPV6_ADDR_MC_SCOPE(&ip6hdr->daddr) !=
 	    IPV6_ADDR_SCOPE_LINKLOCAL)
-		return BATADV_FORW_ALL;
+		return -EINVAL;
 
-	/* We cannot snoop and therefore cannot optimize the
-	 * all-nodes-multicast address
+	/* With one bridge involved, we cannot be certain about
+	 * link-local-all-nodes multicast listener announcements anymore
 	 */
-	if (ipv6_addr_is_ll_all_nodes(&ip6hdr->daddr))
+	if (ipv6_addr_is_ll_all_nodes(&ip6hdr->daddr) &&
+	    atomic_read(&bat_priv->mcast.num_has_bridge))
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * batadv_mcast_forw_mode_check - check for optimized forwarding potential
+ * @skb: the multicast frame to check
+ * @bat_priv: the bat priv with all the soft interface information
+ *
+ * Check whether the given multicast ethernet frame has the potential to
+ * be forwarded with a mode more optimal than classic flooding.
+ *
+ * If so then return 0. Otherwise -EINVAL is returned or -ENOMEM if we are
+ * out of memory.
+ */
+static int batadv_mcast_forw_mode_check(struct sk_buff *skb,
+					struct batadv_priv *bat_priv)
+{
+	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
+
+	if (!atomic_read(&bat_priv->multicast_mode))
+		return -EINVAL;
+
+	if (atomic_read(&bat_priv->mcast.num_no_mla))
+		return -EINVAL;
+
+	switch (ntohs(ethhdr->h_proto)) {
+	case ETH_P_IP:
+		return batadv_mcast_forw_mode_check_ipv4(skb, bat_priv);
+	case ETH_P_IPV6:
+		return batadv_mcast_forw_mode_check_ipv6(skb, bat_priv);
+	default:
+		return -EINVAL;
+	}
+}
+
+/**
+ * batadv_mcast_forw_mode - 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 the forwarding mode as enum batadv_forw_mode.
+ */
+enum batadv_forw_mode
+batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
+{
+	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
+	int ret;
+
+	ret = batadv_mcast_forw_mode_check(skb, bat_priv);
+	if (ret == -ENOMEM)
+		return BATADV_FORW_NONE;
+	else if (ret == -EINVAL)
 		return BATADV_FORW_ALL;
 
-	count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest,
+	ret = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest,
 					    BATADV_NO_FLAGS);
 
-	switch (count) {
+	switch (ret) {
 	case 0:
 		return BATADV_FORW_NONE;
 	case 1:
@@ -263,6 +348,28 @@ batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
 }
 
 /**
+ * batadv_mcast_tvlv_update_flag_counter - update the counter of a flag
+ * @flag: the flag we want to update counters for
+ * @flag_counter: the counter we might update
+ * @new_flags: the new capability bitset of a node
+ * @old_flags: the current, to be updated bitset of a node
+ *
+ * Update the given flag_counter with the help of the new flag information
+ * of a node to reflect how many nodes have the given flag unset.
+ */
+static void batadv_mcast_tvlv_update_flag_counter(uint8_t flag,
+						  atomic_t *flag_counter,
+						  uint8_t new_flags,
+						  int old_flags)
+{
+	if (!(new_flags & flag) &&
+	    (old_flags & flag || old_flags & BATADV_UNINIT_FLAGS))
+		atomic_inc(flag_counter);
+	else if (new_flags & flag && !(old_flags & flag))
+		atomic_dec(flag_counter);
+}
+
+/**
  * 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
@@ -285,13 +392,14 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 	    (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 ||
-	     orig->mcast_flags & BATADV_UNINIT_FLAGS))
-		atomic_inc(&bat_priv->mcast.num_no_mla);
-	else if (mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT &&
-		   !(orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT))
-		atomic_dec(&bat_priv->mcast.num_no_mla);
+	batadv_mcast_tvlv_update_flag_counter(
+					BATADV_MCAST_LISTENER_ANNOUNCEMENT,
+					&bat_priv->mcast.num_no_mla,
+					mcast_flags, orig->mcast_flags);
+	batadv_mcast_tvlv_update_flag_counter(
+					BATADV_MCAST_HAS_NO_BRIDGE,
+					&bat_priv->mcast.num_has_bridge,
+					mcast_flags, orig->mcast_flags);
 
 	orig->mcast_flags = mcast_flags;
 }
diff --git a/packet.h b/packet.h
index 1090977..d10dc17 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -94,6 +94,7 @@ enum batadv_icmp_packettype {
 /* multicast capabilities */
 enum batadv_mcast_flags {
 	BATADV_MCAST_LISTENER_ANNOUNCEMENT = BIT(0),
+	BATADV_MCAST_HAS_NO_BRIDGE = BIT(1),
 };
 
 /* tt data subtypes */
diff --git a/soft-interface.c b/soft-interface.c
index 8fad00e..73c0de1 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -663,6 +663,7 @@ static int batadv_softif_init_late(struct net_device *dev)
 #ifdef CONFIG_BATMAN_ADV_MCAST
 	atomic_set(&bat_priv->multicast_mode, 1);
 	atomic_set(&bat_priv->mcast.num_no_mla, 0);
+	atomic_set(&bat_priv->mcast.num_has_bridge, 0);
 #endif
 	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
 	atomic_set(&bat_priv->gw_sel_class, 20);
diff --git a/types.h b/types.h
index 7535c69..4083746 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -508,6 +508,7 @@ struct batadv_priv_dat {
 struct batadv_priv_mcast {
 	struct hlist_head mla_list;
 	atomic_t num_no_mla;
+	atomic_t num_has_bridge;
 };
 #endif
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [B.A.T.M.A.N.] Basic Multicast Optimizations
  2013-07-03 22:03 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
                   ` (3 preceding siblings ...)
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 4/4] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support Linus Lüssing
@ 2013-07-04  5:06 ` Linus Lüssing
  4 siblings, 0 replies; 10+ messages in thread
From: Linus Lüssing @ 2013-07-04  5:06 UTC (permalink / raw)
  To: b.a.t.m.a.n

And the sysfs file 'mcast_group_awareness' got renamed to
'multicast_mode'. The missing ABI documentation for this file was
added, too.

On Thu, Jul 04, 2013 at 12:03:18AM +0200, Linus Lüssing wrote:
> This is the seventh revision of the basic multicast optimization patches.
> 
> It adds the style changes as suggested by Marek via eMail.
> 
> It also adds various suggestion from IRC (for instance adding kerneldoc for the
> functions touched in translation-table.c, making the counter an atomic
> one to be on the safe side for some "exotic" targets).
> 
> 
> PATCHv7 4/4 is a new one and the result of a longer discussion on IRC:
> Whether to add some support for some IPv4 addresses already for the cost of
> a slightly larger complexity both code and documentation wise
> - for a case which is uncommon for most mesh networks anyway: a mesh
> with no bridges on any node involved.
> 
> So the fourth patch is sort of an RFC patch one, too (although the consense/
> tendency on IRC so far was to add and support this case).
> 
> If everyone is fine with that concept, then I can also easily merge it
> into patches 2/4 and 3/4 (if that is desired).
> 
> Cheers, Linus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv7 2/4] batman-adv: Announce new capability via multicast TVLV
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 2/4] batman-adv: Announce new capability via multicast TVLV Linus Lüssing
@ 2013-07-06 10:57   ` Marek Lindner
  2013-07-06 12:38     ` Linus Lüssing
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Lindner @ 2013-07-06 10:57 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Thursday, July 04, 2013 06:03:20 Linus Lüssing wrote:
> diff --git a/multicast.c b/multicast.c
> index 7ea19ab..4af4bc9 100644
> --- a/net/batman-adv/multicast.c
> +++ b/net/batman-adv/multicast.c
> @@ -169,13 +169,30 @@ void batadv_mcast_mla_tt_update(struct batadv_priv
> *bat_priv) struct net_device *soft_iface = bat_priv->soft_iface;
>         struct hlist_head mcast_list = HLIST_HEAD_INIT;
>         int ret;
> +       static bool enabled;
> +       uint8_t mcast_flags;
>  
>         /* 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->multicast_mode) ||
> -           bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT)
> +           bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT) {
> +               if (enabled) {
> +                       batadv_tvlv_container_unregister(bat_priv,
> +                                                        BATADV_TVLV_MCAST,
> 1); +                       enabled = false;
> +               }
> +
>                 goto update;
> +       }
> +
> +       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_softif_get(soft_iface, &mcast_list);
>         if (ret < 0)

Is there no better way than using a static variable ? Apart from bad 
readibility this totally breaks when using several batX interfaces with 
different settings.


> +/* multicast capabilities */
> +enum batadv_mcast_flags {
> +       BATADV_MCAST_LISTENER_ANNOUNCEMENT = BIT(0),
> +};

Didn't we discuss that using feature names is better ? We talked about things 
like UNICAST, TRACKER, etc ?

Cheers,
Marek

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv7 2/4] batman-adv: Announce new capability via multicast TVLV
  2013-07-06 10:57   ` Marek Lindner
@ 2013-07-06 12:38     ` Linus Lüssing
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Lüssing @ 2013-07-06 12:38 UTC (permalink / raw)
  To: Marek Lindner; +Cc: b.a.t.m.a.n

On Sat, Jul 06, 2013 at 06:57:44PM +0800, Marek Lindner wrote:
> On Thursday, July 04, 2013 06:03:20 Linus Lüssing wrote:
> > diff --git a/multicast.c b/multicast.c
> > index 7ea19ab..4af4bc9 100644
> > --- a/net/batman-adv/multicast.c
> > +++ b/net/batman-adv/multicast.c
> > @@ -169,13 +169,30 @@ void batadv_mcast_mla_tt_update(struct batadv_priv
> > *bat_priv) struct net_device *soft_iface = bat_priv->soft_iface;
> >         struct hlist_head mcast_list = HLIST_HEAD_INIT;
> >         int ret;
> > +       static bool enabled;
> > +       uint8_t mcast_flags;
> >  
> >         /* 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->multicast_mode) ||
> > -           bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT)
> > +           bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT) {
> > +               if (enabled) {
> > +                       batadv_tvlv_container_unregister(bat_priv,
> > +                                                        BATADV_TVLV_MCAST,
> > 1); +                       enabled = false;
> > +               }
> > +
> >                 goto update;
> > +       }
> > +
> > +       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_softif_get(soft_iface, &mcast_list);
> >         if (ret < 0)
> 
> Is there no better way than using a static variable ? Apart from bad 
> readibility this totally breaks when using several batX interfaces with 
> different settings.

Ah, right, didn't think of multiple batX interfaces, that's a bug
indeed. I'm going to fix that.

What do you think about adding new tvlv API like:

---
bool batadv_tvlv_container_is_registered(struct batadv_priv *bat_priv,
					 uint8_t type, uint8_t version)
{
	struct batadv_tvlv_container *tvlv;
	bool ret;

	spin_lock_bh(&bat_priv->tvlv.container_list_lock);
	tvlv = batadv_tvlv_container_get(bat_priv, type, version);
	ret = tvlv ? true : false;
	batadv_tvlv_container_free_ref(tvlv);
	spin_unlock_bh(&bat_priv->tvlv.container_list_lock);

	return ret;
}
----

The disadvantage is that this is a little heavier as it will
perform a list lookup and holds a spin-lock.

If we were going to use something like bat_priv->mcast.tvlv_container_enabled
instead, that would be lighter but less readable.

> 
> 
> > +/* multicast capabilities */
> > +enum batadv_mcast_flags {
> > +       BATADV_MCAST_LISTENER_ANNOUNCEMENT = BIT(0),
> > +};
> 
> Didn't we discuss that using feature names is better ? We talked about things 
> like UNICAST, TRACKER, etc ?

Hm, I only remember discussing the name for sysfs. For the flags
I thought we were only using UNICAST, TRACKER, etc. as
abbreviations for the current names in our IRC discussion, I didn't
interpret that as a suggestion to change these names.

Anyways, let's discuss a renaming of the flags then. I don't like a change
like:

BATADV_MCAST_LISTENER_ANNOUNCEMENT -> BATADV_MCAST_UNICAST

Because for one thing, it isn't up to the node announcing this
flag to deceide whether another, sending node is able use unicast.
Whether unicast can be used depends on a lot more factors.

For another thing this flag is not only adding a unicast feature if
all these factors are met, but also adds the dropping feature
introduced by this patchset.

I'd rather keep the naming for the multicast flags to reflect the
capabilities and settings of the local, announcing node.

Cheers, Linus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv7 4/4] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support
  2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 4/4] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support Linus Lüssing
@ 2013-07-12  9:12   ` Simon Wunderlich
  2013-07-14 16:25     ` Linus Lüssing
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Wunderlich @ 2013-07-12  9:12 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

On Thu, Jul 04, 2013 at 12:03:22AM +0200, Linus Lüssing wrote:
> [...]
> +enum batadv_forw_mode
> +batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
> +{
> +	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
> +	int ret;
> +
> +	ret = batadv_mcast_forw_mode_check(skb, bat_priv);
> +	if (ret == -ENOMEM)
> +		return BATADV_FORW_NONE;
> +	else if (ret == -EINVAL)

Shouldn't this better be "ret < 0", in case you add more error cases later? 

>  		return BATADV_FORW_ALL;
>  
> -	count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest,
> +	ret = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest,
>  					    BATADV_NO_FLAGS);
>  
> -	switch (count) {
> +	switch (ret) {
>  	case 0:
>  		return BATADV_FORW_NONE;
>  	case 1:
> @@ -263,6 +348,28 @@ batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
>  }
>  
>  /**
> + * batadv_mcast_tvlv_update_flag_counter - update the counter of a flag
> + * @flag: the flag we want to update counters for
> + * @flag_counter: the counter we might update
> + * @new_flags: the new capability bitset of a node
> + * @old_flags: the current, to be updated bitset of a node
> + *
> + * Update the given flag_counter with the help of the new flag information
> + * of a node to reflect how many nodes have the given flag unset.
> + */
> +static void batadv_mcast_tvlv_update_flag_counter(uint8_t flag,
> +						  atomic_t *flag_counter,
> +						  uint8_t new_flags,
> +						  int old_flags)

Why are parts of the bitsets uint8_t, some int?

Also, this function should go into one of the previous patches, this has nothing to do with
IPv4 but is just a refactoring.

> +{
> +	if (!(new_flags & flag) &&
> +	    (old_flags & flag || old_flags & BATADV_UNINIT_FLAGS))
> +		atomic_inc(flag_counter);
> +	else if (new_flags & flag && !(old_flags & flag))
> +		atomic_dec(flag_counter);
> +}
> +
> +/**
>   * 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
> @@ -285,13 +392,14 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
>  	    (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 ||
> -	     orig->mcast_flags & BATADV_UNINIT_FLAGS))
> -		atomic_inc(&bat_priv->mcast.num_no_mla);
> -	else if (mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT &&
> -		   !(orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT))
> -		atomic_dec(&bat_priv->mcast.num_no_mla);
> +	batadv_mcast_tvlv_update_flag_counter(
> +					BATADV_MCAST_LISTENER_ANNOUNCEMENT,
> +					&bat_priv->mcast.num_no_mla,
> +					mcast_flags, orig->mcast_flags);
> +	batadv_mcast_tvlv_update_flag_counter(
> +					BATADV_MCAST_HAS_NO_BRIDGE,
> +					&bat_priv->mcast.num_has_bridge,
> +					mcast_flags, orig->mcast_flags);

You should find a shorter name, the alignment looks really ugly. :)

Cheers,
	Simon

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv7 4/4] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support
  2013-07-12  9:12   ` Simon Wunderlich
@ 2013-07-14 16:25     ` Linus Lüssing
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Lüssing @ 2013-07-14 16:25 UTC (permalink / raw)
  To: Simon Wunderlich
  Cc: The list for a Better Approach To Mobile Ad-hoc Networking

On Fri, Jul 12, 2013 at 11:12:16AM +0200, Simon Wunderlich wrote:
> On Thu, Jul 04, 2013 at 12:03:22AM +0200, Linus Lüssing wrote:
> > [...]
> > +enum batadv_forw_mode
> > +batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
> > +{
> > +	struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
> > +	int ret;
> > +
> > +	ret = batadv_mcast_forw_mode_check(skb, bat_priv);
> > +	if (ret == -ENOMEM)
> > +		return BATADV_FORW_NONE;
> > +	else if (ret == -EINVAL)
> 
> Shouldn't this better be "ret < 0", in case you add more error cases later?

Good idea! Looks better, I'm adding that.

> 
> >  		return BATADV_FORW_ALL;
> >  
> > -	count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest,
> > +	ret = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest,
> >  					    BATADV_NO_FLAGS);
> >  
> > -	switch (count) {
> > +	switch (ret) {
> >  	case 0:
> >  		return BATADV_FORW_NONE;
> >  	case 1:
> > @@ -263,6 +348,28 @@ batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv)
> >  }
> >  
> >  /**
> > + * batadv_mcast_tvlv_update_flag_counter - update the counter of a flag
> > + * @flag: the flag we want to update counters for
> > + * @flag_counter: the counter we might update
> > + * @new_flags: the new capability bitset of a node
> > + * @old_flags: the current, to be updated bitset of a node
> > + *
> > + * Update the given flag_counter with the help of the new flag information
> > + * of a node to reflect how many nodes have the given flag unset.
> > + */
> > +static void batadv_mcast_tvlv_update_flag_counter(uint8_t flag,
> > +						  atomic_t *flag_counter,
> > +						  uint8_t new_flags,
> > +						  int old_flags)
> 
> Why are parts of the bitsets uint8_t, some int?

Because the locally stored mcast_flags variable is bigger than the tvlv
mcast_flags (because of the BATADV_UNINIT_FLAGS).

> 
> Also, this function should go into one of the previous patches, this has nothing to do with
> IPv4 but is just a refactoring.

Hm, it's just moving some code to a new function because the
original function got too long for my taste now. Anyways, ok, I'm
going to introduce that function in the previous patch already.

> 
> > +{
> > +	if (!(new_flags & flag) &&
> > +	    (old_flags & flag || old_flags & BATADV_UNINIT_FLAGS))
> > +		atomic_inc(flag_counter);
> > +	else if (new_flags & flag && !(old_flags & flag))
> > +		atomic_dec(flag_counter);
> > +}
> > +
> > +/**
> >   * 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
> > @@ -285,13 +392,14 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
> >  	    (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 ||
> > -	     orig->mcast_flags & BATADV_UNINIT_FLAGS))
> > -		atomic_inc(&bat_priv->mcast.num_no_mla);
> > -	else if (mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT &&
> > -		   !(orig->mcast_flags & BATADV_MCAST_LISTENER_ANNOUNCEMENT))
> > -		atomic_dec(&bat_priv->mcast.num_no_mla);
> > +	batadv_mcast_tvlv_update_flag_counter(
> > +					BATADV_MCAST_LISTENER_ANNOUNCEMENT,
> > +					&bat_priv->mcast.num_no_mla,
> > +					mcast_flags, orig->mcast_flags);
> > +	batadv_mcast_tvlv_update_flag_counter(
> > +					BATADV_MCAST_HAS_NO_BRIDGE,
> > +					&bat_priv->mcast.num_has_bridge,
> > +					mcast_flags, orig->mcast_flags);
> 
> You should find a shorter name, the alignment looks really ugly. :)

Oki doki, fair point :).

> 
> Cheers,
> 	Simon

Cheers, Linus

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-07-14 16:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-03 22:03 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing
2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 1/4] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing
2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 2/4] batman-adv: Announce new capability via multicast TVLV Linus Lüssing
2013-07-06 10:57   ` Marek Lindner
2013-07-06 12:38     ` Linus Lüssing
2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 3/4] batman-adv: Modified forwarding behaviour for multicast packets Linus Lüssing
2013-07-03 22:03 ` [B.A.T.M.A.N.] [PATCHv7 4/4] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support Linus Lüssing
2013-07-12  9:12   ` Simon Wunderlich
2013-07-14 16:25     ` Linus Lüssing
2013-07-04  5:06 ` [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).