From 0a0137da85f063336e99a1f5a3aa69f650ddc808 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 3 Aug 2023 22:44:17 +0300 Subject: [PATCH 1/5] bgpd: Handle cluster attribute the same way as others using setters/getters To be consistent and error-safe. Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 12 +++++------- bgpd/bgp_attr.h | 5 +++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index e6c44a760a..1321ab32c0 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -167,6 +167,9 @@ static struct cluster_list *cluster_intern(struct cluster_list *cluster) static void cluster_unintern(struct cluster_list **cluster) { + if (!*cluster) + return; + if ((*cluster)->refcnt) (*cluster)->refcnt--; @@ -1205,11 +1208,8 @@ void bgp_attr_unintern_sub(struct attr *attr) bgp_attr_set_lcommunity(attr, NULL); cluster = bgp_attr_get_cluster(attr); - if (cluster) { - cluster_unintern(&cluster); - bgp_attr_set_cluster(attr, cluster); - } - UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_CLUSTER_LIST)); + cluster_unintern(&cluster); + bgp_attr_set_cluster(attr, NULL); struct transit *transit = bgp_attr_get_transit(attr); @@ -2214,8 +2214,6 @@ bgp_attr_cluster_list(struct bgp_attr_parser_args *args) /* XXX: Fix cluster_parse to use stream API and then remove this */ stream_forward_getp(peer->curr, length); - attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_CLUSTER_LIST); - return BGP_ATTR_PARSE_PROCEED; cluster_list_ignore: diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 415df2ce53..f1296af159 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -606,6 +606,11 @@ static inline void bgp_attr_set_cluster(struct attr *attr, struct cluster_list *cl) { attr->cluster1 = cl; + + if (cl) + SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_CLUSTER_LIST)); + else + UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_CLUSTER_LIST)); } static inline const struct bgp_route_evpn * From 09b453775541a66439ddb5b0c24c5f8b3a04928c Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 3 Aug 2023 22:48:40 +0300 Subject: [PATCH 2/5] bgpd: Handle transit attributes the same way as others using setters/getters To be consistent and error-safe. Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 1321ab32c0..cca2ca2b0f 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -421,6 +421,9 @@ static struct transit *transit_intern(struct transit *transit) static void transit_unintern(struct transit **transit) { + if (!*transit) + return; + if ((*transit)->refcnt) (*transit)->refcnt--; @@ -1186,6 +1189,7 @@ void bgp_attr_unintern_sub(struct attr *attr) struct cluster_list *cluster; struct lcommunity *lcomm = NULL; struct community *comm = NULL; + struct transit *transit; /* aspath refcount shoud be decrement. */ aspath_unintern(&attr->aspath); @@ -1211,12 +1215,9 @@ void bgp_attr_unintern_sub(struct attr *attr) cluster_unintern(&cluster); bgp_attr_set_cluster(attr, NULL); - struct transit *transit = bgp_attr_get_transit(attr); - - if (transit) { - transit_unintern(&transit); - bgp_attr_set_transit(attr, transit); - } + transit = bgp_attr_get_transit(attr); + transit_unintern(&transit); + bgp_attr_set_transit(attr, NULL); if (attr->encap_subtlvs) encap_unintern(&attr->encap_subtlvs, ENCAP_SUBTLV_TYPE); From 312b8c02a68f20df39353e9a52e7b49310dbfee6 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 3 Aug 2023 22:52:09 +0300 Subject: [PATCH 3/5] bgpd: Handle encap attributes the same way as others using setters/getters To be consistent and error-safe. Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index cca2ca2b0f..ef9bb4f032 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -333,6 +333,10 @@ static void encap_unintern(struct bgp_attr_encap_subtlv **encapp, encap_subtlv_type type) { struct bgp_attr_encap_subtlv *encap = *encapp; + + if (!*encapp) + return; + if (encap->refcnt) encap->refcnt--; @@ -1219,17 +1223,14 @@ void bgp_attr_unintern_sub(struct attr *attr) transit_unintern(&transit); bgp_attr_set_transit(attr, NULL); - if (attr->encap_subtlvs) - encap_unintern(&attr->encap_subtlvs, ENCAP_SUBTLV_TYPE); + encap_unintern(&attr->encap_subtlvs, ENCAP_SUBTLV_TYPE); #ifdef ENABLE_BGP_VNC struct bgp_attr_encap_subtlv *vnc_subtlvs = bgp_attr_get_vnc_subtlvs(attr); - if (vnc_subtlvs) { - encap_unintern(&vnc_subtlvs, VNC_SUBTLV_TYPE); - bgp_attr_set_vnc_subtlvs(attr, vnc_subtlvs); - } + encap_unintern(&vnc_subtlvs, VNC_SUBTLV_TYPE); + bgp_attr_set_vnc_subtlvs(attr, NULL); #endif if (attr->srv6_l3vpn) From fa2749f58eab50ab98cf52120678a513fee4ce89 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 3 Aug 2023 22:53:21 +0300 Subject: [PATCH 4/5] bgpd: Handle srv6 attributes the same way as others using setters/getters To be consistent and error-safe. Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index ef9bb4f032..d31e266c95 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -568,6 +568,9 @@ static void srv6_l3vpn_unintern(struct bgp_attr_srv6_l3vpn **l3vpnp) { struct bgp_attr_srv6_l3vpn *l3vpn = *l3vpnp; + if (!*l3vpnp) + return; + if (l3vpn->refcnt) l3vpn->refcnt--; @@ -603,6 +606,9 @@ static void srv6_vpn_unintern(struct bgp_attr_srv6_vpn **vpnp) { struct bgp_attr_srv6_vpn *vpn = *vpnp; + if (!*vpnp) + return; + if (vpn->refcnt) vpn->refcnt--; @@ -1233,11 +1239,8 @@ void bgp_attr_unintern_sub(struct attr *attr) bgp_attr_set_vnc_subtlvs(attr, NULL); #endif - if (attr->srv6_l3vpn) - srv6_l3vpn_unintern(&attr->srv6_l3vpn); - - if (attr->srv6_vpn) - srv6_vpn_unintern(&attr->srv6_vpn); + srv6_l3vpn_unintern(&attr->srv6_l3vpn); + srv6_vpn_unintern(&attr->srv6_vpn); } /* Free bgp attribute and aspath. */ From dd58cd4d38f7282ba33bbee08c4a84ec50984b29 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 3 Aug 2023 22:54:36 +0300 Subject: [PATCH 5/5] bgpd: Use SET_FLAG when setting AIGP attribute flag Just reuse an existing more-readable code. Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index f1296af159..961e5f1224 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -594,7 +594,7 @@ static inline void bgp_attr_set_aigp_metric(struct attr *attr, uint64_t aigp) attr->aigp_metric = aigp; if (aigp) - attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_AIGP); + SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)); } static inline struct cluster_list *bgp_attr_get_cluster(const struct attr *attr)