On Tue, Jul 03, 2012 at 06:08:29PM +0200, Henning Brauer wrote:
> * Florian Obser <[email protected]> [2012-07-03 17:07]:
> > dammit, I turned this into the fix from hell :/
> >
> > The patch tries to do the right thing when nh->true_nexthop.aid !=
> > nh->nexthop_net.aid I'm pretty sure this should never be the case for
> > AID_INET and AID_INET6 but no idea what AID_VPN_IPv4 does.
> >
> > If nh->true_nexthop.aid != nh->nexthop_net.aid is *never* true the two
> > switch statements can be folded into one.
>
> we already have prefix_compare, let's use that. atm it requires the
Ah, nice, sorry for missing that.
> prefixlength to be passed in and to save us from the af-dependent max
> prefixlen figure out dance I changed prefix_compare to accept prefixlen
> -1 which in turn means maximum.
>
> untested except for compilation.
FWIW it reads correct to me. Benno is going to give it a spin.
>
> Index: rde_rib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.133
> diff -u -p -r1.133 rde_rib.c
> --- rde_rib.c 1 Jul 2012 11:55:13 -0000 1.133
> +++ rde_rib.c 3 Jul 2012 16:06:32 -0000
> @@ -1056,8 +1056,10 @@ void
> nexthop_update(struct kroute_nexthop *msg)
> {
> struct nexthop *nh;
> + struct bgpd_addr oldtrue_nh, oldnh_net;
> struct rde_aspath *asp;
> enum nexthop_state oldstate;
> + u_int8_t oldflags, oldnh_netlen;
>
> nh = nexthop_lookup(&msg->nexthop);
> if (nh == NULL) {
> @@ -1067,6 +1069,11 @@ nexthop_update(struct kroute_nexthop *ms
> }
>
> oldstate = nh->state;
> + oldflags = nh->flags;
> + oldnh_netlen = nh->nexthop_netlen;
> + memcpy(&oldtrue_nh, &nh->true_nexthop, sizeof(oldtrue_nh));
> + memcpy(&oldnh_net, &nh->nexthop_net, sizeof(oldnh_net));
> +
> if (msg->valid)
> nh->state = NEXTHOP_REACH;
> else
> @@ -1087,6 +1094,15 @@ nexthop_update(struct kroute_nexthop *ms
> if (nexthop_delete(nh))
> /* nexthop no longer used */
> return;
> +
> + /* if there is no nexthop change, return early */
> + if (oldstate == nh->state && oldflags == nh->flags &&
> + oldnh_netlen == nh->nexthop_netlen &&
> + oldnh_net.aid == nh->nexthop_net.aid) {
> + if (prefix_compare(&oldtrue_nh, &nh->true_nexthop, -1) == 0 &&
> + prefix_compare(&oldnh_net, &nh->nexthop_net, -1) == 0)
> + return;
> + }
>
> if (rde_noevaluate())
> /*
> Index: util.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 util.c
> --- util.c 20 Sep 2011 21:19:07 -0000 1.14
> +++ util.c 3 Jul 2012 16:06:32 -0000
> @@ -422,16 +422,21 @@ prefix_compare(const struct bgpd_addr *a
>
> switch (a->aid) {
> case AID_INET:
> - if (prefixlen > 32)
> + if (prefixlen < -1 || prefixlen > 32)
> fatalx("prefix_cmp: bad IPv4 prefixlen");
> - mask = htonl(prefixlen2mask(prefixlen));
> + if (prefixlen == -1)
> + mask = 0xffffffff;
> + else
> + mask = htonl(prefixlen2mask(prefixlen));
> aa = ntohl(a->v4.s_addr & mask);
> ba = ntohl(b->v4.s_addr & mask);
> if (aa != ba)
> return (aa - ba);
> return (0);
> case AID_INET6:
> - if (prefixlen > 128)
> + if (prefixlen == -1)
> + prefixlen = 128;
> + else if (prefixlen > 128)
> fatalx("prefix_cmp: bad IPv6 prefixlen");
> for (i = 0; i < prefixlen / 8; i++)
> if (a->v6.s6_addr[i] != b->v6.s6_addr[i])
> @@ -446,13 +451,16 @@ prefix_compare(const struct bgpd_addr *a
> }
> return (0);
> case AID_VPN_IPv4:
> - if (prefixlen > 32)
> - fatalx("prefix_cmp: bad IPv4 VPN prefixlen");
> if (betoh64(a->vpn4.rd) > betoh64(b->vpn4.rd))
> return (1);
> if (betoh64(a->vpn4.rd) < betoh64(b->vpn4.rd))
> return (-1);
> - mask = htonl(prefixlen2mask(prefixlen));
> + if (prefixlen < -1 || prefixlen > 32)
> + fatalx("prefix_cmp: bad IPv4 prefixlen");
> + if (prefixlen == -1)
> + mask = 0xffffffff;
> + else
> + mask = htonl(prefixlen2mask(prefixlen));
> aa = ntohl(a->vpn4.addr.s_addr & mask);
> ba = ntohl(b->vpn4.addr.s_addr & mask);
> if (aa != ba)
