David Lamparter [Thu, 28 Jul 2016 15:23:32 +0000 (17:23 +0200)]
isisd: drop unused per-type metric values
Expense, Error and Delay metrics never quite made it into the real
world. Either way isisd does nothing useful with them, so let's drop
them from the code. If someone wants to implement them, this patch can
still be reverted.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:49 +0000 (17:23 +0200)]
*: get rid of "MTYPE 0"
A few places are using 0 in place of the MTYPE_* argument. The
following rewrite of the alloc tracking won't deal with that, so let's
use MTYPE_TMP instead.
Acked-by: Vincent JARDIN <vincent.jardin@6wind.com> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
[DL: v2: fix XFREE(0, foo) calls too] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:48 +0000 (17:23 +0200)]
pimd: relegate pim_igmp_join.c into a header file
pim_igmp_join.c only exists to make some portability hacks available to
test_igmpv3_join. The function only has 1 call site in each pimd and
the test tool, so it's nicely served as a simple static function in a
header file.
This removes a MTYPE related compiler/linker issue from referencing
lib/if.h in a binary that doesn't link libzebra, as test_igmpv3_join is
now fully independent of lib/.
(Fix by Christian Franke: remove stray leftover ifindex_t)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:43 +0000 (17:23 +0200)]
lib: AgentX: use threads instead of eventloop hack
AgentX fd/timeout handling is rather hackishly monkeyed into thread.c.
Replace with code that uses plain thread_* functions.
NB: Net-SNMP's API rivals Quagga's in terms of age and absence of
documentation. netsnmp_check_outstanding_agent_requests() in particular
seems to be unused and is therefore untested.
The most useful documentation on this is actually the blog post Vincent
Bernat wrote when he originally integrated this into lldpd and Quagga:
https://vincent.bernat.im/en/blog/2012-snmp-event-loop.html
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:42 +0000 (17:23 +0200)]
lib: add thread_add_timer_tv (struct timeval)
Another zoo extension, this adds a timer scheduling function that takes
a struct timeval argument (which is actually what the wrappers boil down
to, yet it's not exposed...)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:41 +0000 (17:23 +0200)]
lib: linklist: add listnode_add_before()
This utility function, to join the zoo that the Quagga linked-list
implementation has accumulated, does an insert-before while returning
the newly allocated node.
It is similar to:
- listnode_add_after(), but
- complementary direction
- returns allocated node
- list_add_node_prev(), but
- supports before == NULL
- returns allocated node
In general, the entire linked-list implementation is in bad shape, and
while it needs a cleanup / rewrite / replacement, this would both cause
significant conflicts and block other cleanups...
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:40 +0000 (17:23 +0200)]
lib: remove a whole bunch of unused time stuff
QUAGGA_CLK_REALTIME and QUAGGA_CLK_REALTIME_STABILISED aren't used
anywhere in the code. Remove. The enum is kept to avoid having to
change the calls everywhere.
Same applies to the workaround code for systems that don't have a
monotonic clock. None of the systems Quagga works on fall into that
category; Linux, BSD and Solaris all do clock_gettime, for OSX we have
mach_absolute_time() - that covers everything.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:38 +0000 (17:23 +0200)]
ospfd: use random() to pick initial DD seq#
While the idea for this came the other way around - removing
quagga_time() - using random() is actually a better idea here. It's
seeded by time to begin with, but if ospfd restarts several times in a
short timespan it won't run straight into the same sequence number.
(Should also update the random seed to include microseconds so restarts
within a second use a different seq#)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:34 +0000 (17:23 +0200)]
bgpd: divorce router-id logic from CLI & zebra
Logic for determining the router-id was spread out over bgp_zebra.c and
bgp_vty.c. Move to bgpd/bgpd.c and have these two call more properly
encapsulated functions.
Significant work by Christian Franke <chris@opensourcerouting.org>.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 28 Jul 2016 15:23:33 +0000 (17:23 +0200)]
bgpd: minor header/API clean up
Adds "const" on:
- peer_update_source_addr_set()
- peer_description_set()
Adds parameter names on:
- bgp_timers_set()
(really confusing, this one, with 2 unexplained args of same type)
Adds new setter:
- peer_afc_set(), calling peer_activate/peer_deactivate.
(intended for API consumers, matches peer->afc)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Don Slice [Thu, 28 Jul 2016 21:35:48 +0000 (14:35 -0700)]
bgpd: Pass correct bgp-name for clear command
Found that original fix for CM-10113 had a significant flaw, that
by deriving the bgp instance from the vty->index, if a clear ip bgp
command was entered from a vty index other then bgp, a crash occurred.
This fix passes the bgp->name to the clear functions so the correct
instance can be derived. Tested manually in both the CM-10113 case
as well as the problem discovered while testing CM-11480.
Ticket: CM-10113 Signed-off-by: Don Slice
Reviewed-by:
bgpd: Fix for CM-11908 BGP: 'Address-family encap' cli issues
Made fix to handle the "Ambigious command" for address-family vpvn6 and vpnv6 unicast.
Rest of the bug analysis is below:
1. Issues with address-family encap/address-family encapv6/address-family vpnv6/address-family vpnv6 unicast need to be added to the ignore list in the test file tests.l3.quagga_cfg_cli_crawler_test. Sid to fix the "Ambiguous command" for vpnv6 as part of fix for this bug.
2. Neighbor <ipaddr/interface> disable-connected-check/ttl. Options should not be listed for interface. Anitha to file bugs for these. And also remove these commands from the test file tests.l3.quagga_cfg_cli_crawler_test.
Ticket: CM-11908
Reviewed By: CCR-4999
Testing Done: Manual, Ran the tests.l3.quagga_cfg_cli_crawler_test
Don Slice [Tue, 26 Jul 2016 13:44:39 +0000 (06:44 -0700)]
lib: Free memory correctly when braces used in parser
When braces (optional parameters) are used in the quagga parser, there
was a small leak on every iteration. Since this construct is primarily
used in the configuation process rather than show commands, it was not
readily apparent. With the addition of the "show ip bgp {json}" form of
the commands, each time one was run, memory was leaked.
Ticket: CM-11435 Signed-off-by: Don Slice
Reviewed By: Donald Sharp
Testing Done: Manual testing, bgp-min and bgp-smoke successful
bgpd: Fix attribute handling upon redistribution metric change
When the metric for a redistributed route is changed through configuration,
the path attribute for the route/routes need to be "re-created" as the hash
entry would change. In the absence of this, the entry would have the correct
values but when a hash lookup is done at a later time (e.g., when trying to
free the entry), it would fail. This patch addresses the "re-creation"
bgpd: Fix for vtysh -m does not mark "end" of router bgp
There was an exit added at the end of the BGP commands after we pulled the code from upstream. This was causing the reload scripts to fail. Removed this exit.
Make vtysh-integrated-config truly the default and fix quagga reload for this.
Ticket: CM-11910
Reviewed By: sharpd, routing-dev slack
Testing Done: Test with nothing in vtysh.conf, add no, remove it etc.
Even though we force integrated config to be the default, we do this by adding
a line to our default vtysh.conf which has integrated config enabled. When we
stopped printing integrated-config as part of wr mem or show running-config, we
broke quagga reload because it was explicitly looking for integrated config.
Furthermore, merely fixing quagga reload wouldn't work because subsequent saves
would result in config being saved to individual files since vtysh.conf no
longer forced the file to be integrated.
This patch fixes both issues. Makes integrated config the default in the code,
rather than via a shipped default file, and fixes quagga reload to look for
the "no integrated-vtysh-config" to deny attempting a reload.
Don Slice [Wed, 20 Jul 2016 12:02:04 +0000 (08:02 -0400)]
bgpd: Print the correct table in "show ip bgp x.x.x.x"
Prior to this change, bgp always identified the routing table used as
the default in the output of "show ip bgp x.x.x.x". This fix changes
the behavior to use the correct table name.
Ticket: CM-10239 Signed-off-by: Don Slice Reviewed-by: Donald Sharp
ospfd: Ensure correct handling of router-id change
Upon router-id change, one object that needs to be updated is the "nbr_self"
structure that is created to contain information about the local router and
is used during DR election, among other things. In the past, the code used to
just change the router-id field of this structure. This is actually not
sufficient - the neighbor has to be deleted and re-added into the tree. This
was fixed upstream and the fix is now available in our tree, but those changes
don't work well with prior Cumulus changes to defer updating the router-id
in the OSPF instance until other cleanup has happened.
Fixed code to update the "nbr_self" structure correctly while continuing to
defer the router_id update in the OSPF structure.
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com> Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-11861
Reviewed By: CCR-4980
Testing Done: Manual, failed test
Don't print empty sections as they clutter the output of show-running
Ticket: CM-11808
Reviewed By: CCR-4971
Testing Done: Usual stuff including doing show running with multiple daemons
Interface and VRF are both sections of the config that could possibly be
empty. This unnecessarily clutters the output of show running. This patch
fixes that by not displaying empty sections of interface, and vrf.
Routemaps have a genuine empty stanza and so we cannot add routemap to this
list. Unfortunately this means a "show running-config ospfd" may have empty
route-maps if the route-maps all correspond to BGP, for example. This
is not a concern for the entire "show running-config".
The trick in fixing this is on the vtysh side rather than on the client side.
The reason for this is that its quite tricky given the number of options to
ensure that a daemon never printed a section header unless there was something
to print. On the vtysh side, however, its easy to check if a section is
empty and not print it.
link-detect is on by default, and has been so since the first release
of Cumulus Linux. So, in the light of not displaying defaults, don't
display link-detect if enabled, only if disabled.
Don't display integrated-vtysh-config as its the default in CL.
Ticket: CM-11832
Reviewed By:
Testing Done: Testing that its not displayed if enabled & only if disabled
In the spirit of not displaying the defaults, we shouldn't display
"service integrated-vtysh-config" as its the default. It also tends to
clutter the output with stuff the user doesn't know or care about. This
patch removes displaying that and only prints it when the option is
disabled.
Don Slice [Mon, 18 Jul 2016 14:32:46 +0000 (10:32 -0400)]
bgpd: Use the correct bgp instance for cli commands issuing clear
Some bgp commands end with doing a bgp_clear_vty, which invalidly
made the assumption that the clear should always be done for the default
instance. This fix derives the correct instance from the vty-index if
one is supplied, and uses the default instance if it is not.
Ticket: CM-10113 Signed-off-by: Don Slice Reviewed-by: Donald Sharp
Simplify BGP unnumbered configuration by eliminating the unessential.
To make BGP configuration as simple as possible, assume the capability
extended-nexthop to be default for interface neighbors. Also allow the
ability to specify remote-as on the same line as neighbor interface to
make BGP unnumbered configuration a single line.
One corner case. This is the first feature for which the default for a
member is different from the default for a peer-group. Since advertising
the capability is only done for interface neighbors, the capability is
not set for the peer-group, but is automatically set for interface
neighbors that belong to that peer-group. So, if you want to disable the
advertisement of this capability for an interface neighbor, you must
do it per each interface neighbor.
The patch is more complicated than it needs to be due to the handling
of quagga reload and appropriate updates to the show running output.
Donald Sharp [Fri, 15 Jul 2016 20:40:01 +0000 (16:40 -0400)]
Merge remote-tracking branch 'origin/cmaster' into cmaster-next
P unnumbered configuration a single line.
One corner case. This is the first feature for which the default for a
member is different from the default for a peer-group. Since advertising
the capability is only done for interface neighbors, the capability is
not set for the peer-group, but is automatically set for interface
neighbors that belong to that peer-group. So, if you want to disable the
advertisement of this capability for an interface neighbor, you must
do it per each interface neighbor.
The patch is more complicated than it needs to be due to the handling
of quagga reload and appropriate updates to the show running output.
Simplify BGP unnumbered configuration by eliminating the unessential.
To make BGP configuration as simple as possible, assume the capability
extended-nexthop to be default for interface neighbors. Also allow the
ability to specify remote-as on the same line as neighbor interface to
make BGP unnumbered configuration a single line.
One corner case. This is the first feature for which the default for a
member is different from the default for a peer-group. Since advertising
the capability is only done for interface neighbors, the capability is
not set for the peer-group, but is automatically set for interface
neighbors that belong to that peer-group. So, if you want to disable the
advertisement of this capability for an interface neighbor, you must
do it per each interface neighbor.
The patch is more complicated than it needs to be due to the handling
of quagga reload and appropriate updates to the show running output.
Don Slice [Fri, 15 Jul 2016 13:33:48 +0000 (06:33 -0700)]
zebra: Eliminate use of imported arp entries as next-hops for other routes
Ticket: CM-8228 Signed-off-by: Donald Slice
Reviewed By:
Testing Done: Manual testing succesful. bgp-min and ospf-smoke successful. redistribute-neighbor-smoke
has the same failures as the base image.
Problem was due to considering imported /32 arp entries as elible next-hops for other routes
(in this case a static route.) This confuses the rib since this next-hop is considered both
recursive and onlink. Disallowed the use of this imported arp entry in next-hop determination.
Donald Sharp [Wed, 13 Jul 2016 18:22:42 +0000 (14:22 -0400)]
pimd: Allow (S,G) pimreg route to time out
When a kernel upcall happens for nocache we create
a (S,G) route to be installed into the kernel.
This code modification starts the ability to
time out the mroute if we stop receiving mcast
packets.
Ticket: CM-11793 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Tue, 12 Jul 2016 19:25:11 +0000 (15:25 -0400)]
pimd: multicast route not removed from kernel when the if goes down
When a multicast route's rpf changes( for whatever reason ) (*,G)
routes were never updating properly. This is because we were
attempting to fing the path to the *, instead of the RP.
Modify the code to check if we are attempting to find a
* route and use the RP instead.
Ticket: CM-11736 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Tue, 12 Jul 2016 19:09:25 +0000 (15:09 -0400)]
pimd: Refactor 'struct static_route' to use channel_oil
The 'struct static_route' data structure duplicated a
decent bit of what is the in the struct channel_oil.
Refactor. This will set us up for further cleanup.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Tue, 12 Jul 2016 15:31:45 +0000 (11:31 -0400)]
pimd: Stale IGMP groups left behind
When a toin IGMPv3 join is received, the code
was always auto creating the igmp group associated
with the received packet. The RFC clearly states
though that if a INCLUDE is received for a group
with 0 sources and we have received nothing the
igmpv3 packet should be ignored.
Ticket: CM-11260 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 11 Jul 2016 19:54:37 +0000 (15:54 -0400)]
pimd: IGMPv3 leave not deleting group entry
After sending a IGMPv3 exclude report for a multicast address
with 0 sources, send an include report for the same group and also 0
sources. This should cause IGMP to GS query and age/delete
the entry.
This fix addresses this issue.
Ticket: CM-11685 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Quentin Young [Mon, 27 Jun 2016 14:51:17 +0000 (14:51 +0000)]
lib: Rewrite ipv4 address and prefix validator
Simplify ipv4 prefix and address matcher / validator to use standard
Linux networking functions instead of a state machine.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com> Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Quentin Young [Mon, 27 Jun 2016 13:14:18 +0000 (13:14 +0000)]
lib: Rewrite ipv6 prefix matcher
Simplify ipv6 prefix matcher / validator to use standard Linux
networking functions instead of a state machine.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com> Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Don Slice [Mon, 11 Jul 2016 19:57:24 +0000 (12:57 -0700)]
all: add default log file if none are defined
Added a default log file named /var/log/quagga/Quagga.log to every daemon
to capture log entries if no log file is defined. This also allows the
capture of logged information prior to reading each daemon's config file.
If a log file is defined manually, it will override this default file name.
Ticket: CM-10987 Signed-off-by: Don Slice
Reviewed By: Donald Sharp
Testing Done: Manual testing
Donald Sharp [Mon, 11 Jul 2016 16:57:28 +0000 (12:57 -0400)]
pimd: static joins no longer worked
Static joins were killed by a previous commit, which
has been backed out. I've recoded the igmp join
code to ignore 224.0.0.0/24 from ourselves a bit
differently now.
Ticket: CM-11751 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Quagga's default "show running" model is to only print the non-default config.
Historically, IPv4 unicast has always had a default 'activate' model unless
its been configured otherwise. In 3.0, we introduced a print of the 'activate'
statement for IPv4 unicast independent of whether it was the default or not.
This causes quagga reload to break as the user doesn't configure 'activate' for
IPv4 unicast, and so any config changes will also not have it. However 'show
running' will display it, causing quagga reload to think that the AFI/SAFI has
been deactivated and bounce the sessions incorrectly.
This patch reverts to the original quagga behavior/model of not printing the
'activate' line for IPv4 unicast if its the default.
bgpd: "neigbor <interface> ttl-security hops" should reject a hops value greater than 1
"neighbor <interface> disable-connected-check" should not be allowed by the parser
Made changes to not allow hops greater than 1 and disable-connected check for neighbor <interface>
Donald Sharp [Wed, 29 Jun 2016 18:30:28 +0000 (14:30 -0400)]
pimd: Pass the appropriate data structure around
Several static functions were passing a list around
when the reality is we are going to need to
look at the group information in order to make an
informated decision.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 29 Jun 2016 12:31:19 +0000 (08:31 -0400)]
pimd: Prevent igmp packet loopback.
pim is joining the 224.0.0.13 and 224.0.0.22 groups
This is causing the creation of (*, 224.0.0.13) and
(*, 224.0.0.22) multicast routes which are immediately
sent to the pim network.
If we are the originator of the igmp report than
there is no need to accept the packet for
processing.
Ticket: CM-11397 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 29 Jun 2016 01:50:49 +0000 (21:50 -0400)]
pimd: Bind pim sockets to interface they are associated with
When pim is receiving packets, each interface's fd is receiving
packets for all interfaces. Modify the code to bind the
pim interface sockets to the interface they were created for.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Don Slice [Mon, 27 Jun 2016 11:34:32 +0000 (04:34 -0700)]
zebra/ospf/ospf6: Fix several memory leaks on if up/down
Resolved several memory leaks caused by ifdown/ifup the vrf device or
a swp port. For bgp/zebra/ospf/ospf6, bouncing the vrf device would cause
a linked list, Interface, and route-table to get leaked. For ospf6,
bouncing the swp device also caused leaks of Connected and Prefix entries.
Ticket: CM-10841 Signed-off-by: Don Slice Reviewed-By: Donald Sharp
Testing Done: Manual testing, bgp and ospf mins passed, smokes had fewer failures than base
Don Slice [Mon, 27 Jun 2016 15:31:57 +0000 (08:31 -0700)]
zebra: resolved problem with show ip route vrf
Repaired damage done by commit upstream, which changed the way show_ip_route
is called to allow for multicast rpf table display. Matched the technique of
the other callers to the new function.
Ticket: CM-11345 Signed-off-by: Don Slice
Reviewed By: Donald Sharp
Testing Done: Manual testing and vrf-min
Donald Sharp [Fri, 24 Jun 2016 00:42:19 +0000 (20:42 -0400)]
pimd: Fix register receive pointer arithmetic
When receiving the register packet from another pim
neighbor at the RP, we were adding an incorrect
amount of bytes to find the start of the ip_hdr
of the encapsulated data. This commit fixes
this issue.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>