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.

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:support tcp-mss for neighbor group (backport #17341) #17388

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion bgpd/bgp_vty.c
Original file line number Diff line number Diff line change
Expand Up @@ -18563,7 +18563,7 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp,
}

/* TCP max segment size */
if (CHECK_FLAG(peer->flags, PEER_FLAG_TCP_MSS))
if (peergroup_flag_check(peer, PEER_FLAG_TCP_MSS))
vty_out(vty, " neighbor %s tcp-mss %d\n", addr, peer->tcp_mss);

/* passive */
Expand Down
66 changes: 61 additions & 5 deletions bgpd/bgpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -3004,6 +3004,9 @@ static void peer_group2peer_config_copy(struct peer_group *group,
bgp_peer_configure_bfd(peer, false);
bgp_peer_config_apply(peer, group);
}
/* peer tcp-mss */
if (!CHECK_FLAG(peer->flags_override, PEER_FLAG_TCP_MSS))
PEER_ATTR_INHERIT(peer, group, tcp_mss);
}

/* Peer group's remote AS configuration. */
Expand Down Expand Up @@ -4677,6 +4680,11 @@ static const struct peer_flag_action peer_flag_action_list[] = {
{PEER_FLAG_CAPABILITY_FQDN, 0, peer_change_none},
{PEER_FLAG_AS_LOOP_DETECTION, 0, peer_change_none},
{PEER_FLAG_EXTENDED_LINK_BANDWIDTH, 0, peer_change_none},
<<<<<<< HEAD
=======
{PEER_FLAG_LONESOUL, 0, peer_change_reset_out},
{PEER_FLAG_TCP_MSS, 0, peer_change_none},
>>>>>>> 9fa56a03c7 (bgpd:support tcp-mss for neighbor group)
{0, 0, 0}};

static const struct peer_flag_action peer_af_flag_action_list[] = {
Expand Down Expand Up @@ -5968,9 +5976,27 @@ void peer_port_unset(struct peer *peer)
*/
void peer_tcp_mss_set(struct peer *peer, uint32_t tcp_mss)
{
struct peer *member;
struct listnode *node, *nnode;

peer_flag_set(peer, PEER_FLAG_TCP_MSS);
peer->tcp_mss = tcp_mss;
SET_FLAG(peer->flags, PEER_FLAG_TCP_MSS);
bgp_tcp_mss_set(peer);

if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
bgp_tcp_mss_set(peer);
return;
}

for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) {
/* Skip peers with overridden configuration. */
if (CHECK_FLAG(member->flags_override, PEER_FLAG_TCP_MSS))
continue;

/* Set flag and configuration on peer-group member. */
SET_FLAG(member->flags, PEER_FLAG_TCP_MSS);
PEER_ATTR_INHERIT(member, peer->group, tcp_mss);
bgp_tcp_mss_set(member);
}
}

/* Reset the TCP-MSS value in the peer structure,
Expand All @@ -5979,9 +6005,39 @@ void peer_tcp_mss_set(struct peer *peer, uint32_t tcp_mss)
*/
void peer_tcp_mss_unset(struct peer *peer)
{
UNSET_FLAG(peer->flags, PEER_FLAG_TCP_MSS);
peer->tcp_mss = 0;
bgp_tcp_mss_set(peer);
struct peer *member;
struct listnode *node, *nnode;

/* Inherit configuration from peer-group if peer is member. */
if (peer_group_active(peer)) {
peer_flag_inherit(peer, PEER_FLAG_TCP_MSS);
PEER_ATTR_INHERIT(peer, peer->group, tcp_mss);
} else {
/* Otherwise remove flag and configuration from peer. */
peer_flag_unset(peer, PEER_FLAG_TCP_MSS);
peer->tcp_mss = 0;
}

/* Skip peer-group mechanics for regular peers. */
if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
bgp_tcp_mss_set(peer);
return;
}

/*
* Remove flag and configuration from all peer-group members, unless
* they are explicitely overriding peer-group configuration.
*/
for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) {
/* Skip peers with overridden configuration. */
if (CHECK_FLAG(member->flags_override, PEER_FLAG_TCP_MSS))
continue;

/* Remove flag and configuration on peer-group member. */
UNSET_FLAG(member->flags, PEER_FLAG_TCP_MSS);
member->tcp_mss = 0;
bgp_tcp_mss_set(member);
}
}

/*
Expand Down
3 changes: 3 additions & 0 deletions tests/topotests/bgp_tcp_mss/r1/bgpd.conf
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
router bgp 65000
no bgp ebgp-requires-policy
neighbor aaa peer-group
neighbor aaa remote-as 65001
neighbor 192.168.254.2 peer-group aaa
neighbor 192.168.255.2 remote-as 65001
neighbor 192.168.255.2 timers 3 10
exit-address-family
Expand Down
3 changes: 3 additions & 0 deletions tests/topotests/bgp_tcp_mss/r1/zebra.conf
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,8 @@
interface r1-eth0
ip address 192.168.255.1/24
!
interface r1-eth1
ip address 192.168.254.1/24
!
ip forwarding
!
3 changes: 3 additions & 0 deletions tests/topotests/bgp_tcp_mss/r2/bgpd.conf
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
router bgp 65001
no bgp ebgp-requires-policy
neighbor aaa peer-group
neighbor aaa remote-as 65000
neighbor 192.168.254.1 peer-group aaa
neighbor 192.168.255.1 remote-as 65000
neighbor 192.168.255.1 timers 3 10
exit-address-family
Expand Down
3 changes: 3 additions & 0 deletions tests/topotests/bgp_tcp_mss/r2/zebra.conf
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,8 @@
interface r2-eth0
ip address 192.168.255.2/24
!
interface r2-eth1
ip address 192.168.254.2/24
!
ip forwarding
!
42 changes: 40 additions & 2 deletions tests/topotests/bgp_tcp_mss/test_bgp_tcp_mss.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ def build_topo(tgen):
switch.add_link(tgen.gears["r1"])
switch.add_link(tgen.gears["r2"])

switch = tgen.add_switch("s2")
switch.add_link(tgen.gears["r1"])
switch.add_link(tgen.gears["r2"])


def setup_module(mod):
tgen = Topogen(build_topo, mod.__name__)
Expand Down Expand Up @@ -78,12 +82,16 @@ def test_bgp_tcp_mss():
router2 = tgen.gears["r2"]

def _bgp_converge(router):
output = json.loads(router.vtysh_cmd("show ip bgp neighbor 192.168.255.2 json"))
output = json.loads(router.vtysh_cmd("show ip bgp neighbor json"))
expected = {
"192.168.255.2": {
"bgpState": "Established",
"addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 0}},
}
},
"192.168.254.2": {
"bgpState": "Established",
"addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 0}},
},
}
return topotest.json_cmp(output, expected)

Expand All @@ -108,7 +116,11 @@ def _bgp_check_neighbor_tcp_mss(router, neigh):

logger.info("Check if neighbor sessions are up in {}".format(router1.name))
test_func = functools.partial(_bgp_converge, router1)
<<<<<<< HEAD
success, result = topotest.run_and_expect(test_func, None, count=15, wait=0.5)
=======
_, result = topotest.run_and_expect(test_func, None, count=15, wait=1)
>>>>>>> 9fa56a03c7 (bgpd:support tcp-mss for neighbor group)
assert result is None, 'Failed to see BGP convergence in "{}"'.format(router1.name)

logger.info("BGP neighbor session is up in {}".format(router1.name))
Expand All @@ -117,19 +129,25 @@ def _bgp_check_neighbor_tcp_mss(router, neigh):
"Configure tcp-mss 500 on {} and reset the session".format(router1.name)
)
_bgp_conf_tcp_mss(router1, "65000", "192.168.255.2")
_bgp_conf_tcp_mss(router1, "65000", "aaa")
_bgp_clear_session(router1)

logger.info(
"Configure tcp-mss 500 on {} and reset the session".format(router2.name)
)
_bgp_conf_tcp_mss(router2, "65001", "192.168.255.1")
_bgp_conf_tcp_mss(router2, "65001", "aaa")
_bgp_clear_session(router2)

logger.info(
"Check if neighbor session is up after reset in {}".format(router1.name)
)
test_func = functools.partial(_bgp_converge, router1)
<<<<<<< HEAD
success, result = topotest.run_and_expect(test_func, None, count=15, wait=0.5)
=======
_, result = topotest.run_and_expect(test_func, None, count=15, wait=1)
>>>>>>> 9fa56a03c7 (bgpd:support tcp-mss for neighbor group)
assert result is None, 'Failed to see BGP convergence after reset in "{}"'.format(
router1.name
)
Expand All @@ -138,7 +156,17 @@ def _bgp_check_neighbor_tcp_mss(router, neigh):
"Verify if TCP MSS value is synced with neighbor in {}".format(router1.name)
)
test_func = functools.partial(_bgp_check_neighbor_tcp_mss, router1, "192.168.255.2")
<<<<<<< HEAD
success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
=======
_, result = topotest.run_and_expect(test_func, None, count=15, wait=1)
assert (
result is None
), 'Failed to sync TCP MSS value over BGP session in "{}"'.format(router1.name)

test_func = functools.partial(_bgp_check_neighbor_tcp_mss, router1, "192.168.254.2")
success, result = topotest.run_and_expect(test_func, None, count=15, wait=1)
>>>>>>> 9fa56a03c7 (bgpd:support tcp-mss for neighbor group)
assert (
result is None
), 'Failed to sync TCP MSS value over BGP session in "{}"'.format(router1.name)
Expand All @@ -148,7 +176,17 @@ def _bgp_check_neighbor_tcp_mss(router, neigh):
"Verify if TCP MSS value is synced with neighbor in {}".format(router2.name)
)
test_func = functools.partial(_bgp_check_neighbor_tcp_mss, router2, "192.168.255.1")
<<<<<<< HEAD
success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
=======
_, result = topotest.run_and_expect(test_func, None, count=15, wait=1)
assert (
result is None
), 'Failed to sync TCP MSS value over BGP session in "{}"'.format(router2.name)

test_func = functools.partial(_bgp_check_neighbor_tcp_mss, router2, "192.168.254.1")
success, result = topotest.run_and_expect(test_func, None, count=15, wait=1)
>>>>>>> 9fa56a03c7 (bgpd:support tcp-mss for neighbor group)
assert (
result is None
), 'Failed to sync TCP MSS value over BGP session in "{}"'.format(router2.name)
Expand Down
Loading