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.

Sign up for GitHub

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

Jump to bottom

Pbrd extra #2051

Merged
merged 14 commits into from
Apr 20, 2018
Merged

Pbrd extra #2051

merged 14 commits into from
Apr 20, 2018

Conversation

donaldsharp
Copy link
Member

A bunch of small bug fixes for PBR that we've discovered.

@LabN-CI
Copy link
Collaborator

LabN-CI commented Apr 11, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/2051 765abe8
Date 04/11/2018
Start 11:15:30
Finish 11:38:27
Run-Time 22:57
Total 1816
Pass 1816
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-04-11-11:15:30.txt
Log autoscript-2018-04-11-11:16:18.log.bz2

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-3287/

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

Warnings Generated during build:

Checkout code: Successful with additional warnings:

Report for pbr_map.c
===============================================
< WARNING: braces {} are not necessary for single statement blocks
< #550: FILE: /tmp/f1-17250/pbr_map.c:550:
< +	for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi)) {
< +		pbr_send_pbr_map(pbrms, pmi, install);
Report for pbr_vty.c
===============================================
< WARNING: line over 80 characters
< #398: FILE: /tmp/f1-17250/pbr_vty.c:398:
< +				"    Seq: %u rule: %u Installed: %" PRIu64 "(%u) Reason: %s\n",
<

CLANG Static Analyzer Summary

  • Github Pull Request 2051, comparing to Git base SHA 08097fe

No Changes in Static Analysis warnings compared to base

20 Static Analyzer issues remaining.

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

@donaldsharp donaldsharp requested a review from riw777 April 17, 2018 15:42
donaldsharp and others added 14 commits April 17, 2018 18:40
@donaldsharp
lib, pbrd: Do not allow v6 LL nexthops to be specified without an int…
cafec8d 
…erface.

Prevent the creation of a v6 LL nexthop that does not include an interface
for proper resolution.

Ticket: CM-20276
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
lib: Clean up delete of a nexthop from a nexthop group
ebee2bc 
The delete was not properly deleting the nexthop from
the nexthop group and it was not properly setting the
nexthop's pointers to NULL.

Ticket: CM-20261
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
zebra: Tell rib_process to actually rethink pbr routes.
4d96fd9 
When I implemented this code change I was only testing against
static routes and with one nexthop.  I missed the fact that
we needed to tell rib_process to actually rethink the nexthops.

Ticket: CM-20274
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
pbrd: Prevent attempt at re-install for several scenarios
2fb7892 
There exists several places we attempt to re-install the
same rule.  Figure out when we need to not make an attempt
at doing anything and do it.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
pbrd: Fix installation and deletion in some cases
9b71ea4 
When a nexthop group is modified do not assume that it
is not installed.  The creation of the pnhgc is enough
to set the installed to false.  If we are reinstalling
it is not needed to set it as not installed.

When a pbrms is being installed/removed check to see if it
is already installed/deleted and do the right thing from
there.

Ticket: CM-20371
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
pbrd, zebra: Fix multiple pbr-policy install
37c606f 
Somewhere along the way the ability to install multiple
pbr-policys for the same pbr-map was lost.

Add this back.  There is a limitation in that we are limited
to 64 interfaces per pbr-policy.

Ticket: CM-20429
Signed-off-by: Donald Sharp sharpd@cumulusnetworks.com>
@donaldsharp
pbrd, zebra: Properly notice rule deletion
0f03639 
When a rule is deleted properly notice it in pbr.

Ticket: CM-20394
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@qlyoung @donaldsharp
pbrd: move debug vty into pbr_vty.c
e14f43c 
Dev docs say that CLI goes in _vty.c files

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
@qlyoung @donaldsharp
pbrd: don't collapse pbr debugs
56c63ee 
While compact, collapsing the various debugs into simply `debug pbr` if
all debugs are on is potentially confusing to users.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
@donaldsharp
pbrd: Allow pbr-policy add/removal to install/remove rules
d02e943 
The pbrm->installed variable was being used only in a couple
of places and it has no real bearing on whether or not
we should install a rule or not.  Remove this value.

Ticket: CM-20429
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
pbrd: Fix unused function
24010e9 
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
lib: Store nexthop information independently
c57bd6b 
Store Nexthop's as the incoming raw data.  This will allow
us to separate the act of inputting the cli from the
act of instantiating the cli.

Ticket: CM-20489
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
pbrd: Use proper decode function for interface
d7b3ad4 
Use a proper decode function for a interface state change.

Ticket: CM-20489
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@donaldsharp
lib: Handle if up/down and vrf enable/disable events
98cbbae 
Properly notice when we get if up/down and vrf enable/disable
events and attempt to properly install nexthops as they
come in.

Ticket: CM20489
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@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-3360/

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

Warnings Generated during build:

Checkout code: Successful with additional warnings:

Report for pbr_map.c
===============================================
< WARNING: braces {} are not necessary for single statement blocks
< #550: FILE: /tmp/f1-29263/pbr_map.c:550:
< +	for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi)) {
< +		pbr_send_pbr_map(pbrms, pmi, install);
Report for pbr_vty.c
===============================================
< WARNING: line over 80 characters
< #398: FILE: /tmp/f1-29263/pbr_vty.c:398:
< +				"    Seq: %u rule: %u Installed: %" PRIu64 "(%u) Reason: %s\n",
<

CLANG Static Analyzer Summary

  • Github Pull Request 2051, comparing to Git base SHA 170f8b9

No Changes in Static Analysis warnings compared to base

6 Static Analyzer issues remaining.

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

@LabN-CI
Copy link
Collaborator

LabN-CI commented Apr 17, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/2051 98cbbae
Date 04/17/2018
Start 18:55:26
Finish 19:18:34
Run-Time 23:08
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-04-17-18:55:26.txt
Log autoscript-2018-04-17-18:56:17.log.bz2

For details, please contact louberger

riw777
riw777 approved these changes Apr 20, 2018
View reviewed changes
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

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

LGTM

@riw777 riw777 merged commit 5998141 into FRRouting:master Apr 20, 2018
@donaldsharp donaldsharp deleted the PBRD_EXTRA branch May 15, 2018 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@riw777 riw777 riw777 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@donaldsharp @LabN-CI @NetDEF-CI @riw777 @qlyoung