Merge remote-tracking branch 'origin/cmaster' into cmaster-next

Conflicts:
debian/changelog
zebra/zebra_rnh.c

vtysh, zebra: Fix link-params to use exit-link-params

Fix the link-params submode to use the 'exit-link-params'
to indicate we are exiting a submode.

Fixup all the relevant bits.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

Merge remote-tracking branch 'opensourcerouting/cmaster-next-linkpar' into cmaster-next

*: Fix up licensing to be right

These files were submitted by Cumulus but did
not properly setup the licensing of them.
This fixes that issue.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: 'show ip bgp neighbor json' some keys are in the wrong place

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Sam Tannous <stannous@cumulusnetworks.com>
Ticket: CM-13511

bgpd: 'show ip bgp neighbor json' some keys are in the wrong place

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-13511

connectionsEstablished, etc are no longer under the addressFamilyInfo
keyword, they are now at the top level

Merge branch 'cmaster' of ssh://stash.cumulusnetworks.com:7999/quag/quagga into cmaster

zebra: remove recursively derived static routes correctly

Problem reported that in certain configs, when a router is initially
booted and the link is bounced, we can end up with a bogus static route
in the table.  This was due to the assumption in zebra_rnh that a static
route would not be recursively resolved through another static route with
a different next-hop. This fix changes this assumption.  Tested manually
and bgp-min, ospf-min, and vrf-min run with no new failures.

Ticket: CM-13328
Signed-off-by: Don Slice
Reviewed-by: CCR-5338

zebra: fix link-params CLI handling

vtysh was breaking in a rather ugly way, and some "no" forms were
missing too.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

Cumulus: Fixup changelog and version string.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

Merge remote-tracking branch 'opensourcerouting/cmaster-next-vty2' into cmaster-next

Conflicts:
watchquagga/watchquagga.c

vtysh: fix config write

Merge remote-tracking branch 'origin/cmaster' into cmaster-next

doc: vtysh doc updates

doc: generic updates

vtysh: improve config-write error reporting

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

lib: add minimal no-config VTY mode

This silences the following warning from watchquagga:
"Can't save to configuration file, using vtysh."
which otherwise appears when doing a "write file" in vtysh when no
integrated-config is in use.

Also make "show memory" available in watchquagga.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

watchquagga: Signal when we are actually up and running

When Quagga is starting up, it is returning immediately.
This is leaving us in a state where systemd believes
Quagga is up and running, while the sytem might actually
not have restarted the code yet.

Modify the code so that when watchquagga starts up
it doesn't start communicating with systemd until
such time that it detects that all daemons are
running.

Additionally modify watchquagga to touch a
file in /var/run/quagga/ that the /usr/lib/quagga/quagga
script looks for for 10 seconds.  If it finds this
Quagga started file then we know watchquagga
has successfully communicated with all daemons.
If after 10 seconds we haven't communicated
with Quagga, continue on for the start and let the
normal start failure code work.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

Revert "vtysh: Make vtysh run as quagga user"

This reverts commit 5dd58b08299e85735f19fba1ee307c509fb19de7.

Changing vtysh uid/gid is now actually counterproductive.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

vtysh: funnel integrated write through watchquagga

Running vtysh as normal user won't have permissions to write
Quagga.conf.  If we're connected to watchquagga, try "write integrated"
first.  In all cases if something fails, try directly.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

vtysh: add watchquagga to target list

Also tag some commands as VTYSH_REALLYALL; these are absolutely
neccessary for correct vtysh operation and will cause "interesting"
breakage if not present on all daemons.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

watchquagga: add "write integrated"

This new command - available for internal use by vtysh and explicit
usage by users - calls "vtysh -w" from watchquagga.  This ensures vtysh
is run with privileges to actually write the integrated-config file.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

watchquagga: add ZLOG_WATCHQUAGGA

watchquagga logs as "NONE", which will also become visible in vtysh for
error messages.  Add "WATCHQUAGGA" log target.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

vtysh: set config file permissions

As vtysh may hopefully be running as root from watchquagga here, let's
try to fix up ownership and permissions for Quagga.conf.  Doing
chown/chmod instead of changing the process's user/group IDs has the
advantage of fixing up preexisting misconfigurations.

Note errors in chmod/chown will print a message but the config is
already written at that point.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

vtysh: add -w option for integrated-config write

This new option is intended to be used both by watchquagga as well as
directly by users.  It performs the collect-configuration operation and
writes out Quagga.conf, regardless of whether integrated-config is
enabled or not.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

vtysh: detangle configuration writes

vtysh has a very convoluted and confusing setup where it isn't even
clear which files are written where (since some filenames come
indirectly from loading config).  Detangle.

This also removes writing vtysh.conf.  The file is intended to be
manually edited since it has some vague security concerns (if PAM is
used).

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

lib: privs: always look up VTY group

Even if we're running without user switch, we should still try to honor
the VTY group.  This applies both to watchquagga (which always runs as
root) as well as "no-userswitch" configurations for other daemons.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

lib: add and use set_cloexec()

watchquagga is already leaking an open file descriptor on its pid file
on fork+exec() invocations; next up is adding vtysh support with even
more fds.  Mark things CLOEXEC before going there.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

Modify build version.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

build: configure.ac - use AC_PROG_CC_C99 instead of cflags to force c99 mode

bgpd: Squash spurious "unknown afi" log messages

* bgp_packet.c: (bgp_update_receive) doesn't differentiate between NLRIs that
  are 0 AFI/SAFI cause they weren't set, and those because a peer sent a
  bogus AFI/SAFI, before sending sending what may be a misleading, spurious
  log message.  Check the .nlri pointer is set and avoid this.

Incorporating a suggestion from: G. Paul Ziemba <unp@ziemba.us>

bgpd: Remove the double-pass parsing of NLRIs

* bgpd parses NLRIs twice, a first pass "sanity check" and then a second pass
  that changes actual state. For most AFI/SAFIs this is done by
  bgp_nlri_sanity_check and bgp_nlri_parse, which are almost identical.

  As the required action on a syntactic error in an NLRI is to NOTIFY and
  shut down the session, it should be acceptable to just do a one pass
  parse.  There is no need to atomically handle the NLRIs.

* bgp_route.h: (bgp_nlri_sanity_check) Delete
* bgp_route.c: (bgp_nlri_parse) Make the prefixlen size check more general
  and don't hard-code AFI/SAFI details, e.g. use prefix_blen library function.

  Add error logs consistent with bgp_nlri_sanity_check as much as possible.

  Add a "defense in depth" type check of the prefixlen against the sizeof
  the (struct prefix) storage - ala bgp_nlri_parse_vpn.
  Update standards text from draft RFC4271 to the actual RFC4271 text.

  Extend the semantic consistency test of IPv6. E.g. it should skip mcast
  NLRIs for unicast safi as v4 does.

* bgp_mplsvpn.{c,h}: Delete bgp_nlri_sanity_check_vpn and make
  bgp_nlri_parse_vpn_body the bgp_nlri_parse_vpn function again.

  (bgp_nlri_parse_vpn) Remove the notifies.  The sanity checks were
  responsible for this, but bgp_update_receive handles sending NOTIFY
  generically for bgp_nlri_parse.

* bgp_attr.c: (bgp_mp_reach_parse,bgp_mp_unreach_parse) Delete sanity check.
  NLRI parsing done after attr parsing by bgp_update_receive.

Arising out of discussions on the need for two-pass NLRI parse with:

Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: Fix non v4 EOR parsing

When we receive a non v4 EOR, we were parsing it but
incorrectly applying the test for the flag for it.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>

bgpd: Regularise bgp_update_receive, add missing notifies and checks

* bgp_packet.c: (bgp_update_receive) Lots of repeated code, doing same
  thing for each AFI/SAFI.  Except when it doesn't, e.g.  the IPv4/VPN
  case was missing the EoR bgp_clear_stale_route call - the only action
  really needed for EoR.

  Make this function a lot more regular, using common, AFI/SAFI
  independent blocks so far as possible.

  Replace the 4 separate bgp_nlris with an array, indexed by an enum.

  The distinct blocks that handle calling bgp_nlri_parse for each
  different AFI/SAFI can now be replaced with a loop.

  Transmogrify the nlri SAFI from the SAFI_MPLS_LABELED_VPN code-point
  used on the wire, to the SAFI_MPLS_VPN safi_t enum we use internally
  as early as possible.

  The existing code was not necessarily sending a NOTIFY for NLRI
  parsing errors, if they arose via bgp_nlri_sanity_check.  Send the
  correct NOTIFY - INVAL_NETWORK for the classic NLRIs and OPT_ATTR_ERR
  for the MP ones.

  EoR can now be handled in one block.  The existing code seemed broken
  for EoR recognition in a number of ways:

  1.  A v4/unicast EoR should be an empty UPDATE.  However, it seemed
     to be treating an UPDATE with attributes, inc.  MP REACH/UNREACH,
     but no classic NLRIs, as a v4/uni EoR.

  2.  For other AFI/SAFIs, it was treating UPDATEs with no classic
     withraw and with a zero-length MP withdraw as EoRs.  However, that
     would mean an UPDATE packet _with_ update NLRIs and a 0-len MP
     withdraw could be classed as an EoR.

  This seems to be loose coding leading to ambiguous protocol
  situations and likely incorrect behaviour, rather than simply being
  liberal.  Be more strict about checking that an UPDATE really is an
  EoR and definitely is not trying to update any NLRIs.

  This same loose EoR parsing was noted by Chris Hall previously on
  list.

  (bgp_nlri_parse) Front end NLRI parse function, to fan-out to the correct
  parser for the AFI/SAFI.

* bgp_route.c: (bgp_nlri_sanity_check) We try convert NLRI safi to
  internal code-point ASAP, adjust switch for that.  Leave the wire
  code point in for defensive coding.

  (bgp_nlri_parse) rename to bgp_nlri_parse_ip.

* tests/bgp_mp_attr_test.c: Can just use bgp_nlri_parse frontend.

bgpd: Regularise BGP NLRI sanity checks a bit

* bgp_route.h: (bgp_nlri_sanity_check) The bulk of the args are equivalent
  to a (struct bgp_nlri), consolidate.
* bgp_route.c: (bgp_nlri_sanity_check) Make this a frontend for all afi/safis.
  Including SAFI_MPLS_LABELED_VPN.
  (bgp_nlri_sanity_check_ip) Regular IP NLRI sanity check based on the
  existing code, and adjusted for (struct bgp_nlri *) arg.
* bgp_attr.c: (bgp_mp_reach_parse) Adjust for passing (struct bgp_nlri *)
  to bgp_nlri_sanity_check.
  Get rid of special-casing to not sanity check VPN.
  (bgp_mp_unreach_parse) Ditto.

* bgp_mplsvpn.c: Use the same VPN parsing code for both the sanity
  check and the actual parse.

  (bgp_nlri_parse_vpn) renamed to bgp_nlri_parse_vpn_body and made
  internal.

  (bgp_nlri_parse_vpn_body) Added (bool) argument to control whether it
  is sanity checking or whether it should update routing state for each
  NLRI.  Send a NOTIFY and reset the session, if there's a parsing
  error, as bgp_nlri_sanity_check_ip does, and as is required by the
  RFC.

  (bgp_nlri_parse_vpn) now a wrapper to call _body with update.

  (bgp_nlri_sanity_check_vpn) wrapper to call parser without
  updating.

* bgp_mplsvpn.h: (bgp_nlri_sanity_check_vpn) export for
  bgp_nlri_sanity_check.

* bgp_packet.c: (bgp_update_receive) Adjust for bgp_nlri_sanity_check
  argument changes.

* test/bgp_mp_attr_test.c: Extend to also test the NLRI parsing functions,
  if the initial MP-attr parsing has succeeded.  Fix the NLRI in the
  VPN cases.  Add further VPN tests.

* tests/bgpd.tests/testbgpmpattr.exp: Add the new test cases.

This commit a joint effort of:

Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>
Paul Jakma <paul.jakma@hpe.com> / <paul@jakma.org>

bgpd: make bgp_nlri_parse_encap conform with other nlri_parse funcs

* bgp_encap.{c,h} (bgp_nlri_parse_encap) afi is already in the NLRI argument.
  update or withdraw is signalled by attr being non-NULL or NULL.

* bgp_packet.c: (update_receive) fixup to match, and also make the attr
  argument conform with NLRI_ATTR_ARG for correct error handling on
  optional, transitive, partial, attributes.

ospfd: Add 'no ...' form of some commands to cli

Also add the ability to specify some or all of the no command.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

ospfd: Only allow ospf RI commands if RI is turned on

When ospf RI commands are entered and we have not turned
on the feature, gracefully tell the user that there is
no going forward.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

ospfd: Dissallow mpls-te commands unless mpls-te has been turned on

If ospf mpls-te has not been turned on via a 'mpls-te on'
dissallow other mpls-te commands from being silently accepted
and ignored.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

zebra: Allow v6 static routes to handle mpls not in kernel

When configuring v6 static routes, allow it to gracefully
ignore mpls commands when mpls is not turned on in the kernel.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

zebra: Disable mpls slightly different

When mpls is not turned on in the kernel, we
are not installing the mpls commands into the cli.
This results in vtysh attempting to run the command
and receiving a 'WTF is this command' back from zebra.
Modify the mpls code to install commands and to check
to see if the command should be accepted based
upon mpls working or not.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>

zebra: If fpm is not turned on hide the cli for it

If the FPM is not enabled properly, hide the cli
for it.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: dynamically grow 'show ip bgp summ' Neighbor column width

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Now that we display hostnames in 'show ip bgp summary' it is really easy
to have the first column be greater than 16 characters which causes a
line wrap. The line wrap makes the output difficult to read.

Before
======
superm-redxp-05# show ip bgp summ
BGP router identifier 6.0.0.11, local AS number 65001 vrf-id 0
BGP table version 56
RIB entries 19, using 2280 bytes of memory
Peers 2, using 41 KiB of memory
Peer groups 1, using 64 bytes of memory

Neighbor        V         AS MsgRcvd MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd
leaf-11-really-long-name(swp1)
                4      65101     307     322        0    0    0 00:14:15 10
leaf-12(swp2)   4      65101     105     121        0    0    0 00:14:14 Idle (Admin)

Total number of neighbors 2
superm-redxp-05#

After
=====
superm-redxp-05# show ip bgp summ
BGP router identifier 6.0.0.11, local AS number 65001 vrf-id 0
BGP table version 10
RIB entries 19, using 2280 bytes of memory
Peers 2, using 41 KiB of memory
Peer groups 1, using 64 bytes of memory

Neighbor                       V         AS MsgRcvd MsgSent   TblVer InQ OutQ  Up/Down State/PfxRcd
leaf-11-really-long-name(swp1) 4      65101      11      12        0   0    0 00:00:08           10
swp2                           4          0       0       0        0   0    0    never Idle (Admin)

Total number of neighbors 2
superm-redxp-05#
superm-redxp-05#

bgpd: fix 'show ip bgp summary' column alignment

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>

vtysh: Allow vtysh to not know about enabled daemons/features

With the way that vtysh works, it compiles in cli even
if there is no support in the protocols.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

tools: Fix bug whereby no searches were made

Bad assignment in header file parsing resulted in all
commands being pruned before searching for installations.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

tools: Be a little more strict when matching command names

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

tools: cmd_check.py checks headers too

Sometimes commands are externed and installed in another
file, so check for a command's name in the header file
corresponding to the file it's defined in before marking
it uninstalled.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

tools: Add tool to check for uninstalled commands

cmd_check.py parses source files looking for commands
that are defined but not installed in any node.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

lib: Fix race condition in shutdown of routemap

When shutting down a daemon that uses an update
timer to handle route map processing, there
exists a race condition where if you change
a route map and then immediately shutdown
quagga before the update timer for the routemap
runs, you will be placed in a infinite loop.

This condition happens because this commit introduces
route map memory free'ing but never tests to see
if the to_be_processed flag has happened or not
before deleting:

commit 289d25016b79ed7908a29af7ba35e18f1ec6ed0a
Author: Lou Berger <lberger@labn.net>
Date:   Wed Apr 10 12:30:04 2013 -0700

    bgpd, lib: memory cleanups for valgrind, plus debug changes

Modify route_map_finish to explicitly turn off the to_be_processed
field.  As that we know we are going away if route_map_finish is
called.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>

Merge remote-tracking branch 'origin/cmaster' into cmaster-next

bgpd: add 'neighbor x.x.x.x allowas-in origin' knob

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Ticket: CM-13207

normal table on spine-1....we do not see 6.0.0.10 (spine-2's loopback)
spine-1 and spine-2 are in AS 65200

superm-redxp-05# show ip bgp
BGP table version is 13, local router ID is 6.0.0.9
Status codes: s suppressed, d damped, h history, * valid, > best, =
multipath,
              i internal, r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete

   Network          Next Hop            Metric LocPrf Weight Path
*> 6.0.0.5/32       swp1            0              0 65101 ?
*> 6.0.0.6/32       swp2            0              0 65101 ?
*> 6.0.0.7/32       swp3            0              0 65104 ?
*> 6.0.0.8/32       swp4            0              0 65104 ?
*> 6.0.0.9/32       0.0.0.0                  0          32768 ?
*= 6.0.0.11/32      swp2                          0 65101 65001 ?
*>                  swp1                          0 65101 65001 ?
*= 6.0.0.12/32      swp2                          0 65101 65002 ?
*>                  swp1                          0 65101 65002 ?
*= 6.0.0.13/32      swp4                          0 65104 65001 ?
*>                  swp3                          0 65104 65001 ?
*= 6.0.0.14/32      swp4                          0 65104 65002 ?
*>                  swp3                          0 65104 65002 ?

Displayed  9 out of 13 total prefixes
superm-redxp-05#

spine-1 with "neighbor x.x.x.x allowas-in origin", we now see 6.0.0.10

superm-redxp-05# show ip bgp
BGP table version is 14, local router ID is 6.0.0.9
Status codes: s suppressed, d damped, h history, * valid, > best, =
multipath,
              i internal, r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete

   Network          Next Hop            Metric LocPrf Weight Path
*> 6.0.0.5/32       swp1            0              0 65101 ?
*> 6.0.0.6/32       swp2            0              0 65101 ?
*> 6.0.0.7/32       swp3            0              0 65104 ?
*> 6.0.0.8/32       swp4            0              0 65104 ?
*  6.0.0.9/32       swp2                          0 65101 65200 ?
*                   swp1                          0 65101 65200 ?
*                   swp3                          0 65104 65200 ?
*                   swp4                          0 65104 65200 ?
*>                  0.0.0.0                  0          32768 ?
*= 6.0.0.10/32      swp2                          0 65101 65200 ?
*>                  swp1                          0 65101 65200 ?
*=                  swp3                          0 65104 65200 ?
*=                  swp4                          0 65104 65200 ?
*= 6.0.0.11/32      swp2                          0 65101 65001 ?
*>                  swp1                          0 65101 65001 ?
*= 6.0.0.12/32      swp2                          0 65101 65002 ?
*>                  swp1                          0 65101 65002 ?
*= 6.0.0.13/32      swp4                          0 65104 65001 ?
*>                  swp3                          0 65104 65001 ?
*= 6.0.0.14/32      swp4                          0 65104 65002 ?
*>                  swp3                          0 65104 65002 ?

Displayed  10 out of 21 total prefixes
superm-redxp-05#

The only as-paths with 65200 that made it through were the ones that
originated from 65200

superm-redxp-05# show ip bgp regexp _65200_
BGP table version is 14, local router ID is 6.0.0.9
Status codes: s suppressed, d damped, h history, * valid, > best, =
multipath,
              i internal, r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete

   Network          Next Hop            Metric LocPrf Weight Path
*  6.0.0.9/32       swp2                          0 65101 65200 ?
*                   swp1                          0 65101 65200 ?
*                   swp3                          0 65104 65200 ?
*                   swp4                          0 65104 65200 ?
*= 6.0.0.10/32      swp2                          0 65101 65200 ?
*>                  swp1                          0 65101 65200 ?
*=                  swp3                          0 65104 65200 ?
*=                  swp4                          0 65104 65200 ?

Displayed  2 out of 21 total prefixes
superm-redxp-05#

bgpd: Fix rfapi qobj use.

We need to tell the system that we've initialized
the qobj memory.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: David Lamparter <equinox@opensourcerouting.org>

bgpd: Fix duplicate installs of rfapi commands

When installing commands into the parser, don't duplicate
the enable/view nodes.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

vtysh: Fix 'show run' output of bgp

The code in vtysh was incorrectly handling
sub-modes for a bgp config when doing a 'show run'

This commit:
commit 65efcfce427e2abb548874ebb1a11a3b2ee7bc17
Author: Lou Berger <lberger@labn.net>
Date:   Sat May 7 14:18:56 2016 -0400

    bgpd: add L3/L2VPN Virtual Network Control feature

Added vnc support to the bgp daemon.  In addition
it modified vtysh_config.c to help vtysh understand
bgp sub-modes.  This caused the output of the show
run bgp command to be displayed incorrectly:

router bgp 65001
 !
 address-family ipv4 unicast
  maximum-paths 4
 exit-address-family
 !
!
 address-family ipv6 unicast
  maximum-paths 3
 exit-address-family
!

Backing out this change allows vtysh to have the
correct display of bgp now.

Ticket: CM-13136
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: David Lamparter <equinox@opensourcerouting.org>

lib: Fix for int64 and json on some systems

When compiling json on systems with json/json.h
that don't have json_object_new_int64, just
use json_object_new_int instead and accept
we might truncate data.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

vtysh: handle case if there is no match in "write terminal $daemon"

While the DEFUN should match the list of clients registered in
vtysh, it seems better to handle the case explicitly instead of
relying on the client list and the DEFUN signature being in sync.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

ripd: print md5 auth digest correctly

The dump of the md5 hash was missing one byte of the hash.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

pimd: don't leak original_s_route on error

original_s_route is allocated on the heap and was not freed during the
error case.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

isisd: Fix size of malloc

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

isisd: fix an error that was probably a result of copypasting

The code should check for the existance of the correct list prior to
accessing it.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: fix memory leaks in show commands

sockunion_str2su allocates a struct sockunion that used to be leaked
in the show commands. Use str2sockunion and keep the information
on the stack instead.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

ospf6d: fix off-by-one on display of spf reasons

The loop should only iterate to array_size - 1.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

ospfd: fix double assignment in ospf_vl_set_timers

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: fix off-by-one in attribute flags handling

bgp_attr_flag_invalid can access beyond the last element of attr_flags_values.
Fix this by initializing attr_flags_values_max to the correct value.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: don't leak memory in community_regexp_include

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: setting nexthop doesn't need inet_pton

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: 'show ip bgp summary json' shows large negative value for "peerUptimeMsec"

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Ticket: CM-13239

bgpd: Fix shutdown order.

When shutting down bgp we were freeing the ecommunity hash
before we unintern'ed everything in it.

This is fallout from:
289d250 bgpd, lib: memory cleanups for valgrind, plus debug changes

Basically we started free'ing data we've never freed before
which caused us to access a hash that had already been freed.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

bgpd: implement admin distance

Until today the admin distance cannot be configured for any IPv6
routing protocol. This patch implements it for bgp.

Signed-off-by: Maitane Zotes <maz@open.ch>
Signed-off-by: Roman Hoog Antink <rha@open.ch>

ospf6d: implement admin distance

Until today the admin distance cannot be configured for any IPv6
routing protocol. This patch implements it for ospf6.

Signed-off-by: Maitane Zotes <maz@open.ch>
Signed-off-by: Roman Hoog Antink <rha@open.ch>

*: Fix duplicate commands from view/enable node consolidation

lib: keep hash of node's commands to detect duplicate installs

* command.h: (struct cmd_node) Add a hash, so duplicate installs of
  a cmd_element to a command node can be detected.  To help catch
  strays from the VIEW/ENABLE node consolidation particularly
  (installs to VIEW automatically install to ENABLE too now).
* command.c: (cmd_hash_{key,cmp}) helpers for the hash - just directly
  on the pointer value is sufficient to catch the main problem.
  (install_node) setup the hash for the command node.
  (install_element) check for duplicate installs.
  The assert on the cmd_parse_format seems misplaced.
  (install_default_basic) separate the basic, VIEW, node default commands
  to here.
  (cmd_init) get rid of dupes, given consolidation.
  (cmd_terminate) clean up the node command hash.

Not done: The (struct cmd_node)'s vector could be replaced with the
cmd hash, however much of the command parser depends heavily on the
vector and it's a lot of work to change.  A vector_lookup_value could
also work, particularly if vector could be backed by a hash.

The duplicate check could be disabled in releases - but useful in
development.  It's a little extra overhead at startup.  The command
initialisation overhead is already something that bites in
micro-benchmarks - makes it easy for other implementations to show
how much faster they are with benchmarks where other load is low
enough that startup time is a factor.

vtysh: fix oversight in vtysh buffer rewrite

end can be NULL and shouldn't be adjusted in that case.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

lib: Fix redist memory free'ing

When shutting down, properly free memory associated
with zclient.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

lib: Add cleanup for routemaps

Some new routemap hash data structures were not being
properly cleaned up.  Add the hash_free code in.

This code does not fix the variety of daemons that
don't actually call route_map_finish.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

*: Consolidate routemap initialization

Consolidate the routemap initialization into one
function.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>

zebra: Move netlink error message under a debug

In some circumstances, the quagga log is being filled with repetitive
error messages reporting "network is down" with RTM_NEWROUTE.  Moved this
particular scenario under "debug zebra kernel" instead of making it an
unprotected error message.  Manually tested using the same script with and
without the fix to verify the message is suppressed.

Ticket: CM-11173
Signed-off-by: Don Slice
Reviewed-by: Donald Sharp

vtysh: refactor vtysh_client_{config,execute}

Triggered by a bugreport / patch by Gautam Kumar <gauta@amazon.com>,
this is a full rewrite vtysh_client_{config,execute}.  (The patch didn't
quite apply anymore.)

vtysh_client_run() now has a buffering implementation that can be read
without losing one's sanity and/or requiring alcoholic beverages.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

bgpd, lib: memory cleanups for valgrind, plus debug changes

Description:
    We use valgrind memcheck quite a bit to spot leaks in
    our work with bgpd. In order to eliminate false positives,
    we added code in the exit path to release the remaining
    allocated memory.

    Bgpd startup log message now includes pid.

Some little tweaks by Paul Jakma <paul.jakma@hpe.com>:

* bgp_mplsvpn.c: (str2prefix_rd) do the cleanup in common code at the end
  and goto it.

[DL: dropped several chunks from original commit which are obsolete by
now on this tree.]

lib/zebra: remove code duplication in redist_del_instance()

Change redist_check_instance() to return a pointer instead of returning 1
on success. This way this function can be reused in redist_del_instance()
instead of duplicating the same logic there.

Also, remove unnecessary call to redist_check_instance() in
zebra_redistribute_delete().

While here, remove unnecessary cast from void* in redist_add_instance().

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

zebra: fix processing of redistribute messages

We should not check/modify client->redist[] when the requested instance
is different than zero.

In the same way, we should not check/modify client->mi_redist[] when
the requested instance is zero.

Failure to respect these conditions can lead to unexpected behavior in
the client daemons.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

ldpd: remove dead code from zsend_redistribute_route()

As a general rule of thumb, we should write functions that do one thing
and that do it well. All callers of zsend_redistribute_route() are already
checking if the route should be redistributed or not (as the comment
says), so we definitely shouldn't bother with that in this function.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

zebra: cleanup redistribution code

Remove code duplication and reduce excessive indentation levels whenever
possible.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

zebra: fix redistribution of default routes

We were always redistributing the default routes (IPv[46]) in
redistribute_update() because the 'client->redist_default' condition
always returns true.

The 'redist_default' member of the 'zserv' structure is a pointer and is
always initialized with vrf_bitmap_init() in the zebra_client_create()
function.

To fix this, use vrf_bitmap_check() instead.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

zebra/ldpd: introduce ZEBRA_ROUTE_ALL wildcard route type

The ZEBRA_ROUTE_ALL route type can be used by a client to request
all routes from zebra. The main motivation for introducing this is
to allow ldpd to get routes from all OSPF instances, not only from
the default one. Without ZEBRA_ROUTE_ALL, ldpd would need to send a
ZEBRA_REDISTRIBUTE_ADD message for each possible OSPF instance (65k),
which doesn't scale very well.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

bgpd: bgp_nexthop_cache not deleted with peers

* Fix mild leak, bgp_nexthop_caches were not deleted when their peer was.
  Not a huge one, but makes valgrinding for other leaks noisier.

  Credit to Lou Berger <lberger@labn.net> for doing the hard work of
  debugging and pinning down the leak, and supplying an initial fix.
  That one didn't quite get the refcounting right, it seemed, hence
  this version.

  This version also keeps bncs pinned so long as the peer is defined, where
  Lou's tried to delete whenever the peer went through bgp_stop. That causes
  lots of zebra traffic if down peers go Active->Connect->Active, etc., so
  leaving bnc's in place until peer_delete seemed better.

* bgp_nht.c: (bgp_unlink_nexthop_by_peer) similar to bgp_unlink_nexthop, but
  by peer.
* bgp_nht.c: (bgp_unlink_nexthop_check) helper to consolidate checking
  if a bnc should be deleted.
  (bgp_unlink_nexthop_by_peer) ensure the bnc->nht_info peer reference
  is removed, and hence allow bncs to be removed by previous.
* bgpd.c: (peer_delete) cleanup the peer's bnc.

bgpd: Fix crash in 'show ip bgp nexthop'

* bgp_nexthop.c: (show_ip_bgp_nexthop_table) the AFIs are sparse, and start
  from 1, check there's a table before derefing so as not to crash.

lib: add event_counter

Signed-off-by: Christian Franke <chris@opensourcerouting.org>

lib: add api method to read current config path

Signed-off-by: Christian Franke <chris@opensourcerouting.org>

bgpd: fix MRT table dumps for locally-originated routes

I've been working on a small patch to correct an issue in the BGP MRT
table dump code. It's a quick'n'easy fix initially, and I'd appreciate
any feedback on making it better :)

Issue:

When the BGP table dump code runs, it generates the peer_index_table.
This walks the list of peers, and dumps out their IP, ASN, address
family, etc. It also sets the peer index number in the peer struct.

Then the code walks the RIB, and for each prefix, writes out RIB
entries, that refer to the peer index number.

However, when it finds prefixes that are locally originated, the
associated peer is the 'self' peer, which wasn't in the list of peers,
never gets an index number assigned, but because it is calloc'd, the
index number is set to 0.

End result: locally-originated routes are associated with whichever peer
happens to be first in the list of remote peers in the index table :)

Example (from one of our route collectors) - these are two of our
originated prefixes (bgpdump output):
TABLE_DUMP2|1457568002|B|12.0.1.63|7018|84.205.80.0/24||IGP|193.0.4.28|0|0||NAG|64512
10.255.255.255|
TABLE_DUMP2|1457568006|B|12.0.1.63|7018|2001:7fb:ff00::/48||IGP|::|0|0||NAG||

The prefixes are announced by us (note it has an empty AS PATH (the
field after the prefix)) but also looks like it was received from AS7018
(12.0.1.63). In fact, the AS7018 peer just happens to be the first peer
in the index table.

Fix:

The simplest fix (which is also the method adopted by both OpenBGPd and
the BIRD mrtdump branch) is to create an empty placeholder 'peer' at the
start of the peer index table, for all the routes which are locally
originated to refer to.

I've attached a patch for this.
Here's a resulting bgpdump output after the patch:
TABLE_DUMP2|1458828539|B|0.0.0.0|0|93.175.150.0/24||IGP|0.0.0.0|0|0||NAG||
Now it is more obvious that the prefix is locally originated.

There are more complicated potential ways of fixing it
1) skip the local routes when dumping the RIB. This leads to questions
about what an MRT table dump *should* contain :)
2) include the 'self' peer in the list of peers used to generate the
index table.
etc etc.

But I'm quite happy with my 'create a fake peer, and associate local
routes with it' method :)

Your thoughts and feedback are welcome!

Regards,

Colin Petrie
Systems Engineer
RIPE NCC RIS Project
Tested-by: NetDEF CI System <cisystem@netdef.org>

zebra: use unsigned args for *addattr

[based on a patch by Paul Jakma changing it to size_t]

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

tests: fix MIN()-related sign warnings

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

zebra: Fix route deletion on *BSD

Fix for not handling RTM_CHANGE correctly.  This patch change it to
delete/add instead.  Using RTM_CHANGE on kernels where it works is better,
but is left as an exercise for developer who has access and will to fix it
on *BSD.

[ed note: collaboration with Martin Winter]

zebra: kernel interface simplification

[DL: picked out from: "atomic FIB updates"]

This simplifies the OS-specific route update API into a single entry
point, kernel_route_rib(), which dispatches the various operations
internally.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>

zebra: remove rt_ioctl kernel interface

None of the BSDs uses ioctls to set routes anymore.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Acked-by: Greg Troxel <gdt@ir.bbn.com>
Acked-by: Feng Lu <lu.feng@6wind.com>
Acked-by: Paul Jakma <paul@jakma.org>

bgpd: Fix crash reported by NetDEF CI

This patch is part of the previously submitted patch set on VPN and
Encap SAFIs.  It fixes an issue identified by NetDEF CI.

Ensure temp stack structures are initialized Add protection against
double frees / post free access to bgp_attr_flush

Signed-off-by: Lou Berger <lberger@labn.net>

bgp: add bgp_isvalid_nexthop helper and additional NHT zebra checks

bgp: ignore NHT when bgpd has never connected to zebra

rfapi: cleanup some warnings