On Thu, Jun 16, 2022 at 07:15:51PM +0200, Claudio Jeker wrote:
> Not much holds us back to switch kroute_find() to use a struct bgpd_addr
> as prefix argument. Only the match code needs some adoption.
> I created applymask() for this which works on bgpd_addrs like
> inet4applymask works on in_addrs.
>
> I also switched a simple case in session.c over to applymask().
> There is another one in bgpctl which I will send out once this is in.
ok
A nit and a question below.
[...]
> +kroute_find(struct ktable *kt, const struct bgpd_addr *prefix,
> + uint8_t prefixlen, uint8_t prio)
> {
> struct kroute_node s;
> struct kroute_node *kn, *tmp;
>
> - s.r.prefix.s_addr = prefix;
> + s.r.prefix= prefix->v4;
Missing space before =.
[...]
> Index: util.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 util.c
> --- util.c 16 Jun 2022 15:33:05 -0000 1.64
> +++ util.c 16 Jun 2022 16:50:47 -0000
> @@ -783,6 +783,26 @@ inet6applymask(struct in6_addr *dest, co
> dest->s6_addr[i] = src->s6_addr[i] & mask.s6_addr[i];
> }
>
> +void
> +applymask(struct bgpd_addr *dest, const struct bgpd_addr *src, int prefixlen)
> +{
> + struct bgpd_addr tmp;
> +
> + /* use temporary storage in case src and dest point to same struct */
> + tmp = *src;
While I'm fine with doing it this way, why is this tmp dance needed? I
would have thought that both inet4applymask() and inet6applymask() work
fine if src and dest point to the same struct. bgpctl's parse.y assumes
that this is the case.
> + switch (src->aid) {
> + case AID_INET:
> + case AID_VPN_IPv4:
> + inet4applymask(&tmp.v4, &src->v4, prefixlen);
> + break;
> + case AID_INET6:
> + case AID_VPN_IPv6:
> + inet6applymask(&tmp.v6, &src->v6, prefixlen);
> + break;
> + }
> + *dest = tmp;
> +}
> +
> /* address family translation functions */
> const struct aid aid_vals[AID_MAX] = AID_VALS;
>
>
