Hello David,
here you have our second batch of patches intended for net-next.
In this patchset you won't find any new features, but quite some code cleanup work, a bunch of code style fixes and also comments corrections by Markus Pargmann.
Moreover you have a patch from Sven Eckelmann removing an unnecessary NULL check in batadv_iv_ogm_update_seqnos().
Please pull or let me know of any problem!
Thanks a lot, Antonio
The following changes since commit df905ceae3883f024282d00824ce33040097ac86:
Merge branch 'sfc-next' (2015-06-02 12:57:39 -0700)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem
for you to fetch changes up to f372d09059a67f57b3d2b5eab8c05b0ddb8ca92f:
batman-adv: Remove unnecessary ret variable in algo_register (2015-06-03 15:57:25 +0200)
---------------------------------------------------------------- Included changes: - code re-arrangement for better reading and understanding - code style fixups - comments corrections - remove unnecessary NULL check in batadv_iv_ogm_update_seqnos() - make boolean functions explicitly return a bool result - remove unnecessary variables in algo_register() and algo_select()
---------------------------------------------------------------- Markus Pargmann (11): 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 missing 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
Sven Eckelmann (1): batman-adv: Remove unnecessary check for orig_ifinfo not NULL
net/batman-adv/bat_iv_ogm.c | 46 ++++++++++++++++++++++----------------------- net/batman-adv/main.c | 29 ++++++++++++---------------- net/batman-adv/main.h | 4 ++-- net/batman-adv/types.h | 7 ++++--- 4 files changed, 41 insertions(+), 45 deletions(-)
From: Markus Pargmann mpa@pengutronix.de
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 Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bat_iv_ogm.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index c5ba7a7..190a642 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -642,19 +642,16 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, if (!batadv_atomic_dec_not_zero(&bat_priv->batman_queue_left)) { batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "batman packet queue full\n"); - goto out; + goto out_free_outgoing; } }
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,7 +692,12 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, send_time - jiffies);
return; -out: +out_free_forw_packet: + kfree(forw_packet_aggr); +out_nomem: + if (!own_packet) + atomic_inc(&bat_priv->batman_queue_left); +out_free_outgoing: batadv_hardif_free_ref(if_outgoing); out_free_incoming: batadv_hardif_free_ref(if_incoming);
From: Markus Pargmann mpa@pengutronix.de
Signed-off-by: Markus Pargmann mpa@pengutronix.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bat_iv_ogm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 190a642..159f30a 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/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,
From: Markus Pargmann mpa@pengutronix.de
CodingStyle describes that either none or both branches of a conditional have to have brackets.
Signed-off-by: Markus Pargmann mpa@pengutronix.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bat_iv_ogm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 159f30a..66ac9e8 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/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);
From: Markus Pargmann mpa@pengutronix.de
Signed-off-by: Markus Pargmann mpa@pengutronix.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 66ac9e8..23c41d1 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -28,7 +28,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
From: Markus Pargmann mpa@pengutronix.de
The kernel coding style says, that there should not be multiple assignments in one row.
Signed-off-by: Markus Pargmann mpa@pengutronix.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bat_iv_ogm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 23c41d1..0019f1b 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -64,7 +64,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;
From: Markus Pargmann mpa@pengutronix.de
This is a small copy paste fix for batadv_ing_buffer_avg.
Signed-off-by: Markus Pargmann mpa@pengutronix.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 0019f1b..9f83137 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -55,7 +55,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 *
From: Markus Pargmann mpa@pengutronix.de
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 Signed-off-by: Antonio Quartulli antonio@meshcoding.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- net/batman-adv/types.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index e95db42..c1000c0 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -183,9 +183,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: set of bitfields (one per hard interface) where each one counts + * the number of our OGMs this orig_node rebroadcasted "back" to us (relative + * to last_real_seqno). Every bitfield is BATADV_TQ_LOCAL_WINDOW_SIZE bits long. + * @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 */
From: Sven Eckelmann sven@narfation.org
orig_ifinfo is dereferenced multiple times in batadv_iv_ogm_update_seqnos before the check for NULL is done. The function also exists at the beginning when orig_ifinfo would have been NULL. This makes the check at the end unnecessary and only confuses the reader/code analyzers.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bat_iv_ogm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 9f83137..4e93d2d 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -1357,8 +1357,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, out: spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock); batadv_orig_node_free_ref(orig_node); - if (orig_ifinfo) - batadv_orig_ifinfo_free_ref(orig_ifinfo); + batadv_orig_ifinfo_free_ref(orig_ifinfo); return ret; }
From: Markus Pargmann mpa@pengutronix.de
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 Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/main.c | 11 +++++++---- net/batman-adv/main.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index fd9333d..a22247a 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -209,10 +209,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) { @@ -223,12 +226,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/net/batman-adv/main.h b/net/batman-adv/main.h index 026ba37..96876d2 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -195,7 +195,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);
From: Markus Pargmann mpa@pengutronix.de
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 Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/main.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h index 96876d2..af0a336 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -279,7 +279,7 @@ static inline void _batadv_dbg(int type __always_unused, * * 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); }
From: Markus Pargmann mpa@pengutronix.de
We can avoid this indirect return variable by directly returning the error values.
Signed-off-by: Markus Pargmann mpa@pengutronix.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/main.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index a22247a..49f07e1 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -513,14 +513,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) */ @@ -534,16 +532,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)
From: Markus Pargmann mpa@pengutronix.de
Remove ret variable and all jumps.
Signed-off-by: Markus Pargmann mpa@pengutronix.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/main.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index 49f07e1..548e405 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -544,17 +544,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)
From: Antonio Quartulli antonio@meshcoding.com Date: Wed, 3 Jun 2015 16:13:56 +0200
here you have our second batch of patches intended for net-next.
In this patchset you won't find any new features, but quite some code cleanup work, a bunch of code style fixes and also comments corrections by Markus Pargmann.
Moreover you have a patch from Sven Eckelmann removing an unnecessary NULL check in batadv_iv_ogm_update_seqnos().
Please pull or let me know of any problem!
Pulled, thanks Antonio.
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
David Miller