Re: Routing socket issue?

[Frank Kardel <kardel%netbsd.org@localhost>]

Hi Roy!


On 01/31/21 03:27, Roy Marples wrote:
> On 30/01/2021 22:01, Frank Kardel wrote:
> > "why it needs to be interested i..."
> >
> > Ntpd needs to know the local address being used when sending to peers 
> > (authentication, which socket to use). That is why it not just reacts to
> >
> > address information but also redetermines to local addresses (and 
> > sockets) are being used for reaching its peers.
> >
> > The interaction with the routing socket is purposely simple. ntpd 
> > just needs to know that *something* has changed. It will then rescan 
> > after a grace period the interfaces and reevaluate
> >
> > the interface/local address/socket setup. It does not need to be 
> > extremely snappy but it needs to happen.
> >
> > Dropping that might delay ntpd's detection of changed local addresses 
> > for peers.
>
> 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.

> 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.

> NOTE TO SELF: our kernel doesn't seem to report RTM_CHGADDR anymore 
> looking at nxr.netbsd.org
>
> 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.
> > 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.
>
> 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.
> Roy
Frank