Timo -

This no longer applies cleanly, can you respin?

thanks!

donald

On Wed, Nov 4, 2015 at 4:25 PM, Donald Sharp <[email protected]>
wrote:

> Acked-by: Donald Sharp <[email protected]>
>
> On Mon, Nov 2, 2015 at 6:50 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 dd75e79..db7e283 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);
>> @@ -1664,7 +1654,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)
>>      {
>> @@ -1810,23 +1800,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.1
>>
>>
>> _______________________________________________
>> 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

Reply via email to