On Wed, Jul 31, 2013 at 10:47:38PM +0200, Antonio Quartulli wrote:
@@ -974,6 +973,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, uint32_t seqno = ntohl(batadv_ogm_packet->seqno); uint8_t *neigh_addr; uint8_t packet_count;
- unsigned long (*bitmap)[];
[...] @@ -1010,13 +1010,13 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
/* if the window moved, set the update flag. */
need_update |= batadv_bit_get_packet(bat_priv,tmp_neigh_node->real_bits,
bitmap = &tmp_neigh_node->bat_iv.real_bits;need_update |= batadv_bit_get_packet(bat_priv, *bitmap, seq_diff, set_mark);
Why referencing and derefencing here? That looks odd, you don't use the bitmap after that, do you?
packet_count = bitmap_weight(tmp_neigh_node->real_bits,
packet_count = bitmap_weight(tmp_neigh_node->bat_iv.real_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
tmp_neigh_node->real_packet_count = packet_count;
tmp_neigh_node->bat_iv.real_packet_count = packet_count;} rcu_read_unlock();
/* ... and is good enough to be considered */
- if (neigh_node->tq_avg < router->tq_avg - BATADV_BONDING_TQ_THRESHOLD)
- if (neigh_node->bat_iv.tq_avg <
router->bat_iv.tq_avg - BATADV_BONDING_TQ_THRESHOLD)
alignment looks a little weird, although this will probably replaced anyway in the later patches?
/**
- struct batadv_neigh_node - structure for single hop neighbors
- @list: list node for batadv_orig_node::neigh_list
- @addr: mac address of neigh node
- struct batadv_neigh_bat_iv - structure for single hop neighbors
- @tq_recv: ring buffer of received TQ values from this neigh node
- @tq_index: ring buffer index
- @tq_avg: averaged tq of all tq values in the ring buffer (tq_recv)
- @last_ttl: last received ttl from this neigh node
- @bonding_list: list node for batadv_orig_node::bond_list
- @last_seen: when last packet via this neighbor was received
- @real_bits: bitfield containing the number of OGMs received from this neigh
- node (relative to orig_node->last_real_seqno)
- @real_packet_count: counted result of real_bits
- @orig_node: pointer to corresponding orig_node
- @if_incoming: pointer to incoming hard interface
- @lq_update_lock: lock protecting tq_recv & tq_index
- @refcount: number of contexts the object is used
*/
- @rcu: struct used for freeing in an RCU-safe manner
-struct batadv_neigh_node {
- struct hlist_node list;
- uint8_t addr[ETH_ALEN];
+struct batadv_neigh_bat_iv { uint8_t tq_recv[BATADV_TQ_GLOBAL_WINDOW_SIZE]; uint8_t tq_index; uint8_t tq_avg;
- uint8_t last_ttl;
- struct list_head bonding_list;
- unsigned long last_seen; DECLARE_BITMAP(real_bits, BATADV_TQ_LOCAL_WINDOW_SIZE); uint8_t real_packet_count;
- spinlock_t lq_update_lock; /* protects tq_recv & tq_index */
I'd like to suggest to split out struct batadv_neigh_bat_iv in an own struct instead of keeping it in batadv_neigh_node. If you want to keep it there, you should at least do some indendation for the bat_iv specific parts (and maybe also the kernel doc?)
Cheers, Simon