Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: add 'default hostname-capability' #15129

Closed
wants to merge 1 commit into from
Closed

bgpd: add 'default hostname-capability' #15129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bgpd/bgp_open.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1896,7 +1896,8 @@ uint16_t bgp_open_capability(struct stream *s, struct peer *peer,
}

/* Hostname capability */
if (cmd_hostname_get()) {
if (CHECK_FLAG(peer->bgp->flags, BGP_FLAG_HOSTNAME_CAPABILITY) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this flag should be on by default where are you setting it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because in the initial commit, I hesitated to change this behavior.
rationale : draft it not supported everywhere.

consequence if I disable this:
topotests may be complicated, especially with IPv6 setups where the link local nexthop is never tested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I would prefer that this behavior does not get turned off by default. It is used and it is very useful for people who have deployed this. I would consider something like datacenter it is turned on and traditional it is not. Reality is that this needs to be a consensus behavior change from the community. Please plan to bring this up in the next weekly meeting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@donaldsharp this is enabled by default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boo on me.

cmd_hostname_get()) {
SET_FLAG(peer->cap, PEER_CAP_HOSTNAME_ADV);
stream_putc(s, BGP_OPEN_OPT_CAP);
rcapp = stream_get_endp(s); /* Ptr to length placeholder */
Expand Down
32 changes: 32 additions & 0 deletions bgpd/bgp_vty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ FRR_CFG_DEFAULT_BOOL(BGP_SOFT_VERSION_CAPABILITY,
{ .val_bool = true, .match_profile = "datacenter", },
{ .val_bool = false },
);
FRR_CFG_DEFAULT_BOOL(BGP_HOSTNAME_CAPABILITY, { .val_bool = true }, );
FRR_CFG_DEFAULT_BOOL(BGP_ENFORCE_FIRST_AS,
{ .val_bool = false, .match_version = "< 9.1", },
{ .val_bool = true },
Expand Down Expand Up @@ -623,6 +624,8 @@ int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name,
if (DFLT_BGP_SOFT_VERSION_CAPABILITY)
SET_FLAG((*bgp)->flags,
BGP_FLAG_SOFT_VERSION_CAPABILITY);
if (DFLT_BGP_HOSTNAME_CAPABILITY)
SET_FLAG((*bgp)->flags, BGP_FLAG_HOSTNAME_CAPABILITY);
if (DFLT_BGP_ENFORCE_FIRST_AS)
SET_FLAG((*bgp)->flags, BGP_FLAG_ENFORCE_FIRST_AS);

Expand Down Expand Up @@ -4287,6 +4290,24 @@ DEFPY (bgp_default_software_version_capability,
return CMD_SUCCESS;
}

DEFPY (bgp_default_hostname_capability,
bgp_default_hostname_capability_cmd,
"[no] bgp default hostname-capability",
NO_STR
BGP_STR
"Configure BGP defaults\n"
"Advertise hostname capability for all neighbors\n")
{
VTY_DECLVAR_CONTEXT(bgp, bgp);

if (no)
UNSET_FLAG(bgp->flags, BGP_FLAG_HOSTNAME_CAPABILITY);
else
SET_FLAG(bgp->flags, BGP_FLAG_HOSTNAME_CAPABILITY);

return CMD_SUCCESS;
}

/* "bgp network import-check" configuration. */
DEFUN (bgp_network_import_check,
bgp_network_import_check_cmd,
Expand Down Expand Up @@ -18829,6 +18850,14 @@ int bgp_config_write(struct vty *vty)
? ""
: "no ");

if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_HOSTNAME_CAPABILITY) !=
SAVE_BGP_HOSTNAME_CAPABILITY)
vty_out(vty, " %sbgp default hostname-capability\n",
CHECK_FLAG(bgp->flags,
BGP_FLAG_HOSTNAME_CAPABILITY)
? ""
: "no ");

/* BGP default subgroup-pkt-queue-max. */
if (bgp->default_subgroup_pkt_queue_max
!= BGP_DEFAULT_SUBGROUP_PKT_QUEUE_MAX)
Expand Down Expand Up @@ -19877,6 +19906,9 @@ void bgp_vty_init(void)
/* bgp default software-version-capability */
install_element(BGP_NODE, &bgp_default_software_version_capability_cmd);

/* bgp default hostname-capability */
install_element(BGP_NODE, &bgp_default_hostname_capability_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);
Expand Down
1 change: 1 addition & 0 deletions bgpd/bgpd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ struct bgp {
#define BGP_FLAG_LU_IPV6_EXPLICIT_NULL (1ULL << 34)
#define BGP_FLAG_SOFT_VERSION_CAPABILITY (1ULL << 35)
#define BGP_FLAG_ENFORCE_FIRST_AS (1ULL << 36)
#define BGP_FLAG_HOSTNAME_CAPABILITY (1ULL << 36)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

37, and align please


/* BGP default address-families.
* New peers inherit enabled afi/safis from bgp instance.
Expand Down
5 changes: 5 additions & 0 deletions doc/user/bgp.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1913,6 +1913,11 @@ Configuring Peers
outputs. It's easier to troubleshoot if you have a number of BGP peers
and a number of routes to check.

.. clicmd:: bgp default hostname-capability

This command enables hostname capability advertisement by default
for all the neighbors. This is enabled by default.

.. clicmd:: bgp default software-version-capability

This command enables software version capability advertisement by default
Expand Down
Loading