Ntpd / gps need some love part II
-
Once I got the GPS sending only the $GPGGA sentence, I haven't had any spikes (so far). If I was ambitious, I'd go back in my clockstats file, and see what the strings looked like around the times the spikes were detected. Hopefully it's fixed though.
I spoke (wrote?) too soon! My spikes are back, so I've got to do more digging; will update if I find anything.
-
I just found a couple of small issues, like typos and html display stuff, fine-tuned a bit SureGPS defaults and noticed that RRD graphs were not plotting on 2.1.3, fixed that.
Thanks for keeping this up! I'm testing 2.2, and I've noticed that the ntp RRD data does not survive an update, unlike the other RRD groups like system, traffic, packets, etc. Note that I mean update from one 2.2 snapshot to the next, not the update from 2.1.3 to 2.2 (which I didn't try).
Sorry I haven't looked into it any deeper; I hate posting a problem without a solution … but your post reminded me.
-
I also noticed that a restart of service sometimes, but a reconfiguration of NTP settings also deletes the entire RRD data, have no clue why.
-
I also noticed that a restart of service sometimes, but a reconfiguration of NTP settings also deletes the entire RRD data, have no clue why.
Found it, I think … in /etc/inc/rrd.inc:
/* NTP, set up the ntpd rrd file */ if (isset($config['ntpd']['statsgraph'])) { /* set up the ntpd rrd file */ if (!file_exists("$rrddbpath$ifname$ntpd")) { $rrdcreate = "$rrdtool create $rrddbpath$ntpd --step $rrdntpdinterval "; $rrdcreate .= "DS:offset:GAUGE:$ntpdvalid:0:1000 "; ...There shouldn't be the '$ifname' in the check for the existing file, since ntpd stats are not per interface. From the previous captive portal stanza, $ifname is set to 'captiveportal' at this point anyway. So '/var/db/rrd/captiveportalntpd.rrd' is of course not found, and a new ntpd.rrd file is created every time through this code. Seems to be the case for both 2.2 and your 2.1.3 patch.
-
Also, I believe that UNKNOWN values need to be written in more data set fields during boot:
/* set up the ntpd rrd file */ if (!file_exists("$rrddbpath$ntpd")) { $rrdcreate = "$rrdtool create $rrddbpath$ntpd --step $rrdntpdinterval "; $rrdcreate .= "DS:offset:GAUGE:$ntpdvalid:0:1000 "; $rrdcreate .= "DS:sjit:GAUGE:$ntpdvalid:0:1000 "; $rrdcreate .= "DS:cjit:GAUGE:$ntpdvalid:0:1000 "; $rrdcreate .= "DS:wander:GAUGE:$ntpdvalid:0:1000 "; $rrdcreate .= "DS:freq:GAUGE:$ntpdvalid:0:1000 "; $rrdcreate .= "DS:disp:GAUGE:$ntpdvalid:0:1000 "; $rrdcreate .= "RRA:MIN:0.5:1:1200 "; $rrdcreate .= "RRA:MIN:0.5:5:720 "; $rrdcreate .= "RRA:MIN:0.5:60:1860 "; $rrdcreate .= "RRA:MIN:0.5:1440:2284 "; $rrdcreate .= "RRA:AVERAGE:0.5:1:1200 "; $rrdcreate .= "RRA:AVERAGE:0.5:5:720 "; $rrdcreate .= "RRA:AVERAGE:0.5:60:1860 "; $rrdcreate .= "RRA:AVERAGE:0.5:1440:2284 "; $rrdcreate .= "RRA:MAX:0.5:1:1200 "; $rrdcreate .= "RRA:MAX:0.5:5:720 "; $rrdcreate .= "RRA:MAX:0.5:60:1860 "; $rrdcreate .= "RRA:MAX:0.5:1440:2284 "; create_new_rrd($rrdcreate); unset($rrdcreate); } /* enter UNKNOWN values in the RRD so it knows we rebooted. */ if($g['booting']) { mwexec("$rrdtool update $rrddbpath$ntpd N:U"); }The update line should be:
mwexec("$rrdtool update $rrddbpath$ntpd N:U:U:U:U:U:U"); -
Can you please attach your modified /etc/inc/rrd.inc?
-
Can you please attach your modified /etc/inc/rrd.inc?
How about a patch?
[2.2-ALPHA][root@pfsense.localdomain]/etc/inc(47): diff -ub rrd.inc rrd_new.inc --- rrd.inc 2014-04-29 11:57:57.000000000 -0400 +++ rrd_new.inc 2014-05-28 12:50:48.000000000 -0400 @@ -849,7 +849,7 @@ /* NTP, set up the ntpd rrd file */ if (isset($config['ntpd']['statsgraph'])) { /* set up the ntpd rrd file */ - if (!file_exists("$rrddbpath$ifname$ntpd")) { + if (!file_exists("$rrddbpath$ntpd")) { $rrdcreate = "$rrdtool create $rrddbpath$ntpd --step $rrdntpdinterval "; $rrdcreate .= "DS:offset:GAUGE:$ntpdvalid:0:1000 "; $rrdcreate .= "DS:sjit:GAUGE:$ntpdvalid:0:1000 "; @@ -876,7 +876,7 @@ /* enter UNKNOWN values in the RRD so it knows we rebooted. */ if($g['booting']) { - mwexec("$rrdtool update $rrddbpath$ntpd N:U"); + mwexec("$rrdtool update $rrddbpath$ntpd N:U:U:U:U:U:U"); } /* the ntp stats gathering function. */ -
Nice catch 8) Seems to work.
-
Here's the entire patch updated and the zipped pack too.
I'll submit it to GitHub too. -
Thank-you Rob.
What adjustments do these commands do? Why did you have to change them? There's a mistake for sure for example on the second line, checksum for that is not 23 but 2D. Didn't check all of them, but PMTK301,2 has the correct checksum of 2E, not 20.
I thought when you tested the checksum in the box below that the correct value would show. Each of the above commands had a different checksum so I changed them all from default.
Apologies; just noticed that I had typed an extra character to one side fat fingering it. Checksum numbers match fine.
Actually I use default settings (the ones which come when you select the model in the GPS pulldown), they all have already correct checksums precalculated. I'd say you should delete the whole thing, select "Generic" from the pulldown, then select "SureGPS" again, to re-load the defaults and press Save.
Will do.
Don't know about the php error you see on that page, have you done any other modifications to your pfSense system? I use this patch on several machines in production, none of them shows this.
I have not done any modifications. I did rebuild it from scratch and instead of uploading old configuration I added my stuff manually (rules and all).
Yeah I do not see it all of the time and thought it might be a Firefox thing.
Tried it on a couple of other PCs running Firefox and GUI looks fine on them.
-
Saw the error in the GUI come up again after I saved configuration.
Warning: substr_count(): Empty substring in /etc/inc/system.inc on line 1457 Warning: substr_count(): Empty substring in /etc/inc/system.inc on line 1458
-
Saw the error in the GUI come up again after I saved configuration.
Warning: substr_count(): Empty substring in /etc/inc/system.inc on line 1457 Warning: substr_count(): Empty substring in /etc/inc/system.inc on line 1458
In the NTP configuration, on the NTP page, do you have any NTP servers configured? Also, in General Setup, what do you see in the "NTP time server" textbox?
-
In the NTP configuration, on the NTP page, do you have any NTP servers configured?
no
Also, in General Setup, what do you see in the "NTP time server" textbox?
pool.ntp.org
-
Thanks, I found the problem for your weirdness and fixed it, attachments below.
It's hardcoded in pfSense's system.php that if for some reason no NTP server is present in the config file, pool.ntp.org is used by default. That's why you were seeing it in General Setup, and when you press Save there, pool.ntp.org goes into the config file.
However, in NTP settings, you could delete all the NTP servers from the list, but the code expects in return when parsing the config file, to exist at least one. That's what generated your error.
I fixed it by adding the same pool.ntp.org server as last resort, if someone deletes all the servers. If you delete all the servers and press Save, the system will automatically add pool.ntp.org, and you will see that in the first row.This results in keeping the original idea in pfSense, that you can't run without at least one NTP server configured. If one still wants to run without any external NTP servers, I guess he/she can use 'localhost' instead or some any dummy IP address or hostname which cannot be resolved. Not too nice though.
-
It's worth a discussion and a decision with pfSense's main devs, to allow no external NTP servers configured, IF there's a local GPS time source. That would involve a simple logic to check if a GPS time source is already configured, and in both NTP settings and in General Setup allow to have empty list of servers.
Moreover, it could even be decided to completely remove the "NTP time server" option from General Setup, since the NTP page already contains advanced configuration for NTP stuff.
Also there's still present the "Default" type of GPS receiver in the list, which is copied from the original pfSense 2.1's code. Dagorlad has put this text on the page:
Note: Default is the configuration of pfSense 2.1 and earlier (not recommended). Select Generic if your GPS is not listed.)
I think we should clear this up, what was the type/model of the GPS receiver used originally when Serial GPS was originally added to pfSense 2.1? I'd add the correct name to reduce confusion and remove the warning text.
Jimp? Can you please consider the above?
-
This results in keeping the original idea in pfSense, that you can't run without at least one NTP server configured. If one still wants to run without any external NTP servers, I guess he/she can use 'localhost' instead or some any dummy IP address or hostname which cannot be resolved. Not too nice though.
I don't remember seeing this documented as a requirement! While it's certainly a good recommendation, should it actually be required?
-
Thank-you Robi!
I am fine using one external NTP server as a failover. A NTP internal only switch that worked would also be a nice thing to have.
Its been a few years here but started initially with an old "surplus" Trimble GPS (well it was made to be inside of a tank) using this one for many years went to using the SureGPS until the USB power port fell off of the board. I baked the board last year and was able to get it to work.
BTW still got some errors using GGA such that I went back to using ZDA/ZDG.
IE: I was seeing bad data using GGA here.
ntpq> cv
assID=0 status=0012 clk_okay, last_clk_18,
device="NMEA GPS Clock", timecode="$GPZDA,123914.000,29,05,2014,,*53",
poll=7, noreply=0, badformat=1, baddata=0, fudgetime2=400.000,
stratum=0, refid=GPS, flags=5 -
Also there's still present the "Default" type of GPS receiver in the list, which is copied from the original pfSense 2.1's code. Dagorlad has put this text on the page:
Note: Default is the configuration of pfSense 2.1 and earlier (not recommended). Select Generic if your GPS is not listed.)
I think we should clear this up, what was the type/model of the GPS receiver used originally when Serial GPS was originally added to pfSense 2.1? I'd add the correct name to reduce confusion and remove the warning text.
Jimp? Can you please consider the above?
It's for a GPS with a uBlox chipset; I mentioned it in my first post about GPS bugs here:
https://forum.pfsense.org/index.php?topic=67189.msg367460#msg367460 I believe JimP later posted that it was added because it was funded by a customer. IMHO it should not be there by defaultAnother outstanding 'design' question is whether we should be saving clockstats by default. They are not used by any installed program or available package AFAIK, and grow without bounds, as there's no cron job to compress/remove/consolidate them. I think anyone who wants them can certainly turn them on. I first brought it up here:
https://forum.pfsense.org/index.php?topic=67189.msg373669#msg373669 and more here:
https://forum.pfsense.org/index.php?topic=67189.msg373841#msg373841 -
A NTP internal only switch that worked would also be a nice thing to have.
+1 on that. If said check box is only on the GPS settings page it's only ever likely to be set by people who understand what they're doing (or think they do ;)).
I thought the clockstats thing had been taken care of? Hmm, clearly I'm not paying enough attention. There was a manual tweak to disable it though right? That didn't make it into the package?
Steve
-
BTW still got some errors using GGA such that I went back to using ZDA/ZDG.
IE: I was seeing bad data using GGA here.
It seems to me that badformat error is due to a bug in ntp, the SureGPS sends the sentences with an extra empty line between them, and it seems to me that the nmea driver gets confused by this. It should either be fixed in ntp-nmea driver, or the SureGPS firmware.
However, the number of badformats is rather small (got about 26 during a day), and since the PPS signal is the actual source for exact timing, I wouldn't worry about it. The time is kept very exact anyway.
