On Sun, Sep 30, 2018 at 01:49:45AM +0000, Job Snijders wrote:
> Dear all,
>
> This small patch exposes the origin validation state in 'bgpctl show
> rib' and 'bgpctl show rib detail'. This will help debugging, and draw
> attention to routing problems.
>
> I know we're weary of spending horizontal space, but I think spending 3
> chars to show the OV state (and as such make it a first class citizen)
> will be worth it. When I tried putting the ovs state in the 'flags'
> column it just didn't look right.
>
> Example output:
>
> $ bgpctl show rib
> flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
> S = Stale, E = Error
> origin validation state: N = not-found, V = valid, ! = invalid
> origin: i = IGP, e = EGP, ? = Incomplete
>
> flags ovs destination gateway lpref med aspath origin
> *> V 192.0.2.0/24 100.64.2.3 100 0 2 i
> *> ! 192.0.2.0/26 100.64.2.3 100 0 2 i
> $ bgpctl show rib detail
>
> BGP routing table entry for 192.0.2.0/24
> 2
> Nexthop 100.64.2.3 (via 100.64.2.3) from 100.64.2.3 (192.147.168.1)
> Origin IGP, metric 0, localpref 100, weight 0, ovs valid, external,
> valid, best
> Last update: 00:02:19 ago
>
> BGP routing table entry for 192.0.2.0/26
> 2
> Nexthop 100.64.2.3 (via 100.64.2.3) from 100.64.2.3 (192.147.168.1)
> Origin IGP, metric 0, localpref 100, weight 0, ovs invalid, external,
> valid, best
> Last update: 00:02:19 ago
>
> OK?
>
I would have prefered to have ovs status in the flags column but OK denis@.
> Kind regards,
>
> Job
>
> diff --git usr.sbin/bgpctl/bgpctl.c usr.sbin/bgpctl/bgpctl.c
> index 7275ea6a2c4..70648ed8f5b 100644
> --- usr.sbin/bgpctl/bgpctl.c
> +++ usr.sbin/bgpctl/bgpctl.c
> @@ -72,8 +72,9 @@ int show_nexthop_msg(struct imsg *);
> void show_interface_head(void);
> int show_interface_msg(struct imsg *);
> void show_rib_summary_head(void);
> -void print_prefix(struct bgpd_addr *, u_int8_t, u_int8_t);
> +void print_prefix(struct bgpd_addr *, u_int8_t, u_int8_t, u_int8_t);
> 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);
> @@ -183,6 +184,7 @@ main(int argc, char *argv[])
> ribreq.neighbor = neighbor;
> ribreq.aid = res->aid;
> ribreq.flags = res->flags;
> + ribreq.validation_state = res->validation_state;
> show_mrt.arg = &ribreq;
> if (!(res->flags & F_CTL_DETAIL))
> show_rib_summary_head();
> @@ -1183,17 +1185,20 @@ show_rib_summary_head(void)
> {
> 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 %-20s %-15s %5s %5s %s\n", "flags", "destination",
> + printf("%-5s %3s %-20s %-15s %5s %5s %s\n", "flags", "ovs",
> "destination",
> "gateway", "lpref", "med", "aspath origin");
> }
>
> void
> -print_prefix(struct bgpd_addr *prefix, u_int8_t prefixlen, u_int8_t flags)
> +print_prefix(struct bgpd_addr *prefix, u_int8_t prefixlen, u_int8_t flags,
> + u_int8_t ovs)
> {
> char *p;
>
> print_flags(flags, 1);
> + printf("%3s ", print_ovs(ovs, 1));
> if (asprintf(&p, "%s/%u", log_addr(prefix), prefixlen) == -1)
> err(1, NULL);
> printf("%-20s", p);
> @@ -1252,6 +1257,19 @@ print_flags(u_int8_t flags, int sum)
> }
> }
>
> +const char *
> +print_ovs(u_int8_t validation_state, int sum)
> +{
> + switch (validation_state) {
> + case ROA_INVALID:
> + return (sum ? "!" : "invalid");
> + case ROA_VALID:
> + return (sum ? "V" : "valid");
> + default:
> + return (sum ? "N" : "not-found");
> + }
> +}
> +
> int
> show_rib_summary_msg(struct imsg *imsg)
> {
> @@ -1309,7 +1327,7 @@ show_rib_brief(struct ctl_show_rib *r, u_char *asdata)
> {
> char *aspath;
>
> - print_prefix(&r->prefix, r->prefixlen, r->flags);
> + print_prefix(&r->prefix, r->prefixlen, r->flags, r->validation_state);
> printf(" %-15s ", log_addr(&r->exit_nexthop));
> printf(" %5u %5u ", r->local_pref, r->med);
>
> @@ -1346,8 +1364,9 @@ show_rib_detail(struct ctl_show_rib *r, u_char *asdata,
> int nodescr, int flag0)
> id.s_addr = htonl(r->remote_id);
> printf("%s)%c", inet_ntoa(id), EOL0(flag0));
>
> - printf(" Origin %s, metric %u, localpref %u, weight %u, ",
> - print_origin(r->origin, 0), r->med, r->local_pref, r->weight);
> + printf(" Origin %s, metric %u, localpref %u, weight %u, ovs %s, ",
> + print_origin(r->origin, 0), r->med, r->local_pref, r->weight,
> + print_ovs(r->validation_state, 0));
> print_flags(r->flags, 0);
>
> now = time(NULL);
> diff --git usr.sbin/bgpctl/parser.h usr.sbin/bgpctl/parser.h
> index fd4847b199c..fd4e96f6efa 100644
> --- usr.sbin/bgpctl/parser.h
> +++ usr.sbin/bgpctl/parser.h
> @@ -70,6 +70,7 @@ struct parse_result {
> char shutcomm[SHUT_COMM_LEN];
> char *irr_outdir;
> int flags;
> + u_int8_t validation_state;
> u_int rtableid;
> enum actions action;
> u_int8_t prefixlen;
> diff --git usr.sbin/bgpd/bgpd.h usr.sbin/bgpd/bgpd.h
> index 8e6a4bcb2c7..2c4ba61d503 100644
> --- usr.sbin/bgpd/bgpd.h
> +++ usr.sbin/bgpd/bgpd.h
> @@ -89,6 +89,15 @@
> #define F_CTL_SSV 0x20000 /* only used by bgpctl */
> #define F_CTL_INVALID 0x40000 /* only used by bgpctl */
>
> +/*
> + * Note that these numeric assignments differ from the numbers commonly
> + * used in route origin validation context.
> + */
> +#define ROA_NOTFOUND 0x0 /* default */
> +#define ROA_INVALID 0x1
> +#define ROA_VALID 0x2
> +#define ROA_MASK 0x3
> +
> /*
> * Limit the number of messages queued in the session engine.
> * The SE will send an IMSG_XOFF messages to the RDE if the high water mark
> @@ -668,6 +677,7 @@ struct ctl_show_rib {
> u_int16_t aspath_len;
> u_int8_t prefixlen;
> u_int8_t origin;
> + u_int8_t validation_state;
> /* plus a aspath_len bytes long aspath */
> };
>
> @@ -768,6 +778,7 @@ struct ctl_show_rib_request {
> struct filter_largecommunity large_community;
> u_int32_t peerid;
> u_int32_t flags;
> + u_int8_t validation_state;
> pid_t pid;
> enum imsg_type type;
> u_int8_t prefixlen;
> diff --git usr.sbin/bgpd/rde.c usr.sbin/bgpd/rde.c
> index 8c4e2bdf017..871e419f53a 100644
> --- usr.sbin/bgpd/rde.c
> +++ usr.sbin/bgpd/rde.c
> @@ -2177,6 +2177,7 @@ rde_dump_rib_as(struct prefix *p, struct rde_aspath
> *asp, pid_t pid, int flags)
> pt_getaddr(p->re->prefix, &rib.prefix);
> rib.prefixlen = p->re->prefix->prefixlen;
> rib.origin = asp->origin;
> + rib.validation_state = p->validation_state;
> rib.flags = 0;
> if (p->re->active == p)
> rib.flags |= F_PREF_ACTIVE;
> diff --git usr.sbin/bgpd/rde.h usr.sbin/bgpd/rde.h
> index 69962d83585..76e78865e76 100644
> --- usr.sbin/bgpd/rde.h
> +++ usr.sbin/bgpd/rde.h
> @@ -36,11 +36,6 @@ enum peer_state {
> PEER_ERR /* error occurred going to PEER_DOWN state */
> };
>
> -#define ROA_NOTFOUND 0x0 /* default */
> -#define ROA_INVALID 0x1
> -#define ROA_VALID 0x2
> -#define ROA_MASK 0x3
> -
> /*
> * How do we identify peers between the session handler and the rde?
> * Currently I assume that we can do that with the neighbor_ip...
>
