[Olsr-dev] [PATCH][BUILD] check_neighbor_link is wrong
Henning Rogge
(spam-protected)
Tue Dec 11 11:02:43 CET 2012
Comments inline.
On 12/10/2012 08:34 PM, Giovanni Di Stasi wrote:
>> I see only two cases where this "mix up links between nodes" will happen:
>> a) a node use the same IPs for multiple interfaces
>> b) a node can see multiple interfaces of a different node
>>
>> The patch solves parts of case b) as long as no VLANs are involved. can
>> we use the interface index instead of the local IP?
>
> The intent of the patch was to fix case b) that may easily arise if you change channels on the interfaces.
> I didn't consider the VLAN case, but from our point of view using the interface index is perfectly fine.
> diff --git a/src/hna_set.c b/src/hna_set.c
> index df8a82d..711e85a 100644
> --- a/src/hna_set.c
> +++ b/src/hna_set.c
> @@ -362,7 +362,7 @@ olsr_print_hna_set(void)
> */
>
> bool
> -olsr_input_hna(union olsr_message *m, struct interface *in_if __attribute__ ((unused)), union olsr_ip_addr *from_addr)
> +olsr_input_hna(union olsr_message *m, struct interface *in_if, union olsr_ip_addr *from_addr)
> {
>
> uint8_t olsr_msgtype;
> @@ -425,7 +425,7 @@ olsr_input_hna(union olsr_message *m, struct interface *in_if __attribute__ ((un
> * is not in the symmetric 1-hop neighborhood of this node, the
> * message MUST be discarded.
> */
> - if (check_neighbor_link(from_addr) != SYM_LINK) {
> + if (check_neighbor_link(&in_if->ip_addr, from_addr) != SYM_LINK) {
> OLSR_PRINTF(2, "Received HNA from NON SYM neighbor %s\n", olsr_ip_to_string(&buf, from_addr));
> return false;
> }
> diff --git a/src/link_set.c b/src/link_set.c
> index aa6a9ea..cba1855 100644
> --- a/src/link_set.c
> +++ b/src/link_set.c
> @@ -184,7 +184,7 @@ get_neighbor_status(const union olsr_ip_addr *address)
> if (!(main_addr = mid_lookup_main_addr(address)))
> main_addr = address;
>
> - /* Loop trough local interfaces to check all possebilities */
> + /* Loop trough local interfaces to check all possbilities */
> for (ifs = ifnet; ifs != NULL; ifs = ifs->int_next) {
> struct mid_address *aliases;
> struct link_entry *lnk = lookup_link_entry(main_addr, NULL, ifs);
> @@ -216,10 +216,14 @@ get_best_link_to_neighbor(const union olsr_ip_addr *remote)
> const union olsr_ip_addr *main_addr;
> struct link_entry *walker, *good_link, *backup_link;
> struct interface *tmp_if;
> - int curr_metric = MAX_IF_METRIC;
> - olsr_linkcost curr_lcost = LINK_COST_BROKEN;
> + int curr_metric_good = MAX_IF_METRIC;
> + int curr_metric_backup = MAX_IF_METRIC;
> + olsr_linkcost curr_lcost_good = LINK_COST_BROKEN;
> + olsr_linkcost curr_lcost_backup = LINK_COST_BROKEN;
> +
> olsr_linkcost tmp_lc;
>
> +
> /* main address lookup */
> main_addr = mid_lookup_main_addr(remote);
>
> @@ -234,13 +238,12 @@ get_best_link_to_neighbor(const union olsr_ip_addr *remote)
>
> /* loop through all links that we have */
> OLSR_FOR_ALL_LINK_ENTRIES(walker) {
> -
> +
> /* if this is not a link to the neighour in question, skip */
> if (!ipequal(&walker->neighbor->neighbor_main_addr, main_addr))
> continue;
> -
> +
> if (olsr_cnf->lq_level == 0) {
> -
> /*
> * handle the non-LQ, RFC-compliant case
> */
> @@ -257,20 +260,22 @@ get_best_link_to_neighbor(const union olsr_ip_addr *remote)
>
> /*
> * is this interface better than anything we had before ?
> - * use the requested remote interface address as a tie-breaker
> + * use the link interface index as a tie-breaker
> */
> - if ((tmp_if->int_metric < curr_metric) || ((tmp_if->int_metric == curr_metric) && ipequal(&walker->local_iface_addr, remote))) {
> -
> - /* memorize the interface's metric */
> - curr_metric = tmp_if->int_metric;
> -
> - /* prefer symmetric links over asymmetric links */
> - if (lookup_link_status(walker) == SYM_LINK) {
> - good_link = walker;
> - } else {
> - backup_link = walker;
> - }
> + if (lookup_link_status(walker) == SYM_LINK) {
> + if (tmp_if->int_metric < curr_metric_good ||
> + (tmp_if->int_metric == curr_metric_good && good_link && tmp_if->if_index < good_link->inter->if_index)){
> + good_link = walker;
> + curr_metric_good = tmp_if->int_metric;
> + }
> + } else {
> + if (tmp_if->int_metric < curr_metric_backup ||
> + (tmp_if->int_metric == curr_metric_backup && backup_link && tmp_if->if_index < backup_link->inter->if_index)){
> + backup_link = walker;
> + curr_metric_backup = tmp_if->int_metric;
> + }
> }
> +
> } else {
>
> /*
> @@ -280,22 +285,31 @@ get_best_link_to_neighbor(const union olsr_ip_addr *remote)
> /* get the link cost */
> tmp_lc = walker->linkcost;
>
> + /* find the interface for the link */
> + tmp_if = walker->if_name ? if_ifwithname(walker->if_name) : if_ifwithaddr(&walker->local_iface_addr);
> +
> + if (!tmp_if) {
> + continue;
> + }
> +
> /*
> * is this link better than anything we had before ?
> - * use the requested remote interface address as a tie-breaker.
> + * use the link interface index as a tie-breaker
> */
> - if ((tmp_lc < curr_lcost) || ((tmp_lc == curr_lcost) && ipequal(&walker->local_iface_addr, remote))) {
> -
> - /* memorize the link quality */
> - curr_lcost = tmp_lc;
> -
> - /* prefer symmetric links over asymmetric links */
> - if (lookup_link_status(walker) == SYM_LINK) {
> - good_link = walker;
> - } else {
> - backup_link = walker;
> - }
> + if (lookup_link_status(walker) == SYM_LINK) {
> + if (tmp_lc < curr_lcost_good ||
> + (tmp_lc == curr_lcost_good && good_link && tmp_if->if_index < good_link->inter->if_index)){
> + good_link = walker;
> + curr_lcost_good=tmp_lc;
> + }
> + } else {
> + if (tmp_lc < curr_lcost_backup ||
> + (tmp_lc == curr_lcost_backup && backup_link && tmp_if->if_index < backup_link->inter->if_index)){
> + backup_link = walker;
> + curr_lcost_backup=tmp_lc;
> + }
> }
> +
> }
> }
I am a little bit scared to look up who use this function if it can
return an asymmetric link. Asymmetric links are useless for outbound
routing.
> OLSR_FOR_ALL_LINK_ENTRIES_END(walker);
> @@ -327,7 +341,7 @@ set_loss_link_multiplier(struct link_entry *entry)
> }
> }
> assert(cfg_inter);
> -
> +
> /* create a null address for comparison */
> memset(&null_addr, 0, sizeof(union olsr_ip_addr));
>
> @@ -627,12 +641,12 @@ add_link_entry(const union olsr_ip_addr *local, const union olsr_ip_addr *remote
> * @return 1 of the link is symmertic 0 if not
> */
> int
> -check_neighbor_link(const union olsr_ip_addr *int_addr)
> +check_neighbor_link(const union olsr_ip_addr * local, const union olsr_ip_addr *remote)
> {
> struct link_entry *link;
>
> OLSR_FOR_ALL_LINK_ENTRIES(link) {
> - if (ipequal(int_addr, &link->neighbor_iface_addr)) {
> + if (ipequal(remote, &link->neighbor_iface_addr) && ipequal(local, &link->local_iface_addr)) {
I was talking about using the interface index (instead of the local IP)
here to detect the "right" link.
> return lookup_link_status(link);
> }
> }
> @@ -720,7 +734,7 @@ update_link_entry(const union olsr_ip_addr *local, const union olsr_ip_addr *rem
> }
>
> /* L_time = max(L_time, L_ASYM_time) */
> - if (entry->link_timer == NULL || (entry->link_timer->timer_clock < entry->ASYM_time)) {
> + if (entry->link_timer && (entry->link_timer->timer_clock < entry->ASYM_time)) {
> olsr_set_link_timer(entry, TIME_DUE(entry->ASYM_time));
> }
>
> diff --git a/src/link_set.h b/src/link_set.h
> index 9ae27f5..f7645e2 100644
> --- a/src/link_set.h
> +++ b/src/link_set.h
> @@ -133,7 +133,7 @@ struct link_entry *lookup_link_entry(const union olsr_ip_addr *, const union ols
> struct link_entry *update_link_entry(const union olsr_ip_addr *, const union olsr_ip_addr *, const struct hello_message *,
> const struct interface *);
>
> -int check_neighbor_link(const union olsr_ip_addr *);
> +int check_neighbor_link(const union olsr_ip_addr *, const union olsr_ip_addr *);
> int replace_neighbor_link_set(const struct neighbor_entry *, struct neighbor_entry *);
> int lookup_link_status(const struct link_entry *);
> void olsr_update_packet_loss_hello_int(struct link_entry *, olsr_reltime);
> diff --git a/src/mid_set.c b/src/mid_set.c
> index c25e0bf..13d8b18 100644
> --- a/src/mid_set.c
> +++ b/src/mid_set.c
> @@ -561,7 +561,7 @@ olsr_print_mid_set(void)
> */
>
> bool
> -olsr_input_mid(union olsr_message *m, struct interface *in_if __attribute__ ((unused)), union olsr_ip_addr *from_addr)
> +olsr_input_mid(union olsr_message *m, struct interface *in_if, union olsr_ip_addr *from_addr)
> {
> struct ipaddr_str buf;
> struct mid_alias *tmp_adr;
> @@ -584,7 +584,7 @@ olsr_input_mid(union olsr_message *m, struct interface *in_if __attribute__ ((un
> * message MUST be discarded.
> */
>
> - if (check_neighbor_link(from_addr) != SYM_LINK) {
> + if (check_neighbor_link(&in_if->ip_addr, from_addr) != SYM_LINK) {
> OLSR_PRINTF(2, "Received MID from NON SYM neighbor %s\n", olsr_ip_to_string(&buf, from_addr));
> olsr_free_mid_packet(&message);
> return false;
> diff --git a/src/tc_set.c b/src/tc_set.c
> index e1480d6..25e98d8 100644
> --- a/src/tc_set.c
> +++ b/src/tc_set.c
> @@ -768,7 +768,7 @@ olsr_calculate_tc_border(uint8_t lower_border, union olsr_ip_addr *lower_border_
> * hence the spot we are looking at.
> */
> bool
> -olsr_input_tc(union olsr_message * msg, struct interface * input_if __attribute__ ((unused)), union olsr_ip_addr * from_addr)
> +olsr_input_tc(union olsr_message * msg, struct interface * input_if, union olsr_ip_addr * from_addr)
> {
> struct ipaddr_str buf;
> uint16_t size, msg_seq, ansn;
> @@ -798,7 +798,7 @@ olsr_input_tc(union olsr_message * msg, struct interface * input_if __attribute_
> * is not in the symmetric 1-hop neighborhood of this node, the
> * message MUST be discarded.
> */
> - if (check_neighbor_link(from_addr) != SYM_LINK) {
> + if (check_neighbor_link(&input_if->ip_addr, from_addr) != SYM_LINK) {
> OLSR_PRINTF(2, "Received TC from NON SYM neighbor %s\n", olsr_ip_to_string(&buf, from_addr));
> return false;
> }
--
Diplom-Informatiker Henning Rogge , Fraunhofer-Institut für
Kommunikation, Informationsverarbeitung und Ergonomie FKIE
Kommunikationssysteme (KOM)
Fraunhofer Straße 20, 53343 Wachtberg, Germany
Telefon +49 228 9435-961, Fax +49 228 9435 685
mailto:(spam-protected) http://www.fkie.fraunhofer.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 6169 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.olsr.org/pipermail/olsr-dev/attachments/20121211/08b7a207/attachment.bin>
More information about the Olsr-dev
mailing list