On Wed, May 29, 2013 at 12:20:48AM +0200, Antonio Quartulli wrote: > From: Antonio Quartulli <[email protected]> > > Signed-off-by: Antonio Quartulli <[email protected]> > --- > gateway_client.c | 74 > +++++++++++++++++++++++++++++++------------------------- > main.h | 1 + > 2 files changed, 42 insertions(+), 33 deletions(-) > > diff --git a/gateway_client.c b/gateway_client.c > index 69488b2..0a9f1d0 100644 > --- a/gateway_client.c > +++ b/gateway_client.c > @@ -113,13 +113,13 @@ void batadv_gw_deselect(struct batadv_priv *bat_priv) > static struct batadv_gw_node * > batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv) > { > - struct batadv_neigh_node *router; > + struct batadv_algo_ops *bao = bat_priv->bat_algo_ops; > struct batadv_gw_node *gw_node, *curr_gw = NULL; > uint32_t max_gw_factor = 0, tmp_gw_factor = 0; > - uint32_t gw_divisor; > - uint8_t max_tq = 0; > - uint8_t tq_avg; > struct batadv_orig_node *orig_node; > + struct batadv_neigh_node *router; > + uint32_t metric, max_metric = 0; > + uint32_t gw_divisor; > > gw_divisor = BATADV_TQ_LOCAL_WINDOW_SIZE * BATADV_TQ_LOCAL_WINDOW_SIZE; > gw_divisor *= 64; > @@ -137,18 +137,19 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv) > if (!atomic_inc_not_zero(&gw_node->refcount)) > goto next; > > - tq_avg = router->bat_iv.tq_avg; > + metric = bao->bat_metric_get(router); > > switch (atomic_read(&bat_priv->gw_sel_class)) { > case 1: /* fast connection */ > - tmp_gw_factor = tq_avg * tq_avg; > + tmp_gw_factor = metric * metric;
Hmm, that is rather strange ... I think fiddling with the metric directly
this way is weird when abstracting. For example:
1.) Assuming we don't know how the metric looks like, we can't just
multiplicate them. A logarithmic scaled metric or even arbritrary
metric would look different compared to the linear metrics as we
use now.
2.) This might overflow because metric is u32 and tmp_gw_factor is too.
It should work for batman IV where the metric is <256, but might
not for BATMAN V.
I think this "bandwidth magic" should be abstracted as well somehow, if
we want to keep on using it that way.
> tmp_gw_factor *= gw_node->bandwidth_down;
> tmp_gw_factor *= 100 * 100;
> tmp_gw_factor /= gw_divisor;
>
> if ((tmp_gw_factor > max_gw_factor) ||
> ((tmp_gw_factor == max_gw_factor) &&
> - (tq_avg > max_tq))) {
> + (bao->bat_metric_compare(metric,
> + max_metric) > 0))) {
> if (curr_gw)
> batadv_gw_node_free_ref(curr_gw);
> curr_gw = gw_node;
> @@ -163,7 +164,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
> * soon as a better gateway appears which has
> * $routing_class more tq points)
> */
> - if (tq_avg > max_tq) {
> + if (bao->bat_metric_compare(metric, max_metric) > 0) {
> if (curr_gw)
> batadv_gw_node_free_ref(curr_gw);
> curr_gw = gw_node;
> @@ -172,8 +173,8 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
> break;
> }
>
> - if (tq_avg > max_tq)
> - max_tq = tq_avg;
> + if (bao->bat_metric_compare(metric, max_metric) > 0)
> + max_metric = metric;
>
> if (tmp_gw_factor > max_gw_factor)
> max_gw_factor = tmp_gw_factor;
> @@ -229,22 +230,24 @@ void batadv_gw_election(struct batadv_priv *bat_priv)
> NULL);
> } else if ((!curr_gw) && (next_gw)) {
> batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
> - "Adding route to gateway %pM (bandwidth: %u.%u/%u.%u
> MBit, tq: %i)\n",
> + "Adding route to gateway %pM (bandwidth: %u.%u/%u.%u
> MBit, metric: %u)\n",
> next_gw->orig_node->orig,
> next_gw->bandwidth_down / 10,
> next_gw->bandwidth_down % 10,
> next_gw->bandwidth_up / 10,
> - next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
> + next_gw->bandwidth_up % 10,
> + bat_priv->bat_algo_ops->bat_metric_get(router));
> batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_ADD,
> gw_addr);
> } else {
> batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
> - "Changing route to gateway %pM (bandwidth:
> %u.%u/%u.%u MBit, tq: %i)\n",
> + "Changing route to gateway %pM (bandwidth:
> %u.%u/%u.%u MBit, metric: %u)\n",
> next_gw->orig_node->orig,
> next_gw->bandwidth_down / 10,
> next_gw->bandwidth_down % 10,
> next_gw->bandwidth_up / 10,
> - next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
> + next_gw->bandwidth_up % 10,
> + bat_priv->bat_algo_ops->bat_metric_get(router));
> batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_CHANGE,
> gw_addr);
> }
> @@ -263,9 +266,10 @@ out:
> void batadv_gw_check_election(struct batadv_priv *bat_priv,
> struct batadv_orig_node *orig_node)
> {
> - struct batadv_orig_node *curr_gw_orig;
> struct batadv_neigh_node *router_gw = NULL, *router_orig = NULL;
> - uint8_t gw_tq_avg, orig_tq_avg;
> + struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
> + struct batadv_orig_node *curr_gw_orig;
> + uint16_t gw_metric, orig_metric;
>
> curr_gw_orig = batadv_gw_get_selected_orig(bat_priv);
> if (!curr_gw_orig)
> @@ -283,23 +287,25 @@ void batadv_gw_check_election(struct batadv_priv
> *bat_priv,
> if (!router_orig)
> goto out;
>
> - gw_tq_avg = router_gw->bat_iv.tq_avg;
> - orig_tq_avg = router_orig->bat_iv.tq_avg;
> + gw_metric = bao->bat_metric_get(router_gw);
> + orig_metric = bao->bat_metric_get(router_orig);
>
> - /* the TQ value has to be better */
> - if (orig_tq_avg < gw_tq_avg)
> + /* the metric has to be better */
> + if (bao->bat_metric_compare(orig_metric, gw_metric) > 0)
> goto out;
>
> /* if the routing class is greater than 3 the value tells us how much
> - * greater the TQ value of the new gateway must be
> + * greater the metric of the new gateway must be.
> + *
> + * FIXME: This comparison is strictly TQ related.
> */
> if ((atomic_read(&bat_priv->gw_sel_class) > 3) &&
> - (orig_tq_avg - gw_tq_avg < atomic_read(&bat_priv->gw_sel_class)))
> + (orig_metric - gw_metric < atomic_read(&bat_priv->gw_sel_class)))
> goto out;
>
> batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
> - "Restarting gateway selection: better gateway found (tq
> curr: %i, tq new: %i)\n",
> - gw_tq_avg, orig_tq_avg);
> + "Restarting gateway selection: better gateway found (metric
> curr: %u, metric new: %u)\n",
> + gw_metric, orig_metric);
>
> deselect:
> batadv_gw_deselect(bat_priv);
> @@ -503,11 +509,11 @@ static int batadv_write_buffer_text(struct batadv_priv
> *bat_priv,
>
> curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
>
> - ret = seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
> + ret = seq_printf(seq, "%s %pM (%10u) %pM [%10s]: %u.%u/%u.%u MBit\n",
> (curr_gw == gw_node ? "=>" : " "),
> gw_node->orig_node->orig,
> - router->bat_iv.tq_avg, router->addr,
> - router->if_incoming->net_dev->name,
> + bat_priv->bat_algo_ops->bat_metric_get(router),
> + router->addr, router->if_incoming->net_dev->name,
> gw_node->bandwidth_down / 10,
> gw_node->bandwidth_down % 10,
> gw_node->bandwidth_up / 10,
> @@ -533,8 +539,8 @@ int batadv_gw_client_seq_print_text(struct seq_file *seq,
> void *offset)
> goto out;
>
> seq_printf(seq,
> - " %-12s (%s/%i) %17s [%10s]: advertised uplink
> bandwidth ... [B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s)]\n",
> - "Gateway", "#", BATADV_TQ_MAX_VALUE, "Nexthop", "outgoingIF",
> + " %-12s (%-4s) %17s [%10s]: advertised uplink bandwidth
> ... [B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s)]\n",
> + "Gateway", "#", "Nexthop", "outgoingIF",
> BATADV_SOURCE_VERSION, primary_if->net_dev->name,
> primary_if->net_dev->dev_addr, net_dev->name);
>
> @@ -707,12 +713,13 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb,
> unsigned int *header_len)
> bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
> struct sk_buff *skb, struct ethhdr *ethhdr)
> {
> + struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
> struct batadv_neigh_node *neigh_curr = NULL, *neigh_old = NULL;
> struct batadv_orig_node *orig_dst_node = NULL;
> struct batadv_gw_node *gw_node = NULL, *curr_gw = NULL;
> bool ret, out_of_range = false;
> unsigned int header_len = 0;
> - uint8_t curr_tq_avg;
> + uint32_t curr_metric;
> unsigned short vid;
>
> vid = batadv_get_vid(skb, 0);
> @@ -739,7 +746,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
> /* If we are a GW then we are our best GW. We can artificially
> * set the tq towards ourself as the maximum value
> */
> - curr_tq_avg = BATADV_TQ_MAX_VALUE;
> + curr_metric = BATADV_MAX_METRIC;
> break;
> case BATADV_GW_MODE_CLIENT:
> curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
> @@ -759,7 +766,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
> if (!neigh_curr)
> goto out;
>
> - curr_tq_avg = neigh_curr->bat_iv.tq_avg;
> + curr_metric = bao->bat_metric_get(neigh_curr);
> break;
> case BATADV_GW_MODE_OFF:
> default:
> @@ -770,7 +777,8 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
> if (!neigh_old)
> goto out;
>
> - if (curr_tq_avg - neigh_old->bat_iv.tq_avg > BATADV_GW_THRESHOLD)
> + if (bao->bat_metric_is_similar(bao->bat_metric_get(neigh_old),
> + curr_metric))
> out_of_range = true;
Hmm ... if you add the abs for metric_is_similar as suggested for the patch
introducing
the function, this one would fail. For BATMAN IV, curr_metric would be
0xffffffff and
the neigh_old would be something < 256, making this function fail for the
BATADV_GW_MODE_SERVER
case.
Actually BATADV_GW_MODE_SERVER could just set out_of_range and goto out, I
don't understand
the purpose of this "artificially setting the tq towards ourself" is good for.
>
> out:
> diff --git a/main.h b/main.h
> index 1d3611f..8d69641 100644
> --- a/main.h
> +++ b/main.h
> @@ -31,6 +31,7 @@
>
> /* B.A.T.M.A.N. parameters */
>
> +#define BATADV_MAX_METRIC 0xffffffff
> #define BATADV_TQ_MAX_VALUE 255
> #define BATADV_JITTER 20
>
> --
> 1.8.1.5
>
signature.asc
Description: Digital signature
