On Mon, Mar 13, 2017 at 02:33:02PM +0100, Mike Belopuhov wrote:
> 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
On the one where we return the delta instead of an absolute time?
>
> > 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:
> >
>
