Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zebra run once #4154

Merged
merged 7 commits into from
Apr 19, 2019
Merged

Zebra run once #4154

merged 7 commits into from
Apr 19, 2019

Conversation

donaldsharp
Copy link
Member

Run nexthop_active_update() one time as part of handling a new route in the system.

@LabN-CI
Copy link
Collaborator

LabN-CI commented Apr 18, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/4154 d67c8d4
Date 04/17/2019
Start 21:45:25
Finish 22:09:12
Run-Time 23:47
Total 1816
Pass 1816
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2019-04-17-21:45:25.txt
Log autoscript-2019-04-17-21:46:11.log.bz2
Memory 438 445 374

For details, please contact louberger

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7276/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.


CLANG Static Analyzer Summary

  • Github Pull Request 4154, comparing to Git base SHA 3c79400

No Changes in Static Analysis warnings compared to base

12 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7276/artifact/shared/static_analysis/index.html

@qlyoung qlyoung added the zebra label Apr 18, 2019
Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the solution very much; just had a couple of questions inline

@FRRouting FRRouting deleted a comment from LabN-CI Apr 18, 2019
@FRRouting FRRouting deleted a comment from NetDEF-CI Apr 18, 2019
The tests are not coming up consistently on my test box.  Add a bit of wait
time to test to allow normal bgp when the first attempt doesn't come up.
Especially since bgp timeouts are 120 seconds with non datacenter compiles.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Add a ability to count the number of nexthops in a nexthop_group.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We are effectively calling nexthop_active_update() on every
route entry being processed for installation at least 2 times.
This is a bit ridiculous.  We need to resolve the nexthops
when we know a route has changed in some manner, so do so.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The NEXTHOP_FLAG_FILTERED went away when we started treating
static routes like every other route in the system.  This was
a special case for handling static route code that just didn't
get finished cleaning up.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The nexthop_active_update command looks at each individual
nexthop and decides if it has changed.  If any nexthop
has changed we will set the re->status to ROUTE_ENTRY_CHANGED
and ROUTE_ENTRY_NEXTHOPS_CHANGED.

Additionally the test for old_nh_num != curr_active
makes no sense because suppose we have several events
we are processing at the same time and a total ecmp
of 16 but 14 are active at the start and 14 are active
at the end but different interfaces are up or down.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We currently run nexthop_active_check multiple times.  Make the
code run once and figure out state from that.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Update the nexthop flag output for the route entry dump to
include all possible flag states be output.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@LabN-CI
Copy link
Collaborator

LabN-CI commented Apr 18, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/4154 df38b09
Date 04/18/2019
Start 15:24:08
Finish 15:47:50
Run-Time 23:42
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2019-04-18-15:24:08.txt
Log autoscript-2019-04-18-15:24:52.log.bz2
Memory 434 445 374

For details, please contact louberger

Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - I'll merge tomorrow in case someone wants to chime in

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7299/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.


CLANG Static Analyzer Summary

  • Github Pull Request 4154, comparing to Git base SHA fd5c2ea
  • Base image data for Git fd5c2ea does not exist - compare skipped

12 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7299/artifact/shared/static_analysis/index.html

@mjstapp mjstapp merged commit 6019e4f into FRRouting:master Apr 19, 2019
@donaldsharp donaldsharp deleted the zebra_run_once branch April 30, 2019 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants