Acked-by: Donald Sharp <[email protected]> On Wed, Dec 23, 2015 at 9:47 AM, Timo Teräs <[email protected]> wrote:
> It simplifies things as we can do atomic replace of route prefix. > And it seems there's some race condition somewhere that can result > in an incorrect change request leaving prefixes in kernel when > they were intended to be replaced/deleted. > > Signed-off-by: Timo Teräs <[email protected]> > --- > zebra/rt_netlink.c | 38 ++++++++++---------------------------- > zebra/rt_netlink.h | 1 + > 2 files changed, 11 insertions(+), 28 deletions(-) > > diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c > index a016d28..abb7367 100644 > --- a/zebra/rt_netlink.c > +++ b/zebra/rt_netlink.c > @@ -667,7 +667,6 @@ netlink_routing_table (struct sockaddr_nl *snl, struct > nlmsghdr *h, > > int index; > int table; > - int metric; > u_int32_t mtu = 0; > > void *dest; > @@ -709,7 +708,6 @@ netlink_routing_table (struct sockaddr_nl *snl, struct > nlmsghdr *h, > flags |= ZEBRA_FLAG_SELFROUTE; > > index = 0; > - metric = 0; > dest = NULL; > gate = NULL; > src = NULL; > @@ -728,9 +726,6 @@ netlink_routing_table (struct sockaddr_nl *snl, struct > nlmsghdr *h, > if (tb[RTA_GATEWAY]) > gate = RTA_DATA (tb[RTA_GATEWAY]); > > - if (tb[RTA_PRIORITY]) > - metric = *(int *) RTA_DATA(tb[RTA_PRIORITY]); > - > if (tb[RTA_METRICS]) > { > struct rtattr *mxrta[RTAX_MAX+1]; > @@ -752,7 +747,7 @@ netlink_routing_table (struct sockaddr_nl *snl, struct > nlmsghdr *h, > > if (!tb[RTA_MULTIPATH]) > rib_add_ipv4 (ZEBRA_ROUTE_KERNEL, flags, &p, gate, src, index, > - vrf_id, table, metric, mtu, 0, SAFI_UNICAST); > + vrf_id, table, 0, mtu, 0, SAFI_UNICAST); > else > { > /* This is a multipath route */ > @@ -767,7 +762,7 @@ netlink_routing_table (struct sockaddr_nl *snl, struct > nlmsghdr *h, > rib->type = ZEBRA_ROUTE_KERNEL; > rib->distance = 0; > rib->flags = flags; > - rib->metric = metric; > + rib->metric = 0; > rib->mtu = mtu; > rib->vrf_id = vrf_id; > rib->table = table; > @@ -819,7 +814,7 @@ netlink_routing_table (struct sockaddr_nl *snl, struct > nlmsghdr *h, > p.prefixlen = rtm->rtm_dst_len; > > rib_add_ipv6 (ZEBRA_ROUTE_KERNEL, flags, &p, gate, index, vrf_id, > - table, metric, mtu, 0, SAFI_UNICAST); > + table, 0, mtu, 0, SAFI_UNICAST); > } > #endif /* HAVE_IPV6 */ > > @@ -854,7 +849,6 @@ netlink_route_change (struct sockaddr_nl *snl, struct > nlmsghdr *h, > > int index; > int table; > - int metric; > u_int32_t mtu = 0; > > void *dest; > @@ -915,7 +909,6 @@ netlink_route_change (struct sockaddr_nl *snl, struct > nlmsghdr *h, > } > > index = 0; > - metric = 0; > dest = NULL; > gate = NULL; > src = NULL; > @@ -936,9 +929,6 @@ netlink_route_change (struct sockaddr_nl *snl, struct > nlmsghdr *h, > > if (h->nlmsg_type == RTM_NEWROUTE) > { > - if (tb[RTA_PRIORITY]) > - metric = *(int *) RTA_DATA(tb[RTA_PRIORITY]); > - > if (tb[RTA_METRICS]) > { > struct rtattr *mxrta[RTAX_MAX+1]; > @@ -971,7 +961,7 @@ netlink_route_change (struct sockaddr_nl *snl, struct > nlmsghdr *h, > { > if (!tb[RTA_MULTIPATH]) > rib_add_ipv4 (ZEBRA_ROUTE_KERNEL, 0, &p, gate, src, index, > vrf_id, > - table, metric, mtu, 0, SAFI_UNICAST); > + table, 0, mtu, 0, SAFI_UNICAST); > else > { > /* This is a multipath route */ > @@ -986,7 +976,7 @@ netlink_route_change (struct sockaddr_nl *snl, struct > nlmsghdr *h, > rib->type = ZEBRA_ROUTE_KERNEL; > rib->distance = 0; > rib->flags = 0; > - rib->metric = metric; > + rib->metric = 0; > rib->mtu = mtu; > rib->vrf_id = vrf_id; > rib->table = table; > @@ -1053,7 +1043,7 @@ netlink_route_change (struct sockaddr_nl *snl, > struct nlmsghdr *h, > > if (h->nlmsg_type == RTM_NEWROUTE) > rib_add_ipv6 (ZEBRA_ROUTE_KERNEL, 0, &p, gate, index, vrf_id, > table, > - metric, mtu, 0, SAFI_UNICAST); > + 0, mtu, 0, SAFI_UNICAST); > else > rib_delete_ipv6 (ZEBRA_ROUTE_KERNEL, 0, &p, gate, index, vrf_id, > SAFI_UNICAST); > @@ -1663,7 +1653,7 @@ netlink_route_multipath (int cmd, struct prefix *p, > struct rib *rib, > addattr_l (&req.n, sizeof req, RTA_DST, &p->u.prefix, bytelen); > > /* Metric. */ > - addattr32 (&req.n, sizeof req, RTA_PRIORITY, rib->metric); > + addattr32 (&req.n, sizeof req, RTA_PRIORITY, NL_DEFAULT_ROUTE_METRIC); > > if (rib->mtu || rib->nexthop_mtu) > { > @@ -1809,23 +1799,15 @@ skip: > int > kernel_route_rib (struct prefix *p, struct rib *old, struct rib *new) > { > - int ret; > - > if (!old && new) > return netlink_route_multipath (RTM_NEWROUTE, p, new, > PREFIX_FAMILY(p)); > if (old && !new) > return netlink_route_multipath (RTM_DELROUTE, p, old, > PREFIX_FAMILY(p)); > > /* Replace, can be done atomically if metric does not change; > - * netlink uses [prefix, tos, priority] to identify prefix */ > - if (old->metric == new->metric) > - return netlink_route_multipath (RTM_NEWROUTE, p, new, > PREFIX_FAMILY(p)); > - > - /* Add + delete so the prefix does not disappear temporarily */ > - ret = netlink_route_multipath (RTM_NEWROUTE, p, new, PREFIX_FAMILY(p)); > - if (netlink_route_multipath (RTM_DELROUTE, p, old, PREFIX_FAMILY(p)) < > 0) > - ret = -1; > - return ret; > + * netlink uses [prefix, tos, priority] to identify prefix. > + * Now metric is not sent to kernel, so we can just do atomic replace. > */ > + return netlink_route_multipath (RTM_NEWROUTE, p, new, PREFIX_FAMILY(p)); > } > > /* Interface address modification. */ > diff --git a/zebra/rt_netlink.h b/zebra/rt_netlink.h > index 40fa8eb..63fbbe7 100644 > --- a/zebra/rt_netlink.h > +++ b/zebra/rt_netlink.h > @@ -25,6 +25,7 @@ > #ifdef HAVE_NETLINK > > #define NL_PKT_BUF_SIZE 8192 > +#define NL_DEFAULT_ROUTE_METRIC 20 > > extern int > addattr32 (struct nlmsghdr *n, size_t maxlen, int type, int data); > -- > 2.6.4 > > > _______________________________________________ > Quagga-dev mailing list > [email protected] > https://lists.quagga.net/mailman/listinfo/quagga-dev
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
