Skip to content

Commit

Permalink
frr: Miscellanewous fixes in bgp and zebra
Browse files Browse the repository at this point in the history
Signed-off-by: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
  • Loading branch information
sudhanshukumar22 committed Jan 11, 2021
1 parent 2816515 commit 1524037
Show file tree
Hide file tree
Showing 13 changed files with 542 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/sonic-frr/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ STG_BRANCH = stg_temp.$(SUFFIX)
$(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :
# Build the package
pushd ./frr
# delete previously created temporary branches, if any
git checkout master
git branch | grep stg_temp | xargs --no-run-if-empty git branch -D
if [ `git branch | grep $(FRR_BRANCH)` ]
then
echo "Branch named $(FRR_BRANCH) already exists"
git branch -D $(FRR_BRANCH)
else
echo "Branch named $(FRR_BRANCH) does not exist"
fi
git checkout -b $(FRR_BRANCH) origin/$(FRR_BRANCH)
stg branch --create $(STG_BRANCH) $(FRR_TAG)
stg import -s ../patch/series
Expand Down
32 changes: 32 additions & 0 deletions src/sonic-frr/patch/0012-BGP-keepalive-timer-wasn-t-reset-in.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
From 2c8210f2b31c2e833df936ceb725d9d685c0d2be Mon Sep 17 00:00:00 2001
From: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
Date: Fri, 8 Jan 2021 23:12:00 -0800
Subject: [PATCH] BGP keepalive timer wasn't reset in case of duplicate
connection. Identified the fix and found the similar FRR PR. Pulling
following ones: 5429, 5368, and 5446.

---
bgpd/bgp_packet.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index f26c8a6a2..e5c15f547 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -506,6 +506,13 @@ void bgp_keepalive_send(struct peer *peer)
{
struct stream *s;

+ if (peer->fd < 0)
+ {
+ if (bgp_debug_keepalive(peer))
+ zlog_debug("%s: Null fd, failed to send KEEPALIVE", peer->host);
+ return;
+ }
+
s = stream_new(BGP_MAX_PACKET_SIZE);

/* Make keepalive packet. */
--
2.12.2

46 changes: 46 additions & 0 deletions src/sonic-frr/patch/0014-Link-local-scope-was-not-set-while.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
From f48d4860b5588df9222ed917447e336cfe899d9e Mon Sep 17 00:00:00 2001
From: Syed Hasan Raza Naqvi <syed.naqvi@broadcom.com>
Date: Wed, 8 Jan 2020 00:44:17 -0800
Subject: [PATCH] Link local scope was not set while binding
socket with local address causing socket errors for bgp ipv6 link local
neighbors.
The corresponding pull request is https://github.com/FRRouting/frr/pull/7434
Change-Id: I6211f545e288164ff1576103f10be503555dfccd
---
bgpd/bgp_network.c | 5 +++++
bgpd/bgp_zebra.c | 3 +++
2 files changed, 8 insertions(+)

diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c
index a4b91f698..4b7b8f2ab 100644
--- a/bgpd/bgp_network.c
+++ b/bgpd/bgp_network.c
@@ -579,6 +579,11 @@ static int bgp_update_address(struct interface *ifp, const union sockunion *dst,
return 1;

prefix2sockunion(sel, addr);
+
+ if (IN6_IS_ADDR_LINKLOCAL(&addr->sin6.sin6_addr)) {
+ addr->sin6.sin6_scope_id = ifp->ifindex;
+ }
+
return 0;
}

diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index dcb9fa8c8..42226d2cc 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -802,6 +802,9 @@ bool bgp_zebra_nexthop_set(union sockunion *local, union sockunion *remote,
? peer->conf_if
: peer->ifname,
peer->bgp->vrf_id);
+ else if (peer->update_if)
+ ifp = if_lookup_by_name(peer->update_if,
+ peer->bgp->vrf_id);
} else if (peer->update_if)
ifp = if_lookup_by_name(peer->update_if,
peer->bgp->vrf_id);
--
2.12.2

30 changes: 30 additions & 0 deletions src/sonic-frr/patch/0015-Holdtime-and-keepalive-parameters-w.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
From 82e00c4162ae70815f8b6c58a3574268720affb2 Mon Sep 17 00:00:00 2001
From: Syed Hasan Raza Naqvi <syed.naqvi@broadcom.com>
Date: Fri, 22 May 2020 17:12:40 -0700
Subject: [PATCH] Holdtime and keepalive parameters weren't
copied from peer-group to peer-group members. Fixed the issue by copying
holdtime and keepalive parameters from peer-group to its members.
The corresponding pull request is https://github.com/FRRouting/frr/pull/7435.
Change-Id: I50b91dde2b8cad2000ce97f1d11f1434c22b4055
---
bgpd/bgpd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 3fb6c9f11..1b67522b1 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -4989,8 +4989,8 @@ int peer_timers_set(struct peer *peer, uint32_t keepalive, uint32_t holdtime)

/* Set flag and configuration on peer-group member. */
SET_FLAG(member->flags, PEER_FLAG_TIMER);
- PEER_ATTR_INHERIT(peer, peer->group, holdtime);
- PEER_ATTR_INHERIT(peer, peer->group, keepalive);
+ PEER_ATTR_INHERIT(member, peer->group, holdtime);
+ PEER_ATTR_INHERIT(member, peer->group, keepalive);
}

return 0;
--
2.12.2

52 changes: 52 additions & 0 deletions src/sonic-frr/patch/0016-Aggregate-member-route-was-enqueued.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
From cedcd601b54f9f561c5bf289e0862215d9820464 Mon Sep 17 00:00:00 2001
From: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
Date: Fri, 8 Jan 2021 23:30:34 -0800
Subject: [PATCH] Aggregate member route was enqueued for recalculation while
bgp instance was deleted. As part of aggregate member route deletion, the
aggregate route is reinstalled with self-peer as source, but self-peer is
already removed. Assert() for null peer pointer is path attribute aborts
bgp.

Fix :
When bgp instance is in the state of deletion, or self-peer is null,
avoid installation of aggregate-prefix as part of aggregate member
delete.
When all of the aggregate members are removed, the aggregate prefix is
automatically removed from zebra.
The pull request is https://github.com/FRRouting/frr/pull/7433/files
---
bgpd/bgp_route.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 0e91b9955..056ea5848 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -6276,14 +6276,19 @@ static void bgp_aggregate_install(
bgp_aggregate_delete(bgp, p, afi, safi, aggregate);
return;
}
-
- new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_AGGREGATE, 0,
+ /* Do not install the aggregate route if BGP is in the
+ * process of termination.
+ */
+ if (bgp->peer_self &&
+ !CHECK_FLAG(bgp->flags, BGP_FLAG_DELETE_IN_PROGRESS)) {
+ new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_AGGREGATE, 0,
bgp->peer_self, attr, dest);

- SET_FLAG(new->flags, BGP_PATH_VALID);
+ SET_FLAG(new->flags, BGP_PATH_VALID);
+ bgp_path_info_add(dest, new);
+ bgp_process(bgp, dest, afi, safi);
+ }

- bgp_path_info_add(dest, new);
- bgp_process(bgp, dest, afi, safi);
} else {
for (pi = orig; pi; pi = pi->next)
if (pi->peer == bgp->peer_self
--
2.12.2

71 changes: 71 additions & 0 deletions src/sonic-frr/patch/0018-When-user-is-config-connect-timer-i.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
From 28c0e4c2aeaf92704f145a1b928086c7842be102 Mon Sep 17 00:00:00 2001
From: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
Date: Sat, 9 Jan 2021 01:32:33 -0800
Subject: [PATCH] When user is config connect timer, it doesn't reflect
immediately. It reflect when next time neighbor is tried to reconnect.

Fix: Code is change to update the connect timer immediately if BGP
session is not in establish state.
---
bgpd/bgpd.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 22962a65c..bb832464e 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -5119,6 +5119,12 @@ int peer_timers_connect_set(struct peer *peer, uint32_t connect)
peer->connect = connect;
peer->v_connect = connect;

+ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) && (peer->status != Established) ) {
+ if (peer_active(peer))
+ BGP_EVENT_ADD(peer, BGP_Stop);
+ BGP_EVENT_ADD(peer, BGP_Start);
+ }
+
/* Skip peer-group mechanics for regular peers. */
if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP))
return 0;
@@ -5136,6 +5142,12 @@ int peer_timers_connect_set(struct peer *peer, uint32_t connect)
SET_FLAG(member->flags, PEER_FLAG_TIMER_CONNECT);
member->connect = connect;
member->v_connect = connect;
+
+ if (member->status != Established) {
+ if (peer_active(member))
+ BGP_EVENT_ADD(member, BGP_Stop);
+ BGP_EVENT_ADD(member, BGP_Start);
+ }
}

return 0;
@@ -5162,6 +5174,12 @@ int peer_timers_connect_unset(struct peer *peer)
else
peer->v_connect = peer->bgp->default_connect_retry;

+ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) && (peer->status != Established) ) {
+ if (peer_active(peer))
+ BGP_EVENT_ADD(peer, BGP_Stop);
+ BGP_EVENT_ADD(peer, BGP_Start);
+ }
+
/* Skip peer-group mechanics for regular peers. */
if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP))
return 0;
@@ -5179,6 +5197,12 @@ int peer_timers_connect_unset(struct peer *peer)
UNSET_FLAG(member->flags, PEER_FLAG_TIMER_CONNECT);
member->connect = 0;
member->v_connect = peer->bgp->default_connect_retry;
+
+ if (member->status != Established) {
+ if (peer_active(member))
+ BGP_EVENT_ADD(member, BGP_Stop);
+ BGP_EVENT_ADD(member, BGP_Start);
+ }
}

return 0;
--
2.12.2

46 changes: 46 additions & 0 deletions src/sonic-frr/patch/0019-when-vrf-add-is-received-add-Vrf-na.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
From 743d562892a6d57b932b3d89c1a416a3827dd20a Mon Sep 17 00:00:00 2001
From: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
Date: Sat, 9 Jan 2021 01:41:40 -0800
Subject: [PATCH] when vrf add is received, add Vrf-name to the interface
database. This is needed while binding the VRF interface to the BGP socket
https://github.com/FRRouting/frr/pull/7432

---
bgpd/bgp_main.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
index fa67cfd56..d8d3867e7 100644
--- a/bgpd/bgp_main.c
+++ b/bgpd/bgp_main.c
@@ -261,16 +261,23 @@ static __attribute__((__noreturn__)) void bgp_exit(int status)
static int bgp_vrf_new(struct vrf *vrf)
{
if (BGP_DEBUG(zebra, ZEBRA))
- zlog_debug("VRF Created: %s(%u)", vrf->name, vrf->vrf_id);
-
- return 0;
+ zlog_info("VRF Creation message received: %s(%u)", vrf->name, vrf->vrf_id);
+ struct interface *ifp;
+ ifp = if_get_by_name(vrf->name, vrf->vrf_id);
+ if (ifp)
+ {
+ if (BGP_DEBUG(zebra, ZEBRA))
+ zlog_info("VRF interface Created: %s(%u) ifindex %d", ifp->name, ifp->vrf_id, ifp->ifindex);
+ }
+ return 0;
}

static int bgp_vrf_delete(struct vrf *vrf)
{
if (BGP_DEBUG(zebra, ZEBRA))
zlog_debug("VRF Deletion: %s(%u)", vrf->name, vrf->vrf_id);
-
+ /* No need to delete vrf here as it will be deleted when BGP client receives
+ ZEBRA_VRF_DELETE message from zebra */
return 0;
}

--
2.12.2

36 changes: 36 additions & 0 deletions src/sonic-frr/patch/0032-Remove-the-assert-since-in-the-case.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
From bacdacefe8ceffeb656d8ac17a64e9d4dd46819f Mon Sep 17 00:00:00 2001
From: Kishore Kunal <kishore.kunal@broadcom.com>
Date: Mon, 27 Jan 2020 14:19:42 -0800
Subject: [PATCH] [JIRA SONIC-16617] Remove the assert since in the case where
l3vni was in the oper down state, it is possoble that the function return len
as zero. Asserts are already present in the called function, so no need to
have assert here. So removing the assert at this location.

Change-Id: Ic119c21223b7a372528e49cf1d26e0f3c69aabbc
---
zebra/zebra_fpm.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/zebra/zebra_fpm.c b/zebra/zebra_fpm.c
index 4c27e1344..0ae88204c 100644
--- a/zebra/zebra_fpm.c
+++ b/zebra/zebra_fpm.c
@@ -996,8 +996,13 @@ static int zfpm_build_route_updates(void)
if (write_msg) {
data_len = zfpm_encode_route(dest, re, (char *)data,
buf_end - data, &msg_type);
-
- assert(data_len);
+ /*
+ * It is possible that zfpm_encode_route return zero, when route is not required to be
+ * send to the FPM, like in case of l3vni, if oper status for the l3vni is not up, route
+ * should not be send to FPM. If there are issue with encoding assert is already present
+ * in the function netlink_route_info_encode.
+ */
+ //assert(data_len);
if (data_len) {
hdr->msg_type = msg_type;
msg_len = fpm_data_len_to_msg_len(data_len);
--
2.12.2

48 changes: 48 additions & 0 deletions src/sonic-frr/patch/0033-While-putting-the-route-for-route-s.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
From 89e6a0d31668044419489a378f725cb8452169fc Mon Sep 17 00:00:00 2001
From: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
Date: Sat, 9 Jan 2021 04:56:24 -0800
Subject: [PATCH] While putting the route for route selection, it is checking
for flag in destination. Which looks like didn't got cleaned correctly,
resetting the flag so that next time route calculation can be triggered in
the Zebra.

---
zebra/rib.h | 1 +
zebra/zebra_vrf.c | 2 ++
2 files changed, 3 insertions(+)

diff --git a/zebra/rib.h b/zebra/rib.h
index 38f802979..f70c0901f 100644
--- a/zebra/rib.h
+++ b/zebra/rib.h
@@ -174,6 +174,7 @@ struct route_entry {
* don't generate routes
*/
#define MQ_SIZE 7
+#define RESET_FROM_ALL_MQ 0x3f
struct meta_queue {
struct list *subq[MQ_SIZE];
uint32_t size; /* sum of lengths of all subqueues */
diff --git a/zebra/zebra_vrf.c b/zebra/zebra_vrf.c
index be4fb29aa..3f95e449e 100644
--- a/zebra/zebra_vrf.c
+++ b/zebra/zebra_vrf.c
@@ -223,6 +223,7 @@ static int zebra_vrf_disable(struct vrf *vrf)
rnode)) {
dest = rib_dest_from_rnode(rnode);
if (dest && rib_dest_vrf(dest) == zvrf) {
+ UNSET_FLAG(dest->flags, RESET_FROM_ALL_MQ);
route_unlock_node(rnode);
list_delete_node(zrouter.mq->subq[i], lnode);
zrouter.mq->size--;
@@ -274,6 +275,7 @@ static int zebra_vrf_delete(struct vrf *vrf)
rnode)) {
dest = rib_dest_from_rnode(rnode);
if (dest && rib_dest_vrf(dest) == zvrf) {
+ UNSET_FLAG(dest->flags, RESET_FROM_ALL_MQ);
route_unlock_node(rnode);
list_delete_node(zrouter.mq->subq[i], lnode);
zrouter.mq->size--;
--
2.12.2

Loading

0 comments on commit 1524037

Please sign in to comment.