Re: bgpd cleanup around mask2prefixlen

[Theo Buehler]

On Tue, Oct 17, 2023 at 04:47:36PM +0200, Claudio Jeker wrote:
> Looking at fixing portable I realized that some bits around mask2prefixlen
> can be cleaned up.
> 
> First in session.c the plen != 0xff check is not needed since it never can
> happen.
> 
> 2nd the checks for sin_len and sin6_len == 0 are also impossible. A
> sockaddr can not have length 0. A default route would at least have
> sin_len == 4 but on OpenBSD even that no longer happens.

Yes, this looks like a mistake must have happened earlier. The only
realistic scenario would be an addr that comes freshly out of memset().
But for those nothing changes and the return 0 remains.

> Last check for RTF_HOST first, then for sa_in != NULL. If RTF_HOST is set
> the netmask is irrelevant.

ok tb.

bgpd cleanup around mask2prefixlen

[Claudio Jeker]

Looking at fixing portable I realized that some bits around mask2prefixlen
can be cleaned up.

First in session.c the plen != 0xff check is not needed since it never can
happen.

2nd the checks for sin_len and sin6_len == 0 are also impossible. A
sockaddr can not have length 0. A default route would at least have
sin_len == 4 but on OpenBSD even that no longer happens.

Last check for RTF_HOST first, then for sa_in != NULL. If RTF_HOST is set
the netmask is irrelevant.

-- 
:wq Claudio

Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.306
diff -u -p -r1.306 kroute.c
--- kroute.c16 Oct 2023 10:25:45 -  1.306
+++ kroute.c17 Oct 2023 13:18:42 -
@@ -2422,8 +2422,6 @@ mask2prefixlen4(struct sockaddr_in *sa_i
 {
in_addr_t ina;
 
-   if (sa_in->sin_len == 0)
-   return (0);
ina = sa_in->sin_addr.s_addr;
if (ina == 0)
return (0);
@@ -2437,8 +2435,6 @@ mask2prefixlen6(struct sockaddr_in6 *sa_
uint8_t *ap, *ep;
u_intl = 0;
 
-   if (sa_in6->sin6_len == 0)
-   return (0);
/*
 * sin6_len is the size of the sockaddr so subtract the offset of
 * the possibly truncated sin6_addr struct.
@@ -3096,20 +3092,20 @@ dispatch_rtmsg_addr(struct rt_msghdr *rt
switch (sa->sa_family) {
case AF_INET:
sa_in = (struct sockaddr_in *)rti_info[RTAX_NETMASK];
-   if (sa_in != NULL) {
-   kf->prefixlen = mask2prefixlen4(sa_in);
-   } else if (rtm->rtm_flags & RTF_HOST)
+   if (rtm->rtm_flags & RTF_HOST)
kf->prefixlen = 32;
+   else if (sa_in != NULL)
+   kf->prefixlen = mask2prefixlen4(sa_in);
else
kf->prefixlen =
prefixlen_classful(kf->prefix.v4.s_addr);
break;
case AF_INET6:
sa_in6 = (struct sockaddr_in6 *)rti_info[RTAX_NETMASK];
-   if (sa_in6 != NULL) {
-   kf->prefixlen = mask2prefixlen6(sa_in6);
-   } else if (rtm->rtm_flags & RTF_HOST)
+   if (rtm->rtm_flags & RTF_HOST)
kf->prefixlen = 128;
+   else if (sa_in6 != NULL)
+   kf->prefixlen = mask2prefixlen6(sa_in6);
else
fatalx("in6 net addr without netmask");
break;
Index: session.c
===
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.449
diff -u -p -r1.449 session.c
--- session.c   16 Oct 2023 10:25:46 -  1.449
+++ session.c   16 Oct 2023 17:14:41 -
@@ -1241,8 +1241,7 @@ get_alternate_addr(struct bgpd_addr *loc
plen = mask2prefixlen(
match->ifa_addr->sa_family,
match->ifa_netmask);
-   if (plen != 0xff &&
-   prefix_compare(local, remote, plen) == 0)
+   if (prefix_compare(local, remote, plen) == 0)
connected = 1;
}
break;