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: Rib Update Event Scheduler For Un-managed Routes, etc. #5184
Conversation
af8ee1b
to
3f3dc6e
Compare
Fixes #4535 |
💚 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-9297/ This is a comment from an automated CI system. |
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-9296/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base2 Static Analyzer issues remaining.See details at |
3f3dc6e
to
822c600
Compare
💚 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-9318/ This is a comment from an automated CI system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the overall idea of making this a scheduled 'event' is great - but I did have a couple of questions
If we need to batch process the rib (all tables or specific vrf), do so as a scheduled thread event rather than immediately handling it. Further, add context to the events so that you narrow down to certain route types you want to reprocess. Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
We can assume that system/kernel routes are valid indeed if this is our first time procesing them. But since we don't get explicit deletion events for kernel routes anymore, we have to be prepared to process them if the nexthop becomes unreachable for instance. Therefore, if the route is not NEW, then don't assume its valid. Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Since we don't have a daemon who's job is to handle kernel routes and we don't get an explicit route delete anymore if nexthops become unreachable from the kernel, zebra must re-process kernel routes itself to make sure they are still valid. Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
822c600
to
2a18114
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanations - looks good to me.
💚 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-9320/ This is a comment from an automated CI system. |
This patch extends the
rib_update()
api to be scheduled as events on thezrouter.master
rather than processing them immediately.Certain situations exists when we need to do a batch process of all the routes in the rib.
One such example is System/Kernel routes. Since they are not
owned
by any daemon, its zebra's job to manage them in its rib. This was not really a problem until recently when the kernel decided to stop sendingroute delete
messages when routes become unreachable (interface down/addr del events).As such, at least in the linux case, we need to re-process all the kernel routes to verify they are still reachable.