On Mon, Aug 26, 2013 at 11:11:37AM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:49 Antonio Quartulli wrote:
list_for_each_entry_rcu(tmp_neigh_node, &primary_orig->bond_list, @@ -502,8 +509,8 @@ batadv_find_ifalter_router(struct batadv_orig_node *primary_orig, if (tmp_neigh_node->if_incoming == recv_if) continue;
if (router &&tmp_neigh_node->bat_iv.tq_avg <= router->bat_iv.tq_avg)
tmp_metric = bao->bat_metric_get(tmp_neigh_node);if (router && (tmp_metric <= router_metric)) continue;This last comparison looks horribly wrong. Isn't that what we have the API for?
we don't have an API for this. In this case we should use the "bat_metric_compare" but we removed that API in this version since it was going to be implemented as a "<=" for all the algorithms we have on the plate now (bat_iv and bat_v). This is the reason why here we use the compare operation directly.
Besides, it seems to me the bat_metric_is_equiv_or_better() call has been misdesigned. Instead of passing the metric and having each function store + process the metric this call should accept 2 neighbor nodes as argument. This should save 2 out of 3 API calls (2 * bao->bat_metric_get()) and some code in all the functions needing the bat_metric_is_equiv_or_better() API. Moreover, it should reduce the impact on performance because each callback will cost us a little bit ..
we could do that, yeah, even if the tests[1] shown that there is not much to gain at the moment. What do you suggest?
Cheers,
[1] http://codepad.org/ZEtQASF3 - test performed using iperf between two clients connected via Ethernet to 2 OM2P-HS.