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

Add Pseudowire management in Zebra #710

Closed
wants to merge 6 commits into from

Conversation

bingen
Copy link
Member

@bingen bingen commented Jun 13, 2017

When Zebra receives a PW from ldpd, it tries to install it in kernel and
in case of failure notifies back to ldpd daemon and enqueues the PW for
retry.

The installation to kernel is not yet implemented as Linux kernel
doesn't have support for it yet. It will be done through a module hook,
so it can be outsourced to a remote dataplane.

Support for OpenBSD's pseudowires and Nexthop tracking for PWs are added too.

ßingen and others added 5 commits June 13, 2017 18:23
When Zebra receives a PW from ldpd, it tries to install it in kernel and
in case of failure notifies back to ldpd daemon and enqueues the PW for
retry.

The installation to kernel is not yet implemented as Linux kernel
doesn't have support for it yet. It will be done through a module hook,
so it can be outsourced to a remote dataplane.

Signed-off-by: ßingen <[email protected]>
Signed-off-by: Renato Westphal <[email protected]>
Implicit-null labels are never installed in the FIB but we need to keep
track of them because of L2/L3 VPNs nexthop resolution.

Signed-off-by: Renato Westphal <[email protected]>
It doesn't make sense to set RIB_ENTRY_NEXTHOPS_CHANGED outside
zebra_rib.c because this flag is always cleared and reevaluated in the
rib_process() function.

With that said, define a new flag to indicate that the MPLS labels
of a nexthop have been changed. Then translate this flag into
RIB_ENTRY_NEXTHOPS_CHANGED in the nexthop_active_update() function.

This allows the NHT code to detect when labels are added/removed from
a nexthop and notify its clients accordingly.

Signed-off-by: Renato Westphal <[email protected]>
Nexthop tracking allows ldpd to ensure that the nexthop of a pseudowire
resolves into an MPLS LSP and only then activate it. Also, whenever the
nexthop resolution for a pseudowire changes, check if it needs to be
updated/installed/uninstalled in the kernel/hardware.

Signed-off-by: Renato Westphal <[email protected]>
@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: FAILED

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

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

IPv4 ldp protocol on Ubuntu 16.04: Successful
Static analyzer (clang): Successful
Addresssanitizer topotest: Successful
IPv4 protocols on Ubuntu 14.04: Successful
IPv6 protocols on Ubuntu 14.04: Successful

Topology tests on Ubuntu 16.04 amd64: Failed

Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1604-920/test

Topology Tests failed for Topology tests on Ubuntu 16.04 amd64
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-920/artifact/TOPOU1604/ErrorLog/log_topotests.txt

Topology Tests memoy analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-920/artifact/TOPOU1604/MemoryLeaks/

CLANG Static Analyzer Summary

  • Github Pull Request 710, comparing to Git base SHA 98f65fd

New warnings:

Static Analysis warning summary compared to base:

  • Fixed warnings: 0
  • New warnings: 1

133 Static Analyzer issues remaining.

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

@rwestphal
Copy link
Member

I'll take a look at the topotest failure tomorrow.

ldpd/ldp_zebra.c Outdated
@@ -153,18 +156,84 @@ kr_delete(struct kroute *kr)
return (zebra_send_mpls_labels(ZEBRA_MPLS_LABELS_DELETE, kr));
}

static int
Copy link
Member

Choose a reason for hiding this comment

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

Does this function Belong in zclient.c to allow other, future, routing protocols to take advantage of it? Should it be under zapi_route as well? Instead of a specialized function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I'll submit another commit in a while.

* Receive PW status update from Zebra and send it to LDE process.
*/
static int
ldp_zebra_read_pw_status_update(int command, struct zclient *zclient,
Copy link
Member

Choose a reason for hiding this comment

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

Again, should this function of decoding the data be in zclient.c?


DEFINE_MTYPE_STATIC(LIB, PW, "Pseudowire")

DEFINE_HOOK(pw_change, (struct zebra_pw_t *pw), (pw))
Copy link
Member

Choose a reason for hiding this comment

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

so pw_change is the hook to the platform specific code to call to install into the kernel. In light of the discussion we had last Wednesday about zebra -> kernel interactions. I don't mind this being a hook, but perhaps the correct place for the hook DEFINE is in a different file? (The declare belongs in rt.h I think )

@rwestphal
Copy link
Member

This should fix the topotest failure: FRRouting/topotests#7

Once this is merged, we can merge the PR in topotest. We can ignore the topotest failure for now.

@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: FAILED

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

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

Fedora24 amd64 build: Successful
FreeBSD11 amd64 build: Successful
NetBSD7 amd64 build: Successful
Ubuntu1604 amd64 build: Successful
NetBSD6 amd64 build: Successful
CentOS7 amd64 build: Successful
CentOS6 amd64 build: Successful
Debian8 amd64 build: Successful
FreeBSD10 amd64 build: Successful
FreeBSD9 amd64 build: Successful
Ubuntu1404 amd64 build: Successful
OmniOS amd64 build: Successful
Ubuntu1204 amd64 build: Successful

OpenBSD60 amd64 build: Failed

Make failed for OpenBSD60 amd64 build:
(see full make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-951/artifact/CI011BUILD/ErrorLog/log_make.txt)

  CC       zebra_mpls_openbsd.o
zebra_mpls_openbsd.c: In function 'kmpw_install':
zebra_mpls_openbsd.c:351: error: 'struct zebra_pw_t' has no member named 'pw_type'
zebra_mpls_openbsd.c:361: error: 'struct zebra_pw_t' has no member named 'pw_type'
zebra_mpls_openbsd.c: In function 'pw_change_openbsd':
zebra_mpls_openbsd.c:430: error: 'PW_SET' undeclared (first use in this function)
zebra_mpls_openbsd.c:430: error: (Each undeclared identifier is reported only once
zebra_mpls_openbsd.c:430: error: for each function it appears in.)
*** Error 1 in zebra (Makefile:746 'zebra_mpls_openbsd.o': @echo "  CC      " zebra_mpls_openbsd.o;gcc -std=gnu99 -DHAVE_CONFIG_H -DSYSCONFD...)

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

@bingen
Copy link
Member Author

bingen commented Jun 15, 2017

Replaced by #724

@bingen bingen closed this Jun 15, 2017
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.

4 participants