Re: ndp delete cloning route

2023-04-04 Thread Alexander Bluhm
On Tue, Apr 04, 2023 at 03:05:43PM +0200, Alexander Bluhm wrote:
> While comparing arp and ndp code I found some stylistic differences.
> Make both similar to catch such bugs easier.

kn@ asked me to commit the style fix upfront.

> if ndp -d does not find a neigbor entry, it removes the cloning
> route instead.  Comparing the arp and ndp code shows that the latter
> has a fallthrough to delete.  Return an error also in this case.

Here is the remaining bugfix.

ok?

bluhm

Index: usr.sbin/ndp/ndp.c
===
RCS file: /cvs/src/usr.sbin/ndp/ndp.c,v
retrieving revision 1.108
diff -u -p -r1.108 ndp.c
--- usr.sbin/ndp/ndp.c  4 Apr 2023 21:18:04 -   1.108
+++ usr.sbin/ndp/ndp.c  4 Apr 2023 21:21:18 -
@@ -436,12 +436,13 @@ delete(const char *host)
if ((rtm->rtm_flags & RTF_GATEWAY) == 0)
goto delete;
}
-   /*
-* IPv4 arp command retries with sin_other = SIN_PROXY here.
-*/
-   warnx("delete: cannot locate %s", host);
-   return 1;
}
+
+   /*
+* IPv4 arp command retries with sin_other = SIN_PROXY here.
+*/
+   warnx("delete: cannot locate %s", host);
+   return 1;
 
 delete:
if (sdl->sdl_family != AF_LINK) {



ndp delete cloning route

2023-04-04 Thread Alexander Bluhm
Hi,

if ndp -d does not find a neigbor entry, it removes the cloning
route instead.  Comparing the arp and ndp code shows that the latter
has a fallthrough to delete.  Return an error also in this case.

While comparing arp and ndp code I found some stylistic differences.
Make both similar to catch such bugs easier.

There is switch (sdl->sdl_type) in arp and missing in ndp.  I cannot
explain this, so I leave it as it is.  Any idea why the logic is
not the same?

ok?

bluhm

Index: usr.sbin/arp/arp.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/arp/arp.c,v
retrieving revision 1.88
diff -u -p -r1.88 arp.c
--- usr.sbin/arp/arp.c  16 Sep 2019 20:49:28 -  1.88
+++ usr.sbin/arp/arp.c  4 Apr 2023 12:39:28 -
@@ -408,17 +408,17 @@ delete(const char *host)
getsocket();
sin_m = blank_sin;  /* struct copy */
if (parse_host(host, >sin_addr))
-   return (1);
+   return 1;
 tryagain:
if (rtget(, )) {
warn("%s", host);
-   return (1);
+   return 1;
}
if (sin->sin_addr.s_addr == sin_m.sin_addr.s_addr) {
if (sdl->sdl_family == AF_LINK && rtm->rtm_flags & RTF_LLINFO) {
if (rtm->rtm_flags & RTF_LOCAL)
-   return (0);
-   if (!(rtm->rtm_flags & RTF_GATEWAY))
+   return 0;
+   if ((rtm->rtm_flags & RTF_GATEWAY) == 0)
switch (sdl->sdl_type) {
case IFT_ETHER:
case IFT_FDDI:
@@ -432,8 +432,8 @@ tryagain:
}
 
if (sin_m.sin_other & SIN_PROXY) {
-   warnx("delete: can't locate %s", host);
-   return (1);
+   warnx("delete: cannot locate %s", host);
+   return 1;
} else {
sin_m.sin_other = SIN_PROXY;
goto tryagain;
@@ -441,12 +441,12 @@ tryagain:
 delete:
if (sdl->sdl_family != AF_LINK) {
printf("cannot locate %s\n", host);
-   return (1);
+   return 1;
}
if (rtmsg(RTM_DELETE))
-   return (1);
+   return 1;
printf("%s (%s) deleted\n", host, inet_ntoa(sin->sin_addr));
-   return (0);
+   return 0;
 }
 
 /*
Index: usr.sbin/ndp/ndp.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/ndp/ndp.c,v
retrieving revision 1.107
diff -u -p -r1.107 ndp.c
--- usr.sbin/ndp/ndp.c  28 Dec 2022 21:30:17 -  1.107
+++ usr.sbin/ndp/ndp.c  4 Apr 2023 12:39:21 -
@@ -137,8 +137,7 @@ static int rdomain;
 int
 main(int argc, char *argv[])
 {
-   int  ch;
-   int  mode = 0;
+   int  ch, mode = 0, error = 0;
char*arg = NULL;
const char  *errstr;
 
@@ -209,7 +208,7 @@ main(int argc, char *argv[])
if (argc != 0) {
usage();
}
-   delete(arg);
+   error = delete(arg);
break;
case 'f':
if (argc != 0)
@@ -232,7 +231,7 @@ main(int argc, char *argv[])
get(argv[0]);
break;
}
-   exit(0);
+   return (error);
 }
 
 /*
@@ -414,46 +413,47 @@ get(const char *host)
 int
 delete(const char *host)
 {
-   struct sockaddr_in6 *sin = _m;
-   struct rt_msghdr *rtm = _rtmsg.m_rtm;
+   struct sockaddr_in6 *sin;
+   struct rt_msghdr *rtm;
struct sockaddr_dl *sdl;
 
+   sin = _m;
+   rtm = _rtmsg.m_rtm;
+
getsocket();
-   sin_m = blank_sin;
+   sin_m = blank_sin;  /* struct copy */
if (parse_host(host, sin))
return 1;
if (rtget(, )) {
-   errx(1, "RTM_GET(%s) failed", host);
+   warn("%s", host);
+   return 1;
}
-
if (IN6_ARE_ADDR_EQUAL(>sin6_addr, _m.sin6_addr) &&
sin->sin6_scope_id == sin_m.sin6_scope_id) {
if (sdl->sdl_family == AF_LINK && rtm->rtm_flags & RTF_LLINFO) {
if (rtm->rtm_flags & RTF_LOCAL)
-   return (0);
-   if (!(rtm->rtm_flags & RTF_GATEWAY))
+   return 0;
+   if ((rtm->rtm_flags & RTF_GATEWAY) == 0)
goto delete;
}
-   /*
-* IPv4 arp command retries with sin_other = SIN_PROXY here.
-*/
-   warnx("delete: cannot delete non-NDP entry");
-   return 1;
}
 
+   /*
+* IPv4 arp command retries with sin_other = SIN_PROXY here.
+*/
+   warnx("delete: cannot locate %s", host);
+   return