bgpd: Show the real next-hop address in addition to hostname in `show bgp`

It's hard to cope with cases when next-hop is changed/unchanged or
peers are non-direct.

It would be better to show the hostname and nexthop IP address (both)
under `show bgp` to quickly identify the source and the real next-hop
of the route.

If `bgp default show-nexthop-hostname` is toggled the output looks like:
```
spine1-debian-9# show bgp
BGP table version is 1, local router ID is 2.2.2.2, vrf id 0
Default local pref 100, local AS 65002
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete

   Network          Next Hop            Metric LocPrf Weight Path
*  2a02:4780::/64   fe80::a00:27ff:fe09:f8a3(exit1-debian-9)
                                             0             0 65001 ?

spine1-debian-9# show ip bgp
BGP table version is 5, local router ID is 2.2.2.2, vrf id 0
Default local pref 100, local AS 65002
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete

   Network          Next Hop            Metric LocPrf Weight Path
*> 10.255.255.0/24  192.168.0.1(exit1-debian-9)
                                             0             0 65001 ?
```

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index ad437c6ba48046d40144540d5cc6a5155ac3427e..8a27f7a39322742ab70b91b967641cd744a1e053 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -7567,8 +7567,7 @@ static char *bgp_nexthop_hostname(struct peer *peer,
                                  struct bgp_nexthop_cache *bnc)
 {
        if (peer->hostname
-           && CHECK_FLAG(peer->bgp->flags, BGP_FLAG_SHOW_HOSTNAME) && bnc
-           && CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED))
+           && CHECK_FLAG(peer->bgp->flags, BGP_FLAG_SHOW_NEXTHOP_HOSTNAME))
                return peer->hostname;
        return NULL;
 }
@@ -7578,6 +7577,7 @@ void route_vty_out(struct vty *vty, const struct prefix *p,
                   struct bgp_path_info *path, int display, safi_t safi,
                   json_object *json_paths)
 {
+       int len;
        struct attr *attr = path->attr;
        json_object *json_path = NULL;
        json_object *json_nexthops = NULL;
@@ -7689,10 +7689,19 @@ void route_vty_out(struct vty *vty, const struct prefix *p,
                                                               : "ipv6");
                        json_object_boolean_true_add(json_nexthop_global,
                                                     "used");
-               } else
-                       vty_out(vty, "%s%s",
-                               nexthop_hostname ? nexthop_hostname : nexthop,
-                               vrf_id_str);
+               } else {
+                       if (nexthop_hostname)
+                               len = vty_out(vty, "%s(%s)%s", nexthop,
+                                             nexthop_hostname, vrf_id_str);
+                       else
+                               len = vty_out(vty, "%s%s", nexthop, vrf_id_str);
+
+                       len = 16 - len;
+                       if (len < 1)
+                               vty_out(vty, "\n%*s", 36, " ");
+                       else
+                               vty_out(vty, "%*s", len, " ");
+               }
        } else if (safi == SAFI_EVPN) {
                if (json_paths) {
                        json_nexthop_global = json_object_new_object();
@@ -7709,11 +7718,20 @@ void route_vty_out(struct vty *vty, const struct prefix *p,
                                               "ipv4");
                        json_object_boolean_true_add(json_nexthop_global,
                                                     "used");
-               } else
-                       vty_out(vty, "%-16s%s",
-                               nexthop_hostname ? nexthop_hostname
-                                                : inet_ntoa(attr->nexthop),
-                               vrf_id_str);
+               } else {
+                       if (nexthop_hostname)
+                               len = vty_out(vty, "%pI4(%s)%s", &attr->nexthop,
+                                             nexthop_hostname, vrf_id_str);
+                       else
+                               len = vty_out(vty, "%pI4%s", &attr->nexthop,
+                                             vrf_id_str);
+
+                       len = 16 - len;
+                       if (len < 1)
+                               vty_out(vty, "\n%*s", 36, " ");
+                       else
+                               vty_out(vty, "%*s", len, " ");
+               }
        } else if (safi == SAFI_FLOWSPEC) {
                if (attr->nexthop.s_addr != INADDR_ANY) {
                        if (json_paths) {
@@ -7734,10 +7752,21 @@ void route_vty_out(struct vty *vty, const struct prefix *p,
                                                        json_nexthop_global,
                                                             "used");
                        } else {
-                               vty_out(vty, "%-16s",
-                                       nexthop_hostname
-                                               ? nexthop_hostname
-                                               : inet_ntoa(attr->nexthop));
+                               if (nexthop_hostname)
+                                       len = vty_out(vty, "%pI4(%s)%s",
+                                                     &attr->nexthop,
+                                                     nexthop_hostname,
+                                                     vrf_id_str);
+                               else
+                                       len = vty_out(vty, "%pI4%s",
+                                                     &attr->nexthop,
+                                                     vrf_id_str);
+
+                               len = 16 - len;
+                               if (len < 1)
+                                       vty_out(vty, "\n%*s", 36, " ");
+                               else
+                                       vty_out(vty, "%*s", len, " ");
                        }
                }
        } else if (p->family == AF_INET && !BGP_ATTR_NEXTHOP_AFI_IP6(attr)) {
@@ -7757,19 +7786,23 @@ void route_vty_out(struct vty *vty, const struct prefix *p,
                        json_object_boolean_true_add(json_nexthop_global,
                                                     "used");
                } else {
-                       char buf[BUFSIZ];
+                       if (nexthop_hostname)
+                               len = vty_out(vty, "%pI4(%s)%s", &attr->nexthop,
+                                             nexthop_hostname, vrf_id_str);
+                       else
+                               len = vty_out(vty, "%pI4%s", &attr->nexthop,
+                                             vrf_id_str);
 
-                       snprintf(buf, sizeof(buf), "%s%s",
-                                nexthop_hostname ? nexthop_hostname
-                                                 : inet_ntoa(attr->nexthop),
-                                vrf_id_str);
-                       vty_out(vty, "%-16s", buf);
+                       len = 16 - len;
+                       if (len < 1)
+                               vty_out(vty, "\n%*s", 36, " ");
+                       else
+                               vty_out(vty, "%*s", len, " ");
                }
        }
 
        /* IPv6 Next Hop */
        else if (p->family == AF_INET6 || BGP_ATTR_NEXTHOP_AFI_IP6(attr)) {
-               int len;
                char buf[BUFSIZ];
 
                if (json_paths) {
@@ -7843,15 +7876,18 @@ void route_vty_out(struct vty *vty, const struct prefix *p,
                                        else
                                                vty_out(vty, "%*s", len, " ");
                                } else {
-                                       len = vty_out(
-                                               vty, "%s%s",
-                                               nexthop_hostname
-                                                       ? nexthop_hostname
-                                                       : inet_ntop(
-                                                                 AF_INET6,
-                                                                 &attr->mp_nexthop_local,
-                                                                 buf, BUFSIZ),
-                                               vrf_id_str);
+                                       if (nexthop_hostname)
+                                               len = vty_out(
+                                                       vty, "%pI6(%s)%s",
+                                                       &attr->mp_nexthop_local,
+                                                       nexthop_hostname,
+                                                       vrf_id_str);
+                                       else
+                                               len = vty_out(
+                                                       vty, "%pI6%s",
+                                                       &attr->mp_nexthop_local,
+                                                       vrf_id_str);
+
                                        len = 16 - len;
 
                                        if (len < 1)
@@ -7860,15 +7896,16 @@ void route_vty_out(struct vty *vty, const struct prefix *p,
                                                vty_out(vty, "%*s", len, " ");
                                }
                        } else {
-                               len = vty_out(
-                                       vty, "%s%s",
-                                       nexthop_hostname
-                                               ? nexthop_hostname
-                                               : inet_ntop(
-                                                         AF_INET6,
-                                                         &attr->mp_nexthop_global,
-                                                         buf, BUFSIZ),
-                                       vrf_id_str);
+                               if (nexthop_hostname)
+                                       len = vty_out(vty, "%pI6(%s)%s",
+                                                     &attr->mp_nexthop_global,
+                                                     nexthop_hostname,
+                                                     vrf_id_str);
+                               else
+                                       len = vty_out(vty, "%pI6%s",
+                                                     &attr->mp_nexthop_global,
+                                                     vrf_id_str);
+
                                len = 16 - len;
 
                                if (len < 1)
@@ -7994,6 +8031,7 @@ void route_vty_out_tmp(struct vty *vty, const struct prefix *p,
 {
        json_object *json_status = NULL;
        json_object *json_net = NULL;
+       int len;
        char buff[BUFSIZ];
 
        /* Route status display. */
@@ -8087,7 +8125,6 @@ void route_vty_out_tmp(struct vty *vty, const struct prefix *p,
                                                inet_ntoa(attr->nexthop));
                        } else if (p->family == AF_INET6
                                   || BGP_ATTR_NEXTHOP_AFI_IP6(attr)) {
-                               int len;
                                char buf[BUFSIZ];
 
                                len = vty_out(
@@ -8833,12 +8870,15 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn,
                                        json_object_string_add(
                                                json_nexthop_global, "hostname",
                                                nexthop_hostname);
-                       } else
-                               vty_out(vty, "    %s",
-                                       nexthop_hostname
-                                               ? nexthop_hostname
-                                               : inet_ntoa(
-                                                         attr->mp_nexthop_global_in));
+                       } else {
+                               if (nexthop_hostname)
+                                       vty_out(vty, "    %pI4(%s)",
+                                               &attr->mp_nexthop_global_in,
+                                               nexthop_hostname);
+                               else
+                                       vty_out(vty, "    %pI4",
+                                               &attr->mp_nexthop_global_in);
+                       }
                } else {
                        if (json_paths) {
                                json_object_string_add(
@@ -8849,11 +8889,15 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn,
                                        json_object_string_add(
                                                json_nexthop_global, "hostname",
                                                nexthop_hostname);
-                       } else
-                               vty_out(vty, "    %s",
-                                       nexthop_hostname
-                                               ? nexthop_hostname
-                                               : inet_ntoa(attr->nexthop));
+                       } else {
+                               if (nexthop_hostname)
+                                       vty_out(vty, "    %pI4(%s)",
+                                               &attr->nexthop,
+                                               nexthop_hostname);
+                               else
+                                       vty_out(vty, "    %pI4",
+                                               &attr->nexthop);
+                       }
                }
 
                if (json_paths)
@@ -8876,12 +8920,13 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn,
                        json_object_string_add(json_nexthop_global, "scope",
                                               "global");
                } else {
-                       vty_out(vty, "    %s",
-                               nexthop_hostname
-                                       ? nexthop_hostname
-                                       : inet_ntop(AF_INET6,
-                                                   &attr->mp_nexthop_global,
-                                                   buf, INET6_ADDRSTRLEN));
+                       if (nexthop_hostname)
+                               vty_out(vty, "    %pI6(%s)",
+                                       &attr->mp_nexthop_global,
+                                       nexthop_hostname);
+                       else
+                               vty_out(vty, "    %pI6",
+                                       &attr->mp_nexthop_global);
                }
        }
 

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index c38db2eb677e5fd84db3f3f9fb7c22dbd6e2db69..fe52b73438a1844d2e43fb17cc4202f258dd6693 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -84,6 +84,10 @@ FRR_CFG_DEFAULT_BOOL(BGP_SHOW_HOSTNAME,
        { .val_bool = true, .match_profile = "datacenter", },
        { .val_bool = false },
 )
+FRR_CFG_DEFAULT_BOOL(BGP_SHOW_NEXTHOP_HOSTNAME,
+       { .val_bool = true, .match_profile = "datacenter", },
+       { .val_bool = false },
+)
 FRR_CFG_DEFAULT_BOOL(BGP_LOG_NEIGHBOR_CHANGES,
        { .val_bool = true, .match_profile = "datacenter", },
        { .val_bool = false },
@@ -422,6 +426,8 @@ int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name,
                        SET_FLAG((*bgp)->flags, BGP_FLAG_IMPORT_CHECK);
                if (DFLT_BGP_SHOW_HOSTNAME)
                        SET_FLAG((*bgp)->flags, BGP_FLAG_SHOW_HOSTNAME);
+               if (DFLT_BGP_SHOW_NEXTHOP_HOSTNAME)
+                       SET_FLAG((*bgp)->flags, BGP_FLAG_SHOW_NEXTHOP_HOSTNAME);
                if (DFLT_BGP_LOG_NEIGHBOR_CHANGES)
                        SET_FLAG((*bgp)->flags, BGP_FLAG_LOG_NEIGHBOR_CHANGES);
                if (DFLT_BGP_DETERMINISTIC_MED)
@@ -3100,6 +3106,32 @@ DEFUN (no_bgp_default_show_hostname,
        return CMD_SUCCESS;
 }
 
+/* Display hostname in certain command outputs */
+DEFUN (bgp_default_show_nexthop_hostname,
+       bgp_default_show_nexthop_hostname_cmd,
+       "bgp default show-nexthop-hostname",
+       "BGP specific commands\n"
+       "Configure BGP defaults\n"
+       "Show hostname for nexthop in certain command outputs\n")
+{
+       VTY_DECLVAR_CONTEXT(bgp, bgp);
+       SET_FLAG(bgp->flags, BGP_FLAG_SHOW_NEXTHOP_HOSTNAME);
+       return CMD_SUCCESS;
+}
+
+DEFUN (no_bgp_default_show_nexthop_hostname,
+       no_bgp_default_show_nexthop_hostname_cmd,
+       "no bgp default show-nexthop-hostname",
+       NO_STR
+       "BGP specific commands\n"
+       "Configure BGP defaults\n"
+       "Show hostname for nexthop in certain command outputs\n")
+{
+       VTY_DECLVAR_CONTEXT(bgp, bgp);
+       UNSET_FLAG(bgp->flags, BGP_FLAG_SHOW_NEXTHOP_HOSTNAME);
+       return CMD_SUCCESS;
+}
+
 /* "bgp network import-check" configuration.  */
 DEFUN (bgp_network_import_check,
        bgp_network_import_check_cmd,
@@ -15280,6 +15312,15 @@ int bgp_config_write(struct vty *vty)
                                        ? ""
                                        : "no ");
 
+               /* BGP default show-nexthop-hostname */
+               if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_SHOW_NEXTHOP_HOSTNAME)
+                   != SAVE_BGP_SHOW_HOSTNAME)
+                       vty_out(vty, " %sbgp default show-nexthop-hostname\n",
+                               CHECK_FLAG(bgp->flags,
+                                          BGP_FLAG_SHOW_NEXTHOP_HOSTNAME)
+                                       ? ""
+                                       : "no ");
+
                /* BGP default subgroup-pkt-queue-max. */
                if (bgp->default_subgroup_pkt_queue_max
                    != BGP_DEFAULT_SUBGROUP_PKT_QUEUE_MAX)
@@ -15910,6 +15951,10 @@ void bgp_vty_init(void)
        install_element(BGP_NODE, &bgp_default_show_hostname_cmd);
        install_element(BGP_NODE, &no_bgp_default_show_hostname_cmd);
 
+       /* bgp default show-nexthop-hostname */
+       install_element(BGP_NODE, &bgp_default_show_nexthop_hostname_cmd);
+       install_element(BGP_NODE, &no_bgp_default_show_nexthop_hostname_cmd);
+
        /* "bgp default subgroup-pkt-queue-max" commands. */
        install_element(BGP_NODE, &bgp_default_subgroup_pkt_queue_max_cmd);
        install_element(BGP_NODE, &no_bgp_default_subgroup_pkt_queue_max_cmd);

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 4a5772a53bc76aaf338e4a26e84911ec7644b2a0..4efc068deadc8d7aa15f45fdf5b778867d612865 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -447,6 +447,7 @@ struct bgp {
 #define BGP_FLAG_SELECT_DEFER_DISABLE     (1 << 23)
 #define BGP_FLAG_GR_DISABLE_EOR           (1 << 24)
 #define BGP_FLAG_EBGP_REQUIRES_POLICY (1 << 25)
+#define BGP_FLAG_SHOW_NEXTHOP_HOSTNAME (1 << 26)
 
        enum global_mode GLOBAL_GR_FSM[BGP_GLOBAL_GR_MODE]
                                      [BGP_GLOBAL_GR_EVENT_CMD];