diff options
author | Donatas Abraitis <donatas@opensourcerouting.org> | 2022-06-06 08:49:37 +0200 |
---|---|---|
committer | Donatas Abraitis <donatas@opensourcerouting.org> | 2022-06-06 09:28:50 +0200 |
commit | 0f05ea43b0c18c890ef0faf81de1d4ad74893d86 (patch) | |
tree | 5a9831f0487ad01f9d04b2ea7a03a7bfffcc64db | |
parent | Merge pull request #11175 from louis-6wind/ip-vrf-exec (diff) | |
download | frr-0f05ea43b0c18c890ef0faf81de1d4ad74893d86.tar.xz frr-0f05ea43b0c18c890ef0faf81de1d4ad74893d86.zip |
bgpd: Initialize attr->local_pref to the configured default value
When we use network/redistribute local_preference is configured inproperly
when using route-maps something like:
```
network 100.100.100.100/32 route-map rm1
network 100.100.100.200/32 route-map rm2
route-map rm1 permit 10
set local-preference +10
route-map rm2 permit 10
set local-preference -10
```
Before:
```
root@spine1-debian-11:~# vtysh -c 'show bgp ipv4 unicast 100.100.100.100/32 json' | jq '.paths[].locPrf'
10
root@spine1-debian-11:~# vtysh -c 'show bgp ipv4 unicast 100.100.100.200/32 json' | jq '.paths[].locPrf'
0
```
After:
```
root@spine1-debian-11:~# vtysh -c 'show bgp ipv4 unicast 100.100.100.100/32 json' | jq '.paths[].locPrf'
110
root@spine1-debian-11:~# vtysh -c 'show bgp ipv4 unicast 100.100.100.200/32 json' | jq '.paths[].locPrf'
90
```
Set local-preference as the default value configured per BGP instance, but
do not set LOCAL_PREF flag by default.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
-rw-r--r-- | bgpd/bgp_attr.c | 4 | ||||
-rw-r--r-- | bgpd/bgp_attr.h | 3 | ||||
-rw-r--r-- | bgpd/bgp_evpn.c | 6 | ||||
-rw-r--r-- | bgpd/bgp_evpn_mh.c | 4 | ||||
-rw-r--r-- | bgpd/bgp_route.c | 6 | ||||
-rw-r--r-- | bgpd/bgp_routemap.c | 2 | ||||
-rw-r--r-- | bgpd/bgp_updgrp_adv.c | 3 | ||||
-rw-r--r-- | bgpd/rfapi/rfapi.c | 2 | ||||
-rw-r--r-- | bgpd/rfapi/vnc_export_bgp.c | 6 | ||||
-rw-r--r-- | tests/topotests/bgp_set_local_preference_add_subtract/r2/bgpd.conf | 4 | ||||
-rw-r--r-- | tests/topotests/bgp_set_local_preference_add_subtract/r3/bgpd.conf | 4 | ||||
-rw-r--r-- | tests/topotests/bgp_set_local_preference_add_subtract/test_bgp_set_local-preference_add_subtract.py | 18 |
12 files changed, 38 insertions, 24 deletions
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index d57281a70..b576d188b 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -951,7 +951,8 @@ struct attr *bgp_attr_intern(struct attr *attr) } /* Make network statement's attribute. */ -struct attr *bgp_attr_default_set(struct attr *attr, uint8_t origin) +struct attr *bgp_attr_default_set(struct attr *attr, struct bgp *bgp, + uint8_t origin) { memset(attr, 0, sizeof(struct attr)); @@ -965,6 +966,7 @@ struct attr *bgp_attr_default_set(struct attr *attr, uint8_t origin) attr->label = MPLS_INVALID_LABEL; attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP); attr->mp_nexthop_len = IPV6_MAX_BYTELEN; + attr->local_pref = bgp->default_local_pref; return attr; } diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 01d993dab..06f350b36 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -393,7 +393,8 @@ extern struct attr *bgp_attr_intern(struct attr *attr); extern void bgp_attr_unintern_sub(struct attr *attr); extern void bgp_attr_unintern(struct attr **pattr); extern void bgp_attr_flush(struct attr *attr); -extern struct attr *bgp_attr_default_set(struct attr *attr, uint8_t origin); +extern struct attr *bgp_attr_default_set(struct attr *attr, struct bgp *bgp, + uint8_t origin); extern struct attr *bgp_attr_aggregate_intern( struct bgp *bgp, uint8_t origin, struct aspath *aspath, struct community *community, struct ecommunity *ecommunity, diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 74395bb0e..3483ece5b 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -1292,7 +1292,7 @@ static int update_evpn_type5_route(struct bgp *bgp_vrf, struct prefix_evpn *evp, attr = *src_attr; else { memset(&attr, 0, sizeof(attr)); - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp_vrf, BGP_ORIGIN_IGP); } /* Advertise Primary IP (PIP) is enabled, send individual @@ -1731,7 +1731,7 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn, memset(&attr, 0, sizeof(attr)); /* Build path-attribute for this route. */ - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = vpn->originator_ip; attr.mp_nexthop_global_in = vpn->originator_ip; attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; @@ -1994,7 +1994,7 @@ void bgp_evpn_update_type2_route_entry(struct bgp *bgp, struct bgpevpn *vpn, * some other values could differ for different routes. The * attributes will be shared in the hash table. */ - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = vpn->originator_ip; attr.mp_nexthop_global_in = vpn->originator_ip; attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; diff --git a/bgpd/bgp_evpn_mh.c b/bgpd/bgp_evpn_mh.c index 92456080e..b42296f4d 100644 --- a/bgpd/bgp_evpn_mh.c +++ b/bgpd/bgp_evpn_mh.c @@ -636,7 +636,7 @@ static int bgp_evpn_type4_route_update(struct bgp *bgp, memset(&attr, 0, sizeof(attr)); /* Build path-attribute for this route. */ - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = es->originator_ip; attr.mp_nexthop_global_in = es->originator_ip; attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; @@ -946,7 +946,7 @@ static int bgp_evpn_type1_route_update(struct bgp *bgp, struct bgp_evpn_es *es, memset(&attr, 0, sizeof(attr)); /* Build path-attribute for this route. */ - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = es->originator_ip; attr.mp_nexthop_global_in = es->originator_ip; attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 5df45aa31..c9269c230 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -5811,7 +5811,7 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p, dest = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi, p, NULL); - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = bgp_static->igpnexthop; attr.med = bgp_static->igpmetric; @@ -6114,7 +6114,7 @@ static void bgp_static_update_safi(struct bgp *bgp, const struct prefix *p, dest = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi, p, &bgp_static->prd); - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = bgp_static->igpnexthop; attr.med = bgp_static->igpmetric; @@ -8315,7 +8315,7 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p, struct bgp_redist *red; /* Make default attribute. */ - bgp_attr_default_set(&attr, BGP_ORIGIN_INCOMPLETE); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_INCOMPLETE); /* * This must not be NULL to satisfy Coverity SA */ diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index c7f5e0433..e0372b31f 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -1968,7 +1968,7 @@ route_set_local_pref(void *rule, const struct prefix *prefix, void *object) path = object; /* Set local preference value. */ - if (path->attr->flag & ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF)) + if (path->attr->local_pref) locpref = path->attr->local_pref; path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF); diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index 87558f9c3..518ce485a 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -812,12 +812,11 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) bgp = peer->bgp; from = bgp->peer_self; - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); /* make coverity happy */ assert(attr.aspath); - attr.local_pref = bgp->default_local_pref; attr.med = 0; attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); diff --git a/bgpd/rfapi/rfapi.c b/bgpd/rfapi/rfapi.c index 3aa886837..35a76e783 100644 --- a/bgpd/rfapi/rfapi.c +++ b/bgpd/rfapi/rfapi.c @@ -627,7 +627,7 @@ void add_vnc_route(struct rfapi_descriptor *rfd, /* cookie, VPN UN addr, peer */ /* Make default attribute. Produces already-interned attr.aspath */ /* Cripes, the memory management of attributes is byzantine */ - bgp_attr_default_set(&attr, BGP_ORIGIN_INCOMPLETE); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_INCOMPLETE); /* * At this point: diff --git a/bgpd/rfapi/vnc_export_bgp.c b/bgpd/rfapi/vnc_export_bgp.c index c479b4d65..7cfa2ed67 100644 --- a/bgpd/rfapi/vnc_export_bgp.c +++ b/bgpd/rfapi/vnc_export_bgp.c @@ -731,7 +731,7 @@ void vnc_direct_bgp_add_prefix(struct bgp *bgp, return; } - bgp_attr_default_set(&attr, BGP_ORIGIN_INCOMPLETE); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_INCOMPLETE); /* TBD set some configured med, see add_vnc_route() */ vnc_zlog_debug_verbose( @@ -980,7 +980,7 @@ void vnc_direct_bgp_add_nve(struct bgp *bgp, struct rfapi_descriptor *rfd) return; } - bgp_attr_default_set(&attr, BGP_ORIGIN_INCOMPLETE); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_INCOMPLETE); /* TBD set some configured med, see add_vnc_route() */ /* @@ -1307,7 +1307,7 @@ static void vnc_direct_bgp_add_group_afi(struct bgp *bgp, return; } - bgp_attr_default_set(&attr, BGP_ORIGIN_INCOMPLETE); + bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_INCOMPLETE); /* TBD set some configured med, see add_vnc_route() */ /* diff --git a/tests/topotests/bgp_set_local_preference_add_subtract/r2/bgpd.conf b/tests/topotests/bgp_set_local_preference_add_subtract/r2/bgpd.conf index 82a01d457..1f85a3578 100644 --- a/tests/topotests/bgp_set_local_preference_add_subtract/r2/bgpd.conf +++ b/tests/topotests/bgp_set_local_preference_add_subtract/r2/bgpd.conf @@ -1,5 +1,7 @@ router bgp 65000 no bgp ebgp-requires-policy + no bgp network import-check + network 10.10.10.2/32 route-map l2 neighbor 192.168.255.1 remote-as 65000 neighbor 192.168.255.1 timers 3 10 address-family ipv4 @@ -9,3 +11,5 @@ router bgp 65000 ! route-map r1-out permit 10 set local-preference +50 +route-map l2 permit 10 + set local-preference +10 diff --git a/tests/topotests/bgp_set_local_preference_add_subtract/r3/bgpd.conf b/tests/topotests/bgp_set_local_preference_add_subtract/r3/bgpd.conf index 65e092b0f..49b8f1ce2 100644 --- a/tests/topotests/bgp_set_local_preference_add_subtract/r3/bgpd.conf +++ b/tests/topotests/bgp_set_local_preference_add_subtract/r3/bgpd.conf @@ -1,7 +1,9 @@ router bgp 65000 no bgp ebgp-requires-policy + no bgp network import-check neighbor 192.168.255.1 remote-as 65000 neighbor 192.168.255.1 timers 3 10 + network 10.10.10.3/32 route-map l3 address-family ipv4 redistribute connected neighbor 192.168.255.1 route-map r1-out out @@ -9,3 +11,5 @@ router bgp 65000 ! route-map r1-out permit 10 set local-preference -50 +route-map l3 permit 10 + set local-preference -10 diff --git a/tests/topotests/bgp_set_local_preference_add_subtract/test_bgp_set_local-preference_add_subtract.py b/tests/topotests/bgp_set_local_preference_add_subtract/test_bgp_set_local-preference_add_subtract.py index d238cc94e..ec9c8a41c 100644 --- a/tests/topotests/bgp_set_local_preference_add_subtract/test_bgp_set_local-preference_add_subtract.py +++ b/tests/topotests/bgp_set_local_preference_add_subtract/test_bgp_set_local-preference_add_subtract.py @@ -89,22 +89,26 @@ def test_bgp_set_local_preference(): expected = { "192.168.255.2": { "bgpState": "Established", - "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 2}}, + "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 3}}, }, "192.168.255.3": { "bgpState": "Established", - "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 2}}, + "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 3}}, }, } return topotest.json_cmp(output, expected) def _bgp_check_local_preference(router): - output = json.loads(router.vtysh_cmd("show ip bgp 172.16.255.254/32 json")) + output = json.loads(router.vtysh_cmd("show bgp ipv4 unicast json")) expected = { - "paths": [ - {"locPrf": 50, "nexthops": [{"ip": "192.168.255.3"}]}, - {"locPrf": 150, "nexthops": [{"ip": "192.168.255.2"}]}, - ] + "routes": { + "10.10.10.2/32": [{"locPrf": 160}], + "10.10.10.3/32": [{"locPrf": 40}], + "172.16.255.254/32": [ + {"locPrf": 50, "nexthops": [{"ip": "192.168.255.3"}]}, + {"locPrf": 150, "nexthops": [{"ip": "192.168.255.2"}]}, + ], + } } return topotest.json_cmp(output, expected) |