[B.A.T.M.A.N.] [PATCH 1/5] batman-adv: ignore protocol packets if the interface did not enable this protocol
Reported-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- bat_iv_ogm.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index eca2b44..819ae49 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1186,6 +1186,7 @@ out: static int bat_iv_ogm_receive(struct sk_buff *skb, struct hard_iface *if_incoming) { + struct bat_priv *bat_priv = netdev_priv(if_incoming->soft_iface); struct batman_ogm_packet *batman_ogm_packet; struct ethhdr *ethhdr; int buff_pos = 0, packet_len; @@ -1196,6 +1197,11 @@ static int bat_iv_ogm_receive(struct sk_buff *skb, if (!ret) return NET_RX_DROP;
+ /* did we receive a B.A.T.M.A.N. IV OGM packet on an interface + * that does not have B.A.T.M.A.N. IV enabled ? */ + if (bat_priv->bat_algo_ops->bat_ogm_emit != bat_iv_ogm_emit) + return NET_RX_DROP; + packet_len = skb_headlen(skb); ethhdr = (struct ethhdr *)skb_mac_header(skb); packet_buff = skb->data;
Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- bat_iv_ogm.c | 18 +++++++++--------- hard-interface.c | 4 ++-- main.c | 2 +- types.h | 5 +++-- 4 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 819ae49..1e92e38 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -93,24 +93,24 @@ static void bat_iv_ogm_iface_disable(struct hard_iface *hard_iface) hard_iface->packet_buff = NULL; }
-static void bat_iv_ogm_primary_iface_set(struct hard_iface *hard_iface) +static void bat_iv_ogm_iface_update_mac(struct hard_iface *hard_iface) { struct batman_ogm_packet *batman_ogm_packet;
batman_ogm_packet = (struct batman_ogm_packet *)hard_iface->packet_buff; - batman_ogm_packet->flags = PRIMARIES_FIRST_HOP; - batman_ogm_packet->header.ttl = TTL; + memcpy(batman_ogm_packet->orig, + hard_iface->net_dev->dev_addr, ETH_ALEN); + memcpy(batman_ogm_packet->prev_sender, + hard_iface->net_dev->dev_addr, ETH_ALEN); }
-static void bat_iv_ogm_update_mac(struct hard_iface *hard_iface) +static void bat_iv_ogm_primary_iface_set(struct hard_iface *hard_iface) { struct batman_ogm_packet *batman_ogm_packet;
batman_ogm_packet = (struct batman_ogm_packet *)hard_iface->packet_buff; - memcpy(batman_ogm_packet->orig, - hard_iface->net_dev->dev_addr, ETH_ALEN); - memcpy(batman_ogm_packet->prev_sender, - hard_iface->net_dev->dev_addr, ETH_ALEN); + batman_ogm_packet->flags = PRIMARIES_FIRST_HOP; + batman_ogm_packet->header.ttl = TTL; }
/* when do we schedule our own ogm to be sent */ @@ -1235,8 +1235,8 @@ static struct bat_algo_ops batman_iv __read_mostly = { .name = "BATMAN IV", .bat_iface_enable = bat_iv_ogm_iface_enable, .bat_iface_disable = bat_iv_ogm_iface_disable, + .bat_iface_update_mac = bat_iv_ogm_iface_update_mac, .bat_primary_iface_set = bat_iv_ogm_primary_iface_set, - .bat_ogm_update_mac = bat_iv_ogm_update_mac, .bat_ogm_schedule = bat_iv_ogm_schedule, .bat_ogm_emit = bat_iv_ogm_emit, }; diff --git a/hard-interface.c b/hard-interface.c index b49bf0d..daa6adc 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -231,7 +231,7 @@ static void hardif_activate_interface(struct hard_iface *hard_iface)
bat_priv = netdev_priv(hard_iface->soft_iface);
- bat_priv->bat_algo_ops->bat_ogm_update_mac(hard_iface); + bat_priv->bat_algo_ops->bat_iface_update_mac(hard_iface); hard_iface->if_status = IF_TO_BE_ACTIVATED;
/** @@ -527,7 +527,7 @@ static int hard_if_event(struct notifier_block *this, check_known_mac_addr(hard_iface->net_dev);
bat_priv = netdev_priv(hard_iface->soft_iface); - bat_priv->bat_algo_ops->bat_ogm_update_mac(hard_iface); + bat_priv->bat_algo_ops->bat_iface_update_mac(hard_iface);
primary_if = primary_if_get_selected(bat_priv); if (!primary_if) diff --git a/main.c b/main.c index 04658e0..75938fb 100644 --- a/main.c +++ b/main.c @@ -326,8 +326,8 @@ int bat_algo_register(struct bat_algo_ops *bat_algo_ops) /* all algorithms must implement all ops (for now) */ if (!bat_algo_ops->bat_iface_enable || !bat_algo_ops->bat_iface_disable || + !bat_algo_ops->bat_iface_update_mac || !bat_algo_ops->bat_primary_iface_set || - !bat_algo_ops->bat_ogm_update_mac || !bat_algo_ops->bat_ogm_schedule || !bat_algo_ops->bat_ogm_emit) { pr_info("Routing algo '%s' does not implement required ops\n", diff --git a/types.h b/types.h index 5ca9b58..04d6cf5 100644 --- a/types.h +++ b/types.h @@ -398,10 +398,11 @@ struct bat_algo_ops { int (*bat_iface_enable)(struct hard_iface *hard_iface); /* de-init routing info when hard-interface is disabled */ void (*bat_iface_disable)(struct hard_iface *hard_iface); + /* (re-)init mac addresses of the protocol information + * belonging to this hard-interface */ + void (*bat_iface_update_mac)(struct hard_iface *hard_iface); /* called when primary interface is selected / changed */ void (*bat_primary_iface_set)(struct hard_iface *hard_iface); - /* init mac addresses of the OGM belonging to this hard-interface */ - void (*bat_ogm_update_mac)(struct hard_iface *hard_iface); /* prepare a new outgoing OGM for the send queue */ void (*bat_ogm_schedule)(struct hard_iface *hard_iface, int tt_num_changes);
On Sunday, March 11, 2012 06:17:50 Marek Lindner wrote:
Signed-off-by: Marek Lindner lindner_marek@yahoo.de
bat_iv_ogm.c | 18 +++++++++--------- hard-interface.c | 4 ++-- main.c | 2 +- types.h | 5 +++-- 4 files changed, 15 insertions(+), 14 deletions(-)
Applied in revision bd83b4c.
Regards, Marek
Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- bat_sysfs.c | 55 ++++++++++++++++++++++++++++--------------------------- 1 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/bat_sysfs.c b/bat_sysfs.c index 3439fb5..91107fd 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -63,7 +63,7 @@ struct bat_attribute bat_attr_##_name = { \ .store = _store, \ };
-#define BAT_ATTR_STORE_BOOL(_name, _post_func) \ +#define BAT_ATTR_SIF_STORE_BOOL(_name, _post_func) \ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ char *buff, size_t count) \ { \ @@ -73,9 +73,9 @@ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ &bat_priv->_name, net_dev); \ }
-#define BAT_ATTR_SHOW_BOOL(_name) \ -ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ - char *buff) \ +#define BAT_ATTR_SIF_SHOW_BOOL(_name) \ +ssize_t show_##_name(struct kobject *kobj, \ + struct attribute *attr, char *buff) \ { \ struct bat_priv *bat_priv = kobj_to_batpriv(kobj); \ return sprintf(buff, "%s\n", \ @@ -83,16 +83,17 @@ ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ "disabled" : "enabled"); \ } \
-/* Use this, if you are going to turn a [name] in bat_priv on or off */ -#define BAT_ATTR_BOOL(_name, _mode, _post_func) \ - static BAT_ATTR_STORE_BOOL(_name, _post_func) \ - static BAT_ATTR_SHOW_BOOL(_name) \ +/* Use this, if you are going to turn a [name] in the soft-interface + * (bat_priv) on or off */ +#define BAT_ATTR_SIF_BOOL(_name, _mode, _post_func) \ + static BAT_ATTR_SIF_STORE_BOOL(_name, _post_func) \ + static BAT_ATTR_SIF_SHOW_BOOL(_name) \ static BAT_ATTR(_name, _mode, show_##_name, store_##_name)
-#define BAT_ATTR_STORE_UINT(_name, _min, _max, _post_func) \ +#define BAT_ATTR_SIF_STORE_UINT(_name, _min, _max, _post_func) \ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ - char *buff, size_t count) \ + char *buff, size_t count) \ { \ struct net_device *net_dev = kobj_to_netdev(kobj); \ struct bat_priv *bat_priv = netdev_priv(net_dev); \ @@ -100,19 +101,19 @@ ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ attr, &bat_priv->_name, net_dev); \ }
-#define BAT_ATTR_SHOW_UINT(_name) \ -ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \ - char *buff) \ +#define BAT_ATTR_SIF_SHOW_UINT(_name) \ +ssize_t show_##_name(struct kobject *kobj, \ + struct attribute *attr, char *buff) \ { \ struct bat_priv *bat_priv = kobj_to_batpriv(kobj); \ return sprintf(buff, "%i\n", atomic_read(&bat_priv->_name)); \ } \
-/* Use this, if you are going to set [name] in bat_priv to unsigned integer - * values only */ -#define BAT_ATTR_UINT(_name, _mode, _min, _max, _post_func) \ - static BAT_ATTR_STORE_UINT(_name, _min, _max, _post_func) \ - static BAT_ATTR_SHOW_UINT(_name) \ +/* Use this, if you are going to set [name] in the soft-interface + * (bat_priv) to an unsigned integer value */ +#define BAT_ATTR_SIF_UINT(_name, _mode, _min, _max, _post_func) \ + static BAT_ATTR_SIF_STORE_UINT(_name, _min, _max, _post_func) \ + static BAT_ATTR_SIF_SHOW_UINT(_name) \ static BAT_ATTR(_name, _mode, show_##_name, store_##_name)
@@ -384,24 +385,24 @@ static ssize_t store_gw_bwidth(struct kobject *kobj, struct attribute *attr, return gw_bandwidth_set(net_dev, buff, count); }
-BAT_ATTR_BOOL(aggregated_ogms, S_IRUGO | S_IWUSR, NULL); -BAT_ATTR_BOOL(bonding, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_SIF_BOOL(aggregated_ogms, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_SIF_BOOL(bonding, S_IRUGO | S_IWUSR, NULL); #ifdef CONFIG_BATMAN_ADV_BLA -BAT_ATTR_BOOL(bridge_loop_avoidance, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_SIF_BOOL(bridge_loop_avoidance, S_IRUGO | S_IWUSR, NULL); #endif -BAT_ATTR_BOOL(fragmentation, S_IRUGO | S_IWUSR, update_min_mtu); -BAT_ATTR_BOOL(ap_isolation, S_IRUGO | S_IWUSR, NULL); +BAT_ATTR_SIF_BOOL(fragmentation, S_IRUGO | S_IWUSR, update_min_mtu); +BAT_ATTR_SIF_BOOL(ap_isolation, S_IRUGO | S_IWUSR, NULL); static BAT_ATTR(vis_mode, S_IRUGO | S_IWUSR, show_vis_mode, store_vis_mode); static BAT_ATTR(routing_algo, S_IRUGO, show_bat_algo, NULL); static BAT_ATTR(gw_mode, S_IRUGO | S_IWUSR, show_gw_mode, store_gw_mode); -BAT_ATTR_UINT(orig_interval, S_IRUGO | S_IWUSR, 2 * JITTER, INT_MAX, NULL); -BAT_ATTR_UINT(hop_penalty, S_IRUGO | S_IWUSR, 0, TQ_MAX_VALUE, NULL); -BAT_ATTR_UINT(gw_sel_class, S_IRUGO | S_IWUSR, 1, TQ_MAX_VALUE, +BAT_ATTR_SIF_UINT(orig_interval, S_IRUGO | S_IWUSR, 2 * JITTER, INT_MAX, NULL); +BAT_ATTR_SIF_UINT(hop_penalty, S_IRUGO | S_IWUSR, 0, TQ_MAX_VALUE, NULL); +BAT_ATTR_SIF_UINT(gw_sel_class, S_IRUGO | S_IWUSR, 1, TQ_MAX_VALUE, post_gw_deselect); static BAT_ATTR(gw_bandwidth, S_IRUGO | S_IWUSR, show_gw_bwidth, store_gw_bwidth); #ifdef CONFIG_BATMAN_ADV_DEBUG -BAT_ATTR_UINT(log_level, S_IRUGO | S_IWUSR, 0, 31, NULL); +BAT_ATTR_SIF_UINT(log_level, S_IRUGO | S_IWUSR, 0, 31, NULL); #endif
static struct bat_attribute *mesh_attrs[] = {
On Sunday, March 11, 2012 06:17:51 Marek Lindner wrote:
Signed-off-by: Marek Lindner lindner_marek@yahoo.de
bat_sysfs.c | 55 ++++++++++++++++++++++++++++--------------------------- 1 files changed, 28 insertions(+), 27 deletions(-)
Applied in revision 7c73d4c.
Regards, Marek
From: Linus Luessing linus.luessing@web.de
This allows us to easily add a sysfs parameter for an unsigned int later, which is not for a batman mesh interface (e.g. bat0), but for a common interface instead. It allows reading and writing an atomic_t in hard_iface (instead of bat_priv compared to the mesh variant).
Developed by Linus during a 6 months trainee study period in Ascom (Switzerland) AG.
Signed-off-by: Linus Luessing linus.luessing@web.de Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- bat_sysfs.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/bat_sysfs.c b/bat_sysfs.c index 91107fd..35c15e9 100644 --- a/bat_sysfs.c +++ b/bat_sysfs.c @@ -117,6 +117,49 @@ ssize_t show_##_name(struct kobject *kobj, \ static BAT_ATTR(_name, _mode, show_##_name, store_##_name)
+#define BAT_ATTR_HIF_STORE_UINT(_name, _min, _max, _post_func) \ +ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \ + char *buff, size_t count) \ +{ \ + struct net_device *net_dev = kobj_to_netdev(kobj); \ + struct hard_iface *hard_iface = hardif_get_by_netdev(net_dev); \ + ssize_t length; \ + \ + if (!hard_iface) \ + return 0; \ + \ + length = __store_uint_attr(buff, count, _min, _max, _post_func, \ + attr, &hard_iface->_name, net_dev); \ + \ + hardif_free_ref(hard_iface); \ + return length; \ +} + +#define BAT_ATTR_HIF_SHOW_UINT(_name) \ +ssize_t show_##_name(struct kobject *kobj, \ + struct attribute *attr, char *buff) \ +{ \ + struct net_device *net_dev = kobj_to_netdev(kobj); \ + struct hard_iface *hard_iface = hardif_get_by_netdev(net_dev); \ + ssize_t length; \ + \ + if (!hard_iface) \ + return 0; \ + \ + length = sprintf(buff, "%i\n", atomic_read(&hard_iface->_name));\ + \ + hardif_free_ref(hard_iface); \ + return length; \ +} + +/* Use this, if you are going to set [name] in hard_iface to an + * unsigned integer value*/ +#define BAT_ATTR_HIF_UINT(_name, _mode, _min, _max, _post_func) \ + static BAT_ATTR_HIF_STORE_UINT(_name, _min, _max, _post_func) \ + static BAT_ATTR_HIF_SHOW_UINT(_name) \ + static BAT_ATTR(_name, _mode, show_##_name, store_##_name) + + static int store_bool_attr(char *buff, size_t count, struct net_device *net_dev, const char *attr_name, atomic_t *attr)
On Sunday, March 11, 2012 06:17:52 Marek Lindner wrote:
From: Linus Luessing linus.luessing@web.de
This allows us to easily add a sysfs parameter for an unsigned int later, which is not for a batman mesh interface (e.g. bat0), but for a common interface instead. It allows reading and writing an atomic_t in hard_iface (instead of bat_priv compared to the mesh variant).
Developed by Linus during a 6 months trainee study period in Ascom (Switzerland) AG.
Applied in revision 4e18e83.
Thanks, Marek
batman-adv would forward OGMs from non-besthops while replacing the the TQ and TTL values with the values from the best hop. In certain corner cases this leads to a temporary routing loop. This patch changes this behavior: Only packets from best next hops are forwarded - TQ and TTL values won't be replaced anymore. However, the protocol needs to rebroadcast OGMs from single hop neighbors regardless of whether or not they are the best hop. To handle this case a new flag is introduced to alert neighboring nodes about the forwarded OGM that is not from my best next hop. It is to be discarded by all nodes except for the one originating the OGM.
Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- bat_iv_ogm.c | 61 ++++++++++++++++++++++++++++++--------------------------- packet.h | 1 + 2 files changed, 33 insertions(+), 29 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 1e92e38..85617c4 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -507,11 +507,10 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node, const struct ethhdr *ethhdr, struct batman_ogm_packet *batman_ogm_packet, bool is_single_hop_neigh, + bool is_from_best_next_hop, struct hard_iface *if_incoming) { struct bat_priv *bat_priv = netdev_priv(if_incoming->soft_iface); - struct neigh_node *router; - uint8_t in_tq, in_ttl, tq_avg = 0; uint8_t tt_num_changes;
if (batman_ogm_packet->header.ttl <= 1) { @@ -519,41 +518,31 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node, return; }
- router = orig_node_get_router(orig_node); + if (!is_from_best_next_hop) { + /** + * Mark the forwarded packet when it is not coming from our best + * next hop. We still need to forward the packet for our neighbor + * link quality detection to work in case the packet originated + * from a single hop neighbor. Otherwise we can simply drop the + * ogm. + */ + if (is_single_hop_neigh) + batman_ogm_packet->flags |= NOT_BEST_NEXT_HOP; + else + return; + }
- in_tq = batman_ogm_packet->tq; - in_ttl = batman_ogm_packet->header.ttl; tt_num_changes = batman_ogm_packet->tt_num_changes;
batman_ogm_packet->header.ttl--; memcpy(batman_ogm_packet->prev_sender, ethhdr->h_source, ETH_ALEN);
- /* rebroadcast tq of our best ranking neighbor to ensure the rebroadcast - * of our best tq value */ - if (router && router->tq_avg != 0) { - - /* rebroadcast ogm of best ranking neighbor as is */ - if (!compare_eth(router->addr, ethhdr->h_source)) { - batman_ogm_packet->tq = router->tq_avg; - - if (router->last_ttl) - batman_ogm_packet->header.ttl = - router->last_ttl - 1; - } - - tq_avg = router->tq_avg; - } - - if (router) - neigh_node_free_ref(router); - /* apply hop penalty */ batman_ogm_packet->tq = hop_penalty(batman_ogm_packet->tq, bat_priv);
bat_dbg(DBG_BATMAN, bat_priv, - "Forwarding packet: tq_orig: %i, tq_avg: %i, tq_forw: %i, ttl_orig: %i, ttl_forw: %i\n", - in_tq, tq_avg, batman_ogm_packet->tq, in_ttl - 1, - batman_ogm_packet->header.ttl); + "Forwarding packet: tq: %i, ttl: %i\n", + batman_ogm_packet->tq, batman_ogm_packet->header.ttl);
batman_ogm_packet->seqno = htonl(batman_ogm_packet->seqno); batman_ogm_packet->tt_crc = htons(batman_ogm_packet->tt_crc); @@ -949,6 +938,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0; int is_broadcast = 0, is_bidirectional; bool is_single_hop_neigh = false; + bool is_from_best_next_hop = false; int is_duplicate; uint32_t if_incoming_seqno;
@@ -1070,6 +1060,13 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, return; }
+ if (batman_ogm_packet->flags & NOT_BEST_NEXT_HOP) { + bat_dbg(DBG_BATMAN, bat_priv, + "Drop packet: ignoring all packets not forwarded from " + "the best next hop (sender: %pM)\n", ethhdr->h_source); + return; + } + orig_node = get_orig_node(bat_priv, batman_ogm_packet->orig); if (!orig_node) return; @@ -1094,6 +1091,10 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, if (router) router_router = orig_node_get_router(router->orig_node);
+ if ((router && router->tq_avg != 0) && + (compare_eth(router->addr, ethhdr->h_source))) + is_from_best_next_hop = true; + /* avoid temporary routing loops */ if (router && router_router && (compare_eth(router->addr, batman_ogm_packet->prev_sender)) && @@ -1144,7 +1145,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
/* mark direct link on incoming interface */ bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet, - is_single_hop_neigh, if_incoming); + is_single_hop_neigh, is_from_best_next_hop, + if_incoming);
bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: rebroadcast neighbor packet with direct link flag\n"); @@ -1167,7 +1169,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: rebroadcast originator packet\n"); bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet, - is_single_hop_neigh, if_incoming); + is_single_hop_neigh, is_from_best_next_hop, + if_incoming);
out_neigh: if ((orig_neigh_node) && (!is_single_hop_neigh)) diff --git a/packet.h b/packet.h index 02b0c87..3c4c533 100644 --- a/packet.h +++ b/packet.h @@ -47,6 +47,7 @@ enum bat_subtype { #define COMPAT_VERSION 14
enum batman_iv_flags { + NOT_BEST_NEXT_HOP = 1 << 3, PRIMARIES_FIRST_HOP = 1 << 4, VIS_SERVER = 1 << 5, DIRECTLINK = 1 << 6
Hey there,
I've tested this patch on my old ring setup which used to create temporary routing loops, and this patch improved the situation.
Feel free to add:
Tested-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de
On Sun, Mar 11, 2012 at 06:17:53AM +0800, Marek Lindner wrote:
batman-adv would forward OGMs from non-besthops while replacing the the TQ and TTL values with the values from the best hop. In certain corner cases this leads to a temporary routing loop. This patch changes this behavior: Only packets from best next hops are forwarded - TQ and TTL values won't be replaced anymore. However, the protocol needs to rebroadcast OGMs from single hop neighbors regardless of whether or not they are the best hop. To handle this case a new flag is introduced to alert neighboring nodes about the forwarded OGM that is not from my best next hop. It is to be discarded by all nodes except for the one originating the OGM.
Signed-off-by: Marek Lindner lindner_marek@yahoo.de
bat_iv_ogm.c | 61 ++++++++++++++++++++++++++++++--------------------------- packet.h | 1 + 2 files changed, 33 insertions(+), 29 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 1e92e38..85617c4 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -507,11 +507,10 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node, const struct ethhdr *ethhdr, struct batman_ogm_packet *batman_ogm_packet, bool is_single_hop_neigh,
bool is_from_best_next_hop, struct hard_iface *if_incoming){ struct bat_priv *bat_priv = netdev_priv(if_incoming->soft_iface);
struct neigh_node *router;
uint8_t in_tq, in_ttl, tq_avg = 0; uint8_t tt_num_changes;
if (batman_ogm_packet->header.ttl <= 1) {
@@ -519,41 +518,31 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node, return; }
- router = orig_node_get_router(orig_node);
- if (!is_from_best_next_hop) {
/*** Mark the forwarded packet when it is not coming from our best* next hop. We still need to forward the packet for our neighbor* link quality detection to work in case the packet originated* from a single hop neighbor. Otherwise we can simply drop the* ogm.*/if (is_single_hop_neigh)batman_ogm_packet->flags |= NOT_BEST_NEXT_HOP;elsereturn;- }
in_tq = batman_ogm_packet->tq;
in_ttl = batman_ogm_packet->header.ttl; tt_num_changes = batman_ogm_packet->tt_num_changes;
batman_ogm_packet->header.ttl--; memcpy(batman_ogm_packet->prev_sender, ethhdr->h_source, ETH_ALEN);
/* rebroadcast tq of our best ranking neighbor to ensure the rebroadcast
* of our best tq value */if (router && router->tq_avg != 0) {
/* rebroadcast ogm of best ranking neighbor as is */if (!compare_eth(router->addr, ethhdr->h_source)) {batman_ogm_packet->tq = router->tq_avg;if (router->last_ttl)batman_ogm_packet->header.ttl =router->last_ttl - 1;}tq_avg = router->tq_avg;}
if (router)
neigh_node_free_ref(router);/* apply hop penalty */ batman_ogm_packet->tq = hop_penalty(batman_ogm_packet->tq, bat_priv);
bat_dbg(DBG_BATMAN, bat_priv,
"Forwarding packet: tq_orig: %i, tq_avg: %i, tq_forw: %i, ttl_orig: %i, ttl_forw: %i\n",in_tq, tq_avg, batman_ogm_packet->tq, in_ttl - 1,batman_ogm_packet->header.ttl);
"Forwarding packet: tq: %i, ttl: %i\n",batman_ogm_packet->tq, batman_ogm_packet->header.ttl);batman_ogm_packet->seqno = htonl(batman_ogm_packet->seqno); batman_ogm_packet->tt_crc = htons(batman_ogm_packet->tt_crc);
@@ -949,6 +938,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0; int is_broadcast = 0, is_bidirectional; bool is_single_hop_neigh = false;
- bool is_from_best_next_hop = false; int is_duplicate; uint32_t if_incoming_seqno;
@@ -1070,6 +1060,13 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, return; }
- if (batman_ogm_packet->flags & NOT_BEST_NEXT_HOP) {
bat_dbg(DBG_BATMAN, bat_priv,"Drop packet: ignoring all packets not forwarded from ""the best next hop (sender: %pM)\n", ethhdr->h_source);return;- }
- orig_node = get_orig_node(bat_priv, batman_ogm_packet->orig); if (!orig_node) return;
@@ -1094,6 +1091,10 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, if (router) router_router = orig_node_get_router(router->orig_node);
- if ((router && router->tq_avg != 0) &&
(compare_eth(router->addr, ethhdr->h_source)))is_from_best_next_hop = true;- /* avoid temporary routing loops */ if (router && router_router && (compare_eth(router->addr, batman_ogm_packet->prev_sender)) &&
@@ -1144,7 +1145,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
/* mark direct link on incoming interface */ bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet,
is_single_hop_neigh, if_incoming);
is_single_hop_neigh, is_from_best_next_hop,if_incoming);bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: rebroadcast neighbor packet with direct link flag\n");
@@ -1167,7 +1169,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: rebroadcast originator packet\n"); bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet,
is_single_hop_neigh, if_incoming);
is_single_hop_neigh, is_from_best_next_hop,if_incoming);out_neigh: if ((orig_neigh_node) && (!is_single_hop_neigh)) diff --git a/packet.h b/packet.h index 02b0c87..3c4c533 100644 --- a/packet.h +++ b/packet.h @@ -47,6 +47,7 @@ enum bat_subtype { #define COMPAT_VERSION 14
enum batman_iv_flags {
- NOT_BEST_NEXT_HOP = 1 << 3, PRIMARIES_FIRST_HOP = 1 << 4, VIS_SERVER = 1 << 5, DIRECTLINK = 1 << 6
-- 1.7.9.1
Hi, I agree with this patch, actually it should solve the loop problem as it is very similar to the proposed modification in my thesis.
In this way I think it is not necessary a global window to average TQ values but only the latest value can be sufficient. Doing this can be re-implemented a mechanism to improve OGM reliability sending old TQ ant TTL (and also sequence number) of the current best router if for example 2 OGM are missing.
Regards.
2012/3/11 Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de:
Hey there,
I've tested this patch on my old ring setup which used to create temporary routing loops, and this patch improved the situation.
Feel free to add:
Tested-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de
On Sun, Mar 11, 2012 at 06:17:53AM +0800, Marek Lindner wrote:
batman-adv would forward OGMs from non-besthops while replacing the the TQ and TTL values with the values from the best hop. In certain corner cases this leads to a temporary routing loop. This patch changes this behavior: Only packets from best next hops are forwarded - TQ and TTL values won't be replaced anymore. However, the protocol needs to rebroadcast OGMs from single hop neighbors regardless of whether or not they are the best hop. To handle this case a new flag is introduced to alert neighboring nodes about the forwarded OGM that is not from my best next hop. It is to be discarded by all nodes except for the one originating the OGM.
Signed-off-by: Marek Lindner lindner_marek@yahoo.de
bat_iv_ogm.c | 61 ++++++++++++++++++++++++++++++--------------------------- packet.h | 1 + 2 files changed, 33 insertions(+), 29 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 1e92e38..85617c4 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -507,11 +507,10 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node, const struct ethhdr *ethhdr, struct batman_ogm_packet *batman_ogm_packet, bool is_single_hop_neigh,
- bool is_from_best_next_hop,
struct hard_iface *if_incoming) { struct bat_priv *bat_priv = netdev_priv(if_incoming->soft_iface);
- struct neigh_node *router;
- uint8_t in_tq, in_ttl, tq_avg = 0;
uint8_t tt_num_changes;
if (batman_ogm_packet->header.ttl <= 1) { @@ -519,41 +518,31 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node, return; }
- router = orig_node_get_router(orig_node);
- if (!is_from_best_next_hop) {
- /**
- * Mark the forwarded packet when it is not coming from our best
- * next hop. We still need to forward the packet for our neighbor
- * link quality detection to work in case the packet originated
- * from a single hop neighbor. Otherwise we can simply drop the
- * ogm.
- */
- if (is_single_hop_neigh)
- batman_ogm_packet->flags |= NOT_BEST_NEXT_HOP;
- else
- return;
- }
- in_tq = batman_ogm_packet->tq;
- in_ttl = batman_ogm_packet->header.ttl;
tt_num_changes = batman_ogm_packet->tt_num_changes;
batman_ogm_packet->header.ttl--; memcpy(batman_ogm_packet->prev_sender, ethhdr->h_source, ETH_ALEN);
- /* rebroadcast tq of our best ranking neighbor to ensure the rebroadcast
- * of our best tq value */
- if (router && router->tq_avg != 0) {
- /* rebroadcast ogm of best ranking neighbor as is */
- if (!compare_eth(router->addr, ethhdr->h_source)) {
- batman_ogm_packet->tq = router->tq_avg;
- if (router->last_ttl)
- batman_ogm_packet->header.ttl =
- router->last_ttl - 1;
- }
- tq_avg = router->tq_avg;
- }
- if (router)
- neigh_node_free_ref(router);
/* apply hop penalty */ batman_ogm_packet->tq = hop_penalty(batman_ogm_packet->tq, bat_priv);
bat_dbg(DBG_BATMAN, bat_priv,
- "Forwarding packet: tq_orig: %i, tq_avg: %i, tq_forw: %i, ttl_orig: %i, ttl_forw: %i\n",
- in_tq, tq_avg, batman_ogm_packet->tq, in_ttl - 1,
- batman_ogm_packet->header.ttl);
- "Forwarding packet: tq: %i, ttl: %i\n",
- batman_ogm_packet->tq, batman_ogm_packet->header.ttl);
batman_ogm_packet->seqno = htonl(batman_ogm_packet->seqno); batman_ogm_packet->tt_crc = htons(batman_ogm_packet->tt_crc); @@ -949,6 +938,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0; int is_broadcast = 0, is_bidirectional; bool is_single_hop_neigh = false;
- bool is_from_best_next_hop = false;
int is_duplicate; uint32_t if_incoming_seqno;
@@ -1070,6 +1060,13 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, return; }
- if (batman_ogm_packet->flags & NOT_BEST_NEXT_HOP) {
- bat_dbg(DBG_BATMAN, bat_priv,
- "Drop packet: ignoring all packets not forwarded from "
- "the best next hop (sender: %pM)\n", ethhdr->h_source);
- return;
- }
orig_node = get_orig_node(bat_priv, batman_ogm_packet->orig); if (!orig_node) return; @@ -1094,6 +1091,10 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, if (router) router_router = orig_node_get_router(router->orig_node);
- if ((router && router->tq_avg != 0) &&
- (compare_eth(router->addr, ethhdr->h_source)))
- is_from_best_next_hop = true;
/* avoid temporary routing loops */ if (router && router_router && (compare_eth(router->addr, batman_ogm_packet->prev_sender)) && @@ -1144,7 +1145,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
/* mark direct link on incoming interface */ bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet,
- is_single_hop_neigh, if_incoming);
- is_single_hop_neigh, is_from_best_next_hop,
- if_incoming);
bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: rebroadcast neighbor packet with direct link flag\n"); @@ -1167,7 +1169,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: rebroadcast originator packet\n"); bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet,
- is_single_hop_neigh, if_incoming);
- is_single_hop_neigh, is_from_best_next_hop,
- if_incoming);
out_neigh: if ((orig_neigh_node) && (!is_single_hop_neigh)) diff --git a/packet.h b/packet.h index 02b0c87..3c4c533 100644 --- a/packet.h +++ b/packet.h @@ -47,6 +47,7 @@ enum bat_subtype { #define COMPAT_VERSION 14
enum batman_iv_flags {
- NOT_BEST_NEXT_HOP = 1 << 3,
PRIMARIES_FIRST_HOP = 1 << 4, VIS_SERVER = 1 << 5, DIRECTLINK = 1 << 6 -- 1.7.9.1
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux)
iEYEARECAAYFAk9dHi8ACgkQrzg/fFk7axbJ+QCg5TaIOetOvSoTjr5vHCHr4k7M /6YAoKC76tkASLbl9TuYFKqLTIHXRK2s =JNhg -----END PGP SIGNATURE-----
Hi,
I agree with this patch, actually it should solve the loop problem as it is very similar to the proposed modification in my thesis.
thanks for the feedback!
In this way I think it is not necessary a global window to average TQ values but only the latest value can be sufficient. Doing this can be re-implemented a mechanism to improve OGM reliability sending old TQ ant TTL (and also sequence number) of the current best router if for example 2 OGM are missing.
I don't quite understand what you are proposing. Are you saying removing the averaging mechanism makes this patch obsolete and allows replacing the TTL/TQ values as the code does now (without the patch) ?
Cheers, Marek
2012/3/12 Marek Lindner lindner_marek@yahoo.de:
Hi,
I agree with this patch, actually it should solve the loop problem as it is very similar to the proposed modification in my thesis.
thanks for the feedback!
In this way I think it is not necessary a global window to average TQ values but only the latest value can be sufficient. Doing this can be re-implemented a mechanism to improve OGM reliability sending old TQ ant TTL (and also sequence number) of the current best router if for example 2 OGM are missing.
I don't quite understand what you are proposing. Are you saying removing the averaging mechanism makes this patch obsolete and allows replacing the TTL/TQ values as the code does now (without the patch) ?
No no this patch is good, the removing of TQ average can be a following step as now average is not forwarded anymore, but only used as a comparative value between path in another point of the source code.
Also the mechanism of forwarding OGM even if it does not come from the best router can be added later, on top of this patch.
Cheers, Marek
On Monday, March 12, 2012 15:49:26 Daniele Furlan wrote:
I don't quite understand what you are proposing. Are you saying removing the averaging mechanism makes this patch obsolete and allows replacing the TTL/TQ values as the code does now (without the patch) ?
No no this patch is good, the removing of TQ average can be a following step as now average is not forwarded anymore, but only used as a comparative value between path in another point of the source code.
This follow-up step needs a volunteer. It is a bigger operation to remove the averaging and ensure everything still works. :-)
Do you mind explaining in simple words how the averaging could cause routing havoc together with the strict OGM forwarding patch ? I did not understand. Or are you in favor of removing the averaging for style / performance reasons?
Cheers, Marek
2012/3/12 Marek Lindner lindner_marek@yahoo.de:
On Monday, March 12, 2012 15:49:26 Daniele Furlan wrote:
I don't quite understand what you are proposing. Are you saying removing the averaging mechanism makes this patch obsolete and allows replacing the TTL/TQ values as the code does now (without the patch) ?
No no this patch is good, the removing of TQ average can be a following step as now average is not forwarded anymore, but only used as a comparative value between path in another point of the source code.
This follow-up step needs a volunteer. It is a bigger operation to remove the averaging and ensure everything still works. :-)
I can try to send some patch next week to start this operation.. :)
Do you mind explaining in simple words how the averaging could cause routing havoc together with the strict OGM forwarding patch ? I did not understand. Or are you in favor of removing the averaging for style / performance reasons?
Actually the average as it is calculated in the current version (without "considering" lost OGMs) is not monotonically decreasing. This cause routing loops when it is forwarded so with this patch the problem does not happen. Remain the fact of non monotonicity and this could cause loop anyway in some corner situation.
Surely if we guarantee monotonicity we obtain LOOP-FRENESS IN ANY CASE and this is an important feature I think!!
Also maintaing only the latest TQ guarantee to have a more updated information about the path. Furthermore an average is already done at every link TQ calculation so it is superfluous to have another window. This reduce also memory and computational requirements.
Cheers, Marek
On Monday, March 12, 2012 16:23:30 Daniele Furlan wrote:
2012/3/12 Marek Lindner lindner_marek@yahoo.de:
On Monday, March 12, 2012 15:49:26 Daniele Furlan wrote:
I don't quite understand what you are proposing. Are you saying removing the averaging mechanism makes this patch obsolete and allows replacing the TTL/TQ values as the code does now (without the patch) ?
No no this patch is good, the removing of TQ average can be a following step as now average is not forwarded anymore, but only used as a comparative value between path in another point of the source code.
This follow-up step needs a volunteer. It is a bigger operation to remove the averaging and ensure everything still works. :-)
I can try to send some patch next week to start this operation.. :)
Wow - cool!
Do you mind explaining in simple words how the averaging could cause routing havoc together with the strict OGM forwarding patch ? I did not understand. Or are you in favor of removing the averaging for style / performance reasons?
Actually the average as it is calculated in the current version (without "considering" lost OGMs) is not monotonically decreasing. This cause routing loops when it is forwarded so with this patch the problem does not happen.
That is what I thought. This patch also enforces monotonicity, thereby eliminating the loop "potential" created by averaging the TQ.
Also maintaing only the latest TQ guarantee to have a more updated information about the path. Furthermore an average is already done at every link TQ calculation so it is superfluous to have another window. This reduce also memory and computational requirements.
Full agreement on this part. At the moment, I was more worried about temporary loops which seems to be addressed with this patch.
Thanks for all your feedback!
Cheers, Marek
On Sunday, March 11, 2012 06:17:53 Marek Lindner wrote:
batman-adv would forward OGMs from non-besthops while replacing the the TQ and TTL values with the values from the best hop. In certain corner cases this leads to a temporary routing loop. This patch changes this behavior: Only packets from best next hops are forwarded - TQ and TTL values won't be replaced anymore. However, the protocol needs to rebroadcast OGMs from single hop neighbors regardless of whether or not they are the best hop. To handle this case a new flag is introduced to alert neighboring nodes about the forwarded OGM that is not from my best next hop. It is to be discarded by all nodes except for the one originating the OGM.
Applied in revision f76d019.
Regards, Marek
On Sunday, March 11, 2012 06:17:49 Marek Lindner wrote:
Reported-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Marek Lindner lindner_marek@yahoo.de
bat_iv_ogm.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
Applied in revision c9619b5.
Regards, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Daniele Furlan -
Marek Lindner -
Simon Wunderlich