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>
This commit is contained in:
Donatas Abraitis 2022-06-06 09:49:37 +03:00
parent 23a1220847
commit 0f05ea43b0
12 changed files with 38 additions and 24 deletions

View File

@ -951,7 +951,8 @@ struct attr *bgp_attr_intern(struct attr *attr)
} }
/* Make network statement's attribute. */ /* 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)); 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->label = MPLS_INVALID_LABEL;
attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP); attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP);
attr->mp_nexthop_len = IPV6_MAX_BYTELEN; attr->mp_nexthop_len = IPV6_MAX_BYTELEN;
attr->local_pref = bgp->default_local_pref;
return attr; return attr;
} }

View File

@ -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_sub(struct attr *attr);
extern void bgp_attr_unintern(struct attr **pattr); extern void bgp_attr_unintern(struct attr **pattr);
extern void bgp_attr_flush(struct attr *attr); 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( extern struct attr *bgp_attr_aggregate_intern(
struct bgp *bgp, uint8_t origin, struct aspath *aspath, struct bgp *bgp, uint8_t origin, struct aspath *aspath,
struct community *community, struct ecommunity *ecommunity, struct community *community, struct ecommunity *ecommunity,

View File

@ -1292,7 +1292,7 @@ static int update_evpn_type5_route(struct bgp *bgp_vrf, struct prefix_evpn *evp,
attr = *src_attr; attr = *src_attr;
else { else {
memset(&attr, 0, sizeof(attr)); 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 /* 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)); memset(&attr, 0, sizeof(attr));
/* Build path-attribute for this route. */ /* 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.nexthop = vpn->originator_ip;
attr.mp_nexthop_global_in = vpn->originator_ip; attr.mp_nexthop_global_in = vpn->originator_ip;
attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; 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 * some other values could differ for different routes. The
* attributes will be shared in the hash table. * 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.nexthop = vpn->originator_ip;
attr.mp_nexthop_global_in = vpn->originator_ip; attr.mp_nexthop_global_in = vpn->originator_ip;
attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4;

View File

@ -636,7 +636,7 @@ static int bgp_evpn_type4_route_update(struct bgp *bgp,
memset(&attr, 0, sizeof(attr)); memset(&attr, 0, sizeof(attr));
/* Build path-attribute for this route. */ /* 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.nexthop = es->originator_ip;
attr.mp_nexthop_global_in = es->originator_ip; attr.mp_nexthop_global_in = es->originator_ip;
attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; 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)); memset(&attr, 0, sizeof(attr));
/* Build path-attribute for this route. */ /* 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.nexthop = es->originator_ip;
attr.mp_nexthop_global_in = es->originator_ip; attr.mp_nexthop_global_in = es->originator_ip;
attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4;

View File

@ -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); 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.nexthop = bgp_static->igpnexthop;
attr.med = bgp_static->igpmetric; 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, dest = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi, p,
&bgp_static->prd); &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.nexthop = bgp_static->igpnexthop;
attr.med = bgp_static->igpmetric; attr.med = bgp_static->igpmetric;
@ -8315,7 +8315,7 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p,
struct bgp_redist *red; struct bgp_redist *red;
/* Make default attribute. */ /* 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 * This must not be NULL to satisfy Coverity SA
*/ */

View File

@ -1968,7 +1968,7 @@ route_set_local_pref(void *rule, const struct prefix *prefix, void *object)
path = object; path = object;
/* Set local preference value. */ /* 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; locpref = path->attr->local_pref;
path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF); path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF);

View File

@ -812,12 +812,11 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw)
bgp = peer->bgp; bgp = peer->bgp;
from = bgp->peer_self; from = bgp->peer_self;
bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP);
/* make coverity happy */ /* make coverity happy */
assert(attr.aspath); assert(attr.aspath);
attr.local_pref = bgp->default_local_pref;
attr.med = 0; attr.med = 0;
attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC);

View File

@ -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 */ /* Make default attribute. Produces already-interned attr.aspath */
/* Cripes, the memory management of attributes is byzantine */ /* 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: * At this point:

View File

@ -731,7 +731,7 @@ void vnc_direct_bgp_add_prefix(struct bgp *bgp,
return; 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() */ /* TBD set some configured med, see add_vnc_route() */
vnc_zlog_debug_verbose( vnc_zlog_debug_verbose(
@ -980,7 +980,7 @@ void vnc_direct_bgp_add_nve(struct bgp *bgp, struct rfapi_descriptor *rfd)
return; 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() */ /* 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; 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() */ /* TBD set some configured med, see add_vnc_route() */
/* /*

View File

@ -1,5 +1,7 @@
router bgp 65000 router bgp 65000
no bgp ebgp-requires-policy 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 remote-as 65000
neighbor 192.168.255.1 timers 3 10 neighbor 192.168.255.1 timers 3 10
address-family ipv4 address-family ipv4
@ -9,3 +11,5 @@ router bgp 65000
! !
route-map r1-out permit 10 route-map r1-out permit 10
set local-preference +50 set local-preference +50
route-map l2 permit 10
set local-preference +10

View File

@ -1,7 +1,9 @@
router bgp 65000 router bgp 65000
no bgp ebgp-requires-policy no bgp ebgp-requires-policy
no bgp network import-check
neighbor 192.168.255.1 remote-as 65000 neighbor 192.168.255.1 remote-as 65000
neighbor 192.168.255.1 timers 3 10 neighbor 192.168.255.1 timers 3 10
network 10.10.10.3/32 route-map l3
address-family ipv4 address-family ipv4
redistribute connected redistribute connected
neighbor 192.168.255.1 route-map r1-out out neighbor 192.168.255.1 route-map r1-out out
@ -9,3 +11,5 @@ router bgp 65000
! !
route-map r1-out permit 10 route-map r1-out permit 10
set local-preference -50 set local-preference -50
route-map l3 permit 10
set local-preference -10

View File

@ -89,22 +89,26 @@ def test_bgp_set_local_preference():
expected = { expected = {
"192.168.255.2": { "192.168.255.2": {
"bgpState": "Established", "bgpState": "Established",
"addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 2}}, "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 3}},
}, },
"192.168.255.3": { "192.168.255.3": {
"bgpState": "Established", "bgpState": "Established",
"addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 2}}, "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 3}},
}, },
} }
return topotest.json_cmp(output, expected) return topotest.json_cmp(output, expected)
def _bgp_check_local_preference(router): 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 = { expected = {
"paths": [ "routes": {
{"locPrf": 50, "nexthops": [{"ip": "192.168.255.3"}]}, "10.10.10.2/32": [{"locPrf": 160}],
{"locPrf": 150, "nexthops": [{"ip": "192.168.255.2"}]}, "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) return topotest.json_cmp(output, expected)