* [B.A.T.M.A.N.] Basic Multicast Optimizations
@ 2013-06-10 6:28 Linus Lüssing
2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Linus Lüssing @ 2013-06-10 6:28 UTC (permalink / raw)
To: b.a.t.m.a.n
This is the third revision of the basic multicast optimization patches.
It includes one functional and some compat fixes, and some style improvements,
thanks to Simons feedback:
PATCHv3 1/3:
* the pmc_rcu functions got removed - they weren't actually used in the
submitted revision - so there's no more patch for net to export
for_each_pmc_rcu() needed anymore. Should also fix Simon's compat error.
* Compat code for netdev_for_each_mc_addr() added.
(I needed to introduce a batadv_hw_addr because I wasn't able to achieve
backwards compatibility with a netdev_hw_addr - my "official reasoning" will
be that netdev_hw_addr is unnecessarily bloated, struct batadv_hw_addr
has just the right size to do the job)
* Compat fix for IFF_BRIDGE_PORT
* Removed primary_if->soft_iface in batadv_mcast_mla_tt_update() as we
can always use bat_priv->soft_iface instead.
PATCHv3 2/3
* Renamed num_non_aware to a more specific num_no_mla - because we will need
things like a num_no_tracker for instance with the multicast tracker feature
in the future.
* Fixed removal of orig_node's without BATADV_MCAST_LISTENER_ANNOUNCEMENT
flag, update num_no_mla properly now.
PATCHv3 3/3
* Renamed batadv_mcast_flood() to batadv_mcast_forw_mode() because it does not
only show whether to flood or not (=drop), but also whether to forward via
unicast. In the future with the multicast tracker feature a fourth return value
is going to be added, making the name "batadv_mcast_flood()" even less fitting.
* Simon's style suggestions for batadv_mcast_forw_mode() and
batadv_interface_tx().
* batadv_mcast_forw_mode() now returns an enum (thanks to Marek and Antonio for
the suggestion).
I did not change:
> I don't quite understand why you return -1, maybe the packet could still
> be forwarded even if it could not be pulled?
Because if it returns -1 then something is wrong, for instance we could be
out of memory. If we are out of memory then we probably won't be able to
forward any packet. Also we shouldn't allocate any more memory for one thing
but I think it is also better to drop/free packets to save some memory
to increase the possibility of the system to recover without crashing.
Cheers, Linus
^ permalink raw reply [flat|nested] 9+ messages in thread* [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table 2013-06-10 6:28 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing @ 2013-06-10 6:28 ` Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Announce new capability via multicast TVLV Linus Lüssing ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Linus Lüssing @ 2013-06-10 6:28 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 | 209 ++++++++++++++++++++++++++++++++++++++++++++++++ multicast.h | 43 ++++++++++ soft-interface.c | 3 + sysfs.c | 6 ++ translation-table.c | 22 ++++- types.h | 12 +++ 12 files changed, 321 insertions(+), 5 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 4f4aabb..f154c5b 100644 --- a/Makefile.kbuild +++ b/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_OPTIMIZATIONS) += multicast.o diff --git a/compat.h b/compat.h index dbf1926..ddc1b29 100644 --- a/compat.h +++ b/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..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 5d30fe0..fc5789c 100644 --- a/main.c +++ b/main.c @@ -32,6 +32,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" @@ -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 1f0b135..87db514 100644 --- a/main.h +++ b/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..d6d8b62 --- /dev/null +++ b/multicast.c @@ -0,0 +1,209 @@ +/* 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" + +struct batadv_hw_addr { + struct list_head list; + unsigned char addr[ETH_ALEN]; +}; + +/** + * 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; + 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); + 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 batadv_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 batadv_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 batadv_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 batadv_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 net_device *soft_iface = bat_priv->soft_iface; + struct list_head mcast_list; + int ret; + static bool enabled; + + INIT_LIST_HEAD(&mcast_list); + + /* Avoid attaching MLAs, if multicast optimization is disabled + * or there is a bridge on top of our soft interface (TODO) + */ + if (!atomic_read(&bat_priv->mcast_group_awareness) || + bat_priv->soft_iface->priv_flags & IFF_BRIDGE_PORT) { + if (enabled) + enabled = false; + + goto update; + } + + if (!enabled) + enabled = true; + + ret = batadv_mcast_mla_local_collect(soft_iface, &mcast_list); + if (ret < 0) + goto out; + +update: + batadv_mcast_mla_tt_clean(bat_priv, &mcast_list); + batadv_mcast_mla_tt_add(bat_priv, &mcast_list); + +out: + batadv_mcast_mla_collect_free(&mcast_list); +} + +/** + * 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 d4fdc61..52244b0 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -456,6 +456,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 b70ae52..cc4e7e5 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 7580ee5..0be7b15 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> @@ -277,14 +278,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; @@ -335,8 +338,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, @@ -922,6 +928,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, @@ -2348,6 +2359,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 304f3d0..8175424 100644 --- a/types.h +++ b/types.h @@ -501,6 +501,12 @@ struct batadv_priv_dat { }; #endif +#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS +struct batadv_priv_mcast { + struct list_head mla_list; +}; +#endif + /** * struct batadv_priv_nc - per mesh interface network coding private data * @work: work queue callback item for cleanup @@ -591,6 +597,9 @@ struct batadv_priv { #ifdef CONFIG_BATMAN_ADV_DAT atomic_t distributed_arp_table; #endif +#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS + atomic_t mcast_group_awareness; +#endif atomic_t gw_mode; atomic_t gw_sel_class; atomic_t orig_interval; @@ -625,6 +634,9 @@ struct batadv_priv { #ifdef CONFIG_BATMAN_ADV_DAT struct batadv_priv_dat dat; #endif +#ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS + struct batadv_priv_mcast mcast; +#endif #ifdef CONFIG_BATMAN_ADV_NC atomic_t network_coding; struct batadv_priv_nc nc; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Announce new capability via multicast TVLV 2013-06-10 6:28 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing @ 2013-06-10 6:28 ` Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Modified forwarding behaviour for multicast packets Linus Lüssing 2013-06-10 7:06 ` [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing 3 siblings, 0 replies; 9+ messages in thread From: Linus Lüssing @ 2013-06-10 6:28 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 ++++ multicast.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- multicast.h | 14 +++++++++++ originator.c | 6 +++++ packet.h | 7 ++++++ soft-interface.c | 1 + types.h | 4 ++++ 7 files changed, 102 insertions(+), 2 deletions(-) diff --git a/main.c b/main.c index fc5789c..968f77a 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 d6d8b62..6375347 100644 --- a/multicast.c +++ b/multicast.c @@ -170,6 +170,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); @@ -178,14 +179,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) @@ -200,10 +209,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) { + 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 3b68e3b..aa58e4b 100644 --- a/multicast.h +++ b/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_OPTIMIZATIONS */ #endif /* _NET_BATMAN_ADV_MULTICAST_H_ */ diff --git a/originator.c b/originator.c index a591dc5..5e827fa 100644 --- a/originator.c +++ b/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_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 e3ae41a..0d616f9 100644 --- a/packet.h +++ b/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 @@ -137,6 +142,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, @@ -144,6 +150,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 52244b0..bc0cc35 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -458,6 +458,7 @@ static int batadv_softif_init_late(struct net_device *dev) #endif #ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS atomic_set(&bat_priv->mcast_group_awareness, 1); + atomic_set(&bat_priv->mcast.num_no_mla, 0); #endif atomic_set(&bat_priv->ap_isolation, 0); atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF); diff --git a/types.h b/types.h index 8175424..0586693 100644 --- a/types.h +++ b/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_OPTIMIZATIONS + uint8_t mcast_flags; +#endif uint8_t capabilities; atomic_t last_ttvn; uint32_t tt_crc; @@ -504,6 +507,7 @@ struct batadv_priv_dat { #ifdef CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS struct batadv_priv_mcast { struct list_head mla_list; + atomic_t num_no_mla; }; #endif -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Modified forwarding behaviour for multicast packets 2013-06-10 6:28 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Announce new capability via multicast TVLV Linus Lüssing @ 2013-06-10 6:28 ` Linus Lüssing 2013-06-10 7:06 ` [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing 3 siblings, 0 replies; 9+ messages in thread From: Linus Lüssing @ 2013-06-10 6:28 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 | 49 ++++++++++++++++++++++++++++++++++++++ multicast.h | 23 ++++++++++++++++++ soft-interface.c | 11 +++++++++ translation-table.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ translation-table.h | 1 + 5 files changed, 150 insertions(+) diff --git a/multicast.c b/multicast.c index 6375347..e4bcdb2 100644 --- a/multicast.c +++ b/multicast.c @@ -21,6 +21,7 @@ #include "originator.h" #include "hard-interface.h" #include "translation-table.h" +#include "multicast.h" struct batadv_hw_addr { struct list_head list; @@ -209,6 +210,54 @@ 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->mcast_group_awareness)) + 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; + + count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest); + + 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 diff --git a/multicast.h b/multicast.h index aa58e4b..6388793 100644 --- a/multicast.h +++ b/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_OPTIMIZATIONS void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv); +enum batadv_forw_mode +batadv_mcast_forw_mode(struct sk_buff *skb, struct batadv_priv *bat_priv); + int batadv_mcast_init(struct batadv_priv *bat_priv); void batadv_mcast_free(struct batadv_priv *bat_priv); @@ -37,6 +54,12 @@ static inline void batadv_mcast_mla_tt_update(struct batadv_priv *bat_priv) return; } +static inline 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 bc0cc35..7ff5a02 100644 --- a/soft-interface.c +++ b/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" @@ -156,6 +157,7 @@ static int batadv_interface_tx(struct sk_buff *skb, bool do_bcast = false; uint32_t seqno; unsigned long brd_delay = 1; + enum batadv_forw_mode mode; if (atomic_read(&bat_priv->mesh_state) != BATADV_MESH_ACTIVE) goto dropped; @@ -218,6 +220,15 @@ static int batadv_interface_tx(struct sk_buff *skb, 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 */ diff --git a/translation-table.c b/translation-table.c index 0be7b15..d14ee64 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [B.A.T.M.A.N.] Basic Multicast Optimizations 2013-06-10 6:28 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing ` (2 preceding siblings ...) 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Modified forwarding behaviour for multicast packets Linus Lüssing @ 2013-06-10 7:06 ` Linus Lüssing 3 siblings, 0 replies; 9+ messages in thread From: Linus Lüssing @ 2013-06-10 7:06 UTC (permalink / raw) To: b.a.t.m.a.n Hrm, forgot to add the v3 an the patch naming. And forgot to actually add -static inline batadv_forw_mode +static inline enum batadv_forw_mode to the according commit. Will send a v4 in a sec. Cheers, Linus On Mon, Jun 10, 2013 at 08:28:18AM +0200, Linus Lüssing wrote: > This is the third revision of the basic multicast optimization patches. > It includes one functional and some compat fixes, and some style improvements, > thanks to Simons feedback: > > PATCHv3 1/3: > * the pmc_rcu functions got removed - they weren't actually used in the > submitted revision - so there's no more patch for net to export > for_each_pmc_rcu() needed anymore. Should also fix Simon's compat error. > * Compat code for netdev_for_each_mc_addr() added. > (I needed to introduce a batadv_hw_addr because I wasn't able to achieve > backwards compatibility with a netdev_hw_addr - my "official reasoning" will > be that netdev_hw_addr is unnecessarily bloated, struct batadv_hw_addr > has just the right size to do the job) > * Compat fix for IFF_BRIDGE_PORT > * Removed primary_if->soft_iface in batadv_mcast_mla_tt_update() as we > can always use bat_priv->soft_iface instead. > > PATCHv3 2/3 > * Renamed num_non_aware to a more specific num_no_mla - because we will need > things like a num_no_tracker for instance with the multicast tracker feature > in the future. > * Fixed removal of orig_node's without BATADV_MCAST_LISTENER_ANNOUNCEMENT > flag, update num_no_mla properly now. > > PATCHv3 3/3 > * Renamed batadv_mcast_flood() to batadv_mcast_forw_mode() because it does not > only show whether to flood or not (=drop), but also whether to forward via > unicast. In the future with the multicast tracker feature a fourth return value > is going to be added, making the name "batadv_mcast_flood()" even less fitting. > * Simon's style suggestions for batadv_mcast_forw_mode() and > batadv_interface_tx(). > * batadv_mcast_forw_mode() now returns an enum (thanks to Marek and Antonio for > the suggestion). > > I did not change: > > I don't quite understand why you return -1, maybe the packet could still > > be forwarded even if it could not be pulled? > > Because if it returns -1 then something is wrong, for instance we could be > out of memory. If we are out of memory then we probably won't be able to > forward any packet. Also we shouldn't allocate any more memory for one thing > but I think it is also better to drop/free packets to save some memory > to increase the possibility of the system to recover without crashing. > > Cheers, Linus > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [B.A.T.M.A.N.] Basic Multicast Optimizations @ 2013-05-11 17:23 Linus Lüssing 2013-05-11 17:23 ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing 0 siblings, 1 reply; 9+ messages in thread From: Linus Lüssing @ 2013-05-11 17:23 UTC (permalink / raw) To: b.a.t.m.a.n This set of patches is the first one for a more efficient, group aware multicast forwarding infrastructure in batman-adv. This initial set mainly consists of the integration of announcing the location of multicast listeners via the translation table mechanism to be able to find out which batman-adv nodes are actually interested in certain multicast traffic. As well as the signalizing of this announcement capability via a new multicast TVLV. Finally some basic multicast forwarding opitimizations are introduced: If all nodes signalized the MLA capability then link-local IPv6 traffic will be dropped if there is no interested listener or gets forwarded via a batman-adv unicast packet if there is just one node interested in the data. For now, these optimizations only apply if all nodes in the mesh have no bridge interface on top their bat0 interface. However making it possible with bridges, too, and other features are on the roadmap. See the according wiki page for details [1]. [1]: http://www.open-mesh.org/projects/batman-adv/wiki/Multicast-ideas-updated#Roadmap ^ permalink raw reply [flat|nested] 9+ messages in thread
* [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table 2013-05-11 17:23 Linus Lüssing @ 2013-05-11 17:23 ` Linus Lüssing 2013-05-11 22:55 ` Antonio Quartulli 0 siblings, 1 reply; 9+ messages in thread From: Linus Lüssing @ 2013-05-11 17:23 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.c | 21 +++++ compat.h | 18 ++++ gen-compat-autoconf.sh | 1 + main.c | 6 ++ main.h | 9 ++ multicast.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++ multicast.h | 43 ++++++++++ soft-interface.c | 3 + sysfs.c | 6 ++ translation-table.c | 17 +++- types.h | 12 +++ 13 files changed, 358 insertions(+), 3 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..39c683d 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 +#define UINT8_MAX 255 + +/* identical to the one used in net/ipv4/igmp.c */ +#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..88bde61 --- /dev/null +++ b/multicast.c @@ -0,0 +1,222 @@ +/* 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" + +/* should match batadv_ogm_packet's mcast_num_mla */ +#define BATADV_MLA_MAX UINT8_MAX + +/** + * batadv_mcast_mla_local_collect - Collects local multicast listeners + * @dev: The device to collect multicast addresses from + * @mcast_list: A list to put found addresses into + * @num_mla_max: The maximum number of items to add + * + * Collects up to num_mla_max multicast addresses of the local multicast + * listeners on the given interface, dev, in the given mcast_list. + * + * Returns -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, + int num_mla_max) { + struct netdev_hw_addr *mc_list_entry, *new; + int num_mla = 0, ret = 0; + + netif_addr_lock_bh(dev); + netdev_for_each_mc_addr(mc_list_entry, dev) { + if (num_mla >= num_mla_max) { + pr_warn("Too many local multicast listener announcements here, just adding %i\n", + num_mla_max); + break; + } + + new = kmalloc(sizeof(struct netdev_hw_addr), GFP_ATOMIC); + if (!new) { + ret = -ENOMEM; + break; + } + + memcpy(&new->addr, &mc_list_entry->addr, ETH_ALEN); + list_add(&new->list, mcast_list); + num_mla++; + } + netif_addr_unlock_bh(dev); + + return ret < 0 ? ret : num_mla; +} + +/** + * batadv_mcast_mla_is_duplicate - Checks whether an address is in a list + * @mcast_addr: The multicast address to check + * @mcast_list: The list with multicast addresses to search in + * + * Returns 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 (!memcmp(mcast_entry->addr, mcast_addr, ETH_ALEN)) + return true; + + return false; +} + +/** + * batadv_mcast_mla_collect_free - Frees a list of multicast addresses + * @mcast_list: The list to free + * + * Removes and frees 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 - Cleans 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 + * + * This method retracts the announcement of any multicast listener from the + * translation table except the ones listed in the given mcast_list. + */ +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 (batadv_mcast_mla_is_duplicate(mcast_entry->addr, + mcast_list)) + continue; + + batadv_tt_local_remove(bat_priv, mcast_entry->addr, + "mcast TT outdated", 0); + + list_del(&mcast_entry->list); + kfree(mcast_entry); + } +} + +/** + * batadv_mcast_mla_tt_add - Adds multicast listener announcements + * @soft_iface: the soft interface where MLAs get added to + * @mcast_list: A list of addresses which are going to get added + * + * Adds 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 net_device *soft_iface, + struct list_head *mcast_list) +{ + struct netdev_hw_addr *mcast_entry, *tmp; + struct batadv_priv *bat_priv = netdev_priv(soft_iface); + + 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(soft_iface, mcast_entry->addr, + BATADV_NULL_IFINDEX); + list_move_tail(&mcast_entry->list, &bat_priv->mcast.mla_list); + } +} + +/** + * batadv_mcast_mla_tt_update - Updates the own MLAs + * @bat_priv: the bat priv with all the soft interface information + * + * Updates the own multicast listener announcements in the translation + * table. Also takes 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, + BATADV_MLA_MAX); + if (ret < 0) + goto out; + +update: + batadv_mcast_mla_tt_clean(bat_priv, &mcast_list); + batadv_mcast_mla_tt_add(soft_iface, &mcast_list); + +out: + if (primary_if) + batadv_hardif_free_ref(primary_if); + + batadv_mcast_mla_collect_free(&mcast_list); +} + +/** + * batadv_mcast_free - Frees the multicast optimizaitons structures + * @bat_priv: the bat priv with all the soft interface information + */ +void batadv_mcast_free(struct batadv_priv *bat_priv) +{ + struct list_head mcast_list; + + INIT_LIST_HEAD(&mcast_list); + + batadv_mcast_mla_tt_clean(bat_priv, &mcast_list); +} 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..37e7d47 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> @@ -281,7 +282,8 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, bool roamed_back = false; tt_local = batadv_tt_local_hash_find(bat_priv, addr); - tt_global = batadv_tt_global_hash_find(bat_priv, addr); + tt_global = is_multicast_ether_addr(addr) ? NULL : + batadv_tt_global_hash_find(bat_priv, addr); if (tt_local) { tt_local->last_seen = jiffies; @@ -332,8 +334,10 @@ 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 +923,10 @@ 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 +2354,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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table 2013-05-11 17:23 ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing @ 2013-05-11 22:55 ` Antonio Quartulli 2013-05-16 18:16 ` Linus Lüssing 0 siblings, 1 reply; 9+ messages in thread From: Antonio Quartulli @ 2013-05-11 22:55 UTC (permalink / raw) To: The list for a Better Approach To Mobile Ad-hoc Networking [-- Attachment #1: Type: text/plain, Size: 9242 bytes --] Hey Linus, here you have some comments inline On Sat, May 11, 2013 at 07:23:25PM +0200, Linus Lüssing wrote: > diff --git a/main.h b/main.h > index 795345f..39c683d 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 > > +#define UINT8_MAX 255 I think you should define it as 255U, otherwise it would get the int type and may generate warnings somewhere (or also unexpected result sometime..) > + > +/* identical to the one used in net/ipv4/igmp.c */ > +#define for_each_pmc_rcu(in_dev, pmc) \ > + for (pmc = rcu_dereference(in_dev->mc_list); \ > + pmc != NULL; \ > + pmc = rcu_dereference(pmc->next_rcu)) > + Since it is exactly the same code reported in net/ipv4/igmp.c and since it is a define, don't you thin kit is worth trying to send a patch to netdev to move the define in a proper header file so that we/everybody can re-use it? > + > +/** > + * batadv_mcast_mla_local_collect - Collects local multicast listeners please start the description with a small letter (like we do everywhere..just keep the same style) > + * @dev: The device to collect multicast addresses from don't put tabs between the name of the arg and the description. don't use the capital letter. Just: @dev: the device.. would be nice :) > +static int batadv_mcast_mla_local_collect(struct net_device *dev, > + struct list_head *mcast_list, > + int num_mla_max) { The opening parenthesis of a function must go at the beginning of the next line. > + struct netdev_hw_addr *mc_list_entry, *new; > + int num_mla = 0, ret = 0; > + > + netif_addr_lock_bh(dev); > + netdev_for_each_mc_addr(mc_list_entry, dev) { > + if (num_mla >= num_mla_max) { > + pr_warn("Too many local multicast listener announcements here, just adding %i\n", > + num_mla_max); why pr_warn and not just a debug message? Is this a symptom of a possible malfunctioning? > + break; > + } > + > + new = kmalloc(sizeof(struct netdev_hw_addr), GFP_ATOMIC); please use sizeof(*new) here. we agreed on this some time ago. This helps because if we decide to change the type of new in the future then we do not need to change all the kmalloc invocations. > +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 (!memcmp(mcast_entry->addr, mcast_addr, ETH_ALEN)) in main.h we have batadv_compare_eth(), you can re-use it here > +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 (batadv_mcast_mla_is_duplicate(mcast_entry->addr, > + mcast_list)) > + continue; > + > + batadv_tt_local_remove(bat_priv, mcast_entry->addr, > + "mcast TT outdated", 0); ^^^ parameter is bool, please use "false" instead of 0 > +static void batadv_mcast_mla_tt_add(struct net_device *soft_iface, > + struct list_head *mcast_list) > +{ > + struct netdev_hw_addr *mcast_entry, *tmp; > + struct batadv_priv *bat_priv = netdev_priv(soft_iface); why not simply pass bat_priv as first argument (as you do in all the other functions) and then use bat_priv->soft_iface when needed ? (this soft_iface member has been added recently to save code for lazy people :)) > +/** > + * batadv_mcast_mla_tt_update - Updates the own MLAs > + * @bat_priv: the bat priv with all the soft interface information > + * > + * Updates the own multicast listener announcements in the translation For some reason we agreed on not using the third person while describing the function. For consistency I'd suggest you to do the same your kernel doc. > + * table. Also takes 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) */ /* comments should * be closed * like this */ /* not like * this */ (this is something checkpatch should also tell you (but I guess only when your code is in the net/ folder inside the kernel tree...yes it is a netdev rule only :)) > +/** > + * batadv_mcast_free - Frees the multicast optimizaitons structures > + * @bat_priv: the bat priv with all the soft interface information > + */ > +void batadv_mcast_free(struct batadv_priv *bat_priv) > +{ > + struct list_head mcast_list; > + > + INIT_LIST_HEAD(&mcast_list); > + > + batadv_mcast_mla_tt_clean(bat_priv, &mcast_list); can't you make batadv_mcast_mla_tt_clean() take a NULL second argument and in that case skip the check in the loop? would be cleaner I guess? what do you think? > diff --git a/translation-table.c b/translation-table.c > index df6b5cd..37e7d47 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> > > @@ -281,7 +282,8 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, > bool roamed_back = false; > > tt_local = batadv_tt_local_hash_find(bat_priv, addr); > - tt_global = batadv_tt_global_hash_find(bat_priv, addr); > + tt_global = is_multicast_ether_addr(addr) ? NULL : > + batadv_tt_global_hash_find(bat_priv, addr); what about: tt_global = NULL; (you can put this directly in the declaration) if (!is_multicast_ether_addr(addr)) tt_global = batadv_tt_global_hash_find(bat_priv, addr); it is a bit easier to read, no? > > if (tt_local) { > tt_local->last_seen = jiffies; > @@ -332,8 +334,10 @@ 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 */ comment style like above > + 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 +923,10 @@ 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 +2354,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 Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto "Che" Guevara [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table 2013-05-11 22:55 ` Antonio Quartulli @ 2013-05-16 18:16 ` Linus Lüssing 2013-05-16 19:36 ` Antonio Quartulli 0 siblings, 1 reply; 9+ messages in thread From: Linus Lüssing @ 2013-05-16 18:16 UTC (permalink / raw) To: The list for a Better Approach To Mobile Ad-hoc Networking Hi Antonio, On Sun, May 12, 2013 at 12:55:25AM +0200, Antonio Quartulli wrote: > Hey Linus, > > here you have some comments inline > > On Sat, May 11, 2013 at 07:23:25PM +0200, Linus Lüssing wrote: > > diff --git a/main.h b/main.h > > index 795345f..39c683d 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 > > > > +#define UINT8_MAX 255 > > I think you should define it as 255U, otherwise it would get the int type and > may generate warnings somewhere (or also unexpected result sometime..) ok > > > + > > +/* identical to the one used in net/ipv4/igmp.c */ > > +#define for_each_pmc_rcu(in_dev, pmc) \ > > + for (pmc = rcu_dereference(in_dev->mc_list); \ > > + pmc != NULL; \ > > + pmc = rcu_dereference(pmc->next_rcu)) > > + > > Since it is exactly the same code reported in net/ipv4/igmp.c and since it is a > define, don't you thin kit is worth trying to send a patch to netdev to move the > define in a proper header file so that we/everybody can re-use it? Yes. Hm, I think I'd prefer adding the potential user in netdev first and then moving it to a header file. I'll add an explicit 'TODO' in the comment. > > > + > > +/** > > + * batadv_mcast_mla_local_collect - Collects local multicast listeners > > please start the description with a small letter (like we do everywhere..just > keep the same style) > > > + * @dev: The device to collect multicast addresses from > > don't put tabs between the name of the arg and the description. > don't use the capital letter. Just: > > @dev: the device.. > > would be nice :) ok > > > +static int batadv_mcast_mla_local_collect(struct net_device *dev, > > + struct list_head *mcast_list, > > + int num_mla_max) { > > The opening parenthesis of a function must go at the beginning of the next line. > > > + struct netdev_hw_addr *mc_list_entry, *new; > > + int num_mla = 0, ret = 0; > > + > > + netif_addr_lock_bh(dev); > > + netdev_for_each_mc_addr(mc_list_entry, dev) { > > + if (num_mla >= num_mla_max) { > > + pr_warn("Too many local multicast listener announcements here, just adding %i\n", > > + num_mla_max); > > why pr_warn and not just a debug message? Is this a symptom of a possible > malfunctioning? It was intended as a warning for a malfunction. But actually - I think I can remove it and the limitation of number of entries, it's more a relic of the old announcing approach. It should be up to the TT mechanism to issue such warnings now instead. > > > > + break; > > + } > > + > > + new = kmalloc(sizeof(struct netdev_hw_addr), GFP_ATOMIC); > > please use sizeof(*new) here. we agreed on this some time ago. This helps > because if we decide to change the type of new in the future then we do not need > to change all the kmalloc invocations. ok > > > +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 (!memcmp(mcast_entry->addr, mcast_addr, ETH_ALEN)) > > in main.h we have batadv_compare_eth(), you can re-use it here ok > > > +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 (batadv_mcast_mla_is_duplicate(mcast_entry->addr, > > + mcast_list)) > > + continue; > > + > > + batadv_tt_local_remove(bat_priv, mcast_entry->addr, > > + "mcast TT outdated", 0); > ^^^ > > parameter is bool, please use "false" instead of 0 ok > > > +static void batadv_mcast_mla_tt_add(struct net_device *soft_iface, > > + struct list_head *mcast_list) > > +{ > > + struct netdev_hw_addr *mcast_entry, *tmp; > > + struct batadv_priv *bat_priv = netdev_priv(soft_iface); > > why not simply pass bat_priv as first argument (as you do in all the other > functions) and then use bat_priv->soft_iface when needed ? > (this soft_iface member has been added recently to save code for lazy people :)) ok > > > +/** > > + * batadv_mcast_mla_tt_update - Updates the own MLAs > > + * @bat_priv: the bat priv with all the soft interface information > > + * > > + * Updates the own multicast listener announcements in the translation > > For some reason we agreed on not using the third person while describing the > function. For consistency I'd suggest you to do the same your kernel doc. I don't quite understand what you mean, talking in the first or second person seems strange, like "I update the multicast..." or "You update the multicast...". Do you have an example? > > > + * table. Also takes 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) */ > > /* comments should > * be closed > * like this > */ > > /* not like > * this */ > > (this is something checkpatch should also tell you (but I guess only when your > code is in the net/ folder inside the kernel tree...yes it is a netdev rule only > :)) Ah! Didn't know about that, thanks. Yes, I was only checking raw git-format-patch'ed patch files from the batman repository. > > > +/** > > + * batadv_mcast_free - Frees the multicast optimizaitons structures > > + * @bat_priv: the bat priv with all the soft interface information > > + */ > > +void batadv_mcast_free(struct batadv_priv *bat_priv) > > +{ > > + struct list_head mcast_list; > > + > > + INIT_LIST_HEAD(&mcast_list); > > + > > + batadv_mcast_mla_tt_clean(bat_priv, &mcast_list); > > can't you make batadv_mcast_mla_tt_clean() take a NULL second argument and in > that case skip the check in the loop? would be cleaner I guess? what do you > think? Sounds good, will change that! > > > > diff --git a/translation-table.c b/translation-table.c > > index df6b5cd..37e7d47 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> > > > > @@ -281,7 +282,8 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, > > bool roamed_back = false; > > > > tt_local = batadv_tt_local_hash_find(bat_priv, addr); > > - tt_global = batadv_tt_global_hash_find(bat_priv, addr); > > + tt_global = is_multicast_ether_addr(addr) ? NULL : > > + batadv_tt_global_hash_find(bat_priv, addr); > > what about: > > tt_global = NULL; (you can put this directly in the declaration) > if (!is_multicast_ether_addr(addr)) > tt_global = batadv_tt_global_hash_find(bat_priv, addr); > > it is a bit easier to read, no? Yes. > > > > > if (tt_local) { > > tt_local->last_seen = jiffies; > > @@ -332,8 +334,10 @@ 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 */ > > comment style like above > > > + 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 +923,10 @@ 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 +2354,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 > > > Cheers, > > > -- > Antonio Quartulli > > ..each of us alone is worth nothing.. > Ernesto "Che" Guevara Cheers, Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table 2013-05-16 18:16 ` Linus Lüssing @ 2013-05-16 19:36 ` Antonio Quartulli 0 siblings, 0 replies; 9+ messages in thread From: Antonio Quartulli @ 2013-05-16 19:36 UTC (permalink / raw) To: The list for a Better Approach To Mobile Ad-hoc Networking [-- Attachment #1: Type: text/plain, Size: 866 bytes --] On Thu, May 16, 2013 at 08:16:40PM +0200, Linus Lüssing wrote: > > > +/** > > > + * batadv_mcast_mla_tt_update - Updates the own MLAs > > > + * @bat_priv: the bat priv with all the soft interface information > > > + * > > > + * Updates the own multicast listener announcements in the translation > > > > For some reason we agreed on not using the third person while describing the > > function. For consistency I'd suggest you to do the same your kernel doc. > > I don't quite understand what you mean, talking in the first or > second person seems strange, like "I update the multicast..." or > "You update the multicast...". Do you have an example? It should look like: "Update the own multicast listener announcements in the translation" Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto "Che" Guevara [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-06-10 7:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-10 6:28 [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Announce new capability via multicast TVLV Linus Lüssing 2013-06-10 6:28 ` [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Modified forwarding behaviour for multicast packets Linus Lüssing 2013-06-10 7:06 ` [B.A.T.M.A.N.] Basic Multicast Optimizations Linus Lüssing -- strict thread matches above, loose matches on Subject: below -- 2013-05-11 17:23 Linus Lüssing 2013-05-11 17:23 ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Multicast Listener Announcements via Translation Table Linus Lüssing 2013-05-11 22:55 ` Antonio Quartulli 2013-05-16 18:16 ` Linus Lüssing 2013-05-16 19:36 ` Antonio Quartulli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox