Skip to content

Commit

Permalink
Simplify BGP unnumbered configuration by eliminating the unessential.
Browse files Browse the repository at this point in the history 
To make BGP configuration as simple as possible, assume the capability
extended-nexthop to be default for interface neighbors. Also allow the
ability to specify remote-as on the same line as neighbor interface to
make BGP unnumbered configuration a single line.

One corner case. This is the first feature for which the default for a
member is different from the default for a peer-group. Since advertising
the capability is only done for interface neighbors, the capability is
not set for the peer-group, but is automatically set for interface
neighbors that belong to that peer-group. So, if you want to disable the
advertisement of this capability for an interface neighbor, you must
do it per each interface neighbor.

The patch is more complicated than it needs to be due to the handling
of quagga reload and appropriate updates to the show running output.

Ticket: CM-11830
Reviewed By: CCR-4958
Testing Done: Usual coterie, including manual

(cherry picked from commit 347914a)
  • Loading branch information
Dinesh G Dutt committed Jul 16, 2016
1 parent 7fb21a9 commit b3a39dc
Show file tree
Hide file tree
Showing 3 changed files with 193 additions and 24 deletions.
93 changes: 84 additions & 9 deletions bgpd/bgp_vty.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2834,9 +2834,11 @@ DEFUN (neighbor_remote_as,

static int
peer_conf_interface_get (struct vty *vty, const char *conf_if, afi_t afi,
safi_t safi, int v6only, const char *peer_group_name)
safi_t safi, int v6only, const char *peer_group_name,
const char *as_str)
{
as_t as;
as_t as = 0;
int as_type = AS_UNSPECIFIED;
struct bgp *bgp;
struct peer *peer;
struct peer_group *group;
Expand All @@ -2852,14 +2854,34 @@ peer_conf_interface_get (struct vty *vty, const char *conf_if, afi_t afi,
return CMD_WARNING;
}

if (as_str)
{
if (strncmp(as_str, "internal", strlen("internal")) == 0)
{
as_type = AS_INTERNAL;
}
else if (strncmp(as_str, "external", strlen("external")) == 0)
{
as_type = AS_EXTERNAL;
}
else
{
/* Get AS number. */
VTY_GET_INTEGER_RANGE ("AS", as, as_str, 1, BGP_AS4_MAX);
as_type = AS_SPECIFIED;
}
}

peer = peer_lookup_by_conf_if (bgp, conf_if);
if (!peer)
{
if (bgp_flag_check (bgp, BGP_FLAG_NO_DEFAULT_IPV4)
&& afi == AFI_IP && safi == SAFI_UNICAST)
peer = peer_create (NULL, conf_if, bgp, bgp->as, 0, AS_UNSPECIFIED, 0, 0, NULL);
peer = peer_create (NULL, conf_if, bgp, bgp->as, as, as_type, 0, 0,
NULL);
else
peer = peer_create (NULL, conf_if, bgp, bgp->as, 0, AS_UNSPECIFIED, afi, safi, NULL);
peer = peer_create (NULL, conf_if, bgp, bgp->as, as, as_type, afi, safi,
NULL);

if (peer && v6only)
SET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY);
Expand All @@ -2870,7 +2892,10 @@ peer_conf_interface_get (struct vty *vty, const char *conf_if, afi_t afi,
* gets deleted later etc.)
*/
if (peer->ifp)
bgp_zebra_initiate_radv (bgp, peer);
{
bgp_zebra_initiate_radv (bgp, peer);
}
peer_flag_set (peer, PEER_FLAG_CAPABILITY_ENHE);
}
else if ((v6only && !CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)) ||
(!v6only && CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)))
Expand Down Expand Up @@ -2917,9 +2942,11 @@ DEFUN (neighbor_interface_config,
"Enable BGP on interface\n")
{
if (argc == 2)
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 0, argv[1]);
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 0,
argv[1], NULL);
else
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 0, NULL);
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 0,
NULL, NULL);
}

ALIAS (neighbor_interface_config,
Expand All @@ -2940,9 +2967,11 @@ DEFUN (neighbor_interface_config_v6only,
"Enable BGP with v6 link-local only\n")
{
if (argc == 2)
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 1, argv[1]);
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 1,
argv[1], NULL);
else
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 1, NULL);
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 1,
NULL, NULL);
}

ALIAS (neighbor_interface_config_v6only,
Expand All @@ -2955,6 +2984,30 @@ ALIAS (neighbor_interface_config_v6only,
"Member of the peer-group\n"
"peer-group name\n")

DEFUN (neighbor_interface_config_remote_as,
neighbor_interface_config_remote_as_cmd,
"neighbor WORD interface remote-as (" CMD_AS_RANGE "|external|internal)",
NEIGHBOR_STR
"Interface name or neighbor tag\n"
"Enable BGP on interface\n"
AS_STR)
{
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 0,
NULL, argv[1]);
}

DEFUN (neighbor_interface_v6only_config_remote_as,
neighbor_interface_v6only_config_remote_as_cmd,
"neighbor WORD interface v6only remote-as (" CMD_AS_RANGE "|external|internal)",
NEIGHBOR_STR
"Interface name or neighbor tag\n"
"Enable BGP on interface\n"
AS_STR)
{
return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 1,
NULL, argv[1]);
}

DEFUN (neighbor_peer_group,
neighbor_peer_group_cmd,
"neighbor WORD peer-group",
Expand Down Expand Up @@ -3105,6 +3158,24 @@ ALIAS (no_neighbor_interface_config,
"Member of the peer-group\n"
"peer-group name\n")

ALIAS (no_neighbor_interface_config,
no_neighbor_interface_config_remote_as_cmd,
"no neighbor WORD interface remote-as (" CMD_AS_RANGE "|internal|external)",
NO_STR
NEIGHBOR_STR
"Interface name\n"
"Configure BGP on interface\n"
AS_STR)

ALIAS (no_neighbor_interface_config,
no_neighbor_interface_config_v6only_remote_as_cmd,
"no neighbor WORD interface v6only remote-as (" CMD_AS_RANGE "|internal|external)",
NO_STR
NEIGHBOR_STR
"Interface name\n"
"Configure BGP on interface\n"
"Enable BGP with v6 link-local only\n"
AS_STR)

DEFUN (no_neighbor_peer_group,
no_neighbor_peer_group_cmd,
Expand Down Expand Up @@ -14596,12 +14667,16 @@ bgp_vty_init (void)
install_element (BGP_NODE, &neighbor_interface_config_v6only_cmd);
install_element (BGP_NODE, &neighbor_interface_config_peergroup_cmd);
install_element (BGP_NODE, &neighbor_interface_config_v6only_peergroup_cmd);
install_element (BGP_NODE, &neighbor_interface_config_remote_as_cmd);
install_element (BGP_NODE, &neighbor_interface_v6only_config_remote_as_cmd);
install_element (BGP_NODE, &no_neighbor_cmd);
install_element (BGP_NODE, &no_neighbor_remote_as_cmd);
install_element (BGP_NODE, &no_neighbor_interface_config_cmd);
install_element (BGP_NODE, &no_neighbor_interface_config_v6only_cmd);
install_element (BGP_NODE, &no_neighbor_interface_config_peergroup_cmd);
install_element (BGP_NODE, &no_neighbor_interface_config_v6only_peergroup_cmd);
install_element (BGP_NODE, &no_neighbor_interface_config_remote_as_cmd);
install_element (BGP_NODE, &no_neighbor_interface_config_v6only_remote_as_cmd);

/* "neighbor peer-group" commands. */
install_element (BGP_NODE, &neighbor_peer_group_cmd);
Expand Down
74 changes: 59 additions & 15 deletions bgpd/bgpd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2546,6 +2546,7 @@ peer_group_bind (struct bgp *bgp, union sockunion *su, struct peer *peer,
int first_member = 0;
afi_t afi;
safi_t safi;
int cap_enhe_preset = 0;

/* Lookup the peer. */
if (!peer)
Expand Down Expand Up @@ -2580,8 +2581,18 @@ peer_group_bind (struct bgp *bgp, union sockunion *su, struct peer *peer,
first_member = 1;
}

if (CHECK_FLAG (peer->flags, PEER_FLAG_CAPABILITY_ENHE))
cap_enhe_preset = 1;

peer_group2peer_config_copy(group, peer);

/*
* Capability extended-nexthop is enabled for an interface neighbor by
* default. So, fix that up here.
*/
if (peer->ifp && cap_enhe_preset)
peer_flag_set (peer, PEER_FLAG_CAPABILITY_ENHE);

for (afi = AFI_IP; afi < AFI_MAX; afi++)
for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
{
Expand Down Expand Up @@ -6269,6 +6280,8 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,
struct peer *g_peer = NULL;
char buf[SU_ADDRSTRLEN];
char *addr;
int if_pg_printed = FALSE;
int if_ras_printed = FALSE;

/* Skip dynamic neighbors. */
if (peer_dynamic_neighbor (peer))
Expand All @@ -6290,7 +6303,25 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,
vty_out (vty, " neighbor %s interface", addr);

if (peer_group_active (peer))
vty_out (vty, " peer-group %s", peer->group->name);
{
vty_out (vty, " peer-group %s", peer->group->name);
if_pg_printed = TRUE;
}
else if (peer->as_type == AS_SPECIFIED)
{
vty_out (vty, " remote-as %u", peer->as);
if_ras_printed = TRUE;
}
else if (peer->as_type == AS_INTERNAL)
{
vty_out (vty, " remote-as internal");
if_ras_printed = TRUE;
}
else if (peer->as_type == AS_EXTERNAL)
{
vty_out (vty, " remote-as external");
if_ras_printed = TRUE;
}

vty_out (vty, "%s", VTY_NEWLINE);
}
Expand All @@ -6301,7 +6332,7 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,
{
g_peer = peer->group->conf;

if (g_peer->as_type == AS_UNSPECIFIED)
if (g_peer->as_type == AS_UNSPECIFIED && !if_ras_printed)
{
if (peer->as_type == AS_SPECIFIED)
{
Expand All @@ -6320,7 +6351,7 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,

/* For swpX peers we displayed the peer-group
* via 'neighbor swpX interface peer-group WORD' */
if (!peer->conf_if)
if (!if_pg_printed)
vty_out (vty, " neighbor %s peer-group %s%s", addr,
peer->group->name, VTY_NEWLINE);
}
Expand All @@ -6335,18 +6366,21 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,
VTY_NEWLINE);
}

if (peer->as_type == AS_SPECIFIED)
{
vty_out (vty, " neighbor %s remote-as %u%s", addr, peer->as,
VTY_NEWLINE);
}
else if (peer->as_type == AS_INTERNAL)
{
vty_out (vty, " neighbor %s remote-as internal%s", addr, VTY_NEWLINE);
}
else if (peer->as_type == AS_EXTERNAL)
if (!if_ras_printed)
{
vty_out (vty, " neighbor %s remote-as external%s", addr, VTY_NEWLINE);
if (peer->as_type == AS_SPECIFIED)
{
vty_out (vty, " neighbor %s remote-as %u%s", addr, peer->as,
VTY_NEWLINE);
}
else if (peer->as_type == AS_INTERNAL)
{
vty_out (vty, " neighbor %s remote-as internal%s", addr, VTY_NEWLINE);
}
else if (peer->as_type == AS_EXTERNAL)
{
vty_out (vty, " neighbor %s remote-as external%s", addr, VTY_NEWLINE);
}
}
}

Expand Down Expand Up @@ -6541,7 +6575,17 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,
}

/* capability extended-nexthop */
if (CHECK_FLAG (peer->flags, PEER_FLAG_CAPABILITY_ENHE))
if (peer->ifp && !CHECK_FLAG (peer->flags, PEER_FLAG_CAPABILITY_ENHE))
{
if (! peer_group_active (peer) ||
! CHECK_FLAG (g_peer->flags, PEER_FLAG_CAPABILITY_ENHE))
{
vty_out (vty, " no neighbor %s capability extended-nexthop%s", addr,
VTY_NEWLINE);
}
}

if (!peer->ifp && CHECK_FLAG (peer->flags, PEER_FLAG_CAPABILITY_ENHE))
{
if (! peer_group_active (peer) ||
! CHECK_FLAG (g_peer->flags, PEER_FLAG_CAPABILITY_ENHE))
Expand Down
50 changes: 50 additions & 0 deletions tools/quagga-reload.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,56 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del):
lines_to_add_to_del.append((ctx_keys, swpx_interface))
lines_to_add_to_del.append((tmp_ctx_keys, swpx_peergroup))

"""
In 3.0.1 we changed how we display neighbor interface command. Older
versions of quagga would display the following:
neighbor swp1 interface
neighbor swp1 remote-as external
neighbor swp1 capability extended-nexthop
but today we display via a single line
neighbor swp1 interface remote-as external
and capability extended-nexthop is no longer needed because we
automatically enable it when the neighbor is of type interface.
This change confuses quagga-reload.py so check to see if we are deleting
neighbor swp1 interface remote-as (external|internal|ASNUM)
and adding
neighbor swp1 interface
neighbor swp1 remote-as (external|internal|ASNUM)
neighbor swp1 capability extended-nexthop
If so then chop the del line and the corresponding add lines
"""
re_swpx_int_remoteas = re.search('neighbor (\S+) interface remote-as (\S+)', line)
re_swpx_int_v6only_remoteas = re.search('neighbor (\S+) interface v6only remote-as (\S+)', line)

if re_swpx_int_remoteas or re_swpx_int_v6only_remoteas:
swpx_interface = None
swpx_remoteas = None

if re_swpx_int_remoteas:
swpx = re_swpx_int_remoteas.group(1)
remoteas = re_swpx_int_remoteas.group(2)
swpx_interface = "neighbor %s interface" % swpx
elif re_swpx_int_v6only_remoteas:
swpx = re_swpx_int_v6only_remoteas.group(1)
remoteas = re_swpx_int_v6only_remoteas.group(2)
swpx_interface = "neighbor %s interface v6only" % swpx

swpx_remoteas = "neighbor %s remote-as %s" % (swpx, remoteas)
found_add_swpx_interface = line_exist(lines_to_add, ctx_keys, swpx_interface)
found_add_swpx_remoteas = line_exist(lines_to_add, ctx_keys, swpx_remoteas)
tmp_ctx_keys = tuple(list(ctx_keys))

if found_add_swpx_interface and found_add_swpx_remoteas:
deleted = True
lines_to_del_to_del.append((ctx_keys, line))
lines_to_add_to_del.append((ctx_keys, swpx_interface))
lines_to_add_to_del.append((tmp_ctx_keys, swpx_remoteas))

if not deleted:
found_add_line = line_exist(lines_to_add, ctx_keys, line)

Expand Down

0 comments on commit b3a39dc

Please sign in to comment.