On Tue, Mar 07, 2017 at 10:36 +0100, Patrick Wildt wrote:
> On Tue, Mar 07, 2017 at 10:17:16AM +0100, Patrick Wildt wrote:
> > Hi,
> >
> > currently the pf status struct contains the time since pf was enabled as
> > seen on the wall clock. This means when time drifts, or is set to some
> > earlier value, the time will be off. If we use time since uptime it
> > always increments and shows how long pf has been running compared to
> > its uptime.
> >
> > Does this make sense? Opinions?
> >
> > Patrick
>
> Alternatively it might be nicer to still use the uptime, but only return
> the delta since it was enabled.
>
I see nothing wrong with this diff. OK mikeb
> diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
> index e241b11f6fc..ab8834e605c 100644
> --- a/sbin/pfctl/pfctl_parser.c
> +++ b/sbin/pfctl/pfctl_parser.c
> @@ -525,7 +525,7 @@ print_status(struct pf_status *s, int opts)
> char buf[PF_MD5_DIGEST_LENGTH * 2 + 1];
> static const char hex[] = "0123456789abcdef";
>
> - runtime = time(NULL) - s->since;
> + runtime = s->since;
> running = s->running ? "Enabled" : "Disabled";
>
> if (s->since) {
> diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
> index 56a43a55ab8..cd5be10e2a1 100644
> --- a/sys/net/pf_ioctl.c
> +++ b/sys/net/pf_ioctl.c
> @@ -994,7 +994,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> error = EEXIST;
> else {
> pf_status.running = 1;
> - pf_status.since = time_second;
> + pf_status.since = time_uptime;
> if (pf_status.stateid == 0) {
> pf_status.stateid = time_second;
> pf_status.stateid = pf_status.stateid << 32;
> @@ -1009,7 +1009,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> error = ENOENT;
> else {
> pf_status.running = 0;
> - pf_status.since = time_second;
> + pf_status.since = time_uptime;
> pf_remove_queues();
> DPFPRINTF(LOG_NOTICE, "pf: stopped");
> }
> @@ -1577,6 +1577,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> case DIOCGETSTATUS: {
> struct pf_status *s = (struct pf_status *)addr;
> bcopy(&pf_status, s, sizeof(struct pf_status));
> + s.since = time_uptime - pf_status.since;
> pfi_update_status(s->ifname, s);
> break;
> }
> @@ -1605,7 +1606,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> bzero(pf_status.counters, sizeof(pf_status.counters));
> bzero(pf_status.fcounters, sizeof(pf_status.fcounters));
> bzero(pf_status.scounters, sizeof(pf_status.scounters));
> - pf_status.since = time_second;
> + pf_status.since = time_uptime;
>
> break;
> }
> diff --git a/usr.bin/systat/pf.c b/usr.bin/systat/pf.c
> index 6e282bb7359..882985e6594 100644
> --- a/usr.bin/systat/pf.c
> +++ b/usr.bin/systat/pf.c
> @@ -229,7 +229,7 @@ print_pf(void)
> if (end > num_disp)
> end = num_disp;
>
> - tm = time(NULL) - s->since;
> + tm = s->since;
>
> ADD_LINE_S("pf", "Status", s->running ? "Enabled" : "Disabled");
> ADD_LINE_A("pf", "Since", tm);
> diff --git a/usr.sbin/snmpd/mib.c b/usr.sbin/snmpd/mib.c
> index f53d9379b07..7fb82f22cb5 100644
> --- a/usr.sbin/snmpd/mib.c
> +++ b/usr.sbin/snmpd/mib.c
> @@ -1650,7 +1650,6 @@ int
> mib_pfinfo(struct oid *oid, struct ber_oid *o, struct ber_element **elm)
> {
> struct pf_status s;
> - time_t runtime;
> char str[11];
>
> if (pf_get_stats(&s))
> @@ -1661,12 +1660,7 @@ mib_pfinfo(struct oid *oid, struct ber_oid *o, struct
> ber_element **elm)
> *elm = ber_add_integer(*elm, s.running);
> break;
> case 2:
> - if (s.since > 0)
> - runtime = time(NULL) - s.since;
> - else
> - runtime = 0;
> - runtime *= 100;
> - *elm = ber_add_integer(*elm, runtime);
> + *elm = ber_add_integer(*elm, s.since * 100);
> ber_set_header(*elm, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
> break;
> case 3:
>
