On Wed, Aug 10, 2022 at 11:59:30AM +0200, Claudio Jeker wrote:
> When introducing prefix_nhvalid(p) the code in network_dump_upcall()
> was not correctly adjusted:
>
> Before:
> if (prefix_nexthop(p) == NULL ||
> prefix_nexthop(p)->state != NEXTHOP_REACH)
> kf.nexthop.aid = kf.prefix.aid;
> else
> kf.nexthop = prefix_nexthop(p)->true_nexthop;
>
> Now:
> if (prefix_nhvalid(p))
> kf.nexthop.aid = kf.prefix.aid;
> else
> kf.nexthop = prefix_nexthop(p)->true_nexthop;
>
> What it should be:
>
> if (prefix_nhvalid(p) && prefix_nexthop(p) != NULL)
> kf.nexthop = prefix_nexthop(p)->exit_nexthop;
> else
> kf.nexthop.aid = kf.prefix.aid;
>
> If the nexthop is valid we want to show it but in most cases the nexthop
> of announced networks is NULL so make sure we don't dereference NULL.
> Also I think it makes more sense to show the exit_nexthop (which is what
> is shown in show rib output by default). It is also what will be sent to
> the peers.
>
> While there adjust the code to be a bit more logical and modern.
This all makes sense and is much nicer this way.
ok
> --
> :wq Claudio
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.560
> diff -u -p -r1.560 rde.c
> --- rde.c 28 Jul 2022 13:11:50 - 1.560
> +++ rde.c 10 Aug 2022 09:56:51 -
> @@ -4247,14 +4247,13 @@ network_dump_upcall(struct rib_entry *re
> continue;
> pt_getaddr(p->pt, &addr);
>
> - bzero(&kf, sizeof(kf));
> - memcpy(&kf.prefix, &addr, sizeof(kf.prefix));
> - if (prefix_nhvalid(p))
> - kf.nexthop.aid = kf.prefix.aid;
> - else
> - memcpy(&kf.nexthop, &prefix_nexthop(p)->true_nexthop,
> - sizeof(kf.nexthop));
> + memset(&kf, 0, sizeof(kf));
> + kf.prefix = addr;
> kf.prefixlen = p->pt->prefixlen;
> + if (prefix_nhvalid(p) && prefix_nexthop(p) != NULL)
> + kf.nexthop = prefix_nexthop(p)->exit_nexthop;
> + else
> + kf.nexthop.aid = kf.prefix.aid;
> if ((asp->flags & F_ANN_DYNAMIC) == 0)
> kf.flags = F_STATIC;
> if (imsg_compose(ibuf_se_ctl, IMSG_CTL_SHOW_NETWORK, 0,
>
