On Fri, Dec 06, 2019 at 05:46:52PM +0100, Claudio Jeker wrote:
> The output processing in bgpctl is not very extensible. And output flags
> like ssv have to hacked into the output in a bad way.
> 
> This is the first bit of a much bigger shuffling action to make the output
> handling more extensible. For now this just moves the header and body
> generation into two functions (show and show_head). The generic response
> handling and the end messages are now handled a bit different. The rest of
> the code was not yet modified to keep the diff small.
> 
> OK?

OK denis@

You can also remove show_rib_summary_head() I guess.

> -- 
> :wq Claudio
> 
> Index: bgpctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.247
> diff -u -p -r1.247 bgpctl.c
> --- bgpctl.c  27 Nov 2019 01:23:30 -0000      1.247
> +++ bgpctl.c  6 Dec 2019 16:38:54 -0000
> @@ -49,10 +49,11 @@ enum neighbor_views {
>  #define EOL0(flag)   ((flag & F_CTL_SSV) ? ';' : '\n')
>  
>  int           main(int, char *[]);
> -char         *fmt_peer(const char *, const struct bgpd_addr *, int, int);
> +int           show(struct imsg *, struct parse_result *);
> +char         *fmt_peer(const char *, const struct bgpd_addr *, int);
>  void          show_summary_head(void);
> -int           show_summary_msg(struct imsg *, int);
> -int           show_summary_terse_msg(struct imsg *, int);
> +int           show_summary_msg(struct imsg *);
> +int           show_summary_terse_msg(struct imsg *);
>  int           show_neighbor_terse(struct imsg *);
>  int           show_neighbor_msg(struct imsg *, enum neighbor_views);
>  void          print_neighbor_capa_mp(struct peer *);
> @@ -76,10 +77,9 @@ const char *        print_origin(u_int8_t, int
>  const char *  print_ovs(u_int8_t, int);
>  void          print_flags(u_int8_t, int);
>  int           show_rib_summary_msg(struct imsg *);
> -int           show_rib_detail_msg(struct imsg *, int, int);
> +int           show_rib_detail_msg(struct imsg *, int);
>  void          show_rib_brief(struct ctl_show_rib *, u_char *, size_t);
> -void          show_rib_detail(struct ctl_show_rib *, u_char *, size_t, int,
> -                 int);
> +void          show_rib_detail(struct ctl_show_rib *, u_char *, size_t, int);
>  void          show_attr(void *, u_int16_t, int);
>  void          show_communities(u_char *, size_t, int);
>  void          show_community(u_char *, u_int16_t);
> @@ -88,7 +88,6 @@ void                 show_ext_community(u_char *, u_in
>  int           show_rib_memory_msg(struct imsg *);
>  void          send_filterset(struct imsgbuf *, struct filter_set_head *);
>  const char   *get_errstr(u_int8_t, u_int8_t);
> -int           show_result(struct imsg *);
>  void          show_mrt_dump_neighbors(struct mrt_rib *, struct mrt_peer *,
>                   void *);
>  void          show_mrt_dump(struct mrt_rib *, struct mrt_peer *, void *);
> @@ -99,11 +98,14 @@ const char        *msg_type(u_int8_t);
>  void          network_bulk(struct parse_result *);
>  const char   *print_auth_method(enum auth_method);
>  int           match_aspath(void *, u_int16_t, struct filter_as *);
> +void          show_head(struct parse_result *);
> +void          show_result(u_int);
>  
>  struct imsgbuf       *ibuf;
>  struct mrt_parser show_mrt = { show_mrt_dump, show_mrt_state, show_mrt_msg };
>  struct mrt_parser net_mrt = { network_mrt_dump, NULL, NULL };
>  int tableid;
> +int nodescr;
>  
>  __dead void
>  usage(void)
> @@ -119,7 +121,7 @@ int
>  main(int argc, char *argv[])
>  {
>       struct sockaddr_un       sun;
> -     int                      fd, n, done, ch, nodescr = 0, verbose = 0;
> +     int                      fd, n, done, ch, verbose = 0;
>       struct imsg              imsg;
>       struct network_config    net;
>       struct parse_result     *res;
> @@ -180,8 +182,8 @@ main(int argc, char *argv[])
>               show_mrt.arg = &ribreq;
>               if (res->flags & F_CTL_NEIGHBORS)
>                       show_mrt.dump = show_mrt_dump_neighbors;
> -             else if (!(res->flags & F_CTL_DETAIL))
> -                     show_rib_summary_head();
> +             else
> +                     show_head(res);
>               mrt_parse(res->mrtfd, &show_mrt, 1);
>               exit(0);
>       default:
> @@ -218,7 +220,6 @@ main(int argc, char *argv[])
>       case SHOW:
>       case SHOW_SUMMARY:
>               imsg_compose(ibuf, IMSG_CTL_SHOW_NEIGHBOR, 0, 0, -1, NULL, 0);
> -             show_summary_head();
>               break;
>       case SHOW_SUMMARY_TERSE:
>               imsg_compose(ibuf, IMSG_CTL_SHOW_TERSE, 0, 0, -1, NULL, 0);
> @@ -241,20 +242,16 @@ main(int argc, char *argv[])
>               } else
>                       imsg_compose(ibuf, IMSG_CTL_KROUTE_ADDR, res->rtableid,
>                           0, -1, &res->addr, sizeof(res->addr));
> -             show_fib_head();
>               break;
>       case SHOW_FIB_TABLES:
>               imsg_compose(ibuf, IMSG_CTL_SHOW_FIB_TABLES, 0, 0, -1, NULL, 0);
> -             show_fib_tables_head();
>               break;
>       case SHOW_NEXTHOP:
>               imsg_compose(ibuf, IMSG_CTL_SHOW_NEXTHOP, res->rtableid, 0, -1,
>                   NULL, 0);
> -             show_nexthop_head();
>               break;
>       case SHOW_INTERFACE:
>               imsg_compose(ibuf, IMSG_CTL_SHOW_INTERFACE, 0, 0, -1, NULL, 0);
> -             show_interface_head();
>               break;
>       case SHOW_NEIGHBOR:
>       case SHOW_NEIGHBOR_TIMERS:
> @@ -284,8 +281,6 @@ main(int argc, char *argv[])
>               ribreq.aid = res->aid;
>               ribreq.flags = res->flags;
>               imsg_compose(ibuf, type, 0, 0, -1, &ribreq, sizeof(ribreq));
> -             if (!(res->flags & F_CTL_DETAIL))
> -                     show_rib_summary_head();
>               break;
>       case SHOW_RIB_MEM:
>               imsg_compose(ibuf, IMSG_CTL_SHOW_RIB_MEM, 0, 0, -1, NULL, 0);
> @@ -368,7 +363,6 @@ main(int argc, char *argv[])
>               strlcpy(ribreq.rib, res->rib, sizeof(ribreq.rib));
>               imsg_compose(ibuf, IMSG_CTL_SHOW_NETWORK, 0, 0, -1,
>                   &ribreq, sizeof(ribreq));
> -             show_network_head();
>               break;
>       case NETWORK_MRT:
>               bzero(&ribreq, sizeof(ribreq));
> @@ -401,6 +395,8 @@ main(int argc, char *argv[])
>               if (msgbuf_write(&ibuf->w) <= 0 && errno != EAGAIN)
>                       err(1, "write error");
>  
> +     show_head(res);
> +
>       while (!done) {
>               if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
>                       err(1, "imsg_read error");
> @@ -413,72 +409,7 @@ main(int argc, char *argv[])
>                       if (n == 0)
>                               break;
>  
> -                     if (imsg.hdr.type == IMSG_CTL_RESULT) {
> -                             done = show_result(&imsg);
> -                             imsg_free(&imsg);
> -                             continue;
> -                     }
> -
> -                     switch (res->action) {
> -                     case SHOW:
> -                     case SHOW_SUMMARY:
> -                             done = show_summary_msg(&imsg, nodescr);
> -                             break;
> -                     case SHOW_SUMMARY_TERSE:
> -                             done = show_summary_terse_msg(&imsg, nodescr);
> -                             break;
> -                     case SHOW_FIB:
> -                     case SHOW_FIB_TABLES:
> -                     case NETWORK_SHOW:
> -                             done = show_fib_msg(&imsg);
> -                             break;
> -                     case SHOW_NEXTHOP:
> -                             done = show_nexthop_msg(&imsg);
> -                             break;
> -                     case SHOW_INTERFACE:
> -                             done = show_interface_msg(&imsg);
> -                             break;
> -                     case SHOW_NEIGHBOR:
> -                             done = show_neighbor_msg(&imsg, NV_DEFAULT);
> -                             break;
> -                     case SHOW_NEIGHBOR_TIMERS:
> -                             done = show_neighbor_msg(&imsg, NV_TIMERS);
> -                             break;
> -                     case SHOW_NEIGHBOR_TERSE:
> -                             done = show_neighbor_terse(&imsg);
> -                             break;
> -                     case SHOW_RIB:
> -                             if (res->flags & F_CTL_DETAIL)
> -                                     done = show_rib_detail_msg(&imsg,
> -                                         nodescr, res->flags);
> -                             else
> -                                     done = show_rib_summary_msg(&imsg);
> -                             break;
> -                     case SHOW_RIB_MEM:
> -                             done = show_rib_memory_msg(&imsg);
> -                             break;
> -                     case NEIGHBOR:
> -                     case NEIGHBOR_UP:
> -                     case NEIGHBOR_DOWN:
> -                     case NEIGHBOR_CLEAR:
> -                     case NEIGHBOR_RREFRESH:
> -                     case NEIGHBOR_DESTROY:
> -                     case NONE:
> -                     case RELOAD:
> -                     case FIB:
> -                     case FIB_COUPLE:
> -                     case FIB_DECOUPLE:
> -                     case NETWORK_ADD:
> -                     case NETWORK_REMOVE:
> -                     case NETWORK_FLUSH:
> -                     case NETWORK_BULK_ADD:
> -                     case NETWORK_BULK_REMOVE:
> -                     case LOG_VERBOSE:
> -                     case LOG_BRIEF:
> -                     case SHOW_MRT:
> -                     case NETWORK_MRT:
> -                             break;
> -                     }
> +                     done = show(&imsg, res);
>                       imsg_free(&imsg);
>               }
>       }
> @@ -488,9 +419,93 @@ main(int argc, char *argv[])
>       exit(0);
>  }
>  
> +int
> +show(struct imsg *imsg, struct parse_result *res)
> +{
> +     u_int rescode;
> +     int done = 0;
> +
> +     switch (imsg->hdr.type) {
> +     case IMSG_CTL_RESULT:
> +             if (imsg->hdr.len != IMSG_HEADER_SIZE + sizeof(rescode)) {
> +                     warnx("got IMSG_CTL_RESULT with wrong len");
> +                     break;
> +             }
> +             memcpy(&rescode, imsg->data, sizeof(rescode));
> +             show_result(rescode);
> +             return (1);
> +     case IMSG_CTL_END:
> +             return (1);
> +     default:
> +             break;
> +     }
> +
> +     switch (res->action) {
> +     case SHOW:
> +     case SHOW_SUMMARY:
> +             done = show_summary_msg(imsg);
> +             break;
> +     case SHOW_SUMMARY_TERSE:
> +             done = show_summary_terse_msg(imsg);
> +             break;
> +     case SHOW_FIB:
> +     case SHOW_FIB_TABLES:
> +     case NETWORK_SHOW:
> +             done = show_fib_msg(imsg);
> +             break;
> +     case SHOW_NEXTHOP:
> +             done = show_nexthop_msg(imsg);
> +             break;
> +     case SHOW_INTERFACE:
> +             done = show_interface_msg(imsg);
> +             break;
> +     case SHOW_NEIGHBOR:
> +             done = show_neighbor_msg(imsg, NV_DEFAULT);
> +             break;
> +     case SHOW_NEIGHBOR_TIMERS:
> +             done = show_neighbor_msg(imsg, NV_TIMERS);
> +             break;
> +     case SHOW_NEIGHBOR_TERSE:
> +             done = show_neighbor_terse(imsg);
> +             break;
> +     case SHOW_RIB:
> +             if (res->flags & F_CTL_DETAIL)
> +                     done = show_rib_detail_msg(imsg,
> +                         res->flags);
> +             else
> +                     done = show_rib_summary_msg(imsg);
> +             break;
> +     case SHOW_RIB_MEM:
> +             done = show_rib_memory_msg(imsg);
> +             break;
> +     case NEIGHBOR:
> +     case NEIGHBOR_UP:
> +     case NEIGHBOR_DOWN:
> +     case NEIGHBOR_CLEAR:
> +     case NEIGHBOR_RREFRESH:
> +     case NEIGHBOR_DESTROY:
> +     case NONE:
> +     case RELOAD:
> +     case FIB:
> +     case FIB_COUPLE:
> +     case FIB_DECOUPLE:
> +     case NETWORK_ADD:
> +     case NETWORK_REMOVE:
> +     case NETWORK_FLUSH:
> +     case NETWORK_BULK_ADD:
> +     case NETWORK_BULK_REMOVE:
> +     case LOG_VERBOSE:
> +     case LOG_BRIEF:
> +     case SHOW_MRT:
> +     case NETWORK_MRT:
> +             break;
> +     }
> +     return (done);
> +}
> +
>  char *
>  fmt_peer(const char *descr, const struct bgpd_addr *remote_addr,
> -    int masklen, int nodescr)
> +    int masklen)
>  {
>       const char      *ip;
>       char            *p;
> @@ -522,7 +537,7 @@ show_summary_head(void)
>  }
>  
>  int
> -show_summary_msg(struct imsg *imsg, int nodescr)
> +show_summary_msg(struct imsg *imsg)
>  {
>       struct peer             *p;
>       char                    *s;
> @@ -533,7 +548,7 @@ show_summary_msg(struct imsg *imsg, int 
>       case IMSG_CTL_SHOW_NEIGHBOR:
>               p = imsg->data;
>               s = fmt_peer(p->conf.descr, &p->conf.remote_addr,
> -                 p->conf.remote_masklen, nodescr);
> +                 p->conf.remote_masklen);
>  
>               a = log_as(p->conf.remote_as);
>               alen = strlen(a);
> @@ -575,7 +590,7 @@ show_summary_msg(struct imsg *imsg, int 
>  }
>  
>  int
> -show_summary_terse_msg(struct imsg *imsg, int nodescr)
> +show_summary_terse_msg(struct imsg *imsg)
>  {
>       struct peer             *p;
>       char                    *s;
> @@ -584,7 +599,7 @@ show_summary_terse_msg(struct imsg *imsg
>       case IMSG_CTL_SHOW_NEIGHBOR:
>               p = imsg->data;
>               s = fmt_peer(p->conf.descr, &p->conf.remote_addr,
> -                 p->conf.remote_masklen, nodescr);
> +                 p->conf.remote_masklen);
>               printf("%s %s %s\n", s, log_as(p->conf.remote_as),
>                   p->conf.template ? "Template" : statenames[p->state]);
>               free(s);
> @@ -1251,7 +1266,7 @@ show_rib_summary_msg(struct imsg *imsg)
>  }
>  
>  int
> -show_rib_detail_msg(struct imsg *imsg, int nodescr, int flag0)
> +show_rib_detail_msg(struct imsg *imsg, int flag0)
>  {
>       struct ctl_show_rib      rib;
>       u_char                  *asdata;
> @@ -1264,7 +1279,7 @@ show_rib_detail_msg(struct imsg *imsg, i
>               asdata = imsg->data;
>               asdata += sizeof(rib);
>               aslen = imsg->hdr.len - IMSG_HEADER_SIZE - sizeof(rib);
> -             show_rib_detail(&rib, asdata, aslen, nodescr, flag0);
> +             show_rib_detail(&rib, asdata, aslen, flag0);
>               break;
>       case IMSG_CTL_SHOW_RIB_COMMUNITIES:
>               ilen = imsg->hdr.len - IMSG_HEADER_SIZE;
> @@ -1308,7 +1323,7 @@ show_rib_brief(struct ctl_show_rib *r, u
>  
>  void
>  show_rib_detail(struct ctl_show_rib *r, u_char *asdata, size_t aslen,
> -    int nodescr, int flag0)
> +    int flag0)
>  {
>       struct in_addr           id;
>       char                    *aspath, *s;
> @@ -1324,7 +1339,7 @@ show_rib_detail(struct ctl_show_rib *r, 
>               printf("    %s%c", aspath, EOL0(flag0));
>       free(aspath);
>  
> -     s = fmt_peer(r->descr, &r->remote_addr, -1, nodescr);
> +     s = fmt_peer(r->descr, &r->remote_addr, -1);
>       printf("    Nexthop %s ", log_addr(&r->exit_nexthop));
>       printf("(via %s) Neighbor %s (", log_addr(&r->true_nexthop), s);
>       free(s);
> @@ -2003,28 +2018,6 @@ get_errstr(u_int8_t errcode, u_int8_t su
>       return (errstr);
>  }
>  
> -int
> -show_result(struct imsg *imsg)
> -{
> -     u_int   rescode;
> -
> -     if (imsg->hdr.len != IMSG_HEADER_SIZE + sizeof(rescode))
> -             errx(1, "got IMSG_CTL_RESULT with wrong len");
> -     memcpy(&rescode, imsg->data, sizeof(rescode));
> -
> -     if (rescode == 0)
> -             printf("request processed\n");
> -     else {
> -             if (rescode >
> -                 sizeof(ctl_res_strerror)/sizeof(ctl_res_strerror[0]))
> -                     printf("unknown result error code %u\n", rescode);
> -             else
> -                     printf("%s\n", ctl_res_strerror[rescode]);
> -     }
> -
> -     return (1);
> -}
> -
>  void
>  network_bulk(struct parse_result *res)
>  {
> @@ -2163,8 +2156,7 @@ show_mrt_dump(struct mrt_rib *mr, struct
>                       continue;
>  
>               if (req->flags & F_CTL_DETAIL) {
> -                     show_rib_detail(&ctl, mre->aspath, mre->aspath_len, 1,
> -                         0);
> +                     show_rib_detail(&ctl, mre->aspath, mre->aspath_len, 0);
>                       for (j = 0; j < mre->nattrs; j++)
>                               show_attr(mre->attrs[j].attr,
>                                   mre->attrs[j].attr_len,
> @@ -2798,4 +2790,69 @@ match_aspath(void *data, u_int16_t len, 
>               }
>       }
>       return (0);
> +}
> +
> +void
> +show_head(struct parse_result *res)
> +{
> +     switch (res->action) {
> +     case SHOW:
> +     case SHOW_SUMMARY:
> +             printf("%-20s %8s %10s %10s %5s %-8s %s\n", "Neighbor", "AS",
> +                 "MsgRcvd", "MsgSent", "OutQ", "Up/Down", "State/PrfRcvd");
> +             break;
> +     case SHOW_FIB:
> +             printf("flags: * = valid, B = BGP, C = Connected, "
> +                 "S = Static, D = Dynamic\n");
> +             printf("       "
> +                 "N = BGP Nexthop reachable via this route\n");
> +             printf("       r = reject route, b = blackhole route\n\n");
> +             printf("flags prio destination          gateway\n");
> +             break;
> +     case SHOW_FIB_TABLES:
> +             printf("%-5s %-20s %-8s\n", "Table", "Description", "State");
> +             break;
> +     case SHOW_NEXTHOP:
> +             printf("Flags: * = nexthop valid\n");
> +             printf("\n  %-15s %-19s%-4s %-15s %-20s\n", "Nexthop", "Route",
> +                  "Prio", "Gateway", "Iface");
> +             break;
> +     case SHOW_INTERFACE:
> +             printf("%-15s%-9s%-9s%-7s%s\n", "Interface", "rdomain",
> +                 "Nexthop", "Flags", "Link state");
> +             break;
> +     case SHOW_RIB:
> +             if (res->flags & F_CTL_DETAIL)
> +                     break;
> +             printf("flags: "
> +                 "* = Valid, > = Selected, I = via IBGP, A = Announced,\n"
> +                 "       S = Stale, E = Error\n");
> +             printf("origin validation state: "
> +                 "N = not-found, V = valid, ! = invalid\n");
> +             printf("origin: i = IGP, e = EGP, ? = Incomplete\n\n");
> +             printf("%-5s %3s %-20s %-15s  %5s %5s %s\n",
> +                 "flags", "ovs", "destination", "gateway", "lpref", "med",
> +                 "aspath origin");
> +             break;
> +     case NETWORK_SHOW:
> +             printf("flags: S = Static\n");
> +             printf("flags prio destination          gateway\n");
> +             break;
> +     default:
> +             break;
> +     }
> +}
> +
> +void
> +show_result(u_int rescode)
> +{
> +     if (rescode == 0)
> +             printf("request processed\n");
> +     else {
> +             if (rescode >
> +                 sizeof(ctl_res_strerror)/sizeof(ctl_res_strerror[0]))
> +                     printf("unknown result error code %u\n", rescode);
> +             else
> +                     printf("%s\n", ctl_res_strerror[rescode]);
> +     }
>  }
> 

Reply via email to