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

ldpd: minor fixes #393

Merged
merged 16 commits into from
Apr 26, 2017
Merged

Conversation

rwestphal
Copy link
Member

Here we have a bunch of minor fixes and code cleanup for ldpd.

I'll send later another PR for stable/3.0 with a subset of these patches.

@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-499/

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.

Get source and apply patch from patchwork: Successful

Building Stage: Failed

Ubuntu1204 amd64 build: Successful
Ubuntu1404 amd64 build: Successful
Debian8 amd64 build: Successful
OmniOS amd64 build: Successful
OpenBSD60 amd64 build: Successful
NetBSD7 amd64 build: Successful
NetBSD6 amd64 build: Successful
CentOS7 amd64 build: Successful
Fedora24 amd64 build: Successful
Ubuntu1604 amd64 build: Successful
CentOS6 amd64 build: Successful

FreeBSD9 amd64 build: Failed

Make failed for FreeBSD9 amd64 build:
(see full make log at /browse/FRR-FRRPULLREQ-499/artifact/CI004BUILD/ErrorLog/log_make.txt)

  CC       accept.o
In file included from accept.c:21:
ldpd.h:153: error: 'LOGIN_NAME_MAX' undeclared here (not in a function)
*** [accept.o] Error code 1
Stop in /usr/home/ci/cibuild.499/frr-source/ldpd.
*** [all] Error code 1
Stop in /usr/home/ci/cibuild.499/frr-source/ldpd.
*** [all-recursive] Error code 1
Stop in /usr/home/ci/cibuild.499/frr-source.

FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-499/artifact/CI004BUILD/config.status/config.status

FreeBSD10 amd64 build: Failed

Make failed for FreeBSD10 amd64 build:
(see full make log at /browse/FRR-FRRPULLREQ-499/artifact/CI003BUILD/ErrorLog/log_make.txt)

  CC       accept.o
In file included from accept.c:21:
./ldpd.h:153:14: error: use of undeclared identifier 'LOGIN_NAME_MAX'
        char             user[LOGIN_NAME_MAX];
                              ^
./ldpd.h:154:15: error: use of undeclared identifier 'LOGIN_NAME_MAX'
        char             group[LOGIN_NAME_MAX];
                               ^
2 errors generated.

FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-499/artifact/CI003BUILD/config.status/config.status

FreeBSD11 amd64 build: Failed

Make failed for FreeBSD11 amd64 build:
(see full make log at /browse/FRR-FRRPULLREQ-499/artifact/CI009BUILD/ErrorLog/log_make.txt)

  CC       accept.o
In file included from accept.c:21:
./ldpd.h:153:14: error: use of undeclared identifier 'LOGIN_NAME_MAX'
        char             user[LOGIN_NAME_MAX];
                              ^
./ldpd.h:154:15: error: use of undeclared identifier 'LOGIN_NAME_MAX'
        char             group[LOGIN_NAME_MAX];
                               ^
2 errors generated.

FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-499/artifact/CI009BUILD/config.status/config.status

This is basically to keep in sync with OpenBSD's ldpd(8) where the same
change was done.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The log_warn() and log_warnx() functions indicate non-critical warnings
and errors, so use LOG_ERR instead of LOG_CRIT.

Keep using LOG_CRIT only in fatal() and fatalx() since these functions
indicate critical errors (when the program needs to exit).

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-507/

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.

Get source and apply patch from patchwork: Successful

Building Stage: Successful

Basic Tests: Failed

Static analyzer (clang): Successful
IPv4 protocols on Ubuntu 14.04: Successful
IPv6 protocols on Ubuntu 14.04: Successful

Topology tests on Ubuntu 16.04: Failed

Topology tests on Ubuntu 16.04: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-507/artifact/TOPOU1604/ErrorLog/
Topology tests on Ubuntu 16.04: No useful log found

IPv4 ldp protocol on Ubuntu 16.04: Failed

RFC Compliance Test ANVL-LDP-1.1 failing:
Test Summary
Establish Hello Adjacency and check that DUT Transport Address
matches configured value
Test Reference
Setup Verification
Test Classification
MUST
Test ANVL-LDP-1.1: !FAILED!
Did not get a Hello Adjacency

RFC Compliance Test ANVL-LDP-1.24 failing:
Test Summary
Send DUT labelled data which DUT should forward
Test Reference
Setup Verification
Test Classification
MUST
Test ANVL-LDP-1.24: !FAILED!
Could not open LDP connection(s)

RFC Compliance Test ANVL-LDP-7.17 failing:
Test Summary
When the last Hello adjacency for a LDP session is deleted, the LSR
terminates the LDP session by sending a Notification message and
closing the transport connection.
Test Reference
RFC 3036, s2.5.5 p20 Maintaining Hello Adjacencies
Test Classification
MUST
Test ANVL-LDP-7.17: !FAILED!
Did not receive Hello Message from 192.168.1.10 to port 646

RFC Compliance Test ANVL-LDP-26.8 failing:
Test Summary
An LSR configured for Independent Control and Downstream Unsolicited
mode sends a mapping message when the LSR recognizes a new FEC via the
forwarding table.
Test Reference
RFC 3036, s3.5.7.1.1 p67 Independent Control Mapping
Test Classification
MUST
Test ANVL-LDP-26.8: !FAILED!
Could not open LDP connection(s)

@donaldsharp
Copy link
Member

Once CI runs correctly I'll push it in.

In order to have separate ASLR/cookies per process, ldpd calls exec()
in the child processes after fork() (this is also known as the fork+exec
model).

This is an important security feature but it makes the initialization
of the child processes a bit more complicated as they're not a copy of
the parent anymore, so all parameters given via command line are lost.

To solve this problem, we were creating an argv array by hand with all
necessary parameters and providing it to the exec() syscall. This works
but it's a very ugly solution. This patch introduces a different approach
to solve the problem: send an IMSG_INIT message to the child processes
with all parameters they need in order to initialize properly. This
makes adding additional initialization parameters much more convenient
and less error prone.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When ldpd fails to start for some reason, like failing to create a pid
file, the child processes call their shutdown functions without being
completely initialized. This patch adds some protections to prevent a
segmentation fault on such circumstances.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This is necessary to guarantee that all log messages sent from the child
processes are received in the parent process right away.

Without this patch, when a child process calls fatal() or fatalx(),
the log messages don't make it to the parent because the child doesn't
have a chance to flush its buffers before exiting.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
If we don't do this, we'll never trigger the backoff exponential timer
since it's impossible to distinguish between Initialization NAK's and
general errors.

Also:
* Implement some missing bits from RFC 5036;
* remove superfluous log message in session_shutdown()
  (send_notification() logs that we're sending a fatal notification).

Regression introduced by commit 8819fc3.

Fixes the following ANVL LDP regressions: 6.19 and 6.21.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
On unstable networks, routes can be lost and relearned very often. If
we deallocate the input label every time a route is lost and allocate
a new one when the route is relearned, a lot of changes are made in vain.

This patch introduces a logic in which labels are preserved for at least
five minutes before being deallocated by the LIB garbage collector. This
is consistent with what other implementations do.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Log deleted routes and simplify the code a bit.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Once we send a Label Withdraw, we can't send a Label Mapping for the
same FEC until we receive a Label Release from the peer. This is due to
some limitations in the LDP algorithms described in Appendix A. ("LDP
Label Distribution Procedures") of RFC 5036.

To workaround this issue, make it possible to schedule the sending of
a Label Mapping as soon as a Label Release is received for the same FEC.

The easiest way to test this patch is by typing the "label local advertise
explicit-null" command. ldpd will withdraw all null labels using a
Wildcard FEC and then send new Label Mappings as soon the corresponding
Label Releases are received.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
It's doesn't make sense to enforce that a targeted-hello is received
on an LDP-enabled interface. It should be possible, for example, to use
LDP only to signal pseudowires and other another protocol (e.g. RSVP-TE)
to create end-to-end LSPs.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When the transport address is changed, all interfaces and targeted
neighbors are temporary disabled in the ldpe process until new sockets
bound to the new transport address are received from the parent.

This patch fixes a problem in which adjacencies weren't being removed
after the associated targeted neighbors were disabled. This was causing
ldpd not to set some MD5 sockoptions for new neighbors are thus preventing
MD5-protected sessions to come up after a change in the transport-address.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
@NetDEF-CI
Copy link
Collaborator

Continous 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-522/

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.

@donaldsharp donaldsharp merged commit e24287f into FRRouting:master Apr 26, 2017
@rwestphal rwestphal deleted the ldpd-minor-fixes branch May 3, 2017 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants