zebra: reduce impact of route-update overload #6853
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
polychaeta
reviewed
Aug 4, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/4eb7b3b72e2e5417b10867c23a856b68/raw/62d9852c300f0e0a7f86f641f77682c496e4b30a/cr_6853_1596546416.diff | git apply
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index aa087790c..b5f1be773 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -3326,9 +3326,12 @@ int dplane_show_provs_helper(struct vty *vty, bool detailed)
out_max = atomic_load_explicit(&prov->dp_out_max,
memory_order_relaxed);
- vty_out(vty, "%s (%u): in: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64", out: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64"\n",
- prov->dp_name, prov->dp_id, in, in_q, in_max,
- out, out_q, out_max);
+ vty_out(vty,
+ "%s (%u): in: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 ", out: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 "\n",
+ prov->dp_name, prov->dp_id, in, in_q, in_max, out,
+ out_q, out_max);
DPLANE_LOCK();
prov = TAILQ_NEXT(prov, dp_prov_link);
@@ -3537,10 +3540,8 @@ void dplane_provider_enqueue_out_ctx(struct zebra_dplane_provider *prov,
/* Maintain out-queue counters */
atomic_fetch_add_explicit(&(prov->dp_out_queued), 1,
memory_order_relaxed);
- curr = atomic_load_explicit(&prov->dp_out_queued,
- memory_order_relaxed);
- high = atomic_load_explicit(&prov->dp_out_max,
- memory_order_relaxed);
+ curr = atomic_load_explicit(&prov->dp_out_queued, memory_order_relaxed);
+ high = atomic_load_explicit(&prov->dp_out_max, memory_order_relaxed);
if (curr > high)
atomic_store_explicit(&prov->dp_out_max, curr,
memory_order_relaxed);
@@ -4246,8 +4247,8 @@ static int dplane_thread_loop(struct thread *event)
/* Locate initial registered provider */
prov = TAILQ_FIRST(&zdplane_info.dg_providers_q);
- prov_q_counter = atomic_load_explicit(&prov->dp_in_queued,
- memory_order_relaxed);
+ prov_q_counter =
+ atomic_load_explicit(&prov->dp_in_queued, memory_order_relaxed);
curr = atomic_load_explicit(&zdplane_info.dg_routes_queued,
memory_order_relaxed);
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 78a4ac129..53c078363 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -2956,10 +2956,10 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
/* Link new re to node.*/
if (IS_ZEBRA_DEBUG_RIB) {
- rnode_debug(rn, re->vrf_id,
- "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
- rn, re, zebra_route_string(re->type), same,
- same_count);
+ rnode_debug(
+ rn, re->vrf_id,
+ "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
+ rn, re, zebra_route_string(re->type), same, same_count);
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
route_entry_dump(p, src_p, re);
@@ -2984,7 +2984,8 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
continue;
if (IS_ZEBRA_DEBUG_RIB)
- rnode_debug(rn, re->vrf_id, "rn %p, removing unneeded re %p",
+ rnode_debug(rn, re->vrf_id,
+ "rn %p, removing unneeded re %p",
rn, re);
rib_unlink(rn, re);
If you are a new contributor to FRR, please see our contributing guidelines.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13461/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
polychaeta
reviewed
Aug 4, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/3c838784ed1693f057d8ef557eeab30c/raw/79dad0940367a7dcc8a4744977e708840c14bf2e/cr_6853_1596554443.diff | git apply
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 77e34050c..f5b16d38e 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -3326,9 +3326,12 @@ int dplane_show_provs_helper(struct vty *vty, bool detailed)
out_max = atomic_load_explicit(&prov->dp_out_max,
memory_order_relaxed);
- vty_out(vty, "%s (%u): in: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64", out: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64"\n",
- prov->dp_name, prov->dp_id, in, in_q, in_max,
- out, out_q, out_max);
+ vty_out(vty,
+ "%s (%u): in: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 ", out: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 "\n",
+ prov->dp_name, prov->dp_id, in, in_q, in_max, out,
+ out_q, out_max);
DPLANE_LOCK();
prov = TAILQ_NEXT(prov, dp_prov_link);
@@ -3537,10 +3540,8 @@ void dplane_provider_enqueue_out_ctx(struct zebra_dplane_provider *prov,
/* Maintain out-queue counters */
atomic_fetch_add_explicit(&(prov->dp_out_queued), 1,
memory_order_relaxed);
- curr = atomic_load_explicit(&prov->dp_out_queued,
- memory_order_relaxed);
- high = atomic_load_explicit(&prov->dp_out_max,
- memory_order_relaxed);
+ curr = atomic_load_explicit(&prov->dp_out_queued, memory_order_relaxed);
+ high = atomic_load_explicit(&prov->dp_out_max, memory_order_relaxed);
if (curr > high)
atomic_store_explicit(&prov->dp_out_max, curr,
memory_order_relaxed);
@@ -4246,8 +4247,8 @@ static int dplane_thread_loop(struct thread *event)
/* Locate initial registered provider */
prov = TAILQ_FIRST(&zdplane_info.dg_providers_q);
- prov_q_counter = atomic_load_explicit(&prov->dp_in_queued,
- memory_order_relaxed);
+ prov_q_counter =
+ atomic_load_explicit(&prov->dp_in_queued, memory_order_relaxed);
/* Don't let the plugin queue build up too far */
if (prov_q_counter > 2 * limit)
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 78a4ac129..53c078363 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -2956,10 +2956,10 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
/* Link new re to node.*/
if (IS_ZEBRA_DEBUG_RIB) {
- rnode_debug(rn, re->vrf_id,
- "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
- rn, re, zebra_route_string(re->type), same,
- same_count);
+ rnode_debug(
+ rn, re->vrf_id,
+ "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
+ rn, re, zebra_route_string(re->type), same, same_count);
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
route_entry_dump(p, src_p, re);
@@ -2984,7 +2984,8 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
continue;
if (IS_ZEBRA_DEBUG_RIB)
- rnode_debug(rn, re->vrf_id, "rn %p, removing unneeded re %p",
+ rnode_debug(rn, re->vrf_id,
+ "rn %p, removing unneeded re %p",
rn, re);
rib_unlink(rn, re);
If you are a new contributor to FRR, please see our contributing guidelines.
|
Pushed to clean up SA warning |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
|
Memory footprint is changing significantly. I have 1 full bgp feed, then also install 1000000 routes from sharpd and then remove them. Before this change we have this: eva# show mem With this change we have this: eva# show mem We need to spend more time understanding this. I would also like to see timing installs of data pre and post this change @mjstapp |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13468/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
|
Interesting - I don't see a difference between master and this branch, with 1M sharpd routes; here's some info. PR: and the detailed memory breakdowns were just about identical too. sharp timings were also very close: master: PR: |
polychaeta
reviewed
Aug 18, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/9000b3820d382463c9408ce32cc98eb3/raw/75d41bac0d841a9c35b83e1945b9c287ef34330e/cr_6853_1597781668.diff | git apply
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index cb5486d9d..bb80de784 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -3446,9 +3446,12 @@ int dplane_show_provs_helper(struct vty *vty, bool detailed)
out_max = atomic_load_explicit(&prov->dp_out_max,
memory_order_relaxed);
- vty_out(vty, "%s (%u): in: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64", out: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64"\n",
- prov->dp_name, prov->dp_id, in, in_q, in_max,
- out, out_q, out_max);
+ vty_out(vty,
+ "%s (%u): in: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 ", out: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 "\n",
+ prov->dp_name, prov->dp_id, in, in_q, in_max, out,
+ out_q, out_max);
DPLANE_LOCK();
prov = TAILQ_NEXT(prov, dp_prov_link);
@@ -3657,10 +3660,8 @@ void dplane_provider_enqueue_out_ctx(struct zebra_dplane_provider *prov,
/* Maintain out-queue counters */
atomic_fetch_add_explicit(&(prov->dp_out_queued), 1,
memory_order_relaxed);
- curr = atomic_load_explicit(&prov->dp_out_queued,
- memory_order_relaxed);
- high = atomic_load_explicit(&prov->dp_out_max,
- memory_order_relaxed);
+ curr = atomic_load_explicit(&prov->dp_out_queued, memory_order_relaxed);
+ high = atomic_load_explicit(&prov->dp_out_max, memory_order_relaxed);
if (curr > high)
atomic_store_explicit(&prov->dp_out_max, curr,
memory_order_relaxed);
@@ -4264,8 +4265,8 @@ static int dplane_thread_loop(struct thread *event)
/* Locate initial registered provider */
prov = TAILQ_FIRST(&zdplane_info.dg_providers_q);
- prov_q_counter = atomic_load_explicit(&prov->dp_in_queued,
- memory_order_relaxed);
+ prov_q_counter =
+ atomic_load_explicit(&prov->dp_in_queued, memory_order_relaxed);
/* Don't let the plugin queue build up too far */
if (prov_q_counter > 2 * limit)
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 2dae5e228..bf0f78088 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -2951,10 +2951,10 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
/* Link new re to node.*/
if (IS_ZEBRA_DEBUG_RIB) {
- rnode_debug(rn, re->vrf_id,
- "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
- rn, re, zebra_route_string(re->type), same,
- same_count);
+ rnode_debug(
+ rn, re->vrf_id,
+ "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
+ rn, re, zebra_route_string(re->type), same, same_count);
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
route_entry_dump(p, src_p, re);
@@ -2979,7 +2979,8 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
continue;
if (IS_ZEBRA_DEBUG_RIB)
- rnode_debug(rn, re->vrf_id, "rn %p, removing unneeded re %p",
+ rnode_debug(rn, re->vrf_id,
+ "rn %p, removing unneeded re %p",
rn, re);
rib_unlink(rn, re);
If you are a new contributor to FRR, please see our contributing guidelines.
|
rebased to fix conflicts |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteTopo tests part 1 on Ubuntu 16.04 amd64: Incomplete(check logs for details)Successful on other platforms/tests
|
|
CI:rerun |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13707/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
mjstapp
force-pushed
the
fix_rib_dups
branch
from
eeb595f to
efa6d92
Compare
polychaeta
reviewed
Sep 3, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/58afcbc4bb749948e9e7d18186e56f75/raw/16fb2b5d78c6834a3070b47900fcfe8f3a38da54/cr_6853_1599139017.diff | git apply
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 53b86092b..b6a275a53 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -3444,9 +3444,12 @@ int dplane_show_provs_helper(struct vty *vty, bool detailed)
out_max = atomic_load_explicit(&prov->dp_out_max,
memory_order_relaxed);
- vty_out(vty, "%s (%u): in: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64", out: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64"\n",
- prov->dp_name, prov->dp_id, in, in_q, in_max,
- out, out_q, out_max);
+ vty_out(vty,
+ "%s (%u): in: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 ", out: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 "\n",
+ prov->dp_name, prov->dp_id, in, in_q, in_max, out,
+ out_q, out_max);
DPLANE_LOCK();
prov = TAILQ_NEXT(prov, dp_prov_link);
@@ -3655,10 +3658,8 @@ void dplane_provider_enqueue_out_ctx(struct zebra_dplane_provider *prov,
/* Maintain out-queue counters */
atomic_fetch_add_explicit(&(prov->dp_out_queued), 1,
memory_order_relaxed);
- curr = atomic_load_explicit(&prov->dp_out_queued,
- memory_order_relaxed);
- high = atomic_load_explicit(&prov->dp_out_max,
- memory_order_relaxed);
+ curr = atomic_load_explicit(&prov->dp_out_queued, memory_order_relaxed);
+ high = atomic_load_explicit(&prov->dp_out_max, memory_order_relaxed);
if (curr > high)
atomic_store_explicit(&prov->dp_out_max, curr,
memory_order_relaxed);
@@ -4262,8 +4263,8 @@ static int dplane_thread_loop(struct thread *event)
/* Locate initial registered provider */
prov = TAILQ_FIRST(&zdplane_info.dg_providers_q);
- prov_q_counter = atomic_load_explicit(&prov->dp_in_queued,
- memory_order_relaxed);
+ prov_q_counter =
+ atomic_load_explicit(&prov->dp_in_queued, memory_order_relaxed);
/* Don't let the plugin queue build up too far */
if (prov_q_counter > 2 * limit)
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 973535a46..26f266cac 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -2989,10 +2989,10 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
/* Link new re to node.*/
if (IS_ZEBRA_DEBUG_RIB) {
- rnode_debug(rn, re->vrf_id,
- "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
- rn, re, zebra_route_string(re->type), same,
- same_count);
+ rnode_debug(
+ rn, re->vrf_id,
+ "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
+ rn, re, zebra_route_string(re->type), same, same_count);
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
route_entry_dump(p, src_p, re);
@@ -3017,7 +3017,8 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
continue;
if (IS_ZEBRA_DEBUG_RIB)
- rnode_debug(rn, re->vrf_id, "rn %p, removing unneeded re %p",
+ rnode_debug(rn, re->vrf_id,
+ "rn %p, removing unneeded re %p",
rn, re);
rib_unlink(rn, re);
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
|
Rebased to newer master |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteAddresssanitizer topotests part 2: Incomplete(check logs for details)Addresssanitizer topotests part 2: Incomplete(check logs for details)Addresssanitizer topotests part 0: Incomplete(check logs for details)Successful on other platforms/tests
|
|
CI:rerun |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-14016/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
polychaeta
reviewed
Oct 27, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/35d71548324de8df0867c5b3d32fa537/raw/5217176c210b16e63d654fbb58b3fa410ddd6cdb/cr_6853_1603803712.diff | git apply
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index e57891d5d..83536f0f2 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -3483,9 +3483,12 @@ int dplane_show_provs_helper(struct vty *vty, bool detailed)
out_max = atomic_load_explicit(&prov->dp_out_max,
memory_order_relaxed);
- vty_out(vty, "%s (%u): in: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64", out: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64"\n",
- prov->dp_name, prov->dp_id, in, in_q, in_max,
- out, out_q, out_max);
+ vty_out(vty,
+ "%s (%u): in: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 ", out: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 "\n",
+ prov->dp_name, prov->dp_id, in, in_q, in_max, out,
+ out_q, out_max);
DPLANE_LOCK();
prov = TAILQ_NEXT(prov, dp_prov_link);
@@ -3694,10 +3697,8 @@ void dplane_provider_enqueue_out_ctx(struct zebra_dplane_provider *prov,
/* Maintain out-queue counters */
atomic_fetch_add_explicit(&(prov->dp_out_queued), 1,
memory_order_relaxed);
- curr = atomic_load_explicit(&prov->dp_out_queued,
- memory_order_relaxed);
- high = atomic_load_explicit(&prov->dp_out_max,
- memory_order_relaxed);
+ curr = atomic_load_explicit(&prov->dp_out_queued, memory_order_relaxed);
+ high = atomic_load_explicit(&prov->dp_out_max, memory_order_relaxed);
if (curr > high)
atomic_store_explicit(&prov->dp_out_max, curr,
memory_order_relaxed);
@@ -4298,8 +4299,8 @@ static int dplane_thread_loop(struct thread *event)
/* Locate initial registered provider */
prov = TAILQ_FIRST(&zdplane_info.dg_providers_q);
- prov_q_counter = atomic_load_explicit(&prov->dp_in_queued,
- memory_order_relaxed);
+ prov_q_counter =
+ atomic_load_explicit(&prov->dp_in_queued, memory_order_relaxed);
/* Don't let the plugin queue build up too far */
if (prov_q_counter > 2 * limit)
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 710e0b35f..d73b10648 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -3004,10 +3004,10 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
/* Link new re to node.*/
if (IS_ZEBRA_DEBUG_RIB) {
- rnode_debug(rn, re->vrf_id,
- "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
- rn, re, zebra_route_string(re->type), same,
- same_count);
+ rnode_debug(
+ rn, re->vrf_id,
+ "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
+ rn, re, zebra_route_string(re->type), same, same_count);
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
route_entry_dump(p, src_p, re);
@@ -3032,7 +3032,8 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
continue;
if (IS_ZEBRA_DEBUG_RIB)
- rnode_debug(rn, re->vrf_id, "rn %p, removing unneeded re %p",
+ rnode_debug(rn, re->vrf_id,
+ "rn %p, removing unneeded re %p",
rn, re);
rib_unlink(rn, re);
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-14999/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
added 2 commits
December 7, 2020 13:54
Zebra accumulates route-entry objects and then processes them as a group. If that rib processing is delayed, because the dataplane/fib programming has built up a queue e.g., zebra can hold multiple deleted route objects in memory. At scale, this can be a problem. Delete unneeded route entries promptly, if they can't contribute to rib processing. Signed-off-by: Mark Stapp <mjs@voltanet.io>
Add the current queue depths for each plugin to the 'show dplane providers' output. Maintain the out-bound queue max counter properly, that was being ignored. Signed-off-by: Mark Stapp <mjs@voltanet.io>
polychaeta
reviewed
Dec 7, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/88492a8e2ae8fcac04c92a0bc1eb99f9/raw/fba1cc3d9062a13c5a3bb0acb4aa907bc6408c48/cr_6853_1607371917.diff | git apply
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 41ff73d77..41221ec28 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -3708,9 +3708,12 @@ int dplane_show_provs_helper(struct vty *vty, bool detailed)
out_max = atomic_load_explicit(&prov->dp_out_max,
memory_order_relaxed);
- vty_out(vty, "%s (%u): in: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64", out: %"PRIu64", q: %"PRIu64", q_max: %"PRIu64"\n",
- prov->dp_name, prov->dp_id, in, in_q, in_max,
- out, out_q, out_max);
+ vty_out(vty,
+ "%s (%u): in: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 ", out: %" PRIu64 ", q: %" PRIu64
+ ", q_max: %" PRIu64 "\n",
+ prov->dp_name, prov->dp_id, in, in_q, in_max, out,
+ out_q, out_max);
DPLANE_LOCK();
prov = TAILQ_NEXT(prov, dp_prov_link);
@@ -3919,10 +3922,8 @@ void dplane_provider_enqueue_out_ctx(struct zebra_dplane_provider *prov,
/* Maintain out-queue counters */
atomic_fetch_add_explicit(&(prov->dp_out_queued), 1,
memory_order_relaxed);
- curr = atomic_load_explicit(&prov->dp_out_queued,
- memory_order_relaxed);
- high = atomic_load_explicit(&prov->dp_out_max,
- memory_order_relaxed);
+ curr = atomic_load_explicit(&prov->dp_out_queued, memory_order_relaxed);
+ high = atomic_load_explicit(&prov->dp_out_max, memory_order_relaxed);
if (curr > high)
atomic_store_explicit(&prov->dp_out_max, curr,
memory_order_relaxed);
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index eb6587f82..d6d6f4257 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -3028,10 +3028,10 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
/* Link new re to node.*/
if (IS_ZEBRA_DEBUG_RIB) {
- rnode_debug(rn, re->vrf_id,
- "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
- rn, re, zebra_route_string(re->type), same,
- same_count);
+ rnode_debug(
+ rn, re->vrf_id,
+ "Inserting route rn %p, re %p (%s) existing %p, same_count %d",
+ rn, re, zebra_route_string(re->type), same, same_count);
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
route_entry_dump(p, src_p, re);
@@ -3056,7 +3056,8 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
continue;
if (IS_ZEBRA_DEBUG_RIB)
- rnode_debug(rn, re->vrf_id, "rn %p, removing unneeded re %p",
+ rnode_debug(rn, re->vrf_id,
+ "rn %p, removing unneeded re %p",
rn, re);
rib_unlink(rn, re);
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
|
Rebase, and include only the rib-trimming changes for now. |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15870/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If zebra is facing an impulse of route updates, try to reduce the memory footprint and reduce redundant work. Free redundant route_nodes promptly. Also improve the dataplane counters we show with the 'show zebra dplane provider' cli.