Merge pull request #13471 from opensourcerouting/fix/aigp_non_transitive

bgpd: AIGP should be non-transitive
This commit is contained in:
Jafar Al-Gharaibeh 2023-05-08 23:25:07 -05:00 committed by GitHub
commit cf19cff64f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 38 additions and 32 deletions

View File

@ -1467,7 +1467,7 @@ const uint8_t attr_flags_values[] = {
[BGP_ATTR_PREFIX_SID] = BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS, [BGP_ATTR_PREFIX_SID] = BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS,
[BGP_ATTR_IPV6_EXT_COMMUNITIES] = [BGP_ATTR_IPV6_EXT_COMMUNITIES] =
BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS,
[BGP_ATTR_AIGP] = BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS, [BGP_ATTR_AIGP] = BGP_ATTR_FLAG_OPTIONAL,
}; };
static const size_t attr_flags_values_max = array_size(attr_flags_values) - 1; static const size_t attr_flags_values_max = array_size(attr_flags_values) - 1;
@ -4740,7 +4740,7 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
*/ */
uint8_t attr_len = BGP_AIGP_TLV_METRIC_LEN; uint8_t attr_len = BGP_AIGP_TLV_METRIC_LEN;
stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS); stream_putc(s, BGP_ATTR_FLAG_OPTIONAL);
stream_putc(s, BGP_ATTR_AIGP); stream_putc(s, BGP_ATTR_AIGP);
stream_putc(s, attr_len); stream_putc(s, attr_len);
stream_put_bgp_aigp_tlv_metric(s, bpi); stream_put_bgp_aigp_tlv_metric(s, bpi);

View File

@ -4,7 +4,8 @@ router bgp 65001
neighbor 10.0.0.1 remote-as internal neighbor 10.0.0.1 remote-as internal
neighbor 10.0.0.1 timers 1 3 neighbor 10.0.0.1 timers 1 3
neighbor 10.0.0.1 timers connect 1 neighbor 10.0.0.1 timers connect 1
neighbor 192.168.24.4 remote-as external neighbor 10.0.0.1 route-reflector-client
neighbor 192.168.24.4 remote-as internal
neighbor 192.168.24.4 timers 1 3 neighbor 192.168.24.4 timers 1 3
neighbor 192.168.24.4 timers connect 1 neighbor 192.168.24.4 timers connect 1
neighbor 192.168.24.4 aigp neighbor 192.168.24.4 aigp

View File

@ -4,7 +4,8 @@ router bgp 65001
neighbor 10.0.0.1 remote-as internal neighbor 10.0.0.1 remote-as internal
neighbor 10.0.0.1 timers 1 3 neighbor 10.0.0.1 timers 1 3
neighbor 10.0.0.1 timers connect 1 neighbor 10.0.0.1 timers connect 1
neighbor 192.168.35.5 remote-as external neighbor 10.0.0.1 route-reflector-client
neighbor 192.168.35.5 remote-as internal
neighbor 192.168.35.5 timers 1 3 neighbor 192.168.35.5 timers 1 3
neighbor 192.168.35.5 timers connect 1 neighbor 192.168.35.5 timers connect 1
neighbor 192.168.35.5 aigp neighbor 192.168.35.5 aigp

View File

@ -1,10 +1,11 @@
router bgp 65002 router bgp 65001
no bgp ebgp-requires-policy no bgp ebgp-requires-policy
no bgp network import-check no bgp network import-check
neighbor 192.168.24.2 remote-as external neighbor 192.168.24.2 remote-as internal
neighbor 192.168.24.2 timers 1 3 neighbor 192.168.24.2 timers 1 3
neighbor 192.168.24.2 timers connect 1 neighbor 192.168.24.2 timers connect 1
neighbor 192.168.24.2 aigp neighbor 192.168.24.2 aigp
neighbor 192.168.24.2 route-reflector-client
neighbor 10.0.0.6 remote-as internal neighbor 10.0.0.6 remote-as internal
neighbor 10.0.0.6 timers 1 3 neighbor 10.0.0.6 timers 1 3
neighbor 10.0.0.6 timers connect 1 neighbor 10.0.0.6 timers connect 1

View File

@ -1,10 +1,11 @@
router bgp 65002 router bgp 65001
no bgp ebgp-requires-policy no bgp ebgp-requires-policy
no bgp network import-check no bgp network import-check
neighbor 192.168.35.3 remote-as external neighbor 192.168.35.3 remote-as internal
neighbor 192.168.35.3 timers 1 3 neighbor 192.168.35.3 timers 1 3
neighbor 192.168.35.3 timers connect 1 neighbor 192.168.35.3 timers connect 1
neighbor 192.168.35.3 aigp neighbor 192.168.35.3 aigp
neighbor 192.168.35.3 route-reflector-client
neighbor 10.0.0.6 remote-as internal neighbor 10.0.0.6 remote-as internal
neighbor 10.0.0.6 timers 1 3 neighbor 10.0.0.6 timers 1 3
neighbor 10.0.0.6 timers connect 1 neighbor 10.0.0.6 timers connect 1

View File

@ -1,4 +1,4 @@
router bgp 65002 router bgp 65001
no bgp ebgp-requires-policy no bgp ebgp-requires-policy
no bgp network import-check no bgp network import-check
neighbor 10.0.0.4 remote-as internal neighbor 10.0.0.4 remote-as internal

View File

@ -1,4 +1,4 @@
router bgp 65002 router bgp 65001
no bgp ebgp-requires-policy no bgp ebgp-requires-policy
no bgp network import-check no bgp network import-check
neighbor 192.168.67.6 remote-as internal neighbor 192.168.67.6 remote-as internal

View File

@ -14,7 +14,7 @@ r6 receives those routes with aigp-metric TLV.
r2 and r3 receives those routes with aigp-metric TLV increased by 20, r2 and r3 receives those routes with aigp-metric TLV increased by 20,
and 30 appropriately. and 30 appropriately.
r1 receives routes with aigp-metric TLV 91,101 and 92,102 appropriately. r1 receives routes with aigp-metric TLV 111,131 and 112,132 appropriately.
""" """
import os import os
@ -109,16 +109,16 @@ def test_bgp_aigp():
expected = { expected = {
"paths": [ "paths": [
{ {
"aigpMetric": 101, "aigpMetric": 111,
"valid": True,
"bestpath": {"selectionReason": "Router ID"},
"nexthops": [{"hostname": "r2", "accessible": True}],
},
{
"aigpMetric": 91,
"valid": True, "valid": True,
"nexthops": [{"hostname": "r3", "accessible": True}], "nexthops": [{"hostname": "r3", "accessible": True}],
}, },
{
"aigpMetric": 131,
"valid": True,
"bestpath": {"selectionReason": "Neighbor IP"},
"nexthops": [{"hostname": "r2", "accessible": True}],
},
] ]
} }
return topotest.json_cmp(output, expected) return topotest.json_cmp(output, expected)
@ -141,28 +141,30 @@ def test_bgp_aigp():
"10.0.0.71/32": { "10.0.0.71/32": {
"paths": [ "paths": [
{ {
"aigpMetric": 101, "aigpMetric": 111,
"bestpath": {"selectionReason": "AIGP"},
"valid": True, "valid": True,
"nexthops": [{"hostname": "r3", "accessible": True}],
}, },
{ {
"aigpMetric": 91, "aigpMetric": 131,
"valid": True, "valid": True,
"bestpath": {"selectionReason": "AIGP"}, "nexthops": [{"hostname": "r2", "accessible": True}],
"nexthops": [{"hostname": "r3", "accessible": True}],
}, },
], ],
}, },
"10.0.0.72/32": { "10.0.0.72/32": {
"paths": [ "paths": [
{ {
"aigpMetric": 102, "aigpMetric": 112,
"bestpath": {"selectionReason": "AIGP"},
"valid": True, "valid": True,
"nexthops": [{"hostname": "r3", "accessible": True}],
}, },
{ {
"aigpMetric": 92, "aigpMetric": 132,
"valid": True, "valid": True,
"bestpath": {"selectionReason": "AIGP"}, "nexthops": [{"hostname": "r2", "accessible": True}],
"nexthops": [{"hostname": "r3", "accessible": True}],
}, },
], ],
}, },
@ -172,7 +174,7 @@ def test_bgp_aigp():
# Initial converge, AIGP is not involved in best-path selection process # Initial converge, AIGP is not involved in best-path selection process
test_func = functools.partial(_bgp_converge) test_func = functools.partial(_bgp_converge)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "can't converge initially" assert result is None, "can't converge initially"
# Enable `bgp bestpath aigp` # Enable `bgp bestpath aigp`
@ -186,27 +188,27 @@ def test_bgp_aigp():
# r4, 10.0.0.71/32 with aigp-metric 71 # r4, 10.0.0.71/32 with aigp-metric 71
test_func = functools.partial(_bgp_check_aigp_metric, r4, "10.0.0.71/32", 71) test_func = functools.partial(_bgp_check_aigp_metric, r4, "10.0.0.71/32", 71)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.0.71/32 is not 71" assert result is None, "aigp-metric for 10.0.0.71/32 is not 71"
# r5, 10.0.0.72/32 with aigp-metric 72 # r5, 10.0.0.72/32 with aigp-metric 72
test_func = functools.partial(_bgp_check_aigp_metric, r5, "10.0.0.72/32", 72) test_func = functools.partial(_bgp_check_aigp_metric, r5, "10.0.0.72/32", 72)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.0.72/32 is not 72" assert result is None, "aigp-metric for 10.0.0.72/32 is not 72"
# r2, 10.0.0.71/32 with aigp-metric 101 (71 + 30) # r2, 10.0.0.71/32 with aigp-metric 101 (71 + 30)
test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 101) test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 101)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.0.71/32 is not 101" assert result is None, "aigp-metric for 10.0.0.71/32 is not 101"
# r3, 10.0.0.72/32 with aigp-metric 92 (72 + 20) # r3, 10.0.0.72/32 with aigp-metric 92 (72 + 20)
test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 92) test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 92)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.0.72/32 is not 92" assert result is None, "aigp-metric for 10.0.0.72/32 is not 92"
# r1, check if AIGP is considered in best-path selection (lowest wins) # r1, check if AIGP is considered in best-path selection (lowest wins)
test_func = functools.partial(_bgp_check_aigp_metric_bestpath) test_func = functools.partial(_bgp_check_aigp_metric_bestpath)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "AIGP attribute is not considered in best-path selection" assert result is None, "AIGP attribute is not considered in best-path selection"