Hi,
this is v2 of this series. I removed the hashtable patches for the moment. The types.h patch is fixed to include linux headers only when compiling for the kernel so that it can still be used in batctl.
Major changes (Patch 1-7): - Compiling debugfs.c only when CONFIG_DEBUG_FS is selected. This reduces the amount of unnecessary code that is executed. At the moment all calls to debugfs functions will result in NOOPs. However there is some more code that we simply don't need without DEBUG_FS. - tvlv is separated from the large main.c file into its own tvlv.c. I don't see a reason to have this set of functions for tvlv inside the main.c file.
Minor changes (Patch 8-26): - Removing unnecessary return value variables - Fixing some comments - Reordering functions to increase readability - Coding style fixes - Declare boolean return types as bool - Add missing includes
Best regards,
Markus
Markus Pargmann (26): batman-adv: debugfs, avoid compiling for !DEBUG_FS batman-adv: Separate logging header batman-adv: iv_ogm, Reduce code duplication batman-adv: iv_ogm, divide and round for ring buffer avg batman-adv: init, Add some error handling batman-adv: tvlv realloc, move error handling into if block batman-adv: split tvlv into a seperate file batman-adv: Makefile, Sort alphabetically batman-adv: iv_ogm_iface_enable, direct return values batman-adv: iv_ogm_aggr_packet, bool return value batman-adv: iv_ogm_send_to_if, declare char* as const batman-adv: iv_ogm_can_aggregate, code readability batman-adv: iv_ogm_orig_update, remove unnecessary brackets batman-adv: iv_ogm_aggregate_new, simplify error handling batman-adv: iv_ogm_queue_add, Simplify expressions batman-adv: iv_ogm_orig_update, style, add missin brackets batman-adv: iv_ogm, Fix dup_status comment batman-adv: iv_ogm, fix coding style batman-adv: iv_ogm, fix comment function name batman-adv: types, Fix comment on bcast_own batman-adv: main, Convert is_my_mac() to bool batman-adv: main, batadv_compare_eth return bool batman-adv: Remove unnecessary ret variable batman-adv: Remove unnecessary ret variable in algo_register batman-adv: packet.h, add some missing includes batman-adv: types.h, add missing include
Makefile.kbuild | 5 +- bat_iv_ogm.c | 239 +++++++++--------- bitarray.c | 1 + bridge_loop_avoidance.c | 1 + debugfs.c | 9 +- debugfs.h | 39 +++ distributed-arp-table.c | 2 + gateway_client.c | 1 + gateway_common.c | 2 + hard-interface.c | 1 + icmp_socket.c | 1 + log.h | 82 +++++++ main.c | 626 +++--------------------------------------------- main.h | 101 +------- multicast.c | 1 + network-coding.c | 2 + originator.c | 1 + packet.h | 5 + routing.c | 2 + send.c | 1 + sysfs.c | 1 + translation-table.c | 2 + tvlv.c | 592 +++++++++++++++++++++++++++++++++++++++++++++ tvlv.h | 62 +++++ types.h | 8 +- 25 files changed, 967 insertions(+), 820 deletions(-) create mode 100644 log.h create mode 100644 tvlv.c create mode 100644 tvlv.h
Normally the debugfs framework will return error pointer with -ENODEV for function calls when DEBUG_FS is not set.
batman does not notice this error code and continues trying to create debugfs files and executes more code. We can avoid this code execution by disabling compiling debugfs.c when DEBUG_FS is not set.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- Makefile.kbuild | 2 +- debugfs.c | 8 -------- debugfs.h | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/Makefile.kbuild b/Makefile.kbuild index eb7d8c0388e4..96fa99b3a409 100644 --- a/Makefile.kbuild +++ b/Makefile.kbuild @@ -20,7 +20,7 @@ obj-$(CONFIG_BATMAN_ADV) += batman-adv.o batman-adv-y += bat_iv_ogm.o batman-adv-y += bitarray.o batman-adv-$(CONFIG_BATMAN_ADV_BLA) += bridge_loop_avoidance.o -batman-adv-y += debugfs.o +batman-adv-$(CONFIG_DEBUG_FS) += debugfs.o batman-adv-$(CONFIG_BATMAN_ADV_DAT) += distributed-arp-table.o batman-adv-y += fragmentation.o batman-adv-y += gateway_client.o diff --git a/debugfs.c b/debugfs.c index d9b9a8eb17fb..026a25ce364c 100644 --- a/debugfs.c +++ b/debugfs.c @@ -483,11 +483,7 @@ rem_attr: debugfs_remove_recursive(hard_iface->debug_dir); hard_iface->debug_dir = NULL; out: -#ifdef CONFIG_DEBUG_FS return -ENOMEM; -#else - return 0; -#endif /* CONFIG_DEBUG_FS */ }
/** @@ -542,11 +538,7 @@ rem_attr: debugfs_remove_recursive(bat_priv->debug_dir); bat_priv->debug_dir = NULL; out: -#ifdef CONFIG_DEBUG_FS return -ENOMEM; -#else - return 0; -#endif /* CONFIG_DEBUG_FS */ }
void batadv_debugfs_del_meshif(struct net_device *dev) diff --git a/debugfs.h b/debugfs.h index 37c4d6ddd04d..5db336a6ef57 100644 --- a/debugfs.h +++ b/debugfs.h @@ -20,6 +20,8 @@
#define BATADV_DEBUGFS_SUBDIR "batman_adv"
+#if IS_ENABLED(CONFIG_DEBUG_FS) + void batadv_debugfs_init(void); void batadv_debugfs_destroy(void); int batadv_debugfs_add_meshif(struct net_device *dev); @@ -27,4 +29,36 @@ void batadv_debugfs_del_meshif(struct net_device *dev); int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface); void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface);
+#else + +static inline void batadv_debugfs_init(void) +{ +} + +static inline void batadv_debugfs_destroy(void) +{ +} + +static inline int batadv_debugfs_add_meshif(struct net_device *dev) +{ + return 0; +} + +static inline void batadv_debugfs_del_meshif(struct net_device *dev) +{ +} + +static inline int batadv_debugfs_add_hardif( + struct batadv_hard_iface *hard_iface) +{ + return 0; +} + +static inline void batadv_debugfs_del_hardif( + struct batadv_hard_iface *hard_iface) +{ +} + +#endif + #endif /* _NET_BATMAN_ADV_DEBUGFS_H_ */
On Friday 26 December 2014 12:41:18 Markus Pargmann wrote:
Normally the debugfs framework will return error pointer with -ENODEV for function calls when DEBUG_FS is not set.
batman does not notice this error code and continues trying to create debugfs files and executes more code. We can avoid this code execution by disabling compiling debugfs.c when DEBUG_FS is not set.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
Makefile.kbuild | 2 +- debugfs.c | 8 -------- debugfs.h | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-)
Applied in revision fe67bb4.
Thanks, Marek
This patch creates a separate header file for logging related functions and definitions.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 1 + bitarray.c | 1 + bridge_loop_avoidance.c | 1 + debugfs.c | 1 + debugfs.h | 5 +++ distributed-arp-table.c | 1 + gateway_client.c | 1 + gateway_common.c | 1 + hard-interface.c | 1 + icmp_socket.c | 1 + log.h | 82 +++++++++++++++++++++++++++++++++++++++++++++++++ main.c | 1 + main.h | 62 ------------------------------------- network-coding.c | 1 + originator.c | 1 + routing.c | 1 + send.c | 1 + sysfs.c | 1 + translation-table.c | 1 + 19 files changed, 103 insertions(+), 62 deletions(-) create mode 100644 log.h
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 00e00e09b000..2d064a71613f 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -25,6 +25,7 @@ #include "send.h" #include "bat_algo.h" #include "network-coding.h" +#include "log.h"
/** * enum batadv_dup_status - duplicate status diff --git a/bitarray.c b/bitarray.c index e3da07a64026..3fbaf540d56d 100644 --- a/bitarray.c +++ b/bitarray.c @@ -17,6 +17,7 @@
#include "main.h" #include "bitarray.h" +#include "log.h"
#include <linux/bitops.h>
diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index 4fc6cab7ed46..d93288377f80 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -22,6 +22,7 @@ #include "bridge_loop_avoidance.h" #include "translation-table.h" #include "send.h" +#include "log.h"
#include <linux/etherdevice.h> #include <linux/crc16.h> diff --git a/debugfs.c b/debugfs.c index 026a25ce364c..3f21e855a13d 100644 --- a/debugfs.c +++ b/debugfs.c @@ -30,6 +30,7 @@ #include "bridge_loop_avoidance.h" #include "distributed-arp-table.h" #include "network-coding.h" +#include "log.h"
static struct dentry *batadv_debugfs;
diff --git a/debugfs.h b/debugfs.h index 5db336a6ef57..1fda474defe8 100644 --- a/debugfs.h +++ b/debugfs.h @@ -20,6 +20,11 @@
#define BATADV_DEBUGFS_SUBDIR "batman_adv"
+#ifdef CONFIG_BATMAN_ADV_DEBUG +int batadv_debug_log(struct batadv_priv *bat_priv, const char *fmt, ...) +__printf(2, 3); +#endif + #if IS_ENABLED(CONFIG_DEBUG_FS)
void batadv_debugfs_init(void); diff --git a/distributed-arp-table.c b/distributed-arp-table.c index aad022dd15df..e2f0677a5a01 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -28,6 +28,7 @@ #include "send.h" #include "types.h" #include "translation-table.h" +#include "log.h"
static void batadv_dat_purge(struct work_struct *work);
diff --git a/gateway_client.c b/gateway_client.c index 27649e85f3f6..d4aa578c78f9 100644 --- a/gateway_client.c +++ b/gateway_client.c @@ -23,6 +23,7 @@ #include "originator.h" #include "translation-table.h" #include "routing.h" +#include "log.h" #include <linux/ip.h> #include <linux/ipv6.h> #include <linux/udp.h> diff --git a/gateway_common.c b/gateway_common.c index 6f5e621f220a..cbef7fd97971 100644 --- a/gateway_common.c +++ b/gateway_common.c @@ -18,6 +18,7 @@ #include "main.h" #include "gateway_common.h" #include "gateway_client.h" +#include "log.h"
/** * batadv_parse_gw_bandwidth - parse supplied string buffer to extract download diff --git a/hard-interface.c b/hard-interface.c index fbda6b54baff..5138f11babb4 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -28,6 +28,7 @@ #include "hash.h" #include "bridge_loop_avoidance.h" #include "gateway_client.h" +#include "log.h"
#include <linux/if_arp.h> #include <linux/if_ether.h> diff --git a/icmp_socket.c b/icmp_socket.c index 161ef8f17d2e..520b2f4739d1 100644 --- a/icmp_socket.c +++ b/icmp_socket.c @@ -23,6 +23,7 @@ #include "hash.h" #include "originator.h" #include "hard-interface.h" +#include "log.h"
static struct batadv_socket_client *batadv_socket_client_hash[256];
diff --git a/log.h b/log.h new file mode 100644 index 000000000000..05b0b9d15883 --- /dev/null +++ b/log.h @@ -0,0 +1,82 @@ +/* Copyright (C) 2007-2014 B.A.T.M.A.N. contributors: + * + * Marek Lindner, Simon Wunderlich + * + * 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, see http://www.gnu.org/licenses/. + */ + +#ifndef _NET_BATMAN_ADV_LOG_H_ +#define _NET_BATMAN_ADV_LOG_H_ + +#include "debugfs.h" + +/** + * enum batadv_dbg_level - available log levels + * @BATADV_DBG_BATMAN: OGM and TQ computations related messages + * @BATADV_DBG_ROUTES: route added / changed / deleted + * @BATADV_DBG_TT: translation table messages + * @BATADV_DBG_BLA: bridge loop avoidance messages + * @BATADV_DBG_DAT: ARP snooping and DAT related messages + * @BATADV_DBG_NC: network coding related messages + * @BATADV_DBG_ALL: the union of all the above log levels + */ +enum batadv_dbg_level { + BATADV_DBG_BATMAN = BIT(0), + BATADV_DBG_ROUTES = BIT(1), + BATADV_DBG_TT = BIT(2), + BATADV_DBG_BLA = BIT(3), + BATADV_DBG_DAT = BIT(4), + BATADV_DBG_NC = BIT(5), + BATADV_DBG_ALL = 63, +}; + +#ifdef CONFIG_BATMAN_ADV_DEBUG +/* possibly ratelimited debug output */ +#define _batadv_dbg(type, bat_priv, ratelimited, fmt, arg...) \ + do { \ + if (atomic_read(&bat_priv->log_level) & type && \ + (!ratelimited || net_ratelimit())) \ + batadv_debug_log(bat_priv, fmt, ## arg);\ + } \ + while (0) +#else /* !CONFIG_BATMAN_ADV_DEBUG */ +__printf(4, 5) +static inline void _batadv_dbg(int type __always_unused, + struct batadv_priv *bat_priv __always_unused, + int ratelimited __always_unused, + const char *fmt __always_unused, ...) +{ +} +#endif + +#define batadv_dbg(type, bat_priv, arg...) \ + _batadv_dbg(type, bat_priv, 0, ## arg) +#define batadv_dbg_ratelimited(type, bat_priv, arg...) \ + _batadv_dbg(type, bat_priv, 1, ## arg) + +#define batadv_info(net_dev, fmt, arg...) \ + do { \ + struct net_device *_netdev = (net_dev); \ + struct batadv_priv *_batpriv = netdev_priv(_netdev); \ + batadv_dbg(BATADV_DBG_ALL, _batpriv, fmt, ## arg); \ + pr_info("%s: " fmt, _netdev->name, ## arg); \ + } while (0) +#define batadv_err(net_dev, fmt, arg...) \ + do { \ + struct net_device *_netdev = (net_dev); \ + struct batadv_priv *_batpriv = netdev_priv(_netdev); \ + batadv_dbg(BATADV_DBG_ALL, _batpriv, fmt, ## arg); \ + pr_err("%s: " fmt, _netdev->name, ## arg); \ + } while (0) + +#endif /* _NET_BATMAN_ADV_LOG_H_ */ diff --git a/main.c b/main.c index 3bcd847a8f9f..a9e09e852c4b 100644 --- a/main.c +++ b/main.c @@ -40,6 +40,7 @@ #include "bat_algo.h" #include "network-coding.h" #include "fragmentation.h" +#include "log.h"
/* List manipulations on hardif_list have to be rtnl_lock()'ed, * list traversals just rcu-locked diff --git a/main.h b/main.h index 52cb0072bfc8..1cd4ebdbe060 100644 --- a/main.h +++ b/main.h @@ -214,68 +214,6 @@ int batadv_algo_select(struct batadv_priv *bat_priv, char *name); int batadv_algo_seq_print_text(struct seq_file *seq, void *offset); __be32 batadv_skb_crc32(struct sk_buff *skb, u8 *payload_ptr);
-/** - * enum batadv_dbg_level - available log levels - * @BATADV_DBG_BATMAN: OGM and TQ computations related messages - * @BATADV_DBG_ROUTES: route added / changed / deleted - * @BATADV_DBG_TT: translation table messages - * @BATADV_DBG_BLA: bridge loop avoidance messages - * @BATADV_DBG_DAT: ARP snooping and DAT related messages - * @BATADV_DBG_NC: network coding related messages - * @BATADV_DBG_ALL: the union of all the above log levels - */ -enum batadv_dbg_level { - BATADV_DBG_BATMAN = BIT(0), - BATADV_DBG_ROUTES = BIT(1), - BATADV_DBG_TT = BIT(2), - BATADV_DBG_BLA = BIT(3), - BATADV_DBG_DAT = BIT(4), - BATADV_DBG_NC = BIT(5), - BATADV_DBG_ALL = 63, -}; - -#ifdef CONFIG_BATMAN_ADV_DEBUG -int batadv_debug_log(struct batadv_priv *bat_priv, const char *fmt, ...) -__printf(2, 3); - -/* possibly ratelimited debug output */ -#define _batadv_dbg(type, bat_priv, ratelimited, fmt, arg...) \ - do { \ - if (atomic_read(&bat_priv->log_level) & type && \ - (!ratelimited || net_ratelimit())) \ - batadv_debug_log(bat_priv, fmt, ## arg);\ - } \ - while (0) -#else /* !CONFIG_BATMAN_ADV_DEBUG */ -__printf(4, 5) -static inline void _batadv_dbg(int type __always_unused, - struct batadv_priv *bat_priv __always_unused, - int ratelimited __always_unused, - const char *fmt __always_unused, ...) -{ -} -#endif - -#define batadv_dbg(type, bat_priv, arg...) \ - _batadv_dbg(type, bat_priv, 0, ## arg) -#define batadv_dbg_ratelimited(type, bat_priv, arg...) \ - _batadv_dbg(type, bat_priv, 1, ## arg) - -#define batadv_info(net_dev, fmt, arg...) \ - do { \ - struct net_device *_netdev = (net_dev); \ - struct batadv_priv *_batpriv = netdev_priv(_netdev); \ - batadv_dbg(BATADV_DBG_ALL, _batpriv, fmt, ## arg); \ - pr_info("%s: " fmt, _netdev->name, ## arg); \ - } while (0) -#define batadv_err(net_dev, fmt, arg...) \ - do { \ - struct net_device *_netdev = (net_dev); \ - struct batadv_priv *_batpriv = netdev_priv(_netdev); \ - batadv_dbg(BATADV_DBG_ALL, _batpriv, fmt, ## arg); \ - pr_err("%s: " fmt, _netdev->name, ## arg); \ - } while (0) - /* returns 1 if they are the same ethernet addr * * note: can't use ether_addr_equal() as it requires aligned memory diff --git a/network-coding.c b/network-coding.c index 127cc4d7380a..93190fc25f76 100644 --- a/network-coding.c +++ b/network-coding.c @@ -24,6 +24,7 @@ #include "originator.h" #include "hard-interface.h" #include "routing.h" +#include "log.h"
static struct lock_class_key batadv_nc_coding_hash_lock_class_key; static struct lock_class_key batadv_nc_decoding_hash_lock_class_key; diff --git a/originator.c b/originator.c index 6c2cb2da9865..3386ae54ede5 100644 --- a/originator.c +++ b/originator.c @@ -28,6 +28,7 @@ #include "network-coding.h" #include "fragmentation.h" #include "multicast.h" +#include "log.h"
/* hash class keys */ static struct lock_class_key batadv_orig_hash_lock_class_key; diff --git a/routing.c b/routing.c index 139d2f65728e..07205591b05f 100644 --- a/routing.c +++ b/routing.c @@ -27,6 +27,7 @@ #include "distributed-arp-table.h" #include "network-coding.h" #include "fragmentation.h" +#include "log.h"
#include <linux/if_vlan.h>
diff --git a/send.c b/send.c index d27161e1088d..043ee89596be 100644 --- a/send.c +++ b/send.c @@ -28,6 +28,7 @@ #include "network-coding.h" #include "fragmentation.h" #include "multicast.h" +#include "log.h"
static void batadv_send_outstanding_bcast_packet(struct work_struct *work);
diff --git a/sysfs.c b/sysfs.c index a63c3ebfbf53..6e39cdef1e1f 100644 --- a/sysfs.c +++ b/sysfs.c @@ -25,6 +25,7 @@ #include "soft-interface.h" #include "gateway_common.h" #include "gateway_client.h" +#include "log.h"
static struct net_device *batadv_kobj_to_netdev(struct kobject *obj) { diff --git a/translation-table.c b/translation-table.c index 84e6f01b734f..be75a2ac543e 100644 --- a/translation-table.c +++ b/translation-table.c @@ -25,6 +25,7 @@ #include "routing.h" #include "bridge_loop_avoidance.h" #include "multicast.h" +#include "log.h"
#include <linux/crc32c.h>
On Friday 26 December 2014 12:41:19 Markus Pargmann wrote:
This patch creates a separate header file for logging related functions and definitions.
Martin's comment still stands. Your commit message states the obvious without giving any clue as to why this is desired or wanted ..
Cheers, Marek
Hi Marek,
On Sun, Dec 28, 2014 at 10:33:58AM +0800, Marek Lindner wrote:
On Friday 26 December 2014 12:41:19 Markus Pargmann wrote:
This patch creates a separate header file for logging related functions and definitions.
Martin's comment still stands. Your commit message states the obvious without giving any clue as to why this is desired or wanted ..
Didn't see Martin's comment as my email somewhere was lost in the to list.
However I am doing this because I think it is better to remove as much code from main.c/h as possible that is not used for init/shutdown. Logging functions are not interesting for the reader of main.c, so I think it should be moved to a separate file.
I will add some similar explanation to the commit message.
Best regards,
Markus
The difference between tq1 and tq2 are calculated the same way in two separate functions.
This patch moves the common code to a seperate function 'batadv_iv_ogm_neigh_diff' which handles everything necessary. The other two functions can then handle errors and use the difference directly.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 80 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 41 insertions(+), 39 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 2d064a71613f..1458ecfa66b8 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1858,36 +1858,27 @@ next: seq_puts(seq, "No batman nodes in range ...\n"); }
-/** - * batadv_iv_ogm_neigh_cmp - compare the metrics of two neighbors - * @neigh1: the first neighbor object of the comparison - * @if_outgoing1: outgoing interface for the first neighbor - * @neigh2: the second neighbor object of the comparison - * @if_outgoing2: outgoing interface for the second neighbor - * - * Returns a value less, equal to or greater than 0 if the metric via neigh1 is - * lower, the same as or higher than the metric via neigh2 - */ -static int batadv_iv_ogm_neigh_cmp(struct batadv_neigh_node *neigh1, - struct batadv_hard_iface *if_outgoing1, - struct batadv_neigh_node *neigh2, - struct batadv_hard_iface *if_outgoing2) +static int batadv_iv_ogm_neigh_diff(struct batadv_neigh_node *neigh1, + struct batadv_hard_iface *if_outgoing1, + struct batadv_neigh_node *neigh2, + struct batadv_hard_iface *if_outgoing2, + int *diff) { struct batadv_neigh_ifinfo *neigh1_ifinfo, *neigh2_ifinfo; uint8_t tq1, tq2; - int diff; + int ret;
neigh1_ifinfo = batadv_neigh_ifinfo_get(neigh1, if_outgoing1); neigh2_ifinfo = batadv_neigh_ifinfo_get(neigh2, if_outgoing2);
if (!neigh1_ifinfo || !neigh2_ifinfo) { - diff = 0; + ret = -EINVAL; goto out; }
tq1 = neigh1_ifinfo->bat_iv.tq_avg; tq2 = neigh2_ifinfo->bat_iv.tq_avg; - diff = tq1 - tq2; + *diff = (int)tq1 - (int)tq2;
out: if (neigh1_ifinfo) @@ -1895,6 +1886,32 @@ out: if (neigh2_ifinfo) batadv_neigh_ifinfo_free_ref(neigh2_ifinfo);
+ return ret; +} + +/** + * batadv_iv_ogm_neigh_cmp - compare the metrics of two neighbors + * @neigh1: the first neighbor object of the comparison + * @if_outgoing1: outgoing interface for the first neighbor + * @neigh2: the second neighbor object of the comparison + * @if_outgoing2: outgoing interface for the second neighbor + * + * Returns a value less, equal to or greater than 0 if the metric via neigh1 is + * lower, the same as or higher than the metric via neigh2 + */ +static int batadv_iv_ogm_neigh_cmp(struct batadv_neigh_node *neigh1, + struct batadv_hard_iface *if_outgoing1, + struct batadv_neigh_node *neigh2, + struct batadv_hard_iface *if_outgoing2) +{ + int ret; + int diff; + + ret = batadv_iv_ogm_neigh_diff(neigh1, if_outgoing1, neigh2, + if_outgoing2, &diff); + if (ret) + return 0; + return diff; }
@@ -1915,30 +1932,15 @@ batadv_iv_ogm_neigh_is_eob(struct batadv_neigh_node *neigh1, struct batadv_neigh_node *neigh2, struct batadv_hard_iface *if_outgoing2) { - struct batadv_neigh_ifinfo *neigh1_ifinfo, *neigh2_ifinfo; - uint8_t tq1, tq2; - bool ret; - - neigh1_ifinfo = batadv_neigh_ifinfo_get(neigh1, if_outgoing1); - neigh2_ifinfo = batadv_neigh_ifinfo_get(neigh2, if_outgoing2); - - /* we can't say that the metric is better */ - if (!neigh1_ifinfo || !neigh2_ifinfo) { - ret = false; - goto out; - } - - tq1 = neigh1_ifinfo->bat_iv.tq_avg; - tq2 = neigh2_ifinfo->bat_iv.tq_avg; - ret = (tq1 - tq2) > -BATADV_TQ_SIMILARITY_THRESHOLD; + int ret; + int diff;
-out: - if (neigh1_ifinfo) - batadv_neigh_ifinfo_free_ref(neigh1_ifinfo); - if (neigh2_ifinfo) - batadv_neigh_ifinfo_free_ref(neigh2_ifinfo); + ret = batadv_iv_ogm_neigh_diff(neigh1, if_outgoing1, neigh2, + if_outgoing2, &diff); + if (ret) + return false;
- return ret; + return diff > -BATADV_TQ_SIMILARITY_THRESHOLD; }
static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
On Friday 26 December 2014 12:41:20 Markus Pargmann wrote:
+static int batadv_iv_ogm_neigh_diff(struct batadv_neigh_node *neigh1,
struct batadv_hard_iface *if_outgoing1,struct batadv_neigh_node *neigh2,struct batadv_hard_iface *if_outgoing2,int *diff){ struct batadv_neigh_ifinfo *neigh1_ifinfo, *neigh2_ifinfo; uint8_t tq1, tq2;
int diff;
int ret;
In the success case 'ret' is never initialized or set to any specific value. We do lack kernel doc for the newly created function 'batadv_iv_ogm_neigh_diff'.
Cheers, Marek
Instead of the normal division which looses precision, use a division with rounding.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 1458ecfa66b8..10eada270015 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -82,7 +82,7 @@ static uint8_t batadv_ring_buffer_avg(const uint8_t lq_recv[]) if (count == 0) return 0;
- return (uint8_t)(sum / count); + return (uint8_t)DIV_ROUND_CLOSEST(sum, count); }
/**
On Friday 26 December 2014 12:41:21 Markus Pargmann wrote:
Instead of the normal division which looses precision, use a division with rounding.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 1458ecfa66b8..10eada270015 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -82,7 +82,7 @@ static uint8_t batadv_ring_buffer_avg(const uint8_t lq_recv[]) if (count == 0) return 0;
return (uint8_t)(sum / count);
return (uint8_t)DIV_ROUND_CLOSEST(sum, count);}
Is DIV_ROUND_CLOSEST available in 2.6.29 ? I could not find a conclusive hint whether or not we need compat code for the out-of-tree module ?
Cheers, Marek
On Sunday 11 January 2015 20:32:49 Marek Lindner wrote:
Is DIV_ROUND_CLOSEST available in 2.6.29 ? I could not find a conclusive hint whether or not we need compat code for the out-of-tree module ?
It was introduced in 2.6.29 in v2.6.28-6240-g9fe0608. So you would only need compat code in marek/compat_old_kernel
Kind regards, Sven
On Friday 26 December 2014 12:41:21 Markus Pargmann wrote:
Instead of the normal division which looses precision, use a division with rounding.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
(sorry for the duplicate, sent the reply for the first patch version, so I'm replying another time on v2 ...)
Why do we need to have more precise rounding here? In doubt, we should rather always round down to avoid any spurious routing loops - the loop free property depends on monotonicity after all, and therefore its better to always round down.
I'm not convinced that this change is safe in that regard, if you think it is please explain further.
bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 1458ecfa66b8..10eada270015 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -82,7 +82,7 @@ static uint8_t batadv_ring_buffer_avg(const uint8_t lq_recv[]) if (count == 0) return 0;
- return (uint8_t)(sum / count);
- return (uint8_t)DIV_ROUND_CLOSEST(sum, count);
}
/**
This patch adds some error handling for the main init function. It checks the return values of all the function calls that provide return values.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- main.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/main.c b/main.c index a9e09e852c4b..65021ea567e8 100644 --- a/main.c +++ b/main.c @@ -59,29 +59,47 @@ static void batadv_recv_handler_init(void);
static int __init batadv_init(void) { + int ret; + INIT_LIST_HEAD(&batadv_hardif_list); INIT_HLIST_HEAD(&batadv_algo_list);
batadv_recv_handler_init();
- batadv_iv_init(); - batadv_nc_init(); + ret = batadv_iv_init(); + if (ret) + return ret; + + ret = batadv_nc_init(); + if (ret) + return ret;
batadv_event_workqueue = create_singlethread_workqueue("bat_events"); - if (!batadv_event_workqueue) return -ENOMEM;
batadv_socket_init(); batadv_debugfs_init();
- register_netdevice_notifier(&batadv_hard_if_notifier); - rtnl_link_register(&batadv_link_ops); + ret = register_netdevice_notifier(&batadv_hard_if_notifier); + if (ret) + goto err_netdev_notifier; + + ret = rtnl_link_register(&batadv_link_ops); + if (ret) + goto err_link_register;
pr_info("B.A.T.M.A.N. advanced %s (compatibility version %i) loaded\n", BATADV_SOURCE_VERSION, BATADV_COMPAT_VERSION);
return 0; + +err_link_register: + unregister_netdevice_notifier(&batadv_hard_if_notifier); +err_netdev_notifier: + batadv_debugfs_destroy(); + + return ret; }
static void __exit batadv_exit(void)
On Friday 26 December 2014 12:41:22 Markus Pargmann wrote:
static int __init batadv_init(void) {
int ret;INIT_LIST_HEAD(&batadv_hardif_list); INIT_HLIST_HEAD(&batadv_algo_list); batadv_recv_handler_init();
batadv_iv_init();batadv_nc_init();
ret = batadv_iv_init();if (ret)return ret;ret = batadv_nc_init();if (ret)return ret; batadv_event_workqueue =create_singlethread_workqueue("bat_events"); - if (!batadv_event_workqueue) return -ENOMEM;
batadv_socket_init(); batadv_debugfs_init();
register_netdevice_notifier(&batadv_hard_if_notifier);rtnl_link_register(&batadv_link_ops);
ret = register_netdevice_notifier(&batadv_hard_if_notifier);if (ret)goto err_netdev_notifier;ret = rtnl_link_register(&batadv_link_ops);if (ret)goto err_link_register; pr_info("B.A.T.M.A.N. advanced %s (compatibility version %i)loaded\n", BATADV_SOURCE_VERSION, BATADV_COMPAT_VERSION);
return 0;
+err_link_register:
unregister_netdevice_notifier(&batadv_hard_if_notifier);+err_netdev_notifier:
batadv_debugfs_destroy();return ret;}
To be truely clean, I guess the workqueue should be destroyed as well ?
Cheers, Marek
On Sun, Jan 11, 2015 at 08:38:10PM +0800, Marek Lindner wrote:
On Friday 26 December 2014 12:41:22 Markus Pargmann wrote:
static int __init batadv_init(void) {
int ret;INIT_LIST_HEAD(&batadv_hardif_list); INIT_HLIST_HEAD(&batadv_algo_list); batadv_recv_handler_init();
batadv_iv_init();batadv_nc_init();
ret = batadv_iv_init();if (ret)return ret;ret = batadv_nc_init();if (ret)return ret; batadv_event_workqueue =create_singlethread_workqueue("bat_events"); - if (!batadv_event_workqueue) return -ENOMEM;
batadv_socket_init(); batadv_debugfs_init();
register_netdevice_notifier(&batadv_hard_if_notifier);rtnl_link_register(&batadv_link_ops);
ret = register_netdevice_notifier(&batadv_hard_if_notifier);if (ret)goto err_netdev_notifier;ret = rtnl_link_register(&batadv_link_ops);if (ret)goto err_link_register; pr_info("B.A.T.M.A.N. advanced %s (compatibility version %i)loaded\n", BATADV_SOURCE_VERSION, BATADV_COMPAT_VERSION);
return 0;
+err_link_register:
unregister_netdevice_notifier(&batadv_hard_if_notifier);+err_netdev_notifier:
batadv_debugfs_destroy();return ret;}
To be truely clean, I guess the workqueue should be destroyed as well ?
Yes, thanks, added destroy_workqueue() for v3.
Best Regards,
Markus
Instead of hiding the normal function flow inside an if block, we should just put the error handling into the if block.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- main.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/main.c b/main.c index 65021ea567e8..00a1107109e8 100644 --- a/main.c +++ b/main.c @@ -838,15 +838,15 @@ static bool batadv_tvlv_realloc_packet_buff(unsigned char **packet_buff, new_buff = kmalloc(min_packet_len + additional_packet_len, GFP_ATOMIC);
/* keep old buffer if kmalloc should fail */ - if (new_buff) { - memcpy(new_buff, *packet_buff, min_packet_len); - kfree(*packet_buff); - *packet_buff = new_buff; - *packet_buff_len = min_packet_len + additional_packet_len; - return true; - } + if (!new_buff) + return false; + + memcpy(new_buff, *packet_buff, min_packet_len); + kfree(*packet_buff); + *packet_buff = new_buff; + *packet_buff_len = min_packet_len + additional_packet_len;
- return false; + return true; }
/**
On Friday 26 December 2014 12:41:23 Markus Pargmann wrote:
Instead of hiding the normal function flow inside an if block, we should just put the error handling into the if block.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
main.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Applied in revision 9aef872.
Thanks, Marek
The main file includes a lot of helper functions. tvlv is a seperate set of functions so we should split them into a seperate file. There is no need for tvlv to be implemented in main.c.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- Makefile.kbuild | 1 + bat_iv_ogm.c | 1 + distributed-arp-table.c | 1 + gateway_common.c | 1 + main.c | 568 ---------------------------------------------- main.h | 35 --- multicast.c | 1 + network-coding.c | 1 + routing.c | 1 + translation-table.c | 1 + tvlv.c | 592 ++++++++++++++++++++++++++++++++++++++++++++++++ tvlv.h | 62 +++++ 12 files changed, 662 insertions(+), 603 deletions(-) create mode 100644 tvlv.c create mode 100644 tvlv.h
diff --git a/Makefile.kbuild b/Makefile.kbuild index 96fa99b3a409..a8c34baddd63 100644 --- a/Makefile.kbuild +++ b/Makefile.kbuild @@ -36,4 +36,5 @@ batman-adv-y += send.o batman-adv-y += soft-interface.o batman-adv-y += sysfs.o batman-adv-y += translation-table.o +batman-adv-y += tvlv.o batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += multicast.o diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 10eada270015..20295c5e5121 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -23,6 +23,7 @@ #include "gateway_client.h" #include "hard-interface.h" #include "send.h" +#include "tvlv.h" #include "bat_algo.h" #include "network-coding.h" #include "log.h" diff --git a/distributed-arp-table.c b/distributed-arp-table.c index e2f0677a5a01..e3deff63d393 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -29,6 +29,7 @@ #include "types.h" #include "translation-table.h" #include "log.h" +#include "tvlv.h"
static void batadv_dat_purge(struct work_struct *work);
diff --git a/gateway_common.c b/gateway_common.c index cbef7fd97971..c2dfcc03ddfa 100644 --- a/gateway_common.c +++ b/gateway_common.c @@ -19,6 +19,7 @@ #include "gateway_common.h" #include "gateway_client.h" #include "log.h" +#include "tvlv.h"
/** * batadv_parse_gw_bandwidth - parse supplied string buffer to extract download diff --git a/main.c b/main.c index 00a1107109e8..cff31bb9bb14 100644 --- a/main.c +++ b/main.c @@ -623,574 +623,6 @@ __be32 batadv_skb_crc32(struct sk_buff *skb, u8 *payload_ptr) }
/** - * batadv_tvlv_handler_free_ref - decrement the tvlv handler refcounter and - * possibly free it - * @tvlv_handler: the tvlv handler to free - */ -static void -batadv_tvlv_handler_free_ref(struct batadv_tvlv_handler *tvlv_handler) -{ - if (atomic_dec_and_test(&tvlv_handler->refcount)) - kfree_rcu(tvlv_handler, rcu); -} - -/** - * batadv_tvlv_handler_get - retrieve tvlv handler from the tvlv handler list - * based on the provided type and version (both need to match) - * @bat_priv: the bat priv with all the soft interface information - * @type: tvlv handler type to look for - * @version: tvlv handler version to look for - * - * Returns tvlv handler if found or NULL otherwise. - */ -static struct batadv_tvlv_handler -*batadv_tvlv_handler_get(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version) -{ - struct batadv_tvlv_handler *tvlv_handler_tmp, *tvlv_handler = NULL; - - rcu_read_lock(); - hlist_for_each_entry_rcu(tvlv_handler_tmp, - &bat_priv->tvlv.handler_list, list) { - if (tvlv_handler_tmp->type != type) - continue; - - if (tvlv_handler_tmp->version != version) - continue; - - if (!atomic_inc_not_zero(&tvlv_handler_tmp->refcount)) - continue; - - tvlv_handler = tvlv_handler_tmp; - break; - } - rcu_read_unlock(); - - return tvlv_handler; -} - -/** - * batadv_tvlv_container_free_ref - decrement the tvlv container refcounter and - * possibly free it - * @tvlv: the tvlv container to free - */ -static void batadv_tvlv_container_free_ref(struct batadv_tvlv_container *tvlv) -{ - if (atomic_dec_and_test(&tvlv->refcount)) - kfree(tvlv); -} - -/** - * batadv_tvlv_container_get - retrieve tvlv container from the tvlv container - * list based on the provided type and version (both need to match) - * @bat_priv: the bat priv with all the soft interface information - * @type: tvlv container type to look for - * @version: tvlv container version to look for - * - * Has to be called with the appropriate locks being acquired - * (tvlv.container_list_lock). - * - * Returns tvlv container if found or NULL otherwise. - */ -static struct batadv_tvlv_container -*batadv_tvlv_container_get(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version) -{ - struct batadv_tvlv_container *tvlv_tmp, *tvlv = NULL; - - hlist_for_each_entry(tvlv_tmp, &bat_priv->tvlv.container_list, list) { - if (tvlv_tmp->tvlv_hdr.type != type) - continue; - - if (tvlv_tmp->tvlv_hdr.version != version) - continue; - - if (!atomic_inc_not_zero(&tvlv_tmp->refcount)) - continue; - - tvlv = tvlv_tmp; - break; - } - - return tvlv; -} - -/** - * batadv_tvlv_container_list_size - calculate the size of the tvlv container - * list entries - * @bat_priv: the bat priv with all the soft interface information - * - * Has to be called with the appropriate locks being acquired - * (tvlv.container_list_lock). - * - * Returns size of all currently registered tvlv containers in bytes. - */ -static uint16_t batadv_tvlv_container_list_size(struct batadv_priv *bat_priv) -{ - struct batadv_tvlv_container *tvlv; - uint16_t tvlv_len = 0; - - hlist_for_each_entry(tvlv, &bat_priv->tvlv.container_list, list) { - tvlv_len += sizeof(struct batadv_tvlv_hdr); - tvlv_len += ntohs(tvlv->tvlv_hdr.len); - } - - return tvlv_len; -} - -/** - * batadv_tvlv_container_remove - remove tvlv container from the tvlv container - * list - * @tvlv: the to be removed tvlv container - * - * Has to be called with the appropriate locks being acquired - * (tvlv.container_list_lock). - */ -static void batadv_tvlv_container_remove(struct batadv_tvlv_container *tvlv) -{ - if (!tvlv) - return; - - hlist_del(&tvlv->list); - - /* first call to decrement the counter, second call to free */ - batadv_tvlv_container_free_ref(tvlv); - batadv_tvlv_container_free_ref(tvlv); -} - -/** - * batadv_tvlv_container_unregister - unregister tvlv container based on the - * provided type and version (both need to match) - * @bat_priv: the bat priv with all the soft interface information - * @type: tvlv container type to unregister - * @version: tvlv container type to unregister - */ -void batadv_tvlv_container_unregister(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version) -{ - struct batadv_tvlv_container *tvlv; - - spin_lock_bh(&bat_priv->tvlv.container_list_lock); - tvlv = batadv_tvlv_container_get(bat_priv, type, version); - batadv_tvlv_container_remove(tvlv); - spin_unlock_bh(&bat_priv->tvlv.container_list_lock); -} - -/** - * batadv_tvlv_container_register - register tvlv type, version and content - * to be propagated with each (primary interface) OGM - * @bat_priv: the bat priv with all the soft interface information - * @type: tvlv container type - * @version: tvlv container version - * @tvlv_value: tvlv container content - * @tvlv_value_len: tvlv container content length - * - * If a container of the same type and version was already registered the new - * content is going to replace the old one. - */ -void batadv_tvlv_container_register(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version, - void *tvlv_value, uint16_t tvlv_value_len) -{ - struct batadv_tvlv_container *tvlv_old, *tvlv_new; - - if (!tvlv_value) - tvlv_value_len = 0; - - tvlv_new = kzalloc(sizeof(*tvlv_new) + tvlv_value_len, GFP_ATOMIC); - if (!tvlv_new) - return; - - tvlv_new->tvlv_hdr.version = version; - tvlv_new->tvlv_hdr.type = type; - tvlv_new->tvlv_hdr.len = htons(tvlv_value_len); - - memcpy(tvlv_new + 1, tvlv_value, ntohs(tvlv_new->tvlv_hdr.len)); - INIT_HLIST_NODE(&tvlv_new->list); - atomic_set(&tvlv_new->refcount, 1); - - spin_lock_bh(&bat_priv->tvlv.container_list_lock); - tvlv_old = batadv_tvlv_container_get(bat_priv, type, version); - batadv_tvlv_container_remove(tvlv_old); - hlist_add_head(&tvlv_new->list, &bat_priv->tvlv.container_list); - spin_unlock_bh(&bat_priv->tvlv.container_list_lock); -} - -/** - * batadv_tvlv_realloc_packet_buff - reallocate packet buffer to accomodate - * requested packet size - * @packet_buff: packet buffer - * @packet_buff_len: packet buffer size - * @min_packet_len: requested packet minimum size - * @additional_packet_len: requested additional packet size on top of minimum - * size - * - * Returns true of the packet buffer could be changed to the requested size, - * false otherwise. - */ -static bool batadv_tvlv_realloc_packet_buff(unsigned char **packet_buff, - int *packet_buff_len, - int min_packet_len, - int additional_packet_len) -{ - unsigned char *new_buff; - - new_buff = kmalloc(min_packet_len + additional_packet_len, GFP_ATOMIC); - - /* keep old buffer if kmalloc should fail */ - if (!new_buff) - return false; - - memcpy(new_buff, *packet_buff, min_packet_len); - kfree(*packet_buff); - *packet_buff = new_buff; - *packet_buff_len = min_packet_len + additional_packet_len; - - return true; -} - -/** - * batadv_tvlv_container_ogm_append - append tvlv container content to given - * OGM packet buffer - * @bat_priv: the bat priv with all the soft interface information - * @packet_buff: ogm packet buffer - * @packet_buff_len: ogm packet buffer size including ogm header and tvlv - * content - * @packet_min_len: ogm header size to be preserved for the OGM itself - * - * The ogm packet might be enlarged or shrunk depending on the current size - * and the size of the to-be-appended tvlv containers. - * - * Returns size of all appended tvlv containers in bytes. - */ -uint16_t batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, - unsigned char **packet_buff, - int *packet_buff_len, - int packet_min_len) -{ - struct batadv_tvlv_container *tvlv; - struct batadv_tvlv_hdr *tvlv_hdr; - uint16_t tvlv_value_len; - void *tvlv_value; - bool ret; - - spin_lock_bh(&bat_priv->tvlv.container_list_lock); - tvlv_value_len = batadv_tvlv_container_list_size(bat_priv); - - ret = batadv_tvlv_realloc_packet_buff(packet_buff, packet_buff_len, - packet_min_len, tvlv_value_len); - - if (!ret) - goto end; - - if (!tvlv_value_len) - goto end; - - tvlv_value = (*packet_buff) + packet_min_len; - - hlist_for_each_entry(tvlv, &bat_priv->tvlv.container_list, list) { - tvlv_hdr = tvlv_value; - tvlv_hdr->type = tvlv->tvlv_hdr.type; - tvlv_hdr->version = tvlv->tvlv_hdr.version; - tvlv_hdr->len = tvlv->tvlv_hdr.len; - tvlv_value = tvlv_hdr + 1; - memcpy(tvlv_value, tvlv + 1, ntohs(tvlv->tvlv_hdr.len)); - tvlv_value = (uint8_t *)tvlv_value + ntohs(tvlv->tvlv_hdr.len); - } - -end: - spin_unlock_bh(&bat_priv->tvlv.container_list_lock); - return tvlv_value_len; -} - -/** - * batadv_tvlv_call_handler - parse the given tvlv buffer to call the - * appropriate handlers - * @bat_priv: the bat priv with all the soft interface information - * @tvlv_handler: tvlv callback function handling the tvlv content - * @ogm_source: flag indicating wether the tvlv is an ogm or a unicast packet - * @orig_node: orig node emitting the ogm packet - * @src: source mac address of the unicast packet - * @dst: destination mac address of the unicast packet - * @tvlv_value: tvlv content - * @tvlv_value_len: tvlv content length - * - * Returns success if handler was not found or the return value of the handler - * callback. - */ -static int batadv_tvlv_call_handler(struct batadv_priv *bat_priv, - struct batadv_tvlv_handler *tvlv_handler, - bool ogm_source, - struct batadv_orig_node *orig_node, - uint8_t *src, uint8_t *dst, - void *tvlv_value, uint16_t tvlv_value_len) -{ - if (!tvlv_handler) - return NET_RX_SUCCESS; - - if (ogm_source) { - if (!tvlv_handler->ogm_handler) - return NET_RX_SUCCESS; - - if (!orig_node) - return NET_RX_SUCCESS; - - tvlv_handler->ogm_handler(bat_priv, orig_node, - BATADV_NO_FLAGS, - tvlv_value, tvlv_value_len); - tvlv_handler->flags |= BATADV_TVLV_HANDLER_OGM_CALLED; - } else { - if (!src) - return NET_RX_SUCCESS; - - if (!dst) - return NET_RX_SUCCESS; - - if (!tvlv_handler->unicast_handler) - return NET_RX_SUCCESS; - - return tvlv_handler->unicast_handler(bat_priv, src, - dst, tvlv_value, - tvlv_value_len); - } - - return NET_RX_SUCCESS; -} - -/** - * batadv_tvlv_containers_process - parse the given tvlv buffer to call the - * appropriate handlers - * @bat_priv: the bat priv with all the soft interface information - * @ogm_source: flag indicating wether the tvlv is an ogm or a unicast packet - * @orig_node: orig node emitting the ogm packet - * @src: source mac address of the unicast packet - * @dst: destination mac address of the unicast packet - * @tvlv_value: tvlv content - * @tvlv_value_len: tvlv content length - * - * Returns success when processing an OGM or the return value of all called - * handler callbacks. - */ -int batadv_tvlv_containers_process(struct batadv_priv *bat_priv, - bool ogm_source, - struct batadv_orig_node *orig_node, - uint8_t *src, uint8_t *dst, - void *tvlv_value, uint16_t tvlv_value_len) -{ - struct batadv_tvlv_handler *tvlv_handler; - struct batadv_tvlv_hdr *tvlv_hdr; - uint16_t tvlv_value_cont_len; - uint8_t cifnotfound = BATADV_TVLV_HANDLER_OGM_CIFNOTFND; - int ret = NET_RX_SUCCESS; - - while (tvlv_value_len >= sizeof(*tvlv_hdr)) { - tvlv_hdr = tvlv_value; - tvlv_value_cont_len = ntohs(tvlv_hdr->len); - tvlv_value = tvlv_hdr + 1; - tvlv_value_len -= sizeof(*tvlv_hdr); - - if (tvlv_value_cont_len > tvlv_value_len) - break; - - tvlv_handler = batadv_tvlv_handler_get(bat_priv, - tvlv_hdr->type, - tvlv_hdr->version); - - ret |= batadv_tvlv_call_handler(bat_priv, tvlv_handler, - ogm_source, orig_node, - src, dst, tvlv_value, - tvlv_value_cont_len); - if (tvlv_handler) - batadv_tvlv_handler_free_ref(tvlv_handler); - tvlv_value = (uint8_t *)tvlv_value + tvlv_value_cont_len; - tvlv_value_len -= tvlv_value_cont_len; - } - - if (!ogm_source) - return ret; - - rcu_read_lock(); - hlist_for_each_entry_rcu(tvlv_handler, - &bat_priv->tvlv.handler_list, list) { - if ((tvlv_handler->flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND) && - !(tvlv_handler->flags & BATADV_TVLV_HANDLER_OGM_CALLED)) - tvlv_handler->ogm_handler(bat_priv, orig_node, - cifnotfound, NULL, 0); - - tvlv_handler->flags &= ~BATADV_TVLV_HANDLER_OGM_CALLED; - } - rcu_read_unlock(); - - return NET_RX_SUCCESS; -} - -/** - * batadv_tvlv_ogm_receive - process an incoming ogm and call the appropriate - * handlers - * @bat_priv: the bat priv with all the soft interface information - * @batadv_ogm_packet: ogm packet containing the tvlv containers - * @orig_node: orig node emitting the ogm packet - */ -void batadv_tvlv_ogm_receive(struct batadv_priv *bat_priv, - struct batadv_ogm_packet *batadv_ogm_packet, - struct batadv_orig_node *orig_node) -{ - void *tvlv_value; - uint16_t tvlv_value_len; - - if (!batadv_ogm_packet) - return; - - tvlv_value_len = ntohs(batadv_ogm_packet->tvlv_len); - if (!tvlv_value_len) - return; - - tvlv_value = batadv_ogm_packet + 1; - - batadv_tvlv_containers_process(bat_priv, true, orig_node, NULL, NULL, - tvlv_value, tvlv_value_len); -} - -/** - * batadv_tvlv_handler_register - register tvlv handler based on the provided - * type and version (both need to match) for ogm tvlv payload and/or unicast - * payload - * @bat_priv: the bat priv with all the soft interface information - * @optr: ogm tvlv handler callback function. This function receives the orig - * node, flags and the tvlv content as argument to process. - * @uptr: unicast tvlv handler callback function. This function receives the - * source & destination of the unicast packet as well as the tvlv content - * to process. - * @type: tvlv handler type to be registered - * @version: tvlv handler version to be registered - * @flags: flags to enable or disable TVLV API behavior - */ -void batadv_tvlv_handler_register(struct batadv_priv *bat_priv, - void (*optr)(struct batadv_priv *bat_priv, - struct batadv_orig_node *orig, - uint8_t flags, - void *tvlv_value, - uint16_t tvlv_value_len), - int (*uptr)(struct batadv_priv *bat_priv, - uint8_t *src, uint8_t *dst, - void *tvlv_value, - uint16_t tvlv_value_len), - uint8_t type, uint8_t version, uint8_t flags) -{ - struct batadv_tvlv_handler *tvlv_handler; - - tvlv_handler = batadv_tvlv_handler_get(bat_priv, type, version); - if (tvlv_handler) { - batadv_tvlv_handler_free_ref(tvlv_handler); - return; - } - - tvlv_handler = kzalloc(sizeof(*tvlv_handler), GFP_ATOMIC); - if (!tvlv_handler) - return; - - tvlv_handler->ogm_handler = optr; - tvlv_handler->unicast_handler = uptr; - tvlv_handler->type = type; - tvlv_handler->version = version; - tvlv_handler->flags = flags; - atomic_set(&tvlv_handler->refcount, 1); - INIT_HLIST_NODE(&tvlv_handler->list); - - spin_lock_bh(&bat_priv->tvlv.handler_list_lock); - hlist_add_head_rcu(&tvlv_handler->list, &bat_priv->tvlv.handler_list); - spin_unlock_bh(&bat_priv->tvlv.handler_list_lock); -} - -/** - * batadv_tvlv_handler_unregister - unregister tvlv handler based on the - * provided type and version (both need to match) - * @bat_priv: the bat priv with all the soft interface information - * @type: tvlv handler type to be unregistered - * @version: tvlv handler version to be unregistered - */ -void batadv_tvlv_handler_unregister(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version) -{ - struct batadv_tvlv_handler *tvlv_handler; - - tvlv_handler = batadv_tvlv_handler_get(bat_priv, type, version); - if (!tvlv_handler) - return; - - batadv_tvlv_handler_free_ref(tvlv_handler); - spin_lock_bh(&bat_priv->tvlv.handler_list_lock); - hlist_del_rcu(&tvlv_handler->list); - spin_unlock_bh(&bat_priv->tvlv.handler_list_lock); - batadv_tvlv_handler_free_ref(tvlv_handler); -} - -/** - * batadv_tvlv_unicast_send - send a unicast packet with tvlv payload to the - * specified host - * @bat_priv: the bat priv with all the soft interface information - * @src: source mac address of the unicast packet - * @dst: destination mac address of the unicast packet - * @type: tvlv type - * @version: tvlv version - * @tvlv_value: tvlv content - * @tvlv_value_len: tvlv content length - */ -void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, uint8_t *src, - uint8_t *dst, uint8_t type, uint8_t version, - void *tvlv_value, uint16_t tvlv_value_len) -{ - struct batadv_unicast_tvlv_packet *unicast_tvlv_packet; - struct batadv_tvlv_hdr *tvlv_hdr; - struct batadv_orig_node *orig_node; - struct sk_buff *skb = NULL; - unsigned char *tvlv_buff; - unsigned int tvlv_len; - ssize_t hdr_len = sizeof(*unicast_tvlv_packet); - bool ret = false; - - orig_node = batadv_orig_hash_find(bat_priv, dst); - if (!orig_node) - goto out; - - tvlv_len = sizeof(*tvlv_hdr) + tvlv_value_len; - - skb = netdev_alloc_skb_ip_align(NULL, ETH_HLEN + hdr_len + tvlv_len); - if (!skb) - goto out; - - skb->priority = TC_PRIO_CONTROL; - skb_reserve(skb, ETH_HLEN); - tvlv_buff = skb_put(skb, sizeof(*unicast_tvlv_packet) + tvlv_len); - unicast_tvlv_packet = (struct batadv_unicast_tvlv_packet *)tvlv_buff; - unicast_tvlv_packet->packet_type = BATADV_UNICAST_TVLV; - unicast_tvlv_packet->version = BATADV_COMPAT_VERSION; - unicast_tvlv_packet->ttl = BATADV_TTL; - unicast_tvlv_packet->reserved = 0; - unicast_tvlv_packet->tvlv_len = htons(tvlv_len); - unicast_tvlv_packet->align = 0; - ether_addr_copy(unicast_tvlv_packet->src, src); - ether_addr_copy(unicast_tvlv_packet->dst, dst); - - tvlv_buff = (unsigned char *)(unicast_tvlv_packet + 1); - tvlv_hdr = (struct batadv_tvlv_hdr *)tvlv_buff; - tvlv_hdr->version = version; - tvlv_hdr->type = type; - tvlv_hdr->len = htons(tvlv_value_len); - tvlv_buff += sizeof(*tvlv_hdr); - memcpy(tvlv_buff, tvlv_value, tvlv_value_len); - - if (batadv_send_skb_to_orig(skb, orig_node, NULL) != NET_XMIT_DROP) - ret = true; - -out: - if (skb && !ret) - kfree_skb(skb); - if (orig_node) - batadv_orig_node_free_ref(orig_node); -} - -/** * batadv_get_vid - extract the VLAN identifier from skb if any * @skb: the buffer containing the packet * @header_len: length of the batman header preceding the ethernet header diff --git a/main.h b/main.h index 1cd4ebdbe060..013de2f7ee11 100644 --- a/main.h +++ b/main.h @@ -287,41 +287,6 @@ static inline uint64_t batadv_sum_counter(struct batadv_priv *bat_priv, * The macro is inspired by the similar macro TCP_SKB_CB() in tcp.h. */ #define BATADV_SKB_CB(__skb) ((struct batadv_skb_cb *)&((__skb)->cb[0])) - -void batadv_tvlv_container_register(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version, - void *tvlv_value, uint16_t tvlv_value_len); -uint16_t batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, - unsigned char **packet_buff, - int *packet_buff_len, - int packet_min_len); -void batadv_tvlv_ogm_receive(struct batadv_priv *bat_priv, - struct batadv_ogm_packet *batadv_ogm_packet, - struct batadv_orig_node *orig_node); -void batadv_tvlv_container_unregister(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version); - -void batadv_tvlv_handler_register(struct batadv_priv *bat_priv, - void (*optr)(struct batadv_priv *bat_priv, - struct batadv_orig_node *orig, - uint8_t flags, - void *tvlv_value, - uint16_t tvlv_value_len), - int (*uptr)(struct batadv_priv *bat_priv, - uint8_t *src, uint8_t *dst, - void *tvlv_value, - uint16_t tvlv_value_len), - uint8_t type, uint8_t version, uint8_t flags); -void batadv_tvlv_handler_unregister(struct batadv_priv *bat_priv, - uint8_t type, uint8_t version); -int batadv_tvlv_containers_process(struct batadv_priv *bat_priv, - bool ogm_source, - struct batadv_orig_node *orig_node, - uint8_t *src, uint8_t *dst, - void *tvlv_buff, uint16_t tvlv_buff_len); -void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, uint8_t *src, - uint8_t *dst, uint8_t type, uint8_t version, - void *tvlv_value, uint16_t tvlv_value_len); unsigned short batadv_get_vid(struct sk_buff *skb, size_t header_len); bool batadv_vlan_ap_isola_get(struct batadv_priv *bat_priv, unsigned short vid);
diff --git a/multicast.c b/multicast.c index b24e4bb64fb5..abc410b7e705 100644 --- a/multicast.c +++ b/multicast.c @@ -20,6 +20,7 @@ #include "originator.h" #include "hard-interface.h" #include "translation-table.h" +#include "tvlv.h"
/** * batadv_mcast_mla_softif_get - get softif multicast listeners diff --git a/network-coding.c b/network-coding.c index 93190fc25f76..68b1f9c880cd 100644 --- a/network-coding.c +++ b/network-coding.c @@ -25,6 +25,7 @@ #include "hard-interface.h" #include "routing.h" #include "log.h" +#include "tvlv.h"
static struct lock_class_key batadv_nc_coding_hash_lock_class_key; static struct lock_class_key batadv_nc_decoding_hash_lock_class_key; diff --git a/routing.c b/routing.c index 07205591b05f..99b31499941b 100644 --- a/routing.c +++ b/routing.c @@ -28,6 +28,7 @@ #include "network-coding.h" #include "fragmentation.h" #include "log.h" +#include "tvlv.h"
#include <linux/if_vlan.h>
diff --git a/translation-table.c b/translation-table.c index be75a2ac543e..2d0bad466b68 100644 --- a/translation-table.c +++ b/translation-table.c @@ -26,6 +26,7 @@ #include "bridge_loop_avoidance.h" #include "multicast.h" #include "log.h" +#include "tvlv.h"
#include <linux/crc32c.h>
diff --git a/tvlv.c b/tvlv.c new file mode 100644 index 000000000000..43f9d9456f42 --- /dev/null +++ b/tvlv.c @@ -0,0 +1,592 @@ +/* Copyright (C) 2007-2014 B.A.T.M.A.N. contributors: + * + * Marek Lindner, Simon Wunderlich + * + * 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, see http://www.gnu.org/licenses/. + */ + +#include <linux/netdevice.h> + +#include "main.h" +#include "tvlv.h" +#include "types.h" +#include "originator.h" +#include "send.h" + +/** + * batadv_tvlv_handler_free_ref - decrement the tvlv handler refcounter and + * possibly free it + * @tvlv_handler: the tvlv handler to free + */ +static void +batadv_tvlv_handler_free_ref(struct batadv_tvlv_handler *tvlv_handler) +{ + if (atomic_dec_and_test(&tvlv_handler->refcount)) + kfree_rcu(tvlv_handler, rcu); +} + +/** + * batadv_tvlv_handler_get - retrieve tvlv handler from the tvlv handler list + * based on the provided type and version (both need to match) + * @bat_priv: the bat priv with all the soft interface information + * @type: tvlv handler type to look for + * @version: tvlv handler version to look for + * + * Returns tvlv handler if found or NULL otherwise. + */ +static struct batadv_tvlv_handler +*batadv_tvlv_handler_get(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version) +{ + struct batadv_tvlv_handler *tvlv_handler_tmp, *tvlv_handler = NULL; + + rcu_read_lock(); + hlist_for_each_entry_rcu(tvlv_handler_tmp, + &bat_priv->tvlv.handler_list, list) { + if (tvlv_handler_tmp->type != type) + continue; + + if (tvlv_handler_tmp->version != version) + continue; + + if (!atomic_inc_not_zero(&tvlv_handler_tmp->refcount)) + continue; + + tvlv_handler = tvlv_handler_tmp; + break; + } + rcu_read_unlock(); + + return tvlv_handler; +} + +/** + * batadv_tvlv_container_free_ref - decrement the tvlv container refcounter and + * possibly free it + * @tvlv: the tvlv container to free + */ +static void batadv_tvlv_container_free_ref(struct batadv_tvlv_container *tvlv) +{ + if (atomic_dec_and_test(&tvlv->refcount)) + kfree(tvlv); +} + +/** + * batadv_tvlv_container_get - retrieve tvlv container from the tvlv container + * list based on the provided type and version (both need to match) + * @bat_priv: the bat priv with all the soft interface information + * @type: tvlv container type to look for + * @version: tvlv container version to look for + * + * Has to be called with the appropriate locks being acquired + * (tvlv.container_list_lock). + * + * Returns tvlv container if found or NULL otherwise. + */ +static struct batadv_tvlv_container +*batadv_tvlv_container_get(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version) +{ + struct batadv_tvlv_container *tvlv_tmp, *tvlv = NULL; + + hlist_for_each_entry(tvlv_tmp, &bat_priv->tvlv.container_list, list) { + if (tvlv_tmp->tvlv_hdr.type != type) + continue; + + if (tvlv_tmp->tvlv_hdr.version != version) + continue; + + if (!atomic_inc_not_zero(&tvlv_tmp->refcount)) + continue; + + tvlv = tvlv_tmp; + break; + } + + return tvlv; +} + +/** + * batadv_tvlv_container_list_size - calculate the size of the tvlv container + * list entries + * @bat_priv: the bat priv with all the soft interface information + * + * Has to be called with the appropriate locks being acquired + * (tvlv.container_list_lock). + * + * Returns size of all currently registered tvlv containers in bytes. + */ +static uint16_t batadv_tvlv_container_list_size(struct batadv_priv *bat_priv) +{ + struct batadv_tvlv_container *tvlv; + uint16_t tvlv_len = 0; + + hlist_for_each_entry(tvlv, &bat_priv->tvlv.container_list, list) { + tvlv_len += sizeof(struct batadv_tvlv_hdr); + tvlv_len += ntohs(tvlv->tvlv_hdr.len); + } + + return tvlv_len; +} + +/** + * batadv_tvlv_container_remove - remove tvlv container from the tvlv container + * list + * @tvlv: the to be removed tvlv container + * + * Has to be called with the appropriate locks being acquired + * (tvlv.container_list_lock). + */ +static void batadv_tvlv_container_remove(struct batadv_tvlv_container *tvlv) +{ + if (!tvlv) + return; + + hlist_del(&tvlv->list); + + /* first call to decrement the counter, second call to free */ + batadv_tvlv_container_free_ref(tvlv); + batadv_tvlv_container_free_ref(tvlv); +} + +/** + * batadv_tvlv_container_unregister - unregister tvlv container based on the + * provided type and version (both need to match) + * @bat_priv: the bat priv with all the soft interface information + * @type: tvlv container type to unregister + * @version: tvlv container type to unregister + */ +void batadv_tvlv_container_unregister(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version) +{ + struct batadv_tvlv_container *tvlv; + + spin_lock_bh(&bat_priv->tvlv.container_list_lock); + tvlv = batadv_tvlv_container_get(bat_priv, type, version); + batadv_tvlv_container_remove(tvlv); + spin_unlock_bh(&bat_priv->tvlv.container_list_lock); +} + +/** + * batadv_tvlv_container_register - register tvlv type, version and content + * to be propagated with each (primary interface) OGM + * @bat_priv: the bat priv with all the soft interface information + * @type: tvlv container type + * @version: tvlv container version + * @tvlv_value: tvlv container content + * @tvlv_value_len: tvlv container content length + * + * If a container of the same type and version was already registered the new + * content is going to replace the old one. + */ +void batadv_tvlv_container_register(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version, + void *tvlv_value, uint16_t tvlv_value_len) +{ + struct batadv_tvlv_container *tvlv_old, *tvlv_new; + + if (!tvlv_value) + tvlv_value_len = 0; + + tvlv_new = kzalloc(sizeof(*tvlv_new) + tvlv_value_len, GFP_ATOMIC); + if (!tvlv_new) + return; + + tvlv_new->tvlv_hdr.version = version; + tvlv_new->tvlv_hdr.type = type; + tvlv_new->tvlv_hdr.len = htons(tvlv_value_len); + + memcpy(tvlv_new + 1, tvlv_value, ntohs(tvlv_new->tvlv_hdr.len)); + INIT_HLIST_NODE(&tvlv_new->list); + atomic_set(&tvlv_new->refcount, 1); + + spin_lock_bh(&bat_priv->tvlv.container_list_lock); + tvlv_old = batadv_tvlv_container_get(bat_priv, type, version); + batadv_tvlv_container_remove(tvlv_old); + hlist_add_head(&tvlv_new->list, &bat_priv->tvlv.container_list); + spin_unlock_bh(&bat_priv->tvlv.container_list_lock); +} + +/** + * batadv_tvlv_realloc_packet_buff - reallocate packet buffer to accomodate + * requested packet size + * @packet_buff: packet buffer + * @packet_buff_len: packet buffer size + * @min_packet_len: requested packet minimum size + * @additional_packet_len: requested additional packet size on top of minimum + * size + * + * Returns true of the packet buffer could be changed to the requested size, + * false otherwise. + */ +static bool batadv_tvlv_realloc_packet_buff(unsigned char **packet_buff, + int *packet_buff_len, + int min_packet_len, + int additional_packet_len) +{ + unsigned char *new_buff; + + new_buff = kmalloc(min_packet_len + additional_packet_len, GFP_ATOMIC); + + /* keep old buffer if kmalloc should fail */ + if (!new_buff) + return false; + + memcpy(new_buff, *packet_buff, min_packet_len); + kfree(*packet_buff); + *packet_buff = new_buff; + *packet_buff_len = min_packet_len + additional_packet_len; + + return true; +} + +/** + * batadv_tvlv_container_ogm_append - append tvlv container content to given + * OGM packet buffer + * @bat_priv: the bat priv with all the soft interface information + * @packet_buff: ogm packet buffer + * @packet_buff_len: ogm packet buffer size including ogm header and tvlv + * content + * @packet_min_len: ogm header size to be preserved for the OGM itself + * + * The ogm packet might be enlarged or shrunk depending on the current size + * and the size of the to-be-appended tvlv containers. + * + * Returns size of all appended tvlv containers in bytes. + */ +uint16_t batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, + unsigned char **packet_buff, + int *packet_buff_len, + int packet_min_len) +{ + struct batadv_tvlv_container *tvlv; + struct batadv_tvlv_hdr *tvlv_hdr; + uint16_t tvlv_value_len; + void *tvlv_value; + bool ret; + + spin_lock_bh(&bat_priv->tvlv.container_list_lock); + tvlv_value_len = batadv_tvlv_container_list_size(bat_priv); + + ret = batadv_tvlv_realloc_packet_buff(packet_buff, packet_buff_len, + packet_min_len, tvlv_value_len); + + if (!ret) + goto end; + + if (!tvlv_value_len) + goto end; + + tvlv_value = (*packet_buff) + packet_min_len; + + hlist_for_each_entry(tvlv, &bat_priv->tvlv.container_list, list) { + tvlv_hdr = tvlv_value; + tvlv_hdr->type = tvlv->tvlv_hdr.type; + tvlv_hdr->version = tvlv->tvlv_hdr.version; + tvlv_hdr->len = tvlv->tvlv_hdr.len; + tvlv_value = tvlv_hdr + 1; + memcpy(tvlv_value, tvlv + 1, ntohs(tvlv->tvlv_hdr.len)); + tvlv_value = (uint8_t *)tvlv_value + ntohs(tvlv->tvlv_hdr.len); + } + +end: + spin_unlock_bh(&bat_priv->tvlv.container_list_lock); + return tvlv_value_len; +} + +/** + * batadv_tvlv_call_handler - parse the given tvlv buffer to call the + * appropriate handlers + * @bat_priv: the bat priv with all the soft interface information + * @tvlv_handler: tvlv callback function handling the tvlv content + * @ogm_source: flag indicating wether the tvlv is an ogm or a unicast packet + * @orig_node: orig node emitting the ogm packet + * @src: source mac address of the unicast packet + * @dst: destination mac address of the unicast packet + * @tvlv_value: tvlv content + * @tvlv_value_len: tvlv content length + * + * Returns success if handler was not found or the return value of the handler + * callback. + */ +static int batadv_tvlv_call_handler(struct batadv_priv *bat_priv, + struct batadv_tvlv_handler *tvlv_handler, + bool ogm_source, + struct batadv_orig_node *orig_node, + uint8_t *src, uint8_t *dst, + void *tvlv_value, uint16_t tvlv_value_len) +{ + if (!tvlv_handler) + return NET_RX_SUCCESS; + + if (ogm_source) { + if (!tvlv_handler->ogm_handler) + return NET_RX_SUCCESS; + + if (!orig_node) + return NET_RX_SUCCESS; + + tvlv_handler->ogm_handler(bat_priv, orig_node, + BATADV_NO_FLAGS, + tvlv_value, tvlv_value_len); + tvlv_handler->flags |= BATADV_TVLV_HANDLER_OGM_CALLED; + } else { + if (!src) + return NET_RX_SUCCESS; + + if (!dst) + return NET_RX_SUCCESS; + + if (!tvlv_handler->unicast_handler) + return NET_RX_SUCCESS; + + return tvlv_handler->unicast_handler(bat_priv, src, + dst, tvlv_value, + tvlv_value_len); + } + + return NET_RX_SUCCESS; +} + +/** + * batadv_tvlv_containers_process - parse the given tvlv buffer to call the + * appropriate handlers + * @bat_priv: the bat priv with all the soft interface information + * @ogm_source: flag indicating wether the tvlv is an ogm or a unicast packet + * @orig_node: orig node emitting the ogm packet + * @src: source mac address of the unicast packet + * @dst: destination mac address of the unicast packet + * @tvlv_value: tvlv content + * @tvlv_value_len: tvlv content length + * + * Returns success when processing an OGM or the return value of all called + * handler callbacks. + */ +int batadv_tvlv_containers_process(struct batadv_priv *bat_priv, + bool ogm_source, + struct batadv_orig_node *orig_node, + uint8_t *src, uint8_t *dst, + void *tvlv_value, uint16_t tvlv_value_len) +{ + struct batadv_tvlv_handler *tvlv_handler; + struct batadv_tvlv_hdr *tvlv_hdr; + uint16_t tvlv_value_cont_len; + uint8_t cifnotfound = BATADV_TVLV_HANDLER_OGM_CIFNOTFND; + int ret = NET_RX_SUCCESS; + + while (tvlv_value_len >= sizeof(*tvlv_hdr)) { + tvlv_hdr = tvlv_value; + tvlv_value_cont_len = ntohs(tvlv_hdr->len); + tvlv_value = tvlv_hdr + 1; + tvlv_value_len -= sizeof(*tvlv_hdr); + + if (tvlv_value_cont_len > tvlv_value_len) + break; + + tvlv_handler = batadv_tvlv_handler_get(bat_priv, + tvlv_hdr->type, + tvlv_hdr->version); + + ret |= batadv_tvlv_call_handler(bat_priv, tvlv_handler, + ogm_source, orig_node, + src, dst, tvlv_value, + tvlv_value_cont_len); + if (tvlv_handler) + batadv_tvlv_handler_free_ref(tvlv_handler); + tvlv_value = (uint8_t *)tvlv_value + tvlv_value_cont_len; + tvlv_value_len -= tvlv_value_cont_len; + } + + if (!ogm_source) + return ret; + + rcu_read_lock(); + hlist_for_each_entry_rcu(tvlv_handler, + &bat_priv->tvlv.handler_list, list) { + if ((tvlv_handler->flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND) && + !(tvlv_handler->flags & BATADV_TVLV_HANDLER_OGM_CALLED)) + tvlv_handler->ogm_handler(bat_priv, orig_node, + cifnotfound, NULL, 0); + + tvlv_handler->flags &= ~BATADV_TVLV_HANDLER_OGM_CALLED; + } + rcu_read_unlock(); + + return NET_RX_SUCCESS; +} + +/** + * batadv_tvlv_ogm_receive - process an incoming ogm and call the appropriate + * handlers + * @bat_priv: the bat priv with all the soft interface information + * @batadv_ogm_packet: ogm packet containing the tvlv containers + * @orig_node: orig node emitting the ogm packet + */ +void batadv_tvlv_ogm_receive(struct batadv_priv *bat_priv, + struct batadv_ogm_packet *batadv_ogm_packet, + struct batadv_orig_node *orig_node) +{ + void *tvlv_value; + uint16_t tvlv_value_len; + + if (!batadv_ogm_packet) + return; + + tvlv_value_len = ntohs(batadv_ogm_packet->tvlv_len); + if (!tvlv_value_len) + return; + + tvlv_value = batadv_ogm_packet + 1; + + batadv_tvlv_containers_process(bat_priv, true, orig_node, NULL, NULL, + tvlv_value, tvlv_value_len); +} + +/** + * batadv_tvlv_handler_register - register tvlv handler based on the provided + * type and version (both need to match) for ogm tvlv payload and/or unicast + * payload + * @bat_priv: the bat priv with all the soft interface information + * @optr: ogm tvlv handler callback function. This function receives the orig + * node, flags and the tvlv content as argument to process. + * @uptr: unicast tvlv handler callback function. This function receives the + * source & destination of the unicast packet as well as the tvlv content + * to process. + * @type: tvlv handler type to be registered + * @version: tvlv handler version to be registered + * @flags: flags to enable or disable TVLV API behavior + */ +void batadv_tvlv_handler_register(struct batadv_priv *bat_priv, + void (*optr)(struct batadv_priv *bat_priv, + struct batadv_orig_node *orig, + uint8_t flags, + void *tvlv_value, + uint16_t tvlv_value_len), + int (*uptr)(struct batadv_priv *bat_priv, + uint8_t *src, uint8_t *dst, + void *tvlv_value, + uint16_t tvlv_value_len), + uint8_t type, uint8_t version, uint8_t flags) +{ + struct batadv_tvlv_handler *tvlv_handler; + + tvlv_handler = batadv_tvlv_handler_get(bat_priv, type, version); + if (tvlv_handler) { + batadv_tvlv_handler_free_ref(tvlv_handler); + return; + } + + tvlv_handler = kzalloc(sizeof(*tvlv_handler), GFP_ATOMIC); + if (!tvlv_handler) + return; + + tvlv_handler->ogm_handler = optr; + tvlv_handler->unicast_handler = uptr; + tvlv_handler->type = type; + tvlv_handler->version = version; + tvlv_handler->flags = flags; + atomic_set(&tvlv_handler->refcount, 1); + INIT_HLIST_NODE(&tvlv_handler->list); + + spin_lock_bh(&bat_priv->tvlv.handler_list_lock); + hlist_add_head_rcu(&tvlv_handler->list, &bat_priv->tvlv.handler_list); + spin_unlock_bh(&bat_priv->tvlv.handler_list_lock); +} + +/** + * batadv_tvlv_handler_unregister - unregister tvlv handler based on the + * provided type and version (both need to match) + * @bat_priv: the bat priv with all the soft interface information + * @type: tvlv handler type to be unregistered + * @version: tvlv handler version to be unregistered + */ +void batadv_tvlv_handler_unregister(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version) +{ + struct batadv_tvlv_handler *tvlv_handler; + + tvlv_handler = batadv_tvlv_handler_get(bat_priv, type, version); + if (!tvlv_handler) + return; + + batadv_tvlv_handler_free_ref(tvlv_handler); + spin_lock_bh(&bat_priv->tvlv.handler_list_lock); + hlist_del_rcu(&tvlv_handler->list); + spin_unlock_bh(&bat_priv->tvlv.handler_list_lock); + batadv_tvlv_handler_free_ref(tvlv_handler); +} + +/** + * batadv_tvlv_unicast_send - send a unicast packet with tvlv payload to the + * specified host + * @bat_priv: the bat priv with all the soft interface information + * @src: source mac address of the unicast packet + * @dst: destination mac address of the unicast packet + * @type: tvlv type + * @version: tvlv version + * @tvlv_value: tvlv content + * @tvlv_value_len: tvlv content length + */ +void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, uint8_t *src, + uint8_t *dst, uint8_t type, uint8_t version, + void *tvlv_value, uint16_t tvlv_value_len) +{ + struct batadv_unicast_tvlv_packet *unicast_tvlv_packet; + struct batadv_tvlv_hdr *tvlv_hdr; + struct batadv_orig_node *orig_node; + struct sk_buff *skb = NULL; + unsigned char *tvlv_buff; + unsigned int tvlv_len; + ssize_t hdr_len = sizeof(*unicast_tvlv_packet); + bool ret = false; + + orig_node = batadv_orig_hash_find(bat_priv, dst); + if (!orig_node) + goto out; + + tvlv_len = sizeof(*tvlv_hdr) + tvlv_value_len; + + skb = netdev_alloc_skb_ip_align(NULL, ETH_HLEN + hdr_len + tvlv_len); + if (!skb) + goto out; + + skb->priority = TC_PRIO_CONTROL; + skb_reserve(skb, ETH_HLEN); + tvlv_buff = skb_put(skb, sizeof(*unicast_tvlv_packet) + tvlv_len); + unicast_tvlv_packet = (struct batadv_unicast_tvlv_packet *)tvlv_buff; + unicast_tvlv_packet->packet_type = BATADV_UNICAST_TVLV; + unicast_tvlv_packet->version = BATADV_COMPAT_VERSION; + unicast_tvlv_packet->ttl = BATADV_TTL; + unicast_tvlv_packet->reserved = 0; + unicast_tvlv_packet->tvlv_len = htons(tvlv_len); + unicast_tvlv_packet->align = 0; + ether_addr_copy(unicast_tvlv_packet->src, src); + ether_addr_copy(unicast_tvlv_packet->dst, dst); + + tvlv_buff = (unsigned char *)(unicast_tvlv_packet + 1); + tvlv_hdr = (struct batadv_tvlv_hdr *)tvlv_buff; + tvlv_hdr->version = version; + tvlv_hdr->type = type; + tvlv_hdr->len = htons(tvlv_value_len); + tvlv_buff += sizeof(*tvlv_hdr); + memcpy(tvlv_buff, tvlv_value, tvlv_value_len); + + if (batadv_send_skb_to_orig(skb, orig_node, NULL) != NET_XMIT_DROP) + ret = true; + +out: + if (skb && !ret) + kfree_skb(skb); + if (orig_node) + batadv_orig_node_free_ref(orig_node); +} diff --git a/tvlv.h b/tvlv.h new file mode 100644 index 000000000000..26436c01bc78 --- /dev/null +++ b/tvlv.h @@ -0,0 +1,62 @@ +/* Copyright (C) 2007-2014 B.A.T.M.A.N. contributors: + * + * Marek Lindner, Simon Wunderlich + * + * 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, see http://www.gnu.org/licenses/. + */ + +#ifndef _NET_BATMAN_ADV_TVLV_H_ +#define _NET_BATMAN_ADV_TVLV_H_ + +#include <linux/types.h> + +struct batadv_priv; +struct batadv_ogm_packet; +struct batadv_orig_node; + +void batadv_tvlv_container_register(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version, + void *tvlv_value, uint16_t tvlv_value_len); +uint16_t batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, + unsigned char **packet_buff, + int *packet_buff_len, + int packet_min_len); +void batadv_tvlv_ogm_receive(struct batadv_priv *bat_priv, + struct batadv_ogm_packet *batadv_ogm_packet, + struct batadv_orig_node *orig_node); +void batadv_tvlv_container_unregister(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version); + +void batadv_tvlv_handler_register(struct batadv_priv *bat_priv, + void (*optr)(struct batadv_priv *bat_priv, + struct batadv_orig_node *orig, + uint8_t flags, + void *tvlv_value, + uint16_t tvlv_value_len), + int (*uptr)(struct batadv_priv *bat_priv, + uint8_t *src, uint8_t *dst, + void *tvlv_value, + uint16_t tvlv_value_len), + uint8_t type, uint8_t version, uint8_t flags); +void batadv_tvlv_handler_unregister(struct batadv_priv *bat_priv, + uint8_t type, uint8_t version); +int batadv_tvlv_containers_process(struct batadv_priv *bat_priv, + bool ogm_source, + struct batadv_orig_node *orig_node, + uint8_t *src, uint8_t *dst, + void *tvlv_buff, uint16_t tvlv_buff_len); +void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, uint8_t *src, + uint8_t *dst, uint8_t type, uint8_t version, + void *tvlv_value, uint16_t tvlv_value_len); + +#endif /* _NET_BATMAN_ADV_TVLV_H_ */
The whole Makefile is sorted, just the multicast rule is not at the right position.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- Makefile.kbuild | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile.kbuild b/Makefile.kbuild index a8c34baddd63..6fb243c5b13c 100644 --- a/Makefile.kbuild +++ b/Makefile.kbuild @@ -29,6 +29,7 @@ batman-adv-y += hard-interface.o batman-adv-y += hash.o batman-adv-y += icmp_socket.o batman-adv-y += main.o +batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += multicast.o batman-adv-$(CONFIG_BATMAN_ADV_NC) += network-coding.o batman-adv-y += originator.o batman-adv-y += routing.o @@ -37,4 +38,3 @@ batman-adv-y += soft-interface.o batman-adv-y += sysfs.o batman-adv-y += translation-table.o batman-adv-y += tvlv.o -batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += multicast.o
On Friday 26 December 2014 12:41:25 Markus Pargmann wrote:
The whole Makefile is sorted, just the multicast rule is not at the right position.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
Makefile.kbuild | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied in revision d857186.
Thanks, Marek
Directly return error values. No need to use a return variable.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 20295c5e5121..92eeed406663 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -310,7 +310,6 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface) struct batadv_ogm_packet *batadv_ogm_packet; unsigned char *ogm_buff; uint32_t random_seqno; - int res = -ENOMEM;
/* randomize initial seqno to avoid collision */ get_random_bytes(&random_seqno, sizeof(random_seqno)); @@ -319,7 +318,7 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface) hard_iface->bat_iv.ogm_buff_len = BATADV_OGM_HLEN; ogm_buff = kmalloc(hard_iface->bat_iv.ogm_buff_len, GFP_ATOMIC); if (!ogm_buff) - goto out; + return -ENOMEM;
hard_iface->bat_iv.ogm_buff = ogm_buff;
@@ -331,10 +330,7 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface) batadv_ogm_packet->reserved = 0; batadv_ogm_packet->tq = BATADV_TQ_MAX_VALUE;
- res = 0; - -out: - return res; + return 0; }
static void batadv_iv_ogm_iface_disable(struct batadv_hard_iface *hard_iface)
On Friday 26 December 2014 12:41:26 Markus Pargmann wrote:
Directly return error values. No need to use a return variable.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
Applied in revision 0cc4569.
Thanks, Marek
This function returns bool values, so it should be defined to return them instead of the whole int range.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 92eeed406663..b267afab65bb 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -394,8 +394,8 @@ static uint8_t batadv_hop_penalty(uint8_t tq, }
/* is there another aggregated packet here? */ -static int batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len, - __be16 tvlv_len) +static bool batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len, + __be16 tvlv_len) { int next_buff_pos = 0;
On Friday 26 December 2014 12:41:27 Markus Pargmann wrote:
This function returns bool values, so it should be defined to return them instead of the whole int range.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Applied in revision ec3b2d9.
Thanks, Marek
This string pointer is later assigned to a constant string, so it should be defined constant at the beginning.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index b267afab65bb..4801619c4da5 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -411,7 +411,7 @@ static void batadv_iv_ogm_send_to_if(struct batadv_forw_packet *forw_packet, struct batadv_hard_iface *hard_iface) { struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface); - char *fwd_str; + const char *fwd_str; uint8_t packet_num; int16_t buff_pos; struct batadv_ogm_packet *batadv_ogm_packet;
On Friday, December 26, 2014 12:41:28 Markus Pargmann wrote:
This string pointer is later assigned to a constant string, so it should be defined constant at the beginning.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied in revision 3de1d1b.
Thanks, Marek
This patch tries to increase code readability by negating the first if block and rearranging some of the other conditional blocks. This way we save an indentation level, we also save some allocation that is not necessary for one of the conditions.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 98 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 50 insertions(+), 48 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 4801619c4da5..1f3880ac8376 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -546,58 +546,60 @@ batadv_iv_ogm_can_aggregate(const struct batadv_ogm_packet *new_bat_ogm_packet, * - the send time is within our MAX_AGGREGATION_MS time * - the resulting packet wont be bigger than * MAX_AGGREGATION_BYTES + * otherwise aggregation is not possible */ - if (time_before(send_time, forw_packet->send_time) && - time_after_eq(aggregation_end_time, forw_packet->send_time) && - (aggregated_bytes <= BATADV_MAX_AGGREGATION_BYTES)) { - /* check aggregation compatibility - * -> direct link packets are broadcasted on - * their interface only - * -> aggregate packet if the current packet is - * a "global" packet as well as the base - * packet - */ - primary_if = batadv_primary_if_get_selected(bat_priv); - if (!primary_if) - goto out; + if (!time_before(send_time, forw_packet->send_time) || + !time_after_eq(aggregation_end_time, forw_packet->send_time) || + aggregated_bytes > BATADV_MAX_AGGREGATION_BYTES) + return false;
- /* packet is not leaving on the same interface. */ - if (forw_packet->if_outgoing != if_outgoing) - goto out; + /* packet is not leaving on the same interface. */ + if (forw_packet->if_outgoing != if_outgoing) + return false;
- /* packets without direct link flag and high TTL - * are flooded through the net - */ - if ((!directlink) && - (!(batadv_ogm_packet->flags & BATADV_DIRECTLINK)) && - (batadv_ogm_packet->ttl != 1) && - - /* own packets originating non-primary - * interfaces leave only that interface - */ - ((!forw_packet->own) || - (forw_packet->if_incoming == primary_if))) { - res = true; - goto out; - } + /* check aggregation compatibility + * -> direct link packets are broadcasted on + * their interface only + * -> aggregate packet if the current packet is + * a "global" packet as well as the base + * packet + */ + primary_if = batadv_primary_if_get_selected(bat_priv); + if (!primary_if) + return false;
- /* if the incoming packet is sent via this one - * interface only - we still can aggregate - */ - if ((directlink) && - (new_bat_ogm_packet->ttl == 1) && - (forw_packet->if_incoming == if_incoming) && - - /* packets from direct neighbors or - * own secondary interface packets - * (= secondary interface packets in general) - */ - (batadv_ogm_packet->flags & BATADV_DIRECTLINK || - (forw_packet->own && - forw_packet->if_incoming != primary_if))) { - res = true; - goto out; - } + /* packets without direct link flag and high TTL + * are flooded through the net + */ + if (!directlink && + !(batadv_ogm_packet->flags & BATADV_DIRECTLINK) && + batadv_ogm_packet->ttl != 1 && + + /* own packets originating non-primary + * interfaces leave only that interface + */ + (!forw_packet->own || + forw_packet->if_incoming == primary_if)) { + res = true; + goto out; + } + + /* if the incoming packet is sent via this one + * interface only - we still can aggregate + */ + if (directlink && + new_bat_ogm_packet->ttl == 1 && + forw_packet->if_incoming == if_incoming && + + /* packets from direct neighbors or + * own secondary interface packets + * (= secondary interface packets in general) + */ + (batadv_ogm_packet->flags & BATADV_DIRECTLINK || + (forw_packet->own && + forw_packet->if_incoming != primary_if))) { + res = true; + goto out; }
out:
On Friday, December 26, 2014 12:41:29 Markus Pargmann wrote:
This patch tries to increase code readability by negating the first if block and rearranging some of the other conditional blocks. This way we save an indentation level, we also save some allocation that is not necessary for one of the conditions.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 98 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 50 insertions(+), 48 deletions(-)
Applied in revision f7ac9a4.
Thanks, Marek
Remove these unnecessary brackets inside a condition.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 1f3880ac8376..3f76f39b2a13 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1081,7 +1081,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, * won't consider it either */ if (router_ifinfo && - (neigh_ifinfo->bat_iv.tq_avg == router_ifinfo->bat_iv.tq_avg)) { + neigh_ifinfo->bat_iv.tq_avg == router_ifinfo->bat_iv.tq_avg) { orig_node_tmp = router->orig_node; spin_lock_bh(&orig_node_tmp->bat_iv.ogm_cnt_lock); if_num = router->if_incoming->if_num;
On Friday, December 26, 2014 12:41:30 Markus Pargmann wrote:
Remove these unnecessary brackets inside a condition.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied in revision 442b546.
Thanks, Marek
It is just a bit easier to put the error handling at one place and let multiple error paths use the same calls.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 3f76f39b2a13..12185f985c79 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -647,14 +647,11 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, }
forw_packet_aggr = kmalloc(sizeof(*forw_packet_aggr), GFP_ATOMIC); - if (!forw_packet_aggr) { - if (!own_packet) - atomic_inc(&bat_priv->batman_queue_left); - goto out; - } + if (!forw_packet_aggr) + goto out_nomem;
- if ((atomic_read(&bat_priv->aggregated_ogms)) && - (packet_len < BATADV_MAX_AGGREGATION_BYTES)) + if (atomic_read(&bat_priv->aggregated_ogms) && + packet_len < BATADV_MAX_AGGREGATION_BYTES) skb_size = BATADV_MAX_AGGREGATION_BYTES; else skb_size = packet_len; @@ -662,12 +659,8 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, skb_size += ETH_HLEN;
forw_packet_aggr->skb = netdev_alloc_skb_ip_align(NULL, skb_size); - if (!forw_packet_aggr->skb) { - if (!own_packet) - atomic_inc(&bat_priv->batman_queue_left); - kfree(forw_packet_aggr); - goto out; - } + if (!forw_packet_aggr->skb) + goto out_free_forw_packet; forw_packet_aggr->skb->priority = TC_PRIO_CONTROL; skb_reserve(forw_packet_aggr->skb, ETH_HLEN);
@@ -699,6 +692,11 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, send_time - jiffies);
return; +out_free_forw_packet: + kfree(forw_packet_aggr); +out_nomem: + if (!own_packet) + atomic_inc(&bat_priv->batman_queue_left); out: batadv_hardif_free_ref(if_outgoing); out_free_incoming:
On Friday, December 26, 2014 12:41:31 Markus Pargmann wrote:
It is just a bit easier to put the error handling at one place and let multiple error paths use the same calls.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)
Applied in revision a467a76.
Thanks, Marek
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 12185f985c79..8e29772ad8ae 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -750,13 +750,13 @@ static void batadv_iv_ogm_queue_add(struct batadv_priv *bat_priv, unsigned long max_aggregation_jiffies;
batadv_ogm_packet = (struct batadv_ogm_packet *)packet_buff; - direct_link = batadv_ogm_packet->flags & BATADV_DIRECTLINK ? 1 : 0; + direct_link = !!(batadv_ogm_packet->flags & BATADV_DIRECTLINK); max_aggregation_jiffies = msecs_to_jiffies(BATADV_MAX_AGGREGATION_MS);
/* find position for the packet in the forward queue */ spin_lock_bh(&bat_priv->forw_bat_list_lock); /* own packets are not to be aggregated */ - if ((atomic_read(&bat_priv->aggregated_ogms)) && (!own_packet)) { + if (atomic_read(&bat_priv->aggregated_ogms) && !own_packet) { hlist_for_each_entry(forw_packet_pos, &bat_priv->forw_bat_list, list) { if (batadv_iv_ogm_can_aggregate(batadv_ogm_packet,
On Friday, December 26, 2014 12:41:32 Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Applied in revision 253a6dd.
Thanks, Marek
CodingStyle describes that either none or both branches of a conditional have to have brackets.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 8e29772ad8ae..32640e0997ed 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1032,9 +1032,10 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, batadv_orig_node_free_ref(orig_tmp); if (!neigh_node) goto unlock; - } else + } else { batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Updating existing last-hop neighbor of originator\n"); + }
rcu_read_unlock(); neigh_ifinfo = batadv_neigh_ifinfo_new(neigh_node, if_outgoing);
On Friday, December 26, 2014 12:41:33 Markus Pargmann wrote:
CodingStyle describes that either none or both branches of a conditional have to have brackets.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Applied in revision 3040f9c.
Thanks, Marek
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 32640e0997ed..ab48865130b0 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -30,7 +30,7 @@
/** * enum batadv_dup_status - duplicate status - * @BATADV_NO_DUP: the packet is a duplicate + * @BATADV_NO_DUP: the packet is no duplicate * @BATADV_ORIG_DUP: OGM is a duplicate in the originator (but not for the * neighbor) * @BATADV_NEIGH_DUP: OGM is a duplicate for the neighbor
On Friday, December 26, 2014 12:41:34 Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied in revision d00159a.
Thanks, Marek
The kernel coding style says, that there should not be multiple assignments in one row.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index ab48865130b0..39ec3eeec41f 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -66,7 +66,9 @@ static void batadv_ring_buffer_set(uint8_t lq_recv[], uint8_t *lq_index, static uint8_t batadv_ring_buffer_avg(const uint8_t lq_recv[]) { const uint8_t *ptr; - uint16_t count = 0, i = 0, sum = 0; + uint16_t count = 0; + uint16_t i = 0; + uint16_t sum = 0;
ptr = lq_recv;
On Friday, December 26, 2014 12:41:35 Markus Pargmann wrote:
The kernel coding style says, that there should not be multiple assignments in one row.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Applied in revision 63c15a5.
Thanks, Marek
This is a small copy paste fix for batadv_ing_buffer_avg.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 39ec3eeec41f..b613ef74b9e0 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -57,7 +57,7 @@ static void batadv_ring_buffer_set(uint8_t lq_recv[], uint8_t *lq_index, }
/** - * batadv_ring_buffer_set - compute the average of all non-zero values stored + * batadv_ring_buffer_avg - compute the average of all non-zero values stored * in the given ring buffer * @lq_recv: pointer to the ring buffer *
On Friday, December 26, 2014 12:41:36 Markus Pargmann wrote:
This is a small copy paste fix for batadv_ing_buffer_avg.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied in revision 11c63fa.
Thanks, Marek
batadv_orig_bat_iv->bcast_own is actually not a bitfield, it is an array. Adjust the comment accordingly.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- types.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/types.h b/types.h index 462a70c0db8f..cd331974321b 100644 --- a/types.h +++ b/types.h @@ -181,9 +181,10 @@ struct batadv_orig_node_vlan {
/** * struct batadv_orig_bat_iv - B.A.T.M.A.N. IV private orig_node members - * @bcast_own: bitfield containing the number of our OGMs this orig_node - * rebroadcasted "back" to us (relative to last_real_seqno) - * @bcast_own_sum: counted result of bcast_own + * @bcast_own: array containing the number of our OGMs this orig_node + * rebroadcasted "back" to us (relative to last_real_seqno) on each hard + * interface + * @bcast_own_sum: sum of bcast_own * @ogm_cnt_lock: lock protecting bcast_own, bcast_own_sum, * neigh_node->bat_iv.real_bits & neigh_node->bat_iv.real_packet_count */
On Friday, December 26, 2014 12:41:37 Markus Pargmann wrote:
batadv_orig_bat_iv->bcast_own is actually not a bitfield, it is an array. Adjust the comment accordingly.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
types.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
Applied in revision eb923e9 (with the modifications discussed on IRC).
Thanks, Marek
It is much clearer to see a bool type as return value than 'int' for functions that are supposed to return true or false.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- main.c | 11 +++++++---- main.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/main.c b/main.c index cff31bb9bb14..d5c79ae1db6f 100644 --- a/main.c +++ b/main.c @@ -228,10 +228,13 @@ void batadv_mesh_free(struct net_device *soft_iface) * interfaces in the current mesh * @bat_priv: the bat priv with all the soft interface information * @addr: the address to check + * + * Returns 'true' if the mac address was found, false otherwise. */ -int batadv_is_my_mac(struct batadv_priv *bat_priv, const uint8_t *addr) +bool batadv_is_my_mac(struct batadv_priv *bat_priv, const uint8_t *addr) { const struct batadv_hard_iface *hard_iface; + bool is_my_mac = false;
rcu_read_lock(); list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) { @@ -242,12 +245,12 @@ int batadv_is_my_mac(struct batadv_priv *bat_priv, const uint8_t *addr) continue;
if (batadv_compare_eth(hard_iface->net_dev->dev_addr, addr)) { - rcu_read_unlock(); - return 1; + is_my_mac = true; + break; } } rcu_read_unlock(); - return 0; + return is_my_mac; }
/** diff --git a/main.h b/main.h index 013de2f7ee11..6cd339090658 100644 --- a/main.h +++ b/main.h @@ -196,7 +196,7 @@ extern struct workqueue_struct *batadv_event_workqueue;
int batadv_mesh_init(struct net_device *soft_iface); void batadv_mesh_free(struct net_device *soft_iface); -int batadv_is_my_mac(struct batadv_priv *bat_priv, const uint8_t *addr); +bool batadv_is_my_mac(struct batadv_priv *bat_priv, const uint8_t *addr); struct batadv_hard_iface * batadv_seq_print_text_primary_if_get(struct seq_file *seq); int batadv_max_header_len(void);
On Friday, December 26, 2014 12:41:38 Markus Pargmann wrote:
It is much clearer to see a bool type as return value than 'int' for functions that are supposed to return true or false.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
main.c | 11 +++++++---- main.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-)
Applied in revision a5b709e.
Thanks, Marek
Declare the returntype of batadv_compare_eth as bool. The function called inside this helper function (ether_addr_equal_unaligned) also uses bool as return value, so there is no need to return int.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- main.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/main.h b/main.h index 6cd339090658..2e70d4dc748d 100644 --- a/main.h +++ b/main.h @@ -218,7 +218,7 @@ __be32 batadv_skb_crc32(struct sk_buff *skb, u8 *payload_ptr); * * note: can't use ether_addr_equal() as it requires aligned memory */ -static inline int batadv_compare_eth(const void *data1, const void *data2) +static inline bool batadv_compare_eth(const void *data1, const void *data2) { return ether_addr_equal_unaligned(data1, data2); }
On Friday, December 26, 2014 12:41:39 Markus Pargmann wrote:
Declare the returntype of batadv_compare_eth as bool. The function called inside this helper function (ether_addr_equal_unaligned) also uses bool as return value, so there is no need to return int.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
main.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied in revision 16af734.
Thanks, Marek
We can avoid this indirect return variable by directly returning the error values.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- main.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/main.c b/main.c index d5c79ae1db6f..2e2ad071a085 100644 --- a/main.c +++ b/main.c @@ -532,14 +532,12 @@ static struct batadv_algo_ops *batadv_algo_get(char *name) int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops) { struct batadv_algo_ops *bat_algo_ops_tmp; - int ret;
bat_algo_ops_tmp = batadv_algo_get(bat_algo_ops->name); if (bat_algo_ops_tmp) { pr_info("Trying to register already registered routing algorithm: %s\n", bat_algo_ops->name); - ret = -EEXIST; - goto out; + return -EEXIST; }
/* all algorithms must implement all ops (for now) */ @@ -553,16 +551,13 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops) !bat_algo_ops->bat_neigh_is_equiv_or_better) { pr_info("Routing algo '%s' does not implement required ops\n", bat_algo_ops->name); - ret = -EINVAL; - goto out; + return -EINVAL; }
INIT_HLIST_NODE(&bat_algo_ops->list); hlist_add_head(&bat_algo_ops->list, &batadv_algo_list); - ret = 0;
-out: - return ret; + return 0; }
int batadv_algo_select(struct batadv_priv *bat_priv, char *name)
On Friday, December 26, 2014 12:41:40 Markus Pargmann wrote:
We can avoid this indirect return variable by directly returning the error values.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
main.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
Applied in revision 32a451a.
Thanks, Marek
Remove ret variable and all jumps.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- main.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/main.c b/main.c index 2e2ad071a085..d67be132e5df 100644 --- a/main.c +++ b/main.c @@ -563,17 +563,14 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops) int batadv_algo_select(struct batadv_priv *bat_priv, char *name) { struct batadv_algo_ops *bat_algo_ops; - int ret = -EINVAL;
bat_algo_ops = batadv_algo_get(name); if (!bat_algo_ops) - goto out; + return -EINVAL;
bat_priv->bat_algo_ops = bat_algo_ops; - ret = 0;
-out: - return ret; + return 0; }
int batadv_algo_seq_print_text(struct seq_file *seq, void *offset)
On Friday, December 26, 2014 12:41:41 Markus Pargmann wrote:
Remove ret variable and all jumps.
Signed-off-by: Markus Pargmann mpa@pengutronix.de
main.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
Applied in revision 16b4833.
Thanks, Marek
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- packet.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/packet.h b/packet.h index facd1feddd4e..5fd0d814b6de 100644 --- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#ifdef __KERNEL__ +#include <linux/bitops.h> +#include <uapi/linux/if_ether.h> +#endif /* __KERNEL__ */ + /** * enum batadv_packettype - types for batman-adv encapsulated packets * @BATADV_IV_OGM: originator messages for B.A.T.M.A.N. IV
Hi Markus,
first of all you should know that I really appreciate your hard work. Thank you very much for this!
On 26/12/14 12:41, Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
packet.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/packet.h b/packet.h index facd1feddd4e..5fd0d814b6de 100644 --- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#ifdef __KERNEL__ +#include <linux/bitops.h> +#include <uapi/linux/if_ether.h> +#endif /* __KERNEL__ */
Unfortunately we can't do this :-(
Patches sent to the kernel cannot contain conditional code on being in the kernel or not (same is true for checks against a particular kernel version - such code can't be in the kernel).
Patches sent to the kernel must assume that they are only for the kernel (and for that particular version we are sending the patches against).
Of course this makes everything a bit trickier for us since we share files between kernel and userspace .....
This is one of the reasons why we have compat.c/h in the batman-adv package: you will see that in those files we had to implement all sort of hackish/dirty tricks in order to keep the code in the other files clean (compat.h/c are not sent to the kernel but are part of the batman-adv out-of-the-tree package).
However, as far as I know, you should be able to simply do:
#include <linux/if_ether.h> (without the uapi subfolder)
and this should happily work for both the kernel and the userspace. Have you tried that? I might be wrong - it's quite some time I haven't used uapi headers.
Otherwise we might need to find a different solution - or live we what we have now :)
Cheers,
/**
- enum batadv_packettype - types for batman-adv encapsulated packets
- @BATADV_IV_OGM: originator messages for B.A.T.M.A.N. IV
Hi Antonio,
On Sat, Dec 27, 2014 at 03:30:53PM +0100, Antonio Quartulli wrote:
Hi Markus,
first of all you should know that I really appreciate your hard work. Thank you very much for this!
On 26/12/14 12:41, Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
packet.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/packet.h b/packet.h index facd1feddd4e..5fd0d814b6de 100644 --- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#ifdef __KERNEL__ +#include <linux/bitops.h> +#include <uapi/linux/if_ether.h> +#endif /* __KERNEL__ */
Unfortunately we can't do this :-(
Patches sent to the kernel cannot contain conditional code on being in the kernel or not (same is true for checks against a particular kernel version - such code can't be in the kernel).
Patches sent to the kernel must assume that they are only for the kernel (and for that particular version we are sending the patches against).
Oh, I checked by grepping through the kernel before changing the patch like this. git grep "^#if.*def.*__KERNEL__" | grep -v include | wc -l 145
So there are 145 uses of an ifdef __KERNEL__ outside of include directories (I excluded include directories as they may be exported to userspace). So I thought it would be ok.
Of course this makes everything a bit trickier for us since we share files between kernel and userspace .....
This is one of the reasons why we have compat.c/h in the batman-adv package: you will see that in those files we had to implement all sort of hackish/dirty tricks in order to keep the code in the other files clean (compat.h/c are not sent to the kernel but are part of the batman-adv out-of-the-tree package).
Yes I saw that. I still don't really understand why this whole out of tree package is necessary. But I am wondering if it wouldn't be easier to develope and build directly in the kernel tree.
However, as far as I know, you should be able to simply do:
#include <linux/if_ether.h> (without the uapi subfolder)
and this should happily work for both the kernel and the userspace. Have you tried that? I might be wrong - it's quite some time I haven't used uapi headers.
Yep, that works for the kernel, thanks. However, bitops.h remains as we don't have bitops.h for the userspace.
Otherwise we might need to find a different solution - or live we what we have now :)
Yeah it works normally as the includes leak through other include files. But I prefer explicit includes. I am thinking about some solution for the bitops.h
Best Regards,
Markus
Markus,
On 27/12/14 18:03, Markus Pargmann wrote:
Hi Antonio,
On Sat, Dec 27, 2014 at 03:30:53PM +0100, Antonio Quartulli wrote:
Hi Markus,
first of all you should know that I really appreciate your hard work. Thank you very much for this!
On 26/12/14 12:41, Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
packet.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/packet.h b/packet.h index facd1feddd4e..5fd0d814b6de 100644 --- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#ifdef __KERNEL__ +#include <linux/bitops.h> +#include <uapi/linux/if_ether.h> +#endif /* __KERNEL__ */
Unfortunately we can't do this :-(
Patches sent to the kernel cannot contain conditional code on being in the kernel or not (same is true for checks against a particular kernel version - such code can't be in the kernel).
Patches sent to the kernel must assume that they are only for the kernel (and for that particular version we are sending the patches against).
Oh, I checked by grepping through the kernel before changing the patch like this. git grep "^#if.*def.*__KERNEL__" | grep -v include | wc -l 145
So there are 145 uses of an ifdef __KERNEL__ outside of include directories (I excluded include directories as they may be exported to userspace). So I thought it would be ok.
these are probably pieces of code that have not yet been cleaned up. Still, this does not allow us to introduce more code like this: David (the networking tree maintainer) would refuse such additions.
Cheers,
On Wed, Dec 31, 2014 at 06:55:25PM +0100, Antonio Quartulli wrote:
Markus,
On 27/12/14 18:03, Markus Pargmann wrote:
Hi Antonio,
On Sat, Dec 27, 2014 at 03:30:53PM +0100, Antonio Quartulli wrote:
Hi Markus,
first of all you should know that I really appreciate your hard work. Thank you very much for this!
On 26/12/14 12:41, Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
packet.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/packet.h b/packet.h index facd1feddd4e..5fd0d814b6de 100644 --- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#ifdef __KERNEL__ +#include <linux/bitops.h> +#include <uapi/linux/if_ether.h> +#endif /* __KERNEL__ */
Unfortunately we can't do this :-(
Patches sent to the kernel cannot contain conditional code on being in the kernel or not (same is true for checks against a particular kernel version - such code can't be in the kernel).
Patches sent to the kernel must assume that they are only for the kernel (and for that particular version we are sending the patches against).
Oh, I checked by grepping through the kernel before changing the patch like this. git grep "^#if.*def.*__KERNEL__" | grep -v include | wc -l 145
So there are 145 uses of an ifdef __KERNEL__ outside of include directories (I excluded include directories as they may be exported to userspace). So I thought it would be ok.
these are probably pieces of code that have not yet been cleaned up. Still, this does not allow us to introduce more code like this: David (the networking tree maintainer) would refuse such additions.
Okay, thanks. I removed the bitops.h include for the moment as I don't have an idea how to keep the header usable for the userspace application.
Best regards,
Markus
On Friday 26 December 2014 12:41:42 Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
packet.h | 5 +++++ 1 file changed, 5 insertions(+)
Again, why is it a problem if these headers are missing ? Something not compiling ? Something else broken ? What are we fixing ?
Cheers, Marek
On Sun, Dec 28, 2014 at 10:35:03AM +0800, Marek Lindner wrote:
On Friday 26 December 2014 12:41:42 Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
packet.h | 5 +++++ 1 file changed, 5 insertions(+)
Again, why is it a problem if these headers are missing ? Something not compiling ? Something else broken ? What are we fixing ?
This is fixing missing includes that I discovered when removing other includes from c files. As this header uses defines from that include file I think it should be explicitly listed. Will add more description to the commit message.
Best regards,
Markus
--- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#ifdef __KERNEL__ +#include <linux/bitops.h> +#include <uapi/linux/if_ether.h> +#endif /* __KERNEL__ */
/**
- enum batadv_packettype - types for batman-adv encapsulated packets
- @BATADV_IV_OGM: originator messages for B.A.T.M.A.N. IV
Ok, so you are including linux/bitops.h for BIT(...) and uapi/linux/if_ether.h for ETH_ALEN? You are missing:
* linux/types.h for uint8_t, __be16 and so on * define for the bitfield byteorder comes from asm/byteorder.h
Most of these files are part of linux-libc-dev. The rest requires some kind of hack. A way to still work in the batctl build would be to include dummy files. An example patch is attached. This builds at least on my Debian system and my OpenWrt build environment.
Kind regards, Sven
On Sat, Mar 21, 2015 at 11:36:10PM +0100, Sven Eckelmann wrote:
--- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#ifdef __KERNEL__ +#include <linux/bitops.h> +#include <uapi/linux/if_ether.h> +#endif /* __KERNEL__ */
/**
- enum batadv_packettype - types for batman-adv encapsulated packets
- @BATADV_IV_OGM: originator messages for B.A.T.M.A.N. IV
Ok, so you are including linux/bitops.h for BIT(...) and uapi/linux/if_ether.h for ETH_ALEN? You are missing:
- linux/types.h for uint8_t, __be16 and so on
- define for the bitfield byteorder comes from asm/byteorder.h
Most of these files are part of linux-libc-dev. The rest requires some kind of hack. A way to still work in the batctl build would be to include dummy files. An example patch is attached. This builds at least on my Debian system and my OpenWrt build environment.
Thanks, I like the idea of dummy files. The patch also looks fine.
Best Regards,
Markus
Kind regards, Sven
diff --git a/Makefile b/Makefile index 0eb71a1..ffbbf72 100755 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ MANPAGE = man/batctl.8
# batctl flags and options CFLAGS += -Wall -W -std=gnu99 -fno-strict-aliasing -MD -MP -CPPFLAGS += -D_GNU_SOURCE +CPPFLAGS += -D_GNU_SOURCE -Ikernelinc LDLIBS += -lm
# disable verbose output diff --git a/kernelinc/linux/bitops.h b/kernelinc/linux/bitops.h new file mode 100644 index 0000000..d41db10 --- /dev/null +++ b/kernelinc/linux/bitops.h @@ -0,0 +1,27 @@ +/*
- Copyright (C) 2009-2014 B.A.T.M.A.N. contributors:
- Marek Lindner mareklindner@neomailbox.ch
- 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 _BATCTL_KERNELINC_LINUX_BITOPS_H +#define _BATCTL_KERNELINC_LINUX_BITOPS_H
+#define BIT(nr) (1UL << (nr))
+#endif diff --git a/main.h b/main.h index 461f3e3..248467c 100644 --- a/main.h +++ b/main.h @@ -36,17 +36,6 @@ #define DEBUG_TABLE_PATH_MAX_LEN 20 #define SETTINGS_PATH_MAX_LEN 25
-#if BYTE_ORDER == BIG_ENDIAN -#define __BIG_ENDIAN_BITFIELD -#elif BYTE_ORDER == LITTLE_ENDIAN -#define __LITTLE_ENDIAN_BITFIELD -#else -#error "unknown endianess" -#endif
-#define __packed __attribute((packed)) /* linux kernel compat */ -#define BIT(nr) (1UL << (nr)) /* linux kernel compat */
extern char module_ver_path[];
#ifndef VLAN_VID_MASK diff --git a/packet.h b/packet.h index b81fbbf..27b4003 100644 --- a/packet.h +++ b/packet.h @@ -18,6 +18,11 @@ #ifndef _NET_BATMAN_ADV_PACKET_H_ #define _NET_BATMAN_ADV_PACKET_H_
+#include <asm/byteorder.h> +#include <linux/bitops.h> +#include <linux/if_ether.h> +#include <linux/types.h>
/**
- enum batadv_packettype - types for batman-adv encapsulated packets
- @BATADV_IV_OGM: originator messages for B.A.T.M.A.N. IV
diff --git a/sys.c b/sys.c index 676bef1..6c0cab4 100644 --- a/sys.c +++ b/sys.c @@ -26,6 +26,7 @@ #include <string.h> #include <errno.h> #include <dirent.h> +#include <linux/bitops.h>
#include "main.h" #include "sys.h"
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- types.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/types.h b/types.h index cd331974321b..7c22dfadff5e 100644 --- a/types.h +++ b/types.h @@ -21,6 +21,7 @@ #include "packet.h" #include "bitarray.h" #include <linux/kernel.h> +#include <linux/netdevice.h>
#ifdef CONFIG_BATMAN_ADV_DAT
On Friday 26 December 2014 12:41:43 Markus Pargmann wrote:
Signed-off-by: Markus Pargmann mpa@pengutronix.de
Same here ..
Cheers, Marek
On Friday 26 December 2014 12:41:17 Markus Pargmann wrote:
Markus Pargmann (26):
[...]
batman-adv: packet.h, add some missing includes batman-adv: types.h, add missing include
Just went through the includes and noticed that a lot more are missing (also in the files which you are already touched). I have attached the current state of my analysis so you can compare it with your notes.
Kind regards, Sven
On Sunday 22 March 2015 01:52:40 Sven Eckelmann wrote:
On Friday 26 December 2014 12:41:17 Markus Pargmann wrote:
Markus Pargmann (26):
[...]
batman-adv: packet.h, add some missing includes batman-adv: types.h, add missing include
Just went through the includes and noticed that a lot more are missing (also in the files which you are already touched). I have attached the current state of my analysis so you can compare it with your notes.
Small disclaimer I forgot:
The patch is currently not finished. Following things are missing:
* #include "types.h" in main.h after "compat.h" (may be removed in the future when I can drop it somehow... but it will most likely be added again even when there is a dependency circle with types.h) * tests with older kernels * an actual description
Kind regards, Sven
Hi Sven,
On Sun, Mar 22, 2015 at 02:34:33AM +0100, Sven Eckelmann wrote:
On Sunday 22 March 2015 01:52:40 Sven Eckelmann wrote:
On Friday 26 December 2014 12:41:17 Markus Pargmann wrote:
Markus Pargmann (26):
[...]
batman-adv: packet.h, add some missing includes batman-adv: types.h, add missing include
Just went through the includes and noticed that a lot more are missing (also in the files which you are already touched). I have attached the current state of my analysis so you can compare it with your notes.
Do you have any tool which makes this include analysis easier?
Small disclaimer I forgot:
The patch is currently not finished. Following things are missing:
- #include "types.h" in main.h after "compat.h" (may be removed in the future when I can drop it somehow... but it will most likely be added again even when there is a dependency circle with types.h)
I hope that types.h can be split up into appropriate topic headers which are selectively included by the files which need it. However I didn't have a deeper look into this yet so it may be more complicated.
- tests with older kernels
I will test your series with an arm build, but I don't think that anything breaks there.
Best Regards,
Markus
On Tuesday 24 March 2015 12:41:07 Markus Pargmann wrote:
Hi Sven,
On Sun, Mar 22, 2015 at 02:34:33AM +0100, Sven Eckelmann wrote:
On Sunday 22 March 2015 01:52:40 Sven Eckelmann wrote:
On Friday 26 December 2014 12:41:17 Markus Pargmann wrote:
Markus Pargmann (26):
[...]
batman-adv: packet.h, add some missing includes batman-adv: types.h, add missing include
Just went through the includes and noticed that a lot more are missing (also in the files which you are already touched). I have attached the current state of my analysis so you can compare it with your notes.
Do you have any tool which makes this include analysis easier?
I've started to test iwyu but the output cannot really be trusted. But at least it can help to spot things.
make -k CC="iwyu -Xiwyu --no_default_mappings -Xiwyu --verbose=1 -Xiwyu -- mapping_file=$HOME/kernel_mappings.iwyu" CONFIG_BATMAN_ADV_NC=y CONFIG_BATMAN_ADV_DEBUG=y
The header files are for example also not all analyzed. So you have to fiddle around with them too.
Kind regards, Sven
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Marek Lindner -
Markus Pargmann -
Simon Wunderlich -
Sven Eckelmann