subject:"svn commit\: r333476 \- head\/sys\/net"

Re: svn commit: r333476 - head/sys/net

2018-05-11 Thread Rodney W. Grimes

> This booted and runs without issue, although obviously I wouldn't want
> to commit it:

Looks pretty close to what I did the first time.
We could also wrap this in my proposed ifdef,
and add a sysctl() to turn it on and off though
it would probably be beter to do that in the code
areas that bde patches this out.

> 
> Index: sys/net/if.c
> ===
> --- sys/net/if.c  (revision 333490)
> +++ sys/net/if.c  (working copy)
> @@ -1814,6 +1814,7 @@
>  ifa_maintain_loopback_route(int cmd, const char *otype, struct ifaddr *ifa,
>  struct sockaddr *ia)
>  {
> +#if 0
#ifdef MAINTAIN_LOOPBACK_ROUTE

>   int error;
>   struct rt_addrinfo info;
>   struct sockaddr_dl null_sdl;
> @@ -1837,6 +1838,9 @@
>   if_printf(ifp, "%s failed: %d\n", otype, error);
>  
>   return (error);
> +#else
> + return (0);
> +#endif
>  }
>  
>  int
> 
> DES
> -- 
> Dag-Erling Sm?rgrav - d...@des.no
> 
> 

-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333476 - head/sys/net

2018-05-11 Thread Dag-Erling Smørgrav

This booted and runs without issue, although obviously I wouldn't want
to commit it:

Index: sys/net/if.c
===
--- sys/net/if.c(revision 333490)
+++ sys/net/if.c(working copy)
@@ -1814,6 +1814,7 @@
 ifa_maintain_loopback_route(int cmd, const char *otype, struct ifaddr *ifa,
 struct sockaddr *ia)
 {
+#if 0
int error;
struct rt_addrinfo info;
struct sockaddr_dl null_sdl;
@@ -1837,6 +1838,9 @@
if_printf(ifp, "%s failed: %d\n", otype, error);
 
return (error);
+#else
+   return (0);
+#endif
 }
 
 int

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333476 - head/sys/net

2018-05-10 Thread Rodney W. Grimes

[ Charset UTF-8 unsupported, converting... ]
> "Rodney W. Grimes"  writes:
> > I do no need or want these routes created by this mechanism on my
> > FreeBSD based routers, they only ever existed originally to use the
> > MTU of the lo0 interface for things that wrongly open an IP address of
> > an interface rather than 127.0.0.1.
> 
> I'm not sure I understand enough of this to make an informed decision.
> Are you saying they serve no practical purpose ever on current systems?
> 
> These are the entries that route all traffic to any of our own addresses
> over lo0, right?  Like the bottom four here?

Correct.

> des@hive ~% netstat -4rn | grep -w lo0
> 127.0.0.1  link#2 UH  lo0
> 192.168.144.15 link#1 UHS lo0
> 192.168.144.16 link#1 UHS lo0
> 192.168.144.19 link#1 UHS lo0
> 192.168.144.30 link#1 UHS lo0
> 
> I'm going to try a kernel with that code #ifdefed out...

We did this back in the day so if you make a connection
to your own IP (your 4 addresses above) then you use
the MTU of lo0, which is usually 16k, instead of the MTU
of the interface, which is usually 1500, or back in the
day of SLIP was often 296.

Eugene Grosbein says there is some use case for it where
he has lots of tunX devices, but I just can not seem to
understand why or what it is that he is doing that requires
these routes.

IIRC Linux has never had these routes.  If you go back
in the history of svn and read whey this was originally
added it was to compensate for the fact that you lost the
/etc/rc installed route if you did an if down/up, which
is actually expected behavior, you loose ALL routes
associtated with the IP of an interface if you down it.

The CURRENT code makes it possible for you to ping
the IP of a DOWNED interface due to this now silly
lo0 route, just more demonstration of how wrong
this idea is.  This was caused by the addition of
pinning these routes so that they stay in effect
even when you down the interface.

-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333476 - head/sys/net

2018-05-10 Thread Dag-Erling Smørgrav

"Rodney W. Grimes"  writes:
> I do no need or want these routes created by this mechanism on my
> FreeBSD based routers, they only ever existed originally to use the
> MTU of the lo0 interface for things that wrongly open an IP address of
> an interface rather than 127.0.0.1.

I'm not sure I understand enough of this to make an informed decision.
Are you saying they serve no practical purpose ever on current systems?

These are the entries that route all traffic to any of our own addresses
over lo0, right?  Like the bottom four here?

des@hive ~% netstat -4rn | grep -w lo0
127.0.0.1  link#2 UH  lo0
192.168.144.15 link#1 UHS lo0
192.168.144.16 link#1 UHS lo0
192.168.144.19 link#1 UHS lo0
192.168.144.30 link#1 UHS lo0

I'm going to try a kernel with that code #ifdefed out...

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333476 - head/sys/net

2018-05-10 Thread Rodney W. Grimes

[ Charset UTF-8 unsupported, converting... ]
> "Rodney W. Grimes"  writes:
> > "Dag-Erling Sm?rgrav"  writes:
> > > In ifa_maintain_loopback_route(), don't needlessly log an error if we
> > > either failed to add a route because it already existed or failed to
> > > remove one because it did not.  We still return an error code, though.
> > Those are the only conditions under which I have ever seen this code
> > log anything.  These usually occur for me when a tunX device is going
> > down, the route gets ripped out long before maintain_loopback ever has
> > a chance to remove it.
> 
> I have a few routers with hundreds of dynamically configured vlan
> interfaces, so I get a *lot* of these...

So do I, and the thing I found best is to just totally rip
out the ifa_maintain_loopback code and let bird deal with
my routes.

I do no need or want these routes created by this mechanism
on my FreeBSD based routers, they only ever existed originally
to use the MTU of the lo0 interface for things that wrongly
open an IP address of an interface rather than 127.0.0.1.


-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333476 - head/sys/net

2018-05-10 Thread Dag-Erling Smørgrav

"Rodney W. Grimes"  writes:
> "Dag-Erling Smørgrav"  writes:
> > In ifa_maintain_loopback_route(), don't needlessly log an error if we
> > either failed to add a route because it already existed or failed to
> > remove one because it did not.  We still return an error code, though.
> Those are the only conditions under which I have ever seen this code
> log anything.  These usually occur for me when a tunX device is going
> down, the route gets ripped out long before maintain_loopback ever has
> a chance to remove it.

I have a few routers with hundreds of dynamically configured vlan
interfaces, so I get a *lot* of these...

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333476 - head/sys/net

2018-05-10 Thread Rodney W. Grimes

> Author: des
> Date: Fri May 11 00:19:49 2018
> New Revision: 333476
> URL: https://svnweb.freebsd.org/changeset/base/333476
> 
> Log:
>   Slight cleanup of interface event logging.
>   
>   Make if_printf() use vlog() instead of vprintf().  This means it can no
>   longer return the number of characters printed, as it used to, but every
>   single call to if_printf() in the entire kernel ignores the return value
>   anyway; just return 0 so we don't have to change the prototype.
>   
>   Consistently use if_printf() throughout sys/net/if.c, instead of a
>   mixture of if_printf() and log().
>   
>   In ifa_maintain_loopback_route(), don't needlessly log an error if we
>   either failed to add a route because it already existed or failed to
>   remove one because it did not.  We still return an error code, though.

Those are the only conditions under which I have ever
seen this code log anything.  These usually occur for
me when a tunX device is going down, the route gets
ripped out long before maintain_loopback ever has a
chance to remove it.

I still feel this whole maintain_loopback_route() is a
implementaton of a hardcoded route policy in the kernel
that we do not need, or want.


>   MFC after:  1 week
> 
> Modified:
>   head/sys/net/if.c
> 
> Modified: head/sys/net/if.c
> ==
> --- head/sys/net/if.c Fri May 11 00:01:43 2018(r333475)
> +++ head/sys/net/if.c Fri May 11 00:19:49 2018(r333476)
> @@ -1832,9 +1832,10 @@ ifa_maintain_loopback_route(int cmd, const char *otype
>  
>   error = rtrequest1_fib(cmd, , NULL, ifp->if_fib);
>  
> - if (error != 0)
> - log(LOG_DEBUG, "%s: %s failed for interface %s: %u\n",
> - __func__, otype, if_name(ifp), error);
> + if (error != 0 &&
> + !(cmd == RTM_ADD && error == EEXIST) &&
> + !(cmd == RTM_DELETE && error == ENOENT))
> + if_printf(ifp, "%s failed: %d\n", otype, error);
>  
>   return (error);
>  }
> @@ -2328,7 +2329,7 @@ do_link_state_change(void *arg, int pending)
>   if (pending > 1)
>   if_printf(ifp, "%d link states coalesced\n", pending);
>   if (log_link_state_change)
> - log(LOG_NOTICE, "%s: link state changed to %s\n", ifp->if_xname,
> + if_printf(ifp, "link state changed to %s\n",
>   (link_state == LINK_STATE_UP) ? "UP" : "DOWN" );
>   EVENTHANDLER_INVOKE(ifnet_link_event, ifp, link_state);
>   CURVNET_RESTORE();
> @@ -2631,8 +2632,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data,
>   else if (ifp->if_pcount == 0)
>   ifp->if_flags &= ~IFF_PROMISC;
>   if (log_promisc_mode_change)
> -log(LOG_INFO, "%s: permanently promiscuous 
> mode %s\n",
> -ifp->if_xname,
> +if_printf(ifp, "permanently promiscuous mode 
> %s\n",
>  ((new_flags & IFF_PPROMISC) ?
>   "enabled" : "disabled"));
>   }
> @@ -2695,8 +2695,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data,
>   rt_ifannouncemsg(ifp, IFAN_DEPARTURE);
>   EVENTHANDLER_INVOKE(ifnet_departure_event, ifp);
>  
> - log(LOG_INFO, "%s: changing name to '%s'\n",
> - ifp->if_xname, new_name);
> + if_printf(ifp, "changing name to '%s'\n", new_name);
>  
>   IF_ADDR_WLOCK(ifp);
>   strlcpy(ifp->if_xname, new_name, sizeof(ifp->if_xname));
> @@ -3199,8 +3198,7 @@ ifpromisc(struct ifnet *ifp, int pswitch)
>   /* If promiscuous mode status has changed, log a message */
>   if (error == 0 && ((ifp->if_flags ^ oldflags) & IFF_PROMISC) &&
>  log_promisc_mode_change)
> - log(LOG_INFO, "%s: promiscuous mode %s\n",
> - ifp->if_xname,
> + if_printf(ifp, "promiscuous mode %s\n",
>   (ifp->if_flags & IFF_PROMISC) ? "enabled" : "disabled");
>   return (error);
>  }
> @@ -3905,16 +3903,16 @@ if_initname(struct ifnet *ifp, const char *name, int u
>  }
>  
>  int
> -if_printf(struct ifnet *ifp, const char * fmt, ...)
> +if_printf(struct ifnet *ifp, const char *fmt, ...)
>  {
> + char if_fmt[256];
>   va_list ap;
> - int retval;
>  
> - retval = printf("%s: ", ifp->if_xname);
> + snprintf(if_fmt, sizeof(if_fmt), "%s: %s", ifp->if_xname, fmt);
>   va_start(ap, fmt);
> - retval += vprintf(fmt, ap);
> + vlog(LOG_INFO, if_fmt, ap);
>   va_end(ap);
> - return (retval);
> + return (0);
>  }
>  
>  void
> 
> 

-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To

svn commit: r333476 - head/sys/net

2018-05-10 Thread Dag-Erling Smørgrav

Author: des
Date: Fri May 11 00:19:49 2018
New Revision: 333476
URL: https://svnweb.freebsd.org/changeset/base/333476

Log:
  Slight cleanup of interface event logging.
  
  Make if_printf() use vlog() instead of vprintf().  This means it can no
  longer return the number of characters printed, as it used to, but every
  single call to if_printf() in the entire kernel ignores the return value
  anyway; just return 0 so we don't have to change the prototype.
  
  Consistently use if_printf() throughout sys/net/if.c, instead of a
  mixture of if_printf() and log().
  
  In ifa_maintain_loopback_route(), don't needlessly log an error if we
  either failed to add a route because it already existed or failed to
  remove one because it did not.  We still return an error code, though.
  
  MFC after:1 week

Modified:
  head/sys/net/if.c

Modified: head/sys/net/if.c
==
--- head/sys/net/if.c   Fri May 11 00:01:43 2018(r333475)
+++ head/sys/net/if.c   Fri May 11 00:19:49 2018(r333476)
@@ -1832,9 +1832,10 @@ ifa_maintain_loopback_route(int cmd, const char *otype
 
error = rtrequest1_fib(cmd, , NULL, ifp->if_fib);
 
-   if (error != 0)
-   log(LOG_DEBUG, "%s: %s failed for interface %s: %u\n",
-   __func__, otype, if_name(ifp), error);
+   if (error != 0 &&
+   !(cmd == RTM_ADD && error == EEXIST) &&
+   !(cmd == RTM_DELETE && error == ENOENT))
+   if_printf(ifp, "%s failed: %d\n", otype, error);
 
return (error);
 }
@@ -2328,7 +2329,7 @@ do_link_state_change(void *arg, int pending)
if (pending > 1)
if_printf(ifp, "%d link states coalesced\n", pending);
if (log_link_state_change)
-   log(LOG_NOTICE, "%s: link state changed to %s\n", ifp->if_xname,
+   if_printf(ifp, "link state changed to %s\n",
(link_state == LINK_STATE_UP) ? "UP" : "DOWN" );
EVENTHANDLER_INVOKE(ifnet_link_event, ifp, link_state);
CURVNET_RESTORE();
@@ -2631,8 +2632,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data,
else if (ifp->if_pcount == 0)
ifp->if_flags &= ~IFF_PROMISC;
if (log_promisc_mode_change)
-log(LOG_INFO, "%s: permanently promiscuous 
mode %s\n",
-ifp->if_xname,
+if_printf(ifp, "permanently promiscuous mode 
%s\n",
 ((new_flags & IFF_PPROMISC) ?
  "enabled" : "disabled"));
}
@@ -2695,8 +2695,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data,
rt_ifannouncemsg(ifp, IFAN_DEPARTURE);
EVENTHANDLER_INVOKE(ifnet_departure_event, ifp);
 
-   log(LOG_INFO, "%s: changing name to '%s'\n",
-   ifp->if_xname, new_name);
+   if_printf(ifp, "changing name to '%s'\n", new_name);
 
IF_ADDR_WLOCK(ifp);
strlcpy(ifp->if_xname, new_name, sizeof(ifp->if_xname));
@@ -3199,8 +3198,7 @@ ifpromisc(struct ifnet *ifp, int pswitch)
/* If promiscuous mode status has changed, log a message */
if (error == 0 && ((ifp->if_flags ^ oldflags) & IFF_PROMISC) &&
 log_promisc_mode_change)
-   log(LOG_INFO, "%s: promiscuous mode %s\n",
-   ifp->if_xname,
+   if_printf(ifp, "promiscuous mode %s\n",
(ifp->if_flags & IFF_PROMISC) ? "enabled" : "disabled");
return (error);
 }
@@ -3905,16 +3903,16 @@ if_initname(struct ifnet *ifp, const char *name, int u
 }
 
 int
-if_printf(struct ifnet *ifp, const char * fmt, ...)
+if_printf(struct ifnet *ifp, const char *fmt, ...)
 {
+   char if_fmt[256];
va_list ap;
-   int retval;
 
-   retval = printf("%s: ", ifp->if_xname);
+   snprintf(if_fmt, sizeof(if_fmt), "%s: %s", ifp->if_xname, fmt);
va_start(ap, fmt);
-   retval += vprintf(fmt, ap);
+   vlog(LOG_INFO, if_fmt, ap);
va_end(ap);
-   return (retval);
+   return (0);
 }
 
 void
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333476 - head/sys/net

Re: svn commit: r333476 - head/sys/net

Re: svn commit: r333476 - head/sys/net

Re: svn commit: r333476 - head/sys/net

Re: svn commit: r333476 - head/sys/net

Re: svn commit: r333476 - head/sys/net

Re: svn commit: r333476 - head/sys/net

svn commit: r333476 - head/sys/net

8 matches

Site Navigation

Mail list logo

Footer information