Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Routing socket issue?



Looks reasonable to me.

I think the the overflows can be triggered when the routing daemon add a longer list of routes like when discovering another part

of the net. This happens e. g. here when my host starts up the VPN an then frr needs to insert 30+ routes it just learned.

So depending on the topology there can be at times route message floods.

Frank


On 01/31/21 09:33, Roy Marples wrote:
Hi Frank :)

On 31/01/2021 07:58, Frank Kardel wrote:
For example I fail to see how RTM_LOSING helps that because it won't change
how ntpd would configure itself.
Well if I read the comment right I am inclined to differ here:
In in_pcs.c we find:
/*
  * Check for alternatives when higher level complains
  * about service problems.  For now, invalidate cached
  * routing information.  If the route was created dynamically
  * (by a redirect), time to try a default gateway again.
  */
in_losing(struct inpcb *inp)

and the call is in tcp_time.c:
     /*
      * If losing, let the lower level know and try for
      * a better route.  Also, if we backed off this far,
      * our srtt estimate is probably bogus.  Clobber it
      * so we'll take the next rtt measurement as our srtt;
      * move the current srtt into rttvar to keep the current
      * retransmit times until then.
      */

As ntpd acts after a grace period the routing engine may have corrected this situation and routing may indeed change. ntpd's interactions with peers can take up to 1024s so it is good to attempt in a best effort way to keep the internal
local address/socket state close to the current state.
It is likely though that there have been routing messages like RTM_CHANGE/ADD/DELETE before that and RTM_LOSING is not providing
additional information at the point.

Right, RTM_LOSING is just informational.
If any routing does change then we get RTM_CHANGE/ADD/DELETE etc.



As NTP doesn't bring interfaces up or down, RFM_IFANNOUNCE is useless as well. If the interface does vanish, any addresses on it will be reported via RTM_DELADDR. RTM_IFINFO is also questionable as commentary in the code is that it only cares about addresses.

Well I read
ntp_io.c
                         /*
                          * we are keen on new and deleted addresses and
                          * if an interface goes up and down or routing
                          * changes
                          */
not as being interested in addresses only.

Also keep in mind that at this point routing messages are processed in a loop and the action here
     timer_interfacetimeout(current_time + UPDATE_GRACE);
just sets the variable for the next interface+local address update run. This is very cheap. The grace period will batch multiple routing message together. An explicit routing message flush is from my point of view code clutter here. as the socket is effectively drained in the loop at the cost of examining the msg_type and setting
a variable. Not much gained here.

OK, we'll keep RTM_IFINFO but drop RTM_IFANNOUNCE.
The point is trying to eliminate the overflow message entirely.

I mean, if you want to argue against any of that then I would suggest why even bother filtering or looking at overflow at all? Shrink the code - any activity on the routing socket, drain it ignoring all error, start the interface update timer.
That would be an option but we should react only on known events. There may be one or two events that could be removed from the list after examination as other messages can cover for them. Keep in mind the this is a portable code section and the code tries to be on the fail safe, robust side for the goal of address/routing tracking so adjusting it to a particular implementation
may break on other os implementations.

Well, Dragonfly (prior to my patches there) and by extension FreeBSD (not checked to see if that changed) both emit RMT_DELADDR before RTM_IFANNOUNCE (ie wrong order) so when they do overflow you never see RTM_IFANNOUNCE to say the interface vanished. Hence there is zero point is listening for it for ntp.



As for the message: IMHO it does not need to be logged at all (DPRINTF/maybe LOGDEBUG at most) because the overflow should and does just trigger ntpd to reevaluate the interface/routing configuration.

This information is not important at all for normal operation as the effects are correctly mitigated.

I changed it to LOG_DEBUG as well as removing RTM_LOSING and RTM_IFANNOUNCE as discussed above.


Great.

BTW: does the current code revert to (fail safe) periodic interface scanning if the routing socket is being disabled (happens when an unexpected error code is returned from read(2))?

No.

The socket is non blocking so the only error to ignore here would be EINTR.
Any other errors are due to bad programming IMO.
Could be bad programming, but I prefer the ntpd being forgiving against hiccups by reverting to periodic scanning when we disable to routing socket. That is a fail safe strategy and would also warrant a log message as it is an unusual event.

EINTR is now ignored.
I'll find time to restore periodic scanning later.

Roy



Home | Main Index | Thread Index | Old Index