pull request: batman-adv 20150603
Subsystems (3)
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(-)
PATCH 1/12 batman-adv: iv_ogm_aggregate_new, simplify error handling
From: Markus Pargmann <[email protected]>
It is just a bit easier to put the error handling at one place and let multiple error paths use the same calls.
--- net/batman-adv/bat_iv_ogm.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)
net/batman-adv/bat_iv_ogm.c +13 -15
@@ -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);
Signature · 1 line
2.4.2
PATCH 2/12 batman-adv: iv_ogm_queue_add, Simplify expressions
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
--- net/batman-adv/bat_iv_ogm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
net/batman-adv/bat_iv_ogm.c +2 -2
@@ -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,
Signature · 1 line
2.4.2
PATCH 3/12 batman-adv: iv_ogm_orig_update, style, add missing brackets
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
CodingStyle describes that either none or both branches of a conditional have to have brackets.
--- net/batman-adv/bat_iv_ogm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
net/batman-adv/bat_iv_ogm.c +2 -1
@@ -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);
Signature · 1 line
2.4.2
PATCH 4/12 batman-adv: iv_ogm, Fix dup_status comment
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
--- net/batman-adv/bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/batman-adv/bat_iv_ogm.c +1 -1
@@ -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
Signature · 1 line
2.4.2
PATCH 5/12 batman-adv: iv_ogm, fix coding style
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
The kernel coding style says, that there should not be multiple assignments in one row.
--- net/batman-adv/bat_iv_ogm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
net/batman-adv/bat_iv_ogm.c +3 -1
@@ -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;
Signature · 1 line
2.4.2
PATCH 6/12 batman-adv: iv_ogm, fix comment function name
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
This is a small copy paste fix for batadv_ing_buffer_avg.
--- net/batman-adv/bat_iv_ogm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/batman-adv/bat_iv_ogm.c +1 -1
@@ -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 *
Signature · 1 line
2.4.2
PATCH 7/12 batman-adv: types, Fix comment on bcast_own
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
batadv_orig_bat_iv->bcast_own is actually not a bitfield, it is an array. Adjust the comment accordingly.
--- net/batman-adv/types.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
net/batman-adv/types.h +4 -3
@@ -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 */
Signature · 1 line
2.4.2
PATCH 8/12 batman-adv: Remove unnecessary check for orig_ifinfo not NULL
In reply to Antonio QuartulliFrom: Sven Eckelmann <[email protected]>
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.
--- net/batman-adv/bat_iv_ogm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
net/batman-adv/bat_iv_ogm.c +1 -2
@@ -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; }
Signature · 1 line
2.4.2
PATCH 9/12 batman-adv: main, Convert is_my_mac() to bool
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
It is much clearer to see a bool type as return value than 'int' for functions that are supposed to return true or false.
--- net/batman-adv/main.c | 11 +++++++---- net/batman-adv/main.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-)
net/batman-adv/main.c +7 -4
@@ -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; }/**
net/batman-adv/main.h +1 -1
@@ -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 );
Signature · 1 line
2.4.2
PATCH 10/12 batman-adv: main, batadv_compare_eth return bool
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
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.
--- net/batman-adv/main.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/batman-adv/main.h +1 -1
@@ -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); }
Signature · 1 line
2.4.2
PATCH 11/12 batman-adv: Remove unnecessary ret variable
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
We can avoid this indirect return variable by directly returning the error values.
--- net/batman-adv/main.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
net/batman-adv/main.c +3 -8
@@ -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)
Signature · 1 line
2.4.2
PATCH 12/12 batman-adv: Remove unnecessary ret variable in algo_register
In reply to Antonio QuartulliFrom: Markus Pargmann <[email protected]>
Remove ret variable and all jumps.
--- net/batman-adv/main.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
net/batman-adv/main.c +2 -5
@@ -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)
Signature · 1 line
2.4.2
David Miller
maintainerFrom: Antonio Quartulli <[email protected]>
Date: Wed, 3 Jun 2015 16:13:56 +0200
Quoted 10 lines
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.