Hi David,
here is a feature/cleanup pull request of batman-adv to go into net-next.
Please pull or let me know of any problem!
Thank you, Simon
The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a:
Linux 4.20-rc1 (2018-11-04 15:37:52 -0800)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batadv-next-for-davem-20181114
for you to fetch changes up to 016fd285682952b943641c074d1cc0d02b3e6889:
batman-adv: enable MCAST by default at compile time (2018-11-12 10:41:51 +0100)
---------------------------------------------------------------- This feature/cleanup patchset includes the following patches:
- Bump version strings, by Simon Wunderlich
- Fixup includes, by Sven Eckelmann (3 patches)
- Separate BATMAN_ADV_DEBUG from DEBUGFS, by Sven Eckelmann
- Fixup tracing log documentation, by Sven Eckelmann
- Use exclusive locks to secure netlink information dump transfers, by Sven Eckelmann (8 patches)
- Move CRC16 dependency, by Sven Eckelmann
- Enable MCAST by default, by Linus Luessing
---------------------------------------------------------------- Linus Lüssing (1): batman-adv: enable MCAST by default at compile time
Simon Wunderlich (1): batman-adv: Start new development cycle
Sven Eckelmann (14): batman-adv: Drop unused lockdep include batman-adv: Add includes for deprecation warning batman-adv: Improve includes for trace functionality batman-adv: Allow to use BATMAN_ADV_DEBUG without BATMAN_ADV_DEBUGFS batman-adv: Fix description for BATMAN_ADV_DEBUG batman-adv: Add inconsistent gateway netlink dump detection batman-adv: Add inconsistent hardif netlink dump detection batman-adv: Store modification counter via hash helpers batman-adv: Add inconsistent backbone netlink dump detection batman-adv: Add inconsistent claim netlink dump detection batman-adv: Add inconsistent dat netlink dump detection batman-adv: Add inconsistent local TT netlink dump detection batman-adv: Add inconsistent multicast netlink dump detection batman-adv: Move CRC16 dependency to BATMAN_ADV_BLA
net/batman-adv/Kconfig | 10 +++-- net/batman-adv/bat_iv_ogm.c | 25 ++++++----- net/batman-adv/bat_v.c | 26 +++++++---- net/batman-adv/bridge_loop_avoidance.c | 82 +++++++++++++++++++--------------- net/batman-adv/debugfs.c | 2 + net/batman-adv/distributed-arp-table.c | 42 +++++++++-------- net/batman-adv/gateway_client.c | 3 ++ net/batman-adv/hard-interface.c | 3 ++ net/batman-adv/hash.c | 2 + net/batman-adv/hash.h | 6 +++ net/batman-adv/log.c | 60 ++++++++++++++----------- net/batman-adv/main.c | 3 ++ net/batman-adv/main.h | 3 +- net/batman-adv/multicast.c | 51 +++++++++++---------- net/batman-adv/netlink.c | 24 +++++----- net/batman-adv/trace.c | 2 - net/batman-adv/trace.h | 6 +++ net/batman-adv/translation-table.c | 41 +++++++++-------- net/batman-adv/types.h | 5 ++- 19 files changed, 236 insertions(+), 160 deletions(-)
Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/main.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h index 2002b70e18db..b68a41190eb0 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -25,7 +25,7 @@ #define BATADV_DRIVER_DEVICE "batman-adv"
#ifndef BATADV_SOURCE_VERSION -#define BATADV_SOURCE_VERSION "2018.4" +#define BATADV_SOURCE_VERSION "2019.0" #endif
/* B.A.T.M.A.N. parameters */
From: Sven Eckelmann sven@narfation.org
The commit dee222c7b20c ("batman-adv: Move OGM rebroadcast stats to orig_ifinfo") removed all used functionality of the include linux/lockdep.h from batadv_iv_ogm.c.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bat_iv_ogm.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index d2227091029f..1d31ac84dec7 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -34,7 +34,6 @@ #include <linux/kernel.h> #include <linux/kref.h> #include <linux/list.h> -#include <linux/lockdep.h> #include <linux/netdevice.h> #include <linux/netlink.h> #include <linux/pkt_sched.h>
From: Sven Eckelmann sven@narfation.org
The commit 00caf6a2b318 ("batman-adv: Mark debugfs functionality as deprecated") introduced various messages to inform the user about the deprecation of the debugfs based functionality. The messages also include the context/task in which this problem was observed.
The datastructures and functions to access this information require special headers. These should be included directly instead of depending on a more complex and fragile include chain.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/debugfs.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c index 8b608a2e2653..d4a7702e48d8 100644 --- a/net/batman-adv/debugfs.c +++ b/net/batman-adv/debugfs.c @@ -19,6 +19,7 @@ #include "debugfs.h" #include "main.h"
+#include <asm/current.h> #include <linux/dcache.h> #include <linux/debugfs.h> #include <linux/err.h> @@ -27,6 +28,7 @@ #include <linux/fs.h> #include <linux/netdevice.h> #include <linux/printk.h> +#include <linux/sched.h> #include <linux/seq_file.h> #include <linux/stat.h> #include <linux/stddef.h>
From: Sven Eckelmann sven@narfation.org
The batadv_dbg trace event uses different functionality and datastructures which are not directly associated with the trace infrastructure. It should not be expected that the trace headers indirectly provide them and instead include the required headers directly.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/trace.c | 2 -- net/batman-adv/trace.h | 6 ++++++ 2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/trace.c b/net/batman-adv/trace.c index 3d57f9981f25..8e1024217cff 100644 --- a/net/batman-adv/trace.c +++ b/net/batman-adv/trace.c @@ -16,7 +16,5 @@ * along with this program; if not, see http://www.gnu.org/licenses/. */
-#include <linux/module.h> - #define CREATE_TRACE_POINTS #include "trace.h" diff --git a/net/batman-adv/trace.h b/net/batman-adv/trace.h index 3acda26a30ca..104784be94d7 100644 --- a/net/batman-adv/trace.h +++ b/net/batman-adv/trace.h @@ -21,7 +21,13 @@
#include "main.h"
+#include <linux/bug.h> +#include <linux/kernel.h> +#include <linux/netdevice.h> +#include <linux/percpu.h> +#include <linux/printk.h> #include <linux/tracepoint.h> +#include <linux/types.h>
#undef TRACE_SYSTEM #define TRACE_SYSTEM batadv
From: Sven Eckelmann sven@narfation.org
The BATMAN_ADV_DEBUGFS portion of batman-adv is marked as deprecated. Thus all required functionality should be available without it. The debug log was already modified to also output via the kernel tracing function but still retained its BATMAN_ADV_DEBUGFS functionality.
Separate the entry point for the debug log from the debugfs portions to make it possible to build with BATMAN_ADV_DEBUG and without BATMAN_ADV_DEBUGFS.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/Kconfig | 2 +- net/batman-adv/log.c | 60 +++++++++++++++++++++++++++----------------------- 2 files changed, 34 insertions(+), 28 deletions(-)
diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig index f75816f58107..7b0e1fcddfa7 100644 --- a/net/batman-adv/Kconfig +++ b/net/batman-adv/Kconfig @@ -100,7 +100,7 @@ config BATMAN_ADV_DEBUGFS
config BATMAN_ADV_DEBUG bool "B.A.T.M.A.N. debugging" - depends on BATMAN_ADV_DEBUGFS + depends on BATMAN_ADV help This is an option for use by developers; most people should say N here. This enables compilation of support for diff --git a/net/batman-adv/log.c b/net/batman-adv/log.c index 6beb5f067810..02e55b78132f 100644 --- a/net/batman-adv/log.c +++ b/net/batman-adv/log.c @@ -43,6 +43,8 @@ #include "debugfs.h" #include "trace.h"
+#ifdef CONFIG_BATMAN_ADV_DEBUGFS + #define BATADV_LOG_BUFF_MASK (batadv_log_buff_len - 1)
static const int batadv_log_buff_len = BATADV_LOG_BUF_LEN; @@ -92,33 +94,6 @@ static int batadv_fdebug_log(struct batadv_priv_debug_log *debug_log, return 0; }
-/** - * batadv_debug_log() - Add debug log entry - * @bat_priv: the bat priv with all the soft interface information - * @fmt: format string - * - * Return: 0 on success or negative error number in case of failure - */ -int batadv_debug_log(struct batadv_priv *bat_priv, const char *fmt, ...) -{ - struct va_format vaf; - va_list args; - - va_start(args, fmt); - - vaf.fmt = fmt; - vaf.va = &args; - - batadv_fdebug_log(bat_priv->debug_log, "[%10u] %pV", - jiffies_to_msecs(jiffies), &vaf); - - trace_batadv_dbg(bat_priv, &vaf); - - va_end(args); - - return 0; -} - static int batadv_log_open(struct inode *inode, struct file *file) { if (!try_module_get(THIS_MODULE)) @@ -259,3 +234,34 @@ void batadv_debug_log_cleanup(struct batadv_priv *bat_priv) kfree(bat_priv->debug_log); bat_priv->debug_log = NULL; } + +#endif /* CONFIG_BATMAN_ADV_DEBUGFS */ + +/** + * batadv_debug_log() - Add debug log entry + * @bat_priv: the bat priv with all the soft interface information + * @fmt: format string + * + * Return: 0 on success or negative error number in case of failure + */ +int batadv_debug_log(struct batadv_priv *bat_priv, const char *fmt, ...) +{ + struct va_format vaf; + va_list args; + + va_start(args, fmt); + + vaf.fmt = fmt; + vaf.va = &args; + +#ifdef CONFIG_BATMAN_ADV_DEBUGFS + batadv_fdebug_log(bat_priv->debug_log, "[%10u] %pV", + jiffies_to_msecs(jiffies), &vaf); +#endif + + trace_batadv_dbg(bat_priv, &vaf); + + va_end(args); + + return 0; +}
From: Sven Eckelmann sven@narfation.org
The debug messages of batman-adv are not printed to the kernel log at all but can be stored (depending on the compile setting) in the tracing buffer or the batadv specific log buffer. There is also no debug module parameter but a batadv netdev specific log_level setting to enable/disable different classes of debug messages at runtime.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig index 7b0e1fcddfa7..082e96060bc2 100644 --- a/net/batman-adv/Kconfig +++ b/net/batman-adv/Kconfig @@ -104,8 +104,9 @@ config BATMAN_ADV_DEBUG help This is an option for use by developers; most people should say N here. This enables compilation of support for - outputting debugging information to the kernel log. The - output is controlled via the module parameter debug. + outputting debugging information to the debugfs log or tracing + buffer. The output is controlled via the batadv netdev specific + log_level setting.
config BATMAN_ADV_TRACING bool "B.A.T.M.A.N. tracing support"
From: Sven Eckelmann sven@narfation.org
The netlink dump functionality transfers a large number of entries from the kernel to userspace. It is rather likely that the transfer has to interrupted and later continued. During that time, it can happen that either new entries are added or removed. The userspace could than either receive some entries multiple times or miss entries.
Commit 670dc2833d14 ("netlink: advertise incomplete dumps") introduced a mechanism to inform userspace about this problem. Userspace can then decide whether it is necessary or not to retry dumping the information again.
The netlink dump functions have to be switched to exclusive locks to avoid changes while the current message is prepared. And an external generation sequence counter is introduced which tracks all modifications of the list.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bat_iv_ogm.c | 24 +++++++++++++++--------- net/batman-adv/bat_v.c | 26 +++++++++++++++++--------- net/batman-adv/gateway_client.c | 3 +++ net/batman-adv/main.c | 2 ++ net/batman-adv/types.h | 5 ++++- 5 files changed, 41 insertions(+), 19 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 1d31ac84dec7..f97e566f0402 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -2584,13 +2584,14 @@ static void batadv_iv_gw_print(struct batadv_priv *bat_priv, * batadv_iv_gw_dump_entry() - Dump a gateway into a message * @msg: Netlink message to dump into * @portid: Port making netlink request - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @bat_priv: The bat priv with all the soft interface information * @gw_node: Gateway to be dumped * * Return: Error code, or 0 on success */ -static int batadv_iv_gw_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, +static int batadv_iv_gw_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_priv *bat_priv, struct batadv_gw_node *gw_node) { @@ -2610,13 +2611,16 @@ static int batadv_iv_gw_dump_entry(struct sk_buff *msg, u32 portid, u32 seq,
curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, - NLM_F_MULTI, BATADV_CMD_GET_GATEWAYS); + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, + BATADV_CMD_GET_GATEWAYS); if (!hdr) { ret = -ENOBUFS; goto out; }
+ genl_dump_check_consistent(cb, hdr); + ret = -EMSGSIZE;
if (curr_gw == gw_node) @@ -2667,13 +2671,15 @@ static void batadv_iv_gw_dump(struct sk_buff *msg, struct netlink_callback *cb, int idx_skip = cb->args[0]; int idx = 0;
- rcu_read_lock(); - hlist_for_each_entry_rcu(gw_node, &bat_priv->gw.gateway_list, list) { + spin_lock_bh(&bat_priv->gw.list_lock); + cb->seq = bat_priv->gw.generation << 1 | 1; + + hlist_for_each_entry(gw_node, &bat_priv->gw.gateway_list, list) { if (idx++ < idx_skip) continue;
- if (batadv_iv_gw_dump_entry(msg, portid, cb->nlh->nlmsg_seq, - bat_priv, gw_node)) { + if (batadv_iv_gw_dump_entry(msg, portid, cb, bat_priv, + gw_node)) { idx_skip = idx - 1; goto unlock; } @@ -2681,7 +2687,7 @@ static void batadv_iv_gw_dump(struct sk_buff *msg, struct netlink_callback *cb,
idx_skip = idx; unlock: - rcu_read_unlock(); + spin_unlock_bh(&bat_priv->gw.list_lock);
cb->args[0] = idx_skip; } diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c index 6baec4e68898..90e33f84d37a 100644 --- a/net/batman-adv/bat_v.c +++ b/net/batman-adv/bat_v.c @@ -27,11 +27,13 @@ #include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/kref.h> +#include <linux/list.h> #include <linux/netdevice.h> #include <linux/netlink.h> #include <linux/rculist.h> #include <linux/rcupdate.h> #include <linux/seq_file.h> +#include <linux/spinlock.h> #include <linux/stddef.h> #include <linux/types.h> #include <linux/workqueue.h> @@ -915,13 +917,14 @@ static void batadv_v_gw_print(struct batadv_priv *bat_priv, * batadv_v_gw_dump_entry() - Dump a gateway into a message * @msg: Netlink message to dump into * @portid: Port making netlink request - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @bat_priv: The bat priv with all the soft interface information * @gw_node: Gateway to be dumped * * Return: Error code, or 0 on success */ -static int batadv_v_gw_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, +static int batadv_v_gw_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_priv *bat_priv, struct batadv_gw_node *gw_node) { @@ -941,13 +944,16 @@ static int batadv_v_gw_dump_entry(struct sk_buff *msg, u32 portid, u32 seq,
curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, - NLM_F_MULTI, BATADV_CMD_GET_GATEWAYS); + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, + BATADV_CMD_GET_GATEWAYS); if (!hdr) { ret = -ENOBUFS; goto out; }
+ genl_dump_check_consistent(cb, hdr); + ret = -EMSGSIZE;
if (curr_gw == gw_node) { @@ -1018,13 +1024,15 @@ static void batadv_v_gw_dump(struct sk_buff *msg, struct netlink_callback *cb, int idx_skip = cb->args[0]; int idx = 0;
- rcu_read_lock(); - hlist_for_each_entry_rcu(gw_node, &bat_priv->gw.gateway_list, list) { + spin_lock_bh(&bat_priv->gw.list_lock); + cb->seq = bat_priv->gw.generation << 1 | 1; + + hlist_for_each_entry(gw_node, &bat_priv->gw.gateway_list, list) { if (idx++ < idx_skip) continue;
- if (batadv_v_gw_dump_entry(msg, portid, cb->nlh->nlmsg_seq, - bat_priv, gw_node)) { + if (batadv_v_gw_dump_entry(msg, portid, cb, bat_priv, + gw_node)) { idx_skip = idx - 1; goto unlock; } @@ -1032,7 +1040,7 @@ static void batadv_v_gw_dump(struct sk_buff *msg, struct netlink_callback *cb,
idx_skip = idx; unlock: - rcu_read_unlock(); + spin_unlock_bh(&bat_priv->gw.list_lock);
cb->args[0] = idx_skip; } diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c index 140c61a3f1ec..9d8e5eda2314 100644 --- a/net/batman-adv/gateway_client.c +++ b/net/batman-adv/gateway_client.c @@ -377,6 +377,7 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv,
kref_get(&gw_node->refcount); hlist_add_head_rcu(&gw_node->list, &bat_priv->gw.gateway_list); + bat_priv->gw.generation++;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Found new gateway %pM -> gw bandwidth: %u.%u/%u.%u MBit\n", @@ -472,6 +473,7 @@ void batadv_gw_node_update(struct batadv_priv *bat_priv, if (!hlist_unhashed(&gw_node->list)) { hlist_del_init_rcu(&gw_node->list); batadv_gw_node_put(gw_node); + bat_priv->gw.generation++; } spin_unlock_bh(&bat_priv->gw.list_lock);
@@ -518,6 +520,7 @@ void batadv_gw_node_free(struct batadv_priv *bat_priv) &bat_priv->gw.gateway_list, list) { hlist_del_init_rcu(&gw_node->list); batadv_gw_node_put(gw_node); + bat_priv->gw.generation++; } spin_unlock_bh(&bat_priv->gw.list_lock); } diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index 69c0d85bceb3..c75e47826949 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -186,6 +186,8 @@ int batadv_mesh_init(struct net_device *soft_iface) INIT_HLIST_HEAD(&bat_priv->softif_vlan_list); INIT_HLIST_HEAD(&bat_priv->tp_list);
+ bat_priv->gw.generation = 0; + ret = batadv_v_mesh_init(bat_priv); if (ret < 0) goto err; diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 45b5592de816..cbe17da36fcb 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1096,12 +1096,15 @@ struct batadv_priv_gw { /** @gateway_list: list of available gateway nodes */ struct hlist_head gateway_list;
- /** @list_lock: lock protecting gateway_list & curr_gw */ + /** @list_lock: lock protecting gateway_list, curr_gw, generation */ spinlock_t list_lock;
/** @curr_gw: pointer to currently selected gateway node */ struct batadv_gw_node __rcu *curr_gw;
+ /** @generation: current (generation) sequence number */ + unsigned int generation; + /** * @mode: gateway operation: off, client or server (see batadv_gw_modes) */
From: Sven Eckelmann sven@narfation.org
The netlink dump functionality transfers a large number of entries from the kernel to userspace. It is rather likely that the transfer has to interrupted and later continued. During that time, it can happen that either new entries are added or removed. The userspace could than either receive some entries multiple times or miss entries.
Commit 670dc2833d14 ("netlink: advertise incomplete dumps") introduced a mechanism to inform userspace about this problem. Userspace can then decide whether it is necessary or not to retry dumping the information again.
The netlink dump functions have to be switched to exclusive locks to avoid changes while the current message is prepared. And an external generation sequence counter is introduced which tracks all modifications of the list.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/hard-interface.c | 3 +++ net/batman-adv/main.c | 1 + net/batman-adv/main.h | 1 + net/batman-adv/netlink.c | 24 ++++++++++++++---------- 4 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 781c5b6e6e8e..508f4416dfc9 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -951,6 +951,7 @@ batadv_hardif_add_interface(struct net_device *net_dev) batadv_check_known_mac_addr(hard_iface->net_dev); kref_get(&hard_iface->refcount); list_add_tail_rcu(&hard_iface->list, &batadv_hardif_list); + batadv_hardif_generation++;
return hard_iface;
@@ -993,6 +994,7 @@ void batadv_hardif_remove_interfaces(void) list_for_each_entry_safe(hard_iface, hard_iface_tmp, &batadv_hardif_list, list) { list_del_rcu(&hard_iface->list); + batadv_hardif_generation++; batadv_hardif_remove_interface(hard_iface); } rtnl_unlock(); @@ -1054,6 +1056,7 @@ static int batadv_hard_if_event(struct notifier_block *this, case NETDEV_UNREGISTER: case NETDEV_PRE_TYPE_CHANGE: list_del_rcu(&hard_iface->list); + batadv_hardif_generation++;
batadv_hardif_remove_interface(hard_iface); break; diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index c75e47826949..d1ed839fd32b 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -74,6 +74,7 @@ * list traversals just rcu-locked */ struct list_head batadv_hardif_list; +unsigned int batadv_hardif_generation; static int (*batadv_rx_handler[256])(struct sk_buff *skb, struct batadv_hard_iface *recv_if);
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h index b68a41190eb0..b572066325e4 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -247,6 +247,7 @@ static inline int batadv_print_vid(unsigned short vid) }
extern struct list_head batadv_hardif_list; +extern unsigned int batadv_hardif_generation;
extern unsigned char batadv_broadcast_addr[]; extern struct workqueue_struct *batadv_event_workqueue; diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c index 0d9459b69bdb..2dc3304cee54 100644 --- a/net/batman-adv/netlink.c +++ b/net/batman-adv/netlink.c @@ -29,11 +29,11 @@ #include <linux/if_ether.h> #include <linux/init.h> #include <linux/kernel.h> +#include <linux/list.h> #include <linux/netdevice.h> #include <linux/netlink.h> #include <linux/printk.h> -#include <linux/rculist.h> -#include <linux/rcupdate.h> +#include <linux/rtnetlink.h> #include <linux/skbuff.h> #include <linux/stddef.h> #include <linux/types.h> @@ -445,23 +445,27 @@ batadv_netlink_tp_meter_cancel(struct sk_buff *skb, struct genl_info *info) * batadv_netlink_dump_hardif_entry() - Dump one hard interface into a message * @msg: Netlink message to dump into * @portid: Port making netlink request - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @hard_iface: Hard interface to dump * * Return: error code, or 0 on success */ static int -batadv_netlink_dump_hardif_entry(struct sk_buff *msg, u32 portid, u32 seq, +batadv_netlink_dump_hardif_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_hard_iface *hard_iface) { struct net_device *net_dev = hard_iface->net_dev; void *hdr;
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, NLM_F_MULTI, + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, BATADV_CMD_GET_HARDIFS); if (!hdr) return -EMSGSIZE;
+ genl_dump_check_consistent(cb, hdr); + if (nla_put_u32(msg, BATADV_ATTR_HARD_IFINDEX, net_dev->ifindex) || nla_put_string(msg, BATADV_ATTR_HARD_IFNAME, @@ -498,7 +502,6 @@ batadv_netlink_dump_hardifs(struct sk_buff *msg, struct netlink_callback *cb) struct batadv_hard_iface *hard_iface; int ifindex; int portid = NETLINK_CB(cb->skb).portid; - int seq = cb->nlh->nlmsg_seq; int skip = cb->args[0]; int i = 0;
@@ -516,23 +519,24 @@ batadv_netlink_dump_hardifs(struct sk_buff *msg, struct netlink_callback *cb) return -ENODEV; }
- rcu_read_lock(); + rtnl_lock(); + cb->seq = batadv_hardif_generation << 1 | 1;
- list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) { + list_for_each_entry(hard_iface, &batadv_hardif_list, list) { if (hard_iface->soft_iface != soft_iface) continue;
if (i++ < skip) continue;
- if (batadv_netlink_dump_hardif_entry(msg, portid, seq, + if (batadv_netlink_dump_hardif_entry(msg, portid, cb, hard_iface)) { i--; break; } }
- rcu_read_unlock(); + rtnl_unlock();
dev_put(soft_iface);
From: Sven Eckelmann sven@narfation.org
Multiple datastructures use the hash helper functions to add and remove entries from the simple hlist based hashes. These are often also dumped to userspace via netlink and thus should have a generation sequence counter.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/hash.c | 2 ++ net/batman-adv/hash.h | 6 ++++++ 2 files changed, 8 insertions(+)
diff --git a/net/batman-adv/hash.c b/net/batman-adv/hash.c index 7b49e4001778..9194f4d891b1 100644 --- a/net/batman-adv/hash.c +++ b/net/batman-adv/hash.c @@ -32,6 +32,8 @@ static void batadv_hash_init(struct batadv_hashtable *hash) INIT_HLIST_HEAD(&hash->table[i]); spin_lock_init(&hash->list_locks[i]); } + + atomic_set(&hash->generation, 0); }
/** diff --git a/net/batman-adv/hash.h b/net/batman-adv/hash.h index 9490a7ca2ba6..0e36fa1c7c3e 100644 --- a/net/batman-adv/hash.h +++ b/net/batman-adv/hash.h @@ -21,6 +21,7 @@
#include "main.h"
+#include <linux/atomic.h> #include <linux/compiler.h> #include <linux/list.h> #include <linux/rculist.h> @@ -58,6 +59,9 @@ struct batadv_hashtable {
/** @size: size of hashtable */ u32 size; + + /** @generation: current (generation) sequence number */ + atomic_t generation; };
/* allocates and clears the hash */ @@ -112,6 +116,7 @@ static inline int batadv_hash_add(struct batadv_hashtable *hash,
/* no duplicate found in list, add new element */ hlist_add_head_rcu(data_node, head); + atomic_inc(&hash->generation);
ret = 0;
@@ -154,6 +159,7 @@ static inline void *batadv_hash_remove(struct batadv_hashtable *hash,
data_save = node; hlist_del_rcu(node); + atomic_inc(&hash->generation); break; } spin_unlock_bh(&hash->list_locks[index]);
From: Sven Eckelmann sven@narfation.org
The netlink dump functionality transfers a large number of entries from the kernel to userspace. It is rather likely that the transfer has to interrupted and later continued. During that time, it can happen that either new entries are added or removed. The userspace could than either receive some entries multiple times or miss entries.
Commit 670dc2833d14 ("netlink: advertise incomplete dumps") introduced a mechanism to inform userspace about this problem. Userspace can then decide whether it is necessary or not to retry dumping the information again.
The netlink dump functions have to be switched to exclusive locks to avoid changes while the current message is prepared. The already existing generation sequence counter from the hash helper can be used for this simple hash.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bridge_loop_avoidance.c | 41 +++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index 5f1aeeded0e3..9f3747346d29 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -2325,14 +2325,15 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset) * netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @primary_if: primary interface * @backbone_gw: entry to dump * * Return: 0 or error code. */ static int -batadv_bla_backbone_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, +batadv_bla_backbone_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_hard_iface *primary_if, struct batadv_bla_backbone_gw *backbone_gw) { @@ -2343,13 +2344,16 @@ batadv_bla_backbone_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, void *hdr; int ret = -EINVAL;
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, - NLM_F_MULTI, BATADV_CMD_GET_BLA_BACKBONE); + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, + BATADV_CMD_GET_BLA_BACKBONE); if (!hdr) { ret = -ENOBUFS; goto out; }
+ genl_dump_check_consistent(cb, hdr); + is_own = batadv_compare_eth(backbone_gw->orig, primary_addr);
spin_lock_bh(&backbone_gw->crc_lock); @@ -2386,28 +2390,33 @@ batadv_bla_backbone_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, * a netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @primary_if: primary interface - * @head: bucket to dump + * @hash: hash to dump + * @bucket: bucket index to dump * @idx_skip: How many entries to skip * * Return: always 0. */ static int -batadv_bla_backbone_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, +batadv_bla_backbone_dump_bucket(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_hard_iface *primary_if, - struct hlist_head *head, int *idx_skip) + struct batadv_hashtable *hash, + unsigned int bucket, int *idx_skip) { struct batadv_bla_backbone_gw *backbone_gw; int idx = 0; int ret = 0;
- rcu_read_lock(); - hlist_for_each_entry_rcu(backbone_gw, head, hash_entry) { + spin_lock_bh(&hash->list_locks[bucket]); + cb->seq = atomic_read(&hash->generation) << 1 | 1; + + hlist_for_each_entry(backbone_gw, &hash->table[bucket], hash_entry) { if (idx++ < *idx_skip) continue;
- ret = batadv_bla_backbone_dump_entry(msg, portid, seq, + ret = batadv_bla_backbone_dump_entry(msg, portid, cb, primary_if, backbone_gw); if (ret) { *idx_skip = idx - 1; @@ -2417,7 +2426,7 @@ batadv_bla_backbone_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq,
*idx_skip = 0; unlock: - rcu_read_unlock(); + spin_unlock_bh(&hash->list_locks[bucket]); return ret; }
@@ -2437,7 +2446,6 @@ int batadv_bla_backbone_dump(struct sk_buff *msg, struct netlink_callback *cb) struct batadv_hashtable *hash; struct batadv_priv *bat_priv; int bucket = cb->args[0]; - struct hlist_head *head; int idx = cb->args[1]; int ifindex; int ret = 0; @@ -2463,11 +2471,8 @@ int batadv_bla_backbone_dump(struct sk_buff *msg, struct netlink_callback *cb) }
while (bucket < hash->size) { - head = &hash->table[bucket]; - - if (batadv_bla_backbone_dump_bucket(msg, portid, - cb->nlh->nlmsg_seq, - primary_if, head, &idx)) + if (batadv_bla_backbone_dump_bucket(msg, portid, cb, primary_if, + hash, bucket, &idx)) break; bucket++; }
From: Sven Eckelmann sven@narfation.org
The netlink dump functionality transfers a large number of entries from the kernel to userspace. It is rather likely that the transfer has to interrupted and later continued. During that time, it can happen that either new entries are added or removed. The userspace could than either receive some entries multiple times or miss entries.
Commit 670dc2833d14 ("netlink: advertise incomplete dumps") introduced a mechanism to inform userspace about this problem. Userspace can then decide whether it is necessary or not to retry dumping the information again.
The netlink dump functions have to be switched to exclusive locks to avoid changes while the current message is prepared. The already existing generation sequence counter from the hash helper can be used for this simple hash.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bridge_loop_avoidance.c | 41 +++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index 9f3747346d29..5fdde2947802 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -2094,14 +2094,15 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset) * to a netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @primary_if: primary interface * @claim: entry to dump * * Return: 0 or error code. */ static int -batadv_bla_claim_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, +batadv_bla_claim_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_hard_iface *primary_if, struct batadv_bla_claim *claim) { @@ -2111,13 +2112,16 @@ batadv_bla_claim_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, void *hdr; int ret = -EINVAL;
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, - NLM_F_MULTI, BATADV_CMD_GET_BLA_CLAIM); + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, + BATADV_CMD_GET_BLA_CLAIM); if (!hdr) { ret = -ENOBUFS; goto out; }
+ genl_dump_check_consistent(cb, hdr); + is_own = batadv_compare_eth(claim->backbone_gw->orig, primary_addr);
@@ -2153,28 +2157,33 @@ batadv_bla_claim_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, * to a netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @primary_if: primary interface - * @head: bucket to dump + * @hash: hash to dump + * @bucket: bucket index to dump * @idx_skip: How many entries to skip * * Return: always 0. */ static int -batadv_bla_claim_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, +batadv_bla_claim_dump_bucket(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_hard_iface *primary_if, - struct hlist_head *head, int *idx_skip) + struct batadv_hashtable *hash, unsigned int bucket, + int *idx_skip) { struct batadv_bla_claim *claim; int idx = 0; int ret = 0;
- rcu_read_lock(); - hlist_for_each_entry_rcu(claim, head, hash_entry) { + spin_lock_bh(&hash->list_locks[bucket]); + cb->seq = atomic_read(&hash->generation) << 1 | 1; + + hlist_for_each_entry(claim, &hash->table[bucket], hash_entry) { if (idx++ < *idx_skip) continue;
- ret = batadv_bla_claim_dump_entry(msg, portid, seq, + ret = batadv_bla_claim_dump_entry(msg, portid, cb, primary_if, claim); if (ret) { *idx_skip = idx - 1; @@ -2184,7 +2193,7 @@ batadv_bla_claim_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq,
*idx_skip = 0; unlock: - rcu_read_unlock(); + spin_unlock_bh(&hash->list_locks[bucket]); return ret; }
@@ -2204,7 +2213,6 @@ int batadv_bla_claim_dump(struct sk_buff *msg, struct netlink_callback *cb) struct batadv_hashtable *hash; struct batadv_priv *bat_priv; int bucket = cb->args[0]; - struct hlist_head *head; int idx = cb->args[1]; int ifindex; int ret = 0; @@ -2230,11 +2238,8 @@ int batadv_bla_claim_dump(struct sk_buff *msg, struct netlink_callback *cb) }
while (bucket < hash->size) { - head = &hash->table[bucket]; - - if (batadv_bla_claim_dump_bucket(msg, portid, - cb->nlh->nlmsg_seq, - primary_if, head, &idx)) + if (batadv_bla_claim_dump_bucket(msg, portid, cb, primary_if, + hash, bucket, &idx)) break; bucket++; }
From: Sven Eckelmann sven@narfation.org
The netlink dump functionality transfers a large number of entries from the kernel to userspace. It is rather likely that the transfer has to interrupted and later continued. During that time, it can happen that either new entries are added or removed. The userspace could than either receive some entries multiple times or miss entries.
Commit 670dc2833d14 ("netlink: advertise incomplete dumps") introduced a mechanism to inform userspace about this problem. Userspace can then decide whether it is necessary or not to retry dumping the information again.
The netlink dump functions have to be switched to exclusive locks to avoid changes while the current message is prepared. The already existing generation sequence counter from the hash helper can be used for this simple hash.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/distributed-arp-table.c | 42 +++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index a60bacf7120b..b9ffe1826527 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -863,23 +863,27 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) * netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @dat_entry: entry to dump * * Return: 0 or error code. */ static int -batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, +batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_dat_entry *dat_entry) { int msecs; void *hdr;
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, - NLM_F_MULTI, BATADV_CMD_GET_DAT_CACHE); + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, + BATADV_CMD_GET_DAT_CACHE); if (!hdr) return -ENOBUFS;
+ genl_dump_check_consistent(cb, hdr); + msecs = jiffies_to_msecs(jiffies - dat_entry->last_update);
if (nla_put_in_addr(msg, BATADV_ATTR_DAT_CACHE_IP4ADDRESS, @@ -901,27 +905,31 @@ batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, * a netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message - * @head: bucket to dump + * @cb: Control block containing additional options + * @hash: hash to dump + * @bucket: bucket index to dump * @idx_skip: How many entries to skip * * Return: 0 or error code. */ static int -batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, - struct hlist_head *head, int *idx_skip) +batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, + struct batadv_hashtable *hash, unsigned int bucket, + int *idx_skip) { struct batadv_dat_entry *dat_entry; int idx = 0;
- rcu_read_lock(); - hlist_for_each_entry_rcu(dat_entry, head, hash_entry) { + spin_lock_bh(&hash->list_locks[bucket]); + cb->seq = atomic_read(&hash->generation) << 1 | 1; + + hlist_for_each_entry(dat_entry, &hash->table[bucket], hash_entry) { if (idx < *idx_skip) goto skip;
- if (batadv_dat_cache_dump_entry(msg, portid, seq, - dat_entry)) { - rcu_read_unlock(); + if (batadv_dat_cache_dump_entry(msg, portid, cb, dat_entry)) { + spin_unlock_bh(&hash->list_locks[bucket]); *idx_skip = idx;
return -EMSGSIZE; @@ -930,7 +938,7 @@ batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, skip: idx++; } - rcu_read_unlock(); + spin_unlock_bh(&hash->list_locks[bucket]);
return 0; } @@ -951,7 +959,6 @@ int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) struct batadv_hashtable *hash; struct batadv_priv *bat_priv; int bucket = cb->args[0]; - struct hlist_head *head; int idx = cb->args[1]; int ifindex; int ret = 0; @@ -977,10 +984,7 @@ int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) }
while (bucket < hash->size) { - head = &hash->table[bucket]; - - if (batadv_dat_cache_dump_bucket(msg, portid, - cb->nlh->nlmsg_seq, head, + if (batadv_dat_cache_dump_bucket(msg, portid, cb, hash, bucket, &idx)) break;
From: Sven Eckelmann sven@narfation.org
The netlink dump functionality transfers a large number of entries from the kernel to userspace. It is rather likely that the transfer has to interrupted and later continued. During that time, it can happen that either new entries are added or removed. The userspace could than either receive some entries multiple times or miss entries.
Commit 670dc2833d14 ("netlink: advertise incomplete dumps") introduced a mechanism to inform userspace about this problem. Userspace can then decide whether it is necessary or not to retry dumping the information again.
The netlink dump functions have to be switched to exclusive locks to avoid changes while the current message is prepared. The already existing generation sequence counter from the hash helper can be used for this simple hash.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/translation-table.c | 41 +++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index d21624c44665..8dcd4968cde7 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1145,14 +1145,15 @@ int batadv_tt_local_seq_print_text(struct seq_file *seq, void *offset) * batadv_tt_local_dump_entry() - Dump one TT local entry into a message * @msg :Netlink message to dump into * @portid: Port making netlink request - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @bat_priv: The bat priv with all the soft interface information * @common: tt local & tt global common data * * Return: Error code, or 0 on success */ static int -batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, +batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_priv *bat_priv, struct batadv_tt_common_entry *common) { @@ -1173,12 +1174,14 @@ batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid, u32 seq,
batadv_softif_vlan_put(vlan);
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, - NLM_F_MULTI, + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, BATADV_CMD_GET_TRANSTABLE_LOCAL); if (!hdr) return -ENOBUFS;
+ genl_dump_check_consistent(cb, hdr); + if (nla_put(msg, BATADV_ATTR_TT_ADDRESS, ETH_ALEN, common->addr) || nla_put_u32(msg, BATADV_ATTR_TT_CRC32, crc) || nla_put_u16(msg, BATADV_ATTR_TT_VID, common->vid) || @@ -1201,34 +1204,39 @@ batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, * batadv_tt_local_dump_bucket() - Dump one TT local bucket into a message * @msg: Netlink message to dump into * @portid: Port making netlink request - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @bat_priv: The bat priv with all the soft interface information - * @head: Pointer to the list containing the local tt entries + * @hash: hash to dump + * @bucket: bucket index to dump * @idx_s: Number of entries to skip * * Return: Error code, or 0 on success */ static int -batadv_tt_local_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, +batadv_tt_local_dump_bucket(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_priv *bat_priv, - struct hlist_head *head, int *idx_s) + struct batadv_hashtable *hash, unsigned int bucket, + int *idx_s) { struct batadv_tt_common_entry *common; int idx = 0;
- rcu_read_lock(); - hlist_for_each_entry_rcu(common, head, hash_entry) { + spin_lock_bh(&hash->list_locks[bucket]); + cb->seq = atomic_read(&hash->generation) << 1 | 1; + + hlist_for_each_entry(common, &hash->table[bucket], hash_entry) { if (idx++ < *idx_s) continue;
- if (batadv_tt_local_dump_entry(msg, portid, seq, bat_priv, + if (batadv_tt_local_dump_entry(msg, portid, cb, bat_priv, common)) { - rcu_read_unlock(); + spin_unlock_bh(&hash->list_locks[bucket]); *idx_s = idx - 1; return -EMSGSIZE; } } - rcu_read_unlock(); + spin_unlock_bh(&hash->list_locks[bucket]);
*idx_s = 0; return 0; @@ -1248,7 +1256,6 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb) struct batadv_priv *bat_priv; struct batadv_hard_iface *primary_if = NULL; struct batadv_hashtable *hash; - struct hlist_head *head; int ret; int ifindex; int bucket = cb->args[0]; @@ -1276,10 +1283,8 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb) hash = bat_priv->tt.local_hash;
while (bucket < hash->size) { - head = &hash->table[bucket]; - - if (batadv_tt_local_dump_bucket(msg, portid, cb->nlh->nlmsg_seq, - bat_priv, head, &idx)) + if (batadv_tt_local_dump_bucket(msg, portid, cb, bat_priv, + hash, bucket, &idx)) break;
bucket++;
From: Sven Eckelmann sven@narfation.org
The netlink dump functionality transfers a large number of entries from the kernel to userspace. It is rather likely that the transfer has to interrupted and later continued. During that time, it can happen that either new entries are added or removed. The userspace could than either receive some entries multiple times or miss entries.
Commit 670dc2833d14 ("netlink: advertise incomplete dumps") introduced a mechanism to inform userspace about this problem. Userspace can then decide whether it is necessary or not to retry dumping the information again.
The netlink dump functions have to be switched to exclusive locks to avoid changes while the current message is prepared. The already existing generation sequence counter from the hash helper can be used for this simple hash.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/multicast.c | 51 +++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c index 86725d792e15..69244e4598f5 100644 --- a/net/batman-adv/multicast.c +++ b/net/batman-adv/multicast.c @@ -1365,22 +1365,26 @@ int batadv_mcast_mesh_info_put(struct sk_buff *msg, * to a netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @orig_node: originator to dump the multicast flags of * * Return: 0 or error code. */ static int -batadv_mcast_flags_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, +batadv_mcast_flags_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_orig_node *orig_node) { void *hdr;
- hdr = genlmsg_put(msg, portid, seq, &batadv_netlink_family, - NLM_F_MULTI, BATADV_CMD_GET_MCAST_FLAGS); + hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, + &batadv_netlink_family, NLM_F_MULTI, + BATADV_CMD_GET_MCAST_FLAGS); if (!hdr) return -ENOBUFS;
+ genl_dump_check_consistent(cb, hdr); + if (nla_put(msg, BATADV_ATTR_ORIG_ADDRESS, ETH_ALEN, orig_node->orig)) { genlmsg_cancel(msg, hdr); @@ -1405,21 +1409,26 @@ batadv_mcast_flags_dump_entry(struct sk_buff *msg, u32 portid, u32 seq, * table to a netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message - * @head: bucket to dump + * @cb: Control block containing additional options + * @hash: hash to dump + * @bucket: bucket index to dump * @idx_skip: How many entries to skip * * Return: 0 or error code. */ static int -batadv_mcast_flags_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, - struct hlist_head *head, long *idx_skip) +batadv_mcast_flags_dump_bucket(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, + struct batadv_hashtable *hash, + unsigned int bucket, long *idx_skip) { struct batadv_orig_node *orig_node; long idx = 0;
- rcu_read_lock(); - hlist_for_each_entry_rcu(orig_node, head, hash_entry) { + spin_lock_bh(&hash->list_locks[bucket]); + cb->seq = atomic_read(&hash->generation) << 1 | 1; + + hlist_for_each_entry(orig_node, &hash->table[bucket], hash_entry) { if (!test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig_node->capa_initialized)) continue; @@ -1427,9 +1436,8 @@ batadv_mcast_flags_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, if (idx < *idx_skip) goto skip;
- if (batadv_mcast_flags_dump_entry(msg, portid, seq, - orig_node)) { - rcu_read_unlock(); + if (batadv_mcast_flags_dump_entry(msg, portid, cb, orig_node)) { + spin_unlock_bh(&hash->list_locks[bucket]); *idx_skip = idx;
return -EMSGSIZE; @@ -1438,7 +1446,7 @@ batadv_mcast_flags_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, skip: idx++; } - rcu_read_unlock(); + spin_unlock_bh(&hash->list_locks[bucket]);
return 0; } @@ -1447,7 +1455,7 @@ batadv_mcast_flags_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, * __batadv_mcast_flags_dump() - dump multicast flags table to a netlink socket * @msg: buffer for the message * @portid: netlink port - * @seq: Sequence number of netlink message + * @cb: Control block containing additional options * @bat_priv: the bat priv with all the soft interface information * @bucket: current bucket to dump * @idx: index in current bucket to the next entry to dump @@ -1455,19 +1463,17 @@ batadv_mcast_flags_dump_bucket(struct sk_buff *msg, u32 portid, u32 seq, * Return: 0 or error code. */ static int -__batadv_mcast_flags_dump(struct sk_buff *msg, u32 portid, u32 seq, +__batadv_mcast_flags_dump(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, struct batadv_priv *bat_priv, long *bucket, long *idx) { struct batadv_hashtable *hash = bat_priv->orig_hash; long bucket_tmp = *bucket; - struct hlist_head *head; long idx_tmp = *idx;
while (bucket_tmp < hash->size) { - head = &hash->table[bucket_tmp]; - - if (batadv_mcast_flags_dump_bucket(msg, portid, seq, head, - &idx_tmp)) + if (batadv_mcast_flags_dump_bucket(msg, portid, cb, hash, + *bucket, &idx_tmp)) break;
bucket_tmp++; @@ -1550,8 +1556,7 @@ int batadv_mcast_flags_dump(struct sk_buff *msg, struct netlink_callback *cb) return ret;
bat_priv = netdev_priv(primary_if->soft_iface); - ret = __batadv_mcast_flags_dump(msg, portid, cb->nlh->nlmsg_seq, - bat_priv, bucket, idx); + ret = __batadv_mcast_flags_dump(msg, portid, cb, bat_priv, bucket, idx);
batadv_hardif_put(primary_if); return ret;
From: Sven Eckelmann sven@narfation.org
The commit ced72933a5e8 ("batman-adv: use CRC32C instead of CRC16 in TT code") switched the translation table code from crc16 to crc32c. The (optional) bridge loop avoidance code is the only user of this function.
batman-adv should only select CRC16 when it is actually using it.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig index 082e96060bc2..d6b94559f888 100644 --- a/net/batman-adv/Kconfig +++ b/net/batman-adv/Kconfig @@ -22,7 +22,6 @@ config BATMAN_ADV tristate "B.A.T.M.A.N. Advanced Meshing Protocol" depends on NET - select CRC16 select LIBCRC32C help B.A.T.M.A.N. (better approach to mobile ad-hoc networking) is @@ -48,6 +47,7 @@ config BATMAN_ADV_BATMAN_V config BATMAN_ADV_BLA bool "Bridge Loop Avoidance" depends on BATMAN_ADV && INET + select CRC16 default y help This option enables BLA (Bridge Loop Avoidance), a mechanism
From: Linus Lüssing linus.luessing@c0d3.blue
Thanks to rigorous testing in wireless community mesh networks several issues with multicast entries in the translation table were found and fixed in the last 1.5 years. Now we see the first larger networks (a few hundred nodes) with a batman-adv version with multicast optimizations enabled arising, with no TT / multicast optimization related issues so far.
Therefore it seems safe to enable multicast optimizations by default.
Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig index d6b94559f888..c386e6981416 100644 --- a/net/batman-adv/Kconfig +++ b/net/batman-adv/Kconfig @@ -82,6 +82,7 @@ config BATMAN_ADV_NC config BATMAN_ADV_MCAST bool "Multicast optimisation" depends on BATMAN_ADV && INET && !(BRIDGE=m && BATMAN_ADV=y) + default y help This option enables the multicast optimisation which aims to reduce the air overhead while improving the reliability of
From: Simon Wunderlich sw@simonwunderlich.de Date: Wed, 14 Nov 2018 15:07:48 +0100
here is a feature/cleanup pull request of batman-adv to go into net-next.
Please pull or let me know of any problem!
Pulled, thanks Simon.
b.a.t.m.a.n@lists.open-mesh.org
-
David Miller -
Simon Wunderlich