From b5e76398de1d4468b4061d9ef57dd3154b2f745e Mon Sep 17 00:00:00 2001 From: Ondrej Zajicek Date: Wed, 19 Aug 2015 11:16:23 +0200 Subject: [PATCH 1/2] OSPF: Fixes some issues with link detection Thanks to Bernardo Figueiredo and Israel G. Lugo for the bugreport. --- proto/ospf/iface.c | 7 +++++-- proto/ospf/packet.c | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/proto/ospf/iface.c b/proto/ospf/iface.c index 9b0f7797..77ce839a 100644 --- a/proto/ospf/iface.c +++ b/proto/ospf/iface.c @@ -493,8 +493,11 @@ ospf_iface_add(struct object_lock *lock) ifa->flood_queue = mb_allocz(ifa->pool, ifa->flood_queue_size * sizeof(void *)); } - /* Do iface UP, unless there is no link and we use link detection */ - ospf_iface_sm(ifa, (ifa->check_link && !(ifa->iface->flags & IF_LINK_UP)) ? ISM_LOOP : ISM_UP); + /* Do iface UP, unless there is no link (then wait in LOOP state) */ + if (!ifa->check_link || (ifa->iface->flags & IF_LINK_UP)) + ospf_iface_sm(ifa, ISM_UP); + else + ospf_iface_chstate(ifa, OSPF_IS_LOOP); } static inline void diff --git a/proto/ospf/packet.c b/proto/ospf/packet.c index fb63e61c..6b8fd7b5 100644 --- a/proto/ospf/packet.c +++ b/proto/ospf/packet.c @@ -231,6 +231,10 @@ ospf_rx_hook(sock *sk, int len) const char *err_dsc = NULL; uint err_val = 0; + /* Should not happen */ + if (ifa->state <= OSPF_IS_LOOP) + return 1; + int src_local, dst_local, dst_mcast; src_local = ipa_in_net(sk->faddr, ifa->addr->prefix, ifa->addr->pxlen); dst_local = ipa_equal(sk->laddr, ifa->addr->ip); From acb04cfdc550697a7171a86ca559fd8c52841acb Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Sat, 17 Oct 2015 14:36:53 +0200 Subject: [PATCH 2/2] Minor changes --- nest/route.h | 34 ++++++++++++++++++++-------------- sysdep/linux/netlink.c | 23 ++++++++++++++++------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/nest/route.h b/nest/route.h index 6067526d..d0f8c688 100644 --- a/nest/route.h +++ b/nest/route.h @@ -174,7 +174,7 @@ struct hostentry { ip_addr addr; /* IP address of host, part of key */ ip_addr link; /* (link-local) IP address of host, used as gw if host is directly attached */ - struct rtable *tab; /* Dependent table, part of key*/ + struct rtable *tab; /* Dependent table, part of key */ struct hostentry *next; /* Next in hash chain */ unsigned hash_key; /* Hash key */ unsigned uc; /* Use count */ @@ -507,19 +507,25 @@ void rta_show(struct cli *, rta *, ea_list *); void rta_set_recursive_next_hop(rtable *dep, rta *a, rtable *tab, ip_addr *gw, ip_addr *ll); /* - * rta_set_recursive_next_hop() acquires hostentry from hostcache and - * fills rta->hostentry field. New hostentry has zero use - * count. Cached rta locks its hostentry (increases its use count), - * uncached rta does not lock it. Hostentry with zero use count is - * removed asynchronously during host cache update, therefore it is - * safe to hold such hostentry temorarily. Hostentry holds a lock for - * a 'source' rta, mainly to share multipath nexthops. There is no - * need to hold a lock for hostentry->dep table, because that table - * contains routes responsible for that hostentry, and therefore is - * non-empty if given hostentry has non-zero use count. The protocol - * responsible for routes with recursive next hops should also hold a - * lock for a table governing that routes (argument tab to - * rta_set_recursive_next_hop()). + * rta_set_recursive_next_hop() acquires hostentry from hostcache and fills + * rta->hostentry field. New hostentry has zero use count. Cached rta locks its + * hostentry (increases its use count), uncached rta does not lock it. Hostentry + * with zero use count is removed asynchronously during host cache update, + * therefore it is safe to hold such hostentry temorarily. Hostentry holds a + * lock for a 'source' rta, mainly to share multipath nexthops. + * + * There is no need to hold a lock for hostentry->dep table, because that table + * contains routes responsible for that hostentry, and therefore is non-empty if + * given hostentry has non-zero use count. If the hostentry has zero use count, + * the entry is removed before dep is referenced. + * + * The protocol responsible for routes with recursive next hops should hold a + * lock for a 'source' table governing that routes (argument tab to + * rta_set_recursive_next_hop()), because its routes reference hostentries + * (through rta) related to the governing table. When all such routes are + * removed, rtas are immediately removed achieving zero uc. Then the 'source' + * table lock could be immediately released, although hostentries may still + * exist - they will be freed together with the 'source' table. */ static inline void rt_lock_hostentry(struct hostentry *he) { if (he) he->uc++; } diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c index 9c9449e2..674d338b 100644 --- a/sysdep/linux/netlink.c +++ b/sysdep/linux/netlink.c @@ -239,6 +239,16 @@ nl_parse_attrs(struct rtattr *a, struct rtattr **k, int ksize) return 1; } +static inline ip4_addr rta_get_u32(struct rtattr *a) +{ return *(u32 *) RTA_DATA(a); } + +static inline ip4_addr rta_get_ip4(struct rtattr *a) +{ return ip4_ntoh(*(ip4_addr *) RTA_DATA(a)); } + +static inline ip6_addr rta_get_ip6(struct rtattr *a) +{ return ip6_ntoh(*(ip6_addr *) RTA_DATA(a)); } + + struct rtattr * nl_add_attr(struct nlmsghdr *h, uint bufsize, uint code, const void *data, uint dlen) { @@ -420,7 +430,7 @@ nl_parse_metrics(struct rtattr *hdr, u32 *metrics, int max) return -1; metrics[0] |= 1 << a->rta_type; - metrics[a->rta_type] = *(u32 *)RTA_DATA(a); + metrics[a->rta_type] = rta_get_u32(a); } if (len > 0) @@ -456,7 +466,7 @@ nl_parse_link(struct nlmsghdr *h, int scan) return; } name = RTA_DATA(a[IFLA_IFNAME]); - memcpy(&mtu, RTA_DATA(a[IFLA_MTU]), sizeof(u32)); + mtu = rta_get_u32(a[IFLA_MTU]); ifi = if_find_by_index(i->ifi_index); if (!new) @@ -831,7 +841,7 @@ nl_parse_route(struct nlmsghdr *h, int scan) } if (a[RTA_OIF]) - memcpy(&oif, RTA_DATA(a[RTA_OIF]), sizeof(oif)); + oif = rta_get_u32(a[RTA_OIF]); p = nl_table_map[i->rtm_table]; /* Do we know this table? */ DBG("KRT: Got %I/%d, type=%d, oif=%d, table=%d, prid=%d, proto=%s\n", dst, i->rtm_dst_len, i->rtm_type, oif, i->rtm_table, i->rtm_protocol, p ? p->p.name : "(none)"); @@ -965,11 +975,10 @@ nl_parse_route(struct nlmsghdr *h, int scan) e->u.krt.src = src; e->u.krt.proto = i->rtm_protocol; e->u.krt.type = i->rtm_type; + e->u.krt.metric = 0; if (a[RTA_PRIORITY]) - memcpy(&e->u.krt.metric, RTA_DATA(a[RTA_PRIORITY]), sizeof(e->u.krt.metric)); - else - e->u.krt.metric = 0; + e->u.krt.metric = rta_get_u32(a[RTA_PRIORITY]); if (a[RTA_PREFSRC]) { @@ -1000,7 +1009,7 @@ nl_parse_route(struct nlmsghdr *h, int scan) ea->attrs[0].id = EA_KRT_REALM; ea->attrs[0].flags = 0; ea->attrs[0].type = EAF_TYPE_INT; - memcpy(&ea->attrs[0].u.data, RTA_DATA(a[RTA_FLOW]), 4); + ea->attrs[0].u.data = rta_get_u32(a[RTA_FLOW]); } if (a[RTA_METRICS])