Ntpd / gps need some love part II
-
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.
If applied patch using system patches package, revert the old patch, and apply this new one, or simply overwrite the files on the system, and press "Save" on the Serial GPS page.
Pushed the fixes to GitHub also, for 2.2.(Edit: attachments removed, read further for updates)
-
Thanks Rob. For the update posted do I need to recalculate the checksum stuff?
I did notice while using FF after the save on the serial GPS page; the page looked a bit weird.
Here is the adjusted checksums for the SureGPS.
$PMTK225,025
$PMTK314,1,1,0,1,0,5,0,0,0,0,0,0,0,0,0,0,0,1,023
$PMTK301,220
$PMTK397,02D
$PMTK1023F
$PMTK313,120
$PMTK513,126
$PMTK319,02B
$PMTK527,0.000E
$PMTK251,960019
-
Love the annotation. ;D
Steve
-
Thanks Rob. For the update posted do I need to recalculate the checksum stuff?
I did notice while using FF after the save on the serial GPS page; the page looked a bit weird.
Here is the adjusted checksums for the SureGPS.
$PMTK225,025
$PMTK314,1,1,0,1,0,5,0,0,0,0,0,0,0,0,0,0,0,1,023
$PMTK301,220
$PMTK397,02D
$PMTK1023F
$PMTK313,120
$PMTK513,126
$PMTK319,02B
$PMTK527,0.000E
$PMTK251,960019What 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.
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.
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.
-
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?
