Skip to content

Commit

Permalink
bgpd: BGP neighbor password change doesn't take effect with a a parti…
Browse files Browse the repository at this point in the history
…cular config on reboot

Description: when vrf add is received, add Vrf-name to the interface database. This is needed while binding the VRF interface to the BGP socket.
In this case, the global bgp config containing vrf is received before zebra sends vrf add message to BGP. When we receive the global bgp vrf message
first, vrf interface is not present in the interface database of BGP. So, while creating the global bgp socket, interface bind to the vrf interface fails.
Also, when setting the password on the bgp socket, first we should apply the password on the socket and then, reset the session or send update to peer.

Problem Description/Summary :
changing the neighbor password resets the session immediately but it doesn't use the password. It continues to operate without password.

Managed to recreate the issue with below reduced config.

Setup:
Sonic1------Sonic2

Test Steps:
1. Configure eBGP session between Sonic-1 and Sonic2 in default vrf using Peer-group.
2. Create a dummy non-default vrf BGP instance on Sonic1 device.
3. Save and reload Sonic1 device.
4. Once the device is up, try to configure 'password <string>' on Sonic1 BGP neighbor alone.
5. The eBGP session should go down and must not come up until matching password is configured on Sonic2 device. But it comes up.

Expected Behavior :
The eBGP session should go down and must not come up until matching password is configured on Sonic2 device.

Signed-off-by: Preetham Singh preetham.singh@broadcom.com
  • Loading branch information
sudhanshukumar22 committed Nov 2, 2020
1 parent d65c0a6 commit df1cf38
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 17 deletions.
13 changes: 13 additions & 0 deletions bgpd/bgp_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,26 @@ static int bgp_vrf_new(struct vrf *vrf)
if (BGP_DEBUG(zebra, ZEBRA))
zlog_debug("VRF Created: %s(%u)", vrf->name, vrf->vrf_id);

zlog_info("VRF Creation message received: %s(%u)", vrf->name,
vrf->vrf_id);
struct interface *ifp;
ifp = if_get_by_name(vrf->name, vrf->vrf_id);
if (ifp) {
zlog_info("VRF interface Created: %s(%u) ifindex %d", ifp->name,
ifp->vrf_id, ifp->ifindex);
}

return 0;
}

static int bgp_vrf_delete(struct vrf *vrf)
{
if (BGP_DEBUG(zebra, ZEBRA))
zlog_debug("VRF Deletion: %s(%u)", vrf->name, vrf->vrf_id);
zlog_info("VRF Deletion message received: %s(%u)", vrf->name,
vrf->vrf_id);
/* No need to delete vrf here as it will be deleted when BGP client
receives ZEBRA_VRF_DELETE message from zebra */

return 0;
}
Expand Down
38 changes: 21 additions & 17 deletions bgpd/bgpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -5649,21 +5649,22 @@ int peer_password_set(struct peer *peer, const char *password)

/* Check if handling a regular peer. */
if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
/* Send notification or reset peer depending on state. */
if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send(peer, BGP_NOTIFY_CEASE,
BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
bgp_session_reset(peer);

/*
* Attempt to install password on socket and skip peer-group
* mechanics.
*/
if (BGP_PEER_SU_UNSPEC(peer))
return BGP_SUCCESS;
return (bgp_md5_set(peer) >= 0) ? BGP_SUCCESS
: BGP_ERR_TCPSIG_FAILED;
ret = (bgp_md5_set(peer) >= 0) ? BGP_SUCCESS
: BGP_ERR_TCPSIG_FAILED;
/* Send notification or reset peer depending on state. */
if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send(peer, BGP_NOTIFY_CEASE,
BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
bgp_session_reset(peer);
return ret;
}

/*
Expand All @@ -5685,16 +5686,17 @@ int peer_password_set(struct peer *peer, const char *password)
XFREE(MTYPE_PEER_PASSWORD, member->password);
member->password = XSTRDUP(MTYPE_PEER_PASSWORD, password);

/* Attempt to install password on socket. */
if (!BGP_PEER_SU_UNSPEC(member) && bgp_md5_set(member) < 0)
ret = BGP_ERR_TCPSIG_FAILED;

/* Send notification or reset peer depending on state. */
if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status))
bgp_notify_send(member, BGP_NOTIFY_CEASE,
BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
bgp_session_reset(member);

/* Attempt to install password on socket. */
if (!BGP_PEER_SU_UNSPEC(member) && bgp_md5_set(member) < 0)
ret = BGP_ERR_TCPSIG_FAILED;
}

/* Set flag and configuration on all peer-group listen ranges */
Expand Down Expand Up @@ -5730,16 +5732,17 @@ int peer_password_unset(struct peer *peer)

/* Check if handling a regular peer. */
if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
/* Attempt to uninstall password on socket. */
if (!BGP_PEER_SU_UNSPEC(peer))
bgp_md5_unset(peer);

/* Send notification or reset peer depending on state. */
if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send(peer, BGP_NOTIFY_CEASE,
BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
bgp_session_reset(peer);

/* Attempt to uninstall password on socket. */
if (!BGP_PEER_SU_UNSPEC(peer))
bgp_md5_unset(peer);
/* Skip peer-group mechanics for regular peers. */
return 0;
}
Expand All @@ -5757,16 +5760,17 @@ int peer_password_unset(struct peer *peer)
UNSET_FLAG(member->flags, PEER_FLAG_PASSWORD);
XFREE(MTYPE_PEER_PASSWORD, member->password);

/* Attempt to uninstall password on socket. */
if (!BGP_PEER_SU_UNSPEC(member))
bgp_md5_unset(member);

/* Send notification or reset peer depending on state. */
if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status))
bgp_notify_send(member, BGP_NOTIFY_CEASE,
BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
bgp_session_reset(member);

/* Attempt to uninstall password on socket. */
if (!BGP_PEER_SU_UNSPEC(member))
bgp_md5_unset(member);
}

/* Set flag and configuration on all peer-group listen ranges */
Expand Down

0 comments on commit df1cf38

Please sign in to comment.