-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
lib, ripnd: Isolate aggreagate pointer to using protocol #2715
Conversation
#include "ripngd/ripng_debug.h" | ||
|
||
/* If RFC2133 definition is used. */ | ||
#ifndef IPV6_JOIN_GROUP | ||
#define IPV6_JOIN_GROUP IPV6_ADD_MEMBERSHIP | ||
#define IPV6_JOIN_GROUP IPV6_ADD_MEMBERSHIP |
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.
Intentional whitespace removal it was annoying me.
🛑 Basic BGPD CI results: FAILUREResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an EXPERIMENTAL automated CI system. Get source and apply patch from patchwork: SuccessfulBuilding Stage: FailedUbuntu1204 amd64 build: Successful FreeBSD9 amd64 build: FailedMake failed for FreeBSD9 amd64 build:
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI004BUILD/config.status/config.status Ubuntu 16.04 i386: FailedUbuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/U1604I386/config.status/config.status
CentOS7 amd64 build: FailedMake failed for CentOS7 amd64 build:
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI005BUILD/config.status/config.status Ubuntu 18.04 amd64 build: FailedUbuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/U1804AMD64/config.status/config.status
FreeBSD10 amd64 build: FailedMake failed for FreeBSD10 amd64 build:
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI003BUILD/config.status/config.status Ubuntu1404 amd64 build: FailedMake failed for Ubuntu1404 amd64 build:
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI001BUILD/config.status/config.status Debian8 amd64 build: FailedMake failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI008BLD/config.status/config.status OmniOS amd64 build: FailedMake failed for OmniOS amd64 build:
OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI010BUILD/config.status/config.status OpenBSD60 amd64 build: FailedMake failed for OpenBSD60 amd64 build:
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI011BUILD/config.status/config.status Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI021BUILD/config.status/config.status
NetBSD7 amd64 build: FailedMake failed for NetBSD7 amd64 build:
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI012BUILD/config.status/config.status Ubuntu1604 amd64 build: FailedMake failed for Ubuntu1604 amd64 build:
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI014BUILD/config.status/config.status FreeBSD11 amd64 build: FailedMake failed for FreeBSD11 amd64 build:
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI009BUILD/config.status/config.status CentOS6 amd64 build: FailedMake failed for CentOS6 amd64 build:
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI006BUILD/config.status/config.status NetBSD6 amd64 build: FailedMake failed for NetBSD6 amd64 build:
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI007BUILD/config.status/config.status Fedora24 amd64 build: FailedMake failed for Fedora24 amd64 build:
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI015BUILD/config.status/config.status Warnings Generated during build:Checkout code: Successful with additional warnings:FreeBSD9 amd64 build: FailedMake failed for FreeBSD9 amd64 build:
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI004BUILD/config.status/config.status Ubuntu 16.04 i386: FailedUbuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/U1604I386/config.status/config.status
CentOS7 amd64 build: FailedMake failed for CentOS7 amd64 build:
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI005BUILD/config.status/config.status Ubuntu 18.04 amd64 build: FailedUbuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/U1804AMD64/config.status/config.status
FreeBSD10 amd64 build: FailedMake failed for FreeBSD10 amd64 build:
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI003BUILD/config.status/config.status Ubuntu1404 amd64 build: FailedMake failed for Ubuntu1404 amd64 build:
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI001BUILD/config.status/config.status Debian8 amd64 build: FailedMake failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI008BLD/config.status/config.status OmniOS amd64 build: FailedMake failed for OmniOS amd64 build:
OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI010BUILD/config.status/config.status OpenBSD60 amd64 build: FailedMake failed for OpenBSD60 amd64 build:
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI011BUILD/config.status/config.status Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI021BUILD/config.status/config.status
NetBSD7 amd64 build: FailedMake failed for NetBSD7 amd64 build:
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI012BUILD/config.status/config.status Ubuntu1604 amd64 build: FailedMake failed for Ubuntu1604 amd64 build:
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI014BUILD/config.status/config.status FreeBSD11 amd64 build: FailedMake failed for FreeBSD11 amd64 build:
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI009BUILD/config.status/config.status CentOS6 amd64 build: FailedMake failed for CentOS6 amd64 build:
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI006BUILD/config.status/config.status NetBSD6 amd64 build: FailedMake failed for NetBSD6 amd64 build:
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI007BUILD/config.status/config.status Fedora24 amd64 build: FailedMake failed for Fedora24 amd64 build:
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4594/artifact/CI015BUILD/config.status/config.status
|
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an EXPERIMENTAL automated CI system. Get source and apply patch from patchwork: SuccessfulBuilding Stage: FailedFedora24 amd64 build: Successful Ubuntu1404 amd64 build: FailedPackage building failed for Ubuntu1404 amd64 build:
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/CI001BUILD/config.status/config.status Ubuntu1604 amd64 build: FailedUbuntu1604 amd64 build: Unknown Log <log_snapcraft.txt> Ubuntu 18.04 amd64 build: FailedUbuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/U1804AMD64/config.status/config.status
Ubuntu 16.04 i386: FailedUbuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/U1604I386/config.status/config.status Debian8 amd64 build: FailedPackage building failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/CI008BLD/config.status/config.status Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/CI021BUILD/config.status/config.status
Warnings Generated during build:Checkout code: Successful with additional warnings:Ubuntu1404 amd64 build: FailedPackage building failed for Ubuntu1404 amd64 build:
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/CI001BUILD/config.status/config.status Ubuntu1604 amd64 build: FailedUbuntu1604 amd64 build: Unknown Log <log_snapcraft.txt> Ubuntu 18.04 amd64 build: FailedUbuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/U1804AMD64/config.status/config.status
Ubuntu 16.04 i386: FailedUbuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/U1604I386/config.status/config.status Debian8 amd64 build: FailedPackage building failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/CI008BLD/config.status/config.status Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4629/artifact/CI021BUILD/config.status/config.status
|
🛑 Basic BGPD CI results: FAILUREResults table
For details, please contact louberger |
The `void *aggregate` pointer in ROUTE_NODE_FIELDS is only used by ripngd and the bgp rfapi code. As such every single route_node in the system is paying the cost of keeping this pointer around and never using it. Follow the table.h design pattern in bgp_table.h and create a `struct ripng_node` and `struct rfapi_node` pointers that actually have this aggregate pointer and then provide rfapi and ripng specific struct route_node <-> struct rfapi/ripng_node translator functions. On a 64 bit system this should account for 8 bytes of data per route_node which is not insignificant considering a full bgp feed in both bgp and zebra, or evpn routes which make signficant usage of tables. Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
🛑 Basic BGPD CI results: FAILUREResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an EXPERIMENTAL automated CI system. Get source and apply patch from patchwork: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedCentOS 7 rpm pkg check: Successful Topotest tests on Ubuntu 16.04 i386: FailedTopology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-4630/test Topology Tests failed for Topotest tests on Ubuntu 16.04 i386:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4630/artifact/TOPOI386/ErrorLog/log_topotests.txt Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4630/artifact/TOPOI386/MemoryLeaks/Warnings Generated during build:Checkout code: Successful with additional warnings:Topotest tests on Ubuntu 16.04 i386: FailedTopology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-4630/test Topology Tests failed for Topotest tests on Ubuntu 16.04 i386:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-4630/artifact/TOPOI386/ErrorLog/log_topotests.txt
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base5 Static Analyzer issues remaining.See details at |
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.
breaks rfapi:
seen in labn CI:
#0 0x000000331ec324f5 in raise () from /lib64/libc.so.6
(gdb) #0 0x000000331ec324f5 in raise () from /lib64/libc.so.6
#1 0x000000331ec33cd5 in abort () from /lib64/libc.so.6
#2 0x00000000004e0055 in _zlog_assert_failed (
assertion=0x50986e "node->lock > 0", file=0x50985f "../lib/table.h",
line=235, function=0x5337c0 "route_unlock_node") at lib/log.c:710
#3 0x0000000000490b06 in route_unlock_node (node=)
at ../lib/table.h:235
#4 0x000000000049184d in rfapi_query (handle=0x16306c70, target=0x18c9ee4,
l2o=, ppNextHopEntry=)
at ../bgpd/rfapi/rfapi_table.h:46
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-4630/ This is a comment from an EXPERIMENTAL automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings:
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base5 Static Analyzer issues remaining.See details at |
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.
LGTM.
abstraction right.
Unfortunately this code is horribly broken. Hence the |
It seems this PR was superseded by #2785. Should we close this? |
The
void *aggregate
pointer in ROUTE_NODE_FIELDS is only used by ripngd.As such every single route_node in the system is paying the cost
of keeping this pointer around and never using it. Follow the table.h
design pattern in bgp_table.h and create a
struct ripng_node
pointerthat actually has this aggregate pointer and then provide ripng
specific struct route_node <-> struct ripng_node translator functions.
On a 64 bit system this should account for 8 bytes of data per route_node
which is not insignificant considering a full bgp feed in both bgp and
zebra, or evpn routes which make signficant usage of tables.
Signed-off-by: Donald Sharp sharpd@cumulusnetworks.com