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>

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 162b929f32540c26c862a7bdcc1188c67da9cf1d..c3b5a800704ff75722037f1f847fa8dcf264ccdf 100644 (file)
--- 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 781bfdec3256bc2aa15705a79482c367806e1550..a0838197baa52b0302e819cff37eebc89c3965a5 100644 (file)
--- 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 de246b07ee60d2dac3becf0f1e61286068c51ce7..269b48ce57711b715bef4fa912e06e473d8b0f53 100644 (file)
--- 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 680bf4a4454cce601a5a5534e93720d1e3ce955a..7c5821829703fb88078c6490d82da27792647d5d 100644 (file)
--- 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 546f1ca1ae6317a4b82fea60083d51d79dca471e..02cebd96d2396d22181d6833c04c0218a1c15c0b 100644 (file)
--- 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 2ebdb2a246900d82198d517bef8f11352301f2cc..bf05f5a54fd5b2a0bab4325085a10cabc33476f4 100644 (file)
--- 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 fc1a8bdf337ccb812db6de3b9e333dc0e74ea935..c9ae6f137b551d2131847262abc090169952effe 100644 (file)
--- 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 8bed5156b75096fc0127b64fc4a27d45e70b1e2c..356ff131a4729862f6641e3d6b88e36c2b2dd0bd 100644 (file)
--- 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 534ddcc64bce92098af1464ae666f4b3881a1cac..b441d506e476829ee92e1ec8aa91acadf045b2ed 100644 (file)
--- 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 82a01d457044e76e511c8766b6857292d439f497..1f85a3578bc8caba121466470fb343d7a1cf94f1 100644 (file)
--- 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 65e092b0f251eb63c4f09324329175325a1b59f4..49b8f1ce2a0ef6a073cea500534f9e3dbb8eb88a 100644 (file)
--- 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 d238cc94ec84d0885a766f62385752cd4747a1bd..ec9c8a41c904edfae00734ea8231338f3f47bb17 100644 (file)
--- 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)