On Fri, Apr 30, 2021 at 10:03:49PM -0500, Scott Cheloha wrote:
> > On Apr 28, 2021, at 03:47, Claudio Jeker <[email protected]> wrote:
> >
> > There are various time fields in the JSON output.
> > last_read, last_write, last_updown on sessions, last_update for rib
> > entries and last_change for sets. Currently the value is the fmt_timeframe
> > string (which looks something like 7w3d12h) and is hard to parse for
> > machines. Include an additional _sec value which is the same value in
> > seconds.
> >
> > OK?
>
> One thing bugs me about this. Below.
>
> > --
> > :wq Claudio
> >
> > Index: bgpctl.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> > retrieving revision 1.266
> > diff -u -p -r1.266 bgpctl.c
> > --- bgpctl.c 15 Apr 2021 14:12:05 -0000 1.266
> > +++ bgpctl.c 28 Apr 2021 08:34:53 -0000
> > @@ -510,6 +510,20 @@ show(struct imsg *imsg, struct parse_res
> > return (0);
> > }
> >
> > +time_t
> > +get_monotime(time_t t)
> > +{
> > + struct timespec ts;
> > +
> > + if (t == 0)
> > + return -1;
> > + if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0)
> > + err(1, "clock_gettime");
> > + if (t > ts.tv_sec) /* time in the future is not possible */
> > + t = ts.tv_sec;
>
> Why are you checking for this? How would
> it happen?
>
> Would it be better to err(3) if this happens?
> So we catch the false assumption instead of
> just ignoring it?
While probably not possible this is portable software and the timestamps
are generated in a different process then bgpctl. I think just printing 0
here is better than using errx(1, "give me back my delorian") error.
> > + return (ts.tv_sec - t);
> > +}
> > +
> > char *
> > fmt_peer(const char *descr, const struct bgpd_addr *remote_addr,
> > int masklen)
> > @@ -596,16 +610,12 @@ fmt_timeframe(time_t t)
> > const char *
> > fmt_monotime(time_t t)
> > {
> > - struct timespec ts;
> > + t = get_monotime(t);
> >
> > - if (t == 0)
> > + if (t == -1)
> > return ("Never");
> >
> > - if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0)
> > - err(1, "clock_gettime");
> > - if (t > ts.tv_sec) /* time in the future is not possible */
> > - t = ts.tv_sec;
> > - return (fmt_timeframe(ts.tv_sec - t));
> > + return (fmt_timeframe(t));
> > }
> >
> > const char *
> > Index: bgpctl.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.h,v
> > retrieving revision 1.10
> > diff -u -p -r1.10 bgpctl.h
> > --- bgpctl.h 15 Apr 2021 14:12:05 -0000 1.10
> > +++ bgpctl.h 28 Apr 2021 08:34:33 -0000
> > @@ -41,6 +41,7 @@ extern const size_t pt_sizes[];
> >
> > #define EOL0(flag) ((flag & F_CTL_SSV) ? ';' : '\n')
> >
> > +time_t get_monotime(time_t);
> > char *fmt_peer(const char *, const struct bgpd_addr *, int);
> > const char *fmt_timeframe(time_t);
> > const char *fmt_monotime(time_t);
> > Index: output_json.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpctl/output_json.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 output_json.c
> > --- output_json.c 15 Apr 2021 14:12:05 -0000 1.9
> > +++ output_json.c 28 Apr 2021 08:38:42 -0000
> > @@ -104,7 +104,9 @@ json_neighbor_stats(struct peer *p)
> > {
> > json_do_object("stats");
> > json_do_printf("last_read", "%s", fmt_monotime(p->stats.last_read));
> > + json_do_int("last_read_sec", get_monotime(p->stats.last_read));
> > json_do_printf("last_write", "%s", fmt_monotime(p->stats.last_write));
> > + json_do_int("last_write_sec", get_monotime(p->stats.last_write));
> >
> > json_do_object("prefixes");
> > json_do_uint("sent", p->stats.prefix_out_cnt);
> > @@ -267,6 +269,7 @@ json_neighbor(struct peer *p, struct par
> > }
> > json_do_printf("state", "%s", statenames[p->state]);
> > json_do_printf("last_updown", "%s", fmt_monotime(p->stats.last_updown));
> > + json_do_int("last_updown_sec", get_monotime(p->stats.last_updown));
> >
> > switch (res->action) {
> > case SHOW:
> > @@ -827,6 +830,7 @@ json_rib(struct ctl_show_rib *r, u_char
> > json_do_uint("localpref", r->local_pref);
> > json_do_uint("weight", r->weight);
> > json_do_printf("last_update", "%s", fmt_timeframe(r->age));
> > + json_do_int("last_update_sec", r->age);
> >
> > /* keep the object open for communities and attribuites */
> > }
> > @@ -927,6 +931,7 @@ json_rib_set(struct ctl_show_set *set)
> > json_do_printf("name", "%s", set->name);
> > json_do_printf("type", "%s", fmt_set_type(set));
> > json_do_printf("last_change", "%s", fmt_monotime(set->lastchange));
> > + json_do_int("last_change_sec", get_monotime(set->lastchange));
> > if (set->type == ASNUM_SET) {
> > json_do_uint("num_ASnum", set->as_cnt);
> > } else {
> >
>
--
:wq Claudio
