Hi,
On Tue, Nov 01, 2011 at 11:52:45AM +0100, Marek Lindner wrote:
On Sunday, October 30, 2011 23:51:03 Simon Wunderlich wrote:
+static const uint8_t announce_mac[6] = {0x43, 0x05, 0x43, 0x05, 0x00, 0x00};
All we ever use are 4 bytes - we could make this shorter. Otherwise please use ETH_ALEN. Please use ETH_ALEN throughout the code instead of 6.
OK, that's right, i'll change it to 4 and use ETH_ALEN at the other place where i missed it.
bat_dbg(DBG_BLA, bat_priv,"handle_announce(): ANNOUNCE vid %d (sent ""by %pM)... CRC = %04x (nw order)\n",vid, backbone_gw->orig, crc);It is not network byte order anymore ..
OK
/* TODO: we could cal something like tt_local_del() here. */Why should we ?
Because when someone claims the adress, the the client has roamed to the mesh.
Usually this is handled by the tt roaming code, this will only fail in a limited horizon scenario where a node does not know the other node the client roamed to. In this case, the tt will time out after a long while.
This is just an idea/optimization to add the tt_local_del(). :)
@@ -634,7 +640,7 @@ static int batman_skb_recv(struct sk_buff *skb, struct net_device *dev, case BAT_TT_QUERY: ret = recv_tt_query(skb, hard_iface); break;
/* Roaming advertisement */
/* bridge roop avoidance query */ case BAT_ROAM_ADV: ret = recv_roam_adv(skb, hard_iface); break;Small typo here but why are you even changing the comment ?
Oh thanks, this is wrong. This comes from an earlier version of the BLA table changes where I used a special packet type.
The comment should not be changed.
atomic_t gw_reselect; struct hard_iface __rcu *primary_if; /* rcu protected pointer */ struct vis_info *my_vis_info;
uint8_t own_orig[6]; /* cache primary hardifs address */};
I'd call this "primary_addr" instead of "own_orig" to be consistent with "primary_if". Furthermore ETH_ALEN should be used for the length.
OK.
+struct backbone_gw {
uint8_t orig[ETH_ALEN];short vid; /* used VLAN ID */struct hlist_node hash_entry;struct bat_priv *bat_priv;unsigned long lasttime; /* last time we heard of this backbone gw */atomic_t request_sent;atomic_t refcount;struct rcu_head rcu;uint16_t crc; /* crc checksum over all claims */+} __packed;
+struct claim {
uint8_t addr[ETH_ALEN];short vid;struct backbone_gw *backbone_gw;unsigned long lasttime; /* last time we heard of claim (locals only)*/
struct rcu_head rcu;atomic_t refcount;struct hlist_node hash_entry;+} __packed;
Why are these structs packed ?
For similar reasons as the tt_global/local_entry issue: The hash functions both access orig and vid from these structures.
However, as there are not that many sharing function I'll split it into choose_claim()/choose_backbone_gw() and remove the __packed attribute.
In this case, it should be a little bit shorter and cleaner than using a common structure (no container_of() stuff).
Thanks for the comments, you'll find the changed stuff as patches in my blaII_dirty repo.
Cheers, Simon