From 5e120ff4f42b1149e83b39bd191972875f5afafc Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Mon, 6 Jun 2022 09:49:37 +0300 Subject: [PATCH] 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 --- bgpd/bgp_attr.c | 4 +++- bgpd/bgp_attr.h | 3 ++- bgpd/bgp_evpn.c | 8 ++++---- bgpd/bgp_evpn_mh.c | 4 ++-- bgpd/bgp_route.c | 6 +++--- bgpd/bgp_routemap.c | 2 +- bgpd/bgp_updgrp_adv.c | 4 +--- bgpd/rfapi/rfapi.c | 2 +- bgpd/rfapi/vnc_export_bgp.c | 6 +++--- .../r2/bgpd.conf | 4 ++++ .../r3/bgpd.conf | 4 ++++ ...st_bgp_set_local-preference_add_subtract.py | 18 +++++++++++------- 12 files changed, 39 insertions(+), 26 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 162b929f32..c3b5a80070 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -941,7 +941,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)); @@ -955,6 +956,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 781bfdec32..a0838197ba 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -394,7 +394,8 @@ extern struct attr *bgp_attr_intern(struct attr *attr); extern void bgp_attr_unintern_sub(struct attr *); extern void bgp_attr_unintern(struct attr **); extern void bgp_attr_flush(struct attr *); -extern struct attr *bgp_attr_default_set(struct attr *attr, uint8_t); +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 de246b07ee..269b48ce57 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -1294,8 +1294,8 @@ static int update_evpn_type5_route(struct bgp *bgp_vrf, struct prefix_evpn *evp, if (src_attr) attr = *src_attr; else { - memset(&attr, 0, sizeof(struct attr)); - bgp_attr_default_set(&attr, BGP_ORIGIN_IGP); + memset(&attr, 0, sizeof(attr)); + bgp_attr_default_set(&attr, bgp_vrf, BGP_ORIGIN_IGP); } /* Advertise Primary IP (PIP) is enabled, send individual @@ -1734,7 +1734,7 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn, memset(&attr, 0, sizeof(struct 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; @@ -1997,7 +1997,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 680bf4a445..7c58218297 100644 --- a/bgpd/bgp_evpn_mh.c +++ b/bgpd/bgp_evpn_mh.c @@ -637,7 +637,7 @@ static int bgp_evpn_type4_route_update(struct bgp *bgp, memset(&attr, 0, sizeof(struct 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; @@ -935,7 +935,7 @@ static int bgp_evpn_type1_route_update(struct bgp *bgp, memset(&attr, 0, sizeof(struct 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 546f1ca1ae..02cebd96d2 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -5822,7 +5822,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; @@ -6125,7 +6125,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; @@ -8325,7 +8325,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 2ebdb2a246..bf05f5a54f 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -1956,7 +1956,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 fc1a8bdf33..c9ae6f137b 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -814,13 +814,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; - if ((afi == AFI_IP6) || peer_cap_enhe(peer, afi, safi)) { /* IPv6 global nexthop must be included. */ attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV6_GLOBAL; diff --git a/bgpd/rfapi/rfapi.c b/bgpd/rfapi/rfapi.c index 8bed5156b7..356ff131a4 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 534ddcc64b..b441d506e4 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 82a01d4570..1f85a3578b 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 65e092b0f2..49b8f1ce2a 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 d238cc94ec..ec9c8a41c9 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) -- 2.39.5