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

update to Quagga 0.99.24.1 #1

Open
wants to merge 488 commits into
base: master
Choose a base branch
from

Conversation

equinox0815
Copy link

This updates quagga to the latest release 0.99.24.1.
Due to the introduction of pimd the olsr/batman patch needed some extra attention.
This also contains a refreshed patch for the remaining changes needed for openwrt.

fabled and others added 30 commits August 18, 2014 01:50
The whole IPv6 stack detection could need refactoring. But this
fixes the linux check to not assume glibc. Fixes build against
musl c-library.

Signed-off-by: Timo Teräs <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
In bgpd/bgp_community_del_val memcpy is used for potentially overlapping
regions which is *not* safe. It may "work" in some cases but is not
guaranteed to work in all cases. The case that I saw fail was on an
x86_64 architecture with the number of bytes being moved/copied equal to
8.

The way the code is written the uint32_t pointers will always differ by
1, which is equivalent to a memcpy/memmove of regions that are 4 bytes
away from one another. So the code failed while copying an 8 byte region
to an address that is 4 bytes lower i.e. overlapping regions.

Interestingly, the same architecture had no problems with a 12 byte
copy.

When the code failed the communities were [200,300,400] and a call was
made to delete the 200 community. The result of this was an array that
looked like [400,400] which was uniquified to [400]. Of course the
expected result should have been [300, 400].

One additional point - in our production environment memmove would not
*link* without including <string.h> but in an isolated quagga git repo
this #include does not seem to be required and I see memmove is used in
vtysh.c without this #include either.

Signed-off-by: David Lamparter <[email protected]>
Whoops, these are in6_addrs, not prefix_ipv6... funnily enough, it does the
right thing either way, if it compiles, which it only does on Linux because
IN6_IS_ADDR_LINKLOCAL contains a cast to the right type.  On BSD there is no
such cast, hence it explodes on trying to compile, trying to access struct
members of in6_addrs while operating on prefix_ipv6...

Fixes: 28a8cfc ("isisd: don't require IPv4 for adjacency")
Signed-off-by: David Lamparter <[email protected]>
On OpenBSD, carp interfaces claim to be PtP interfaces with a 0.0.0.0/0
peer address.  We process those in zebra and try to send them to
clients, at which point they get encoded as all-0.  The client code,
however, decodes that to a NULL pointer instead of 0.0.0.0.  This later
turns into a SEGV when CONNECTED_PREFIX sees that ZEBRA_IFA_PEER is set
and tries to access the peer prefix.

This is a band-aid fix for stable/0.99.23, a long-term solution needs
some conceptual improvements on the entire thing.

(The usefulness of a PtP-to-0.0.0.0/0 is a separate question;  at this
point dropping the peer prefix seems the least intrusive solution.)

Reported-by: Laurent Lavaud <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
* Each node in the routing table is changed into a list, holding
  the multiple equal-cost paths.

* If one of the multiple entries gets less-preferred (greater
  metric or greater distance), it will be directly deleted instead
  of starting a garbage-collection timer for it.
  The garbage-collection timer is started only when the last entry
  in the list gets INFINITY.

* Some new functions are used to maintain the ECMP list. And hence
  rip_rte_process(), rip_redistribute_add() and rip_timeout() are
  significantly simplified.

* rip_zebra_ipv4_add() and rip_zebra_ipv4_delete() now can share
  the common code. The common part is moved to rip_zebra_ipv4_send().

Signed-off-by: Feng Lu <[email protected]>
Reviewed-by: Alain Ritoux <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
Introduce a new command "[no] allow-ecmp" to enable/disable the
ECMP feature in RIP. By default, ECMP is not allowed.

Once ECMP is disabled, only one route entry can exist in the list.

* rip_zebra.c: adjust a debugging information, which shows the number
               of nexthops according to whether ECMP is enabled.
* ripd.c: rip_ecmp_add() will reject the new route if ECMP is not
          allowed and some entry already exists.
          A new configurable command "allow-ecmp" is added to control
          whether ECMP is allowed.
          When ECMP is disabled, rip_ecmp_disable() is called to
          remove the multiple nexthops.
* ripd.h: Add a new member "ecmp" to "struct rip", indicating whether
          ECMP is allowed or not.

Signed-off-by: Feng Lu <[email protected]>
Reviewed-by: Alain Ritoux <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
In _netlink_route_build_multipath():
- Each time when appending a IPv4 gateway in the message, rtnh_len
  is increased by sizeof (struct rtattr) + 4, where we should use
  "bytelen" instead of the hard coding "4".
- As what done for IPv4, we should increase rtnh_len accordingly
  along with adding a IPv6 gateway, or else the IPv6 gateways will
  be lost.

Signed-off-by: Feng Lu <[email protected]>
Reviewed-by: Alain Ritoux <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
the library's thread scheduling functions keep track of the thread
function's name, so far so good.  However, copying the compiler-provided
constant into a buffer inside the thread structure is plain useless.
Also, strip_funcname() was trying to support something that never
happens.

Instead, let's use some bytes here to track where threads are scheduled
from.  Another commit will print that information on crashes.

Ripping out useless stuff:  -64 bytes in the thread structure
Re-add as const ptr:         +8 bytes
Extra debug info:           +12 bytes

Signed-off-by: David Lamparter <[email protected]>
now that we know what thread we're currently executing, let's add that
information to SEGV / assert backtraces.

Signed-off-by: David Lamparter <[email protected]>
* command.h: (config_from_file) Add variable to interface for line
      number reporting.
    * command.c: (config_from_file) Set & increment 'line_num' while parsing.
    * vty.c: (vty_read_file) Report parse errors in the correct order to
      stderr, with added line numbers.
* Fix (a subset of)? files with non-trivial code that are missing GPL headers.

* A few copyright claims added which I am certain apply, but which I had
  missed out on the original commits.

NB: Copyright claims are not exclusive and the addition of any copyright
claim should not be read as implying a lack of any further claims, or
denying the validity of any other claims.  All those with claims of
copyright over any portion of Quagga are welcome to submit them, ideally as
patches to update copyright strings in files.
Fix lots of warnings. Some const and type-pun breaks strict-aliasing
warnings left but much reduced.

* bgp_advertise.h: (struct bgp_advertise_fifo) is functionally identical to
  (struct fifo), so just use that.  Makes it clearer the beginning of
  (struct bgp_advertise) is compatible with with (struct fifo), which seems
  to be enough for gcc.
  Add a BGP_ADV_FIFO_HEAD macro to contain the right cast to try shut up
  type-punning breaks strict aliasing warnings.
* bgp_packet.c: Use BGP_ADV_FIFO_HEAD.
  (bgp_route_refresh_receive) fix an interesting logic error in
  (!ok || (ret != BLAH)) where ret is only well-defined if ok.
* bgp_vty.c: Peer commands should use bgp_vty_return to set their return.
* jhash.{c,h}: Can take const on * args without adding issues & fix warnings.
* libospf.h: LSA sequence numbers use the unsigned range of values, and
  constants need to be set to unsigned, or it causes warnings in ospf6d.
* md5.h: signedness of caddr_t is implementation specific, change to an
  explicit (uint_8 *), fix sign/unsigned comparison warnings.
* vty.c: (vty_log_fixed) const on level is well-intentioned, but not going
  to fly given iov_base.
* workqueue.c: ALL_LIST_ELEMENTS_RO tests for null pointer, which is always
  true for address of static variable.  Correct but pointless warning in
  this case, but use a 2nd pointer to shut it up.
* ospf6_route.h: Add a comment about the use of (struct prefix) to stuff 2
  different 32 bit IDs into in (struct ospf6_route), and the resulting
  type-pun strict-alias breakage warnings this causes.  Need to use 2
  different fields to fix that warning?

general:

* remove unused variables, other than a few cases where they serve a
  sufficiently useful documentary purpose (e.g.  for code that needs
  fixing), or they're required dummies.  In those cases, try mark them as
  unused.
* Remove dead code that can't be reached.
* Quite a few 'no ...' forms of vty commands take arguments, but do not
  check the argument matches the command being negated.  E.g., should
  'distance X <prefix>' succeed if previously 'distance Y <prefix>' was set?
  Or should it be required that the distance match the previously configured
  distance for the prefix?
  Ultimately, probably better to be strict about this.  However, changing
  from slack to strict might expose problems in command aliases and tools.
* Fix uninitialised use of variables.
* Fix sign/unsigned comparison warnings by making signedness of types consistent.
* Mark functions as static where their use is restricted to the same compilation
  unit.
* Add required headers
* Move constants defined in headers into code.
* remove dead, unused functions that have no debug purpose.
* lib/sigevent.c: (program_counter) extend to support more platforms. Joint
  effort with Paul Jakma.
…crash.

* ANVL testing by Martin Winter threw up a crash in bgpd in aspath_dup
  called from bgp_packet_attribute, if attr->aspath was NULL, on an IPv6
  UPDATE.

  This root cause is that the checks for well-known, mandatory attributes
  were being applied only if an UPDATE contained the IPv4 NLRI and the
  peer was configured for v4/unicast (i.e. not deconfigured). This is
  something inherited from GNU Zebra, and never noticed before.

* bgp_attr.c: (bgp_attr_parse) Move the well-known mandatory attribute
  check to here, so that it can be run immediately after all attributes
  are parsed, and before any further processing of attributes that might
  assume the existence of WK/M attributes (e.g. AS4-Path).
  (bgp_attr_munge_as4_attrs) Missing AS_PATH shouldn't happen here anymore,
  but retain a check anyway for robustness - it's definitely a hard error
  though.
* bgp_attr.h: (bgp_attr_check) No longer needs to be exported, make static.
* bgp_packet.c: (bgp_update_receive) Responsibility for well-known check
  now in bgp_attr_parse.
* quagga.texi: I'm getting warnings about stuff in defines.texi not being
  defined when building quagga.info. Seems to be fixed by moving the include
  of defines.texi to the end of the header. Also, the Texinfo docs suggest
  setfilename must go first.
This looks fishy in ospf_make_md5_digest()
if (list_isempty (OSPF_IF_PARAM (oi, auth_crypt)))
    auth_key = (const u_int8_t *) "";
...
MD5Update(&ctx, auth_key, OSPF_AUTH_MD5_SIZE);
auth_key points to a "" string of len 1 which is a lot
smaller that OSPF_AUTH_MD5_SIZE. Is this intentional to
get some random data or just a plain bug?

Anyone using MD5 should have a closer look and decide
what to do.
Acked-by: Feng Lu <[email protected]>
This fix is probably correct on 32bit systems,
but i think it will not work on 64bit systems.
sizeof(signed long) would be 8 and therefore the
cast from u_int32_t will map all the values to
non-negative part of long int.

You would like to use int (like in ospfd) and
change the type of seqnuma, seqnumb to that.

The type int32_t would be even more proper, but
sizeof(int) is 4 on relevant platforms.

Signed-off: Ondrej Zajicek <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Yasuhiro Ohara <[email protected]>
The one place this was being used in BGP is now gone,
can remove deprecated interface.
Acked-by: Feng Lu <[email protected]>
Explain how to be a nice contributor in a handy way.

Signed-off-by: Vincent JARDIN <[email protected]>
Acked-by: Paul Jakma <[email protected]>
* mrlg.cgi: The version we shipped was very much  out of date, remove it.
* mrlg.txt: Add file pointing to the official MRLG site.
eqvinox and others added 30 commits February 6, 2015 22:01
Signed-off-by: David Lamparter <[email protected]>
Reported-by: Alexis Rosen <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
Welcome pimd to the Quagga daemon zoo!

This is a merge of commit 77ae369 ("pimd: Log ifindex found for an
interface when zebra lib reports a new connected address."), with
the intermediate "reconnect" changes removed (c9adf00...d274381).
d274381 is replaced with b162ab7, which includes some changes.  In
addition, 4 reconnect-related changes and 1 cosmetic one have been
bumped out.

The rebase command used to produce the branch that is merged here is:
  git rebase --onto b162ab7 c9adf00 77ae369

Note that 3 patches had their author rewritten from
    "Anonymous SR#108542 <>" (which is not a valid git author ID)
to: "Savannah SR#108542 <[email protected]>" (which is the e-mail address
                               listed in the associated Savannah ticket)

Signed-off-by: David Lamparter <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
this is not a full release version, so neither release notes nor
documentation are updated yet.  Also, signing the tag with my private
GPG key instead of the Quagga one.

Signed-off-by: David Lamparter <[email protected]>
These actually break configure on FreeBSD very subtly, because inet_aton
and __inet_aton are both detected, and then later other tests get
warnings about HAVE_INET_ATON being defined twice.

That said, they're incorrect to begin with since they detect alternative
functions but there is nothing in place to actually use these
alternates.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
This path is deprecated, completely untested, likely broken and will not
be maintained.  Kill it with fire.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
Valar morghulis.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
Valar dohaeris.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
In the 90ies, IPv4 was believed to exist within IPv6, with some kernels
implementing this belief in code...  Our code here is keyed to "#ifdef
LINUX", yet no Linux from the past 10 years had this, making the code
completely useless.

FreeBSD 10.0 does in fact have a "::/96 via ::1 dev lo0 reject" route.
IMHO we shouldn't mess with that, the admin can filter as neccessary
anyway.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
[DL: slightly adjusted commit message to remove misunderstanding]
Acked-by: Paul Jakma <[email protected]>
The only user of this was rib_bogus_ipv6(), which was removed in the
previous commit.  Good riddance.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
IPv6 functions in a separate library... yeah, right.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
None of the BSDs uses ioctls to set routes anymore.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
This switch controlled descending into the solaris/ subdirectory, which
contains package descriptions and init scripts.  If they're not
appropriate, they'd better be removed outright.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
INCLUDES in configure.ac was not used at all, and INCLUDES in
Makefile.am is supposed to be AM_CPPFLAGS these days.

Reduces warnings spewed during bootstrap/autoreconf.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Greg Troxel <[email protected]>
Acked-by: Feng Lu <[email protected]>
Acked-by: Paul Jakma <[email protected]>
Building with IPv6 disabled tends to break rather often and sprinkles
ugly #ifdefs around the code.  All that only to support systems where
the C library doesn't have IPv6 capability.

The year now being 2015, if this is a problem the thing to fix is the C
library.

The implication of this patch is that future patches need not care about
HAVE_IPV6 = 0 and may remove ifdefs gratuitously.  This patch doesn't
remove these ifdefs to not create unneccessary churn.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Paul Jakma <[email protected]>
With --disable-ipv6 gone, the IPv6 detection logic in the tests is not
needed anymore either.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Paul Jakma <[email protected]>
Record the ./configure arguments used and make them user-visible.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Paul Jakma <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
Acked-by: Paul Jakma <[email protected]>
Most distributors enable it anyway, and it's not THAT broken anymore to
mandate disabling it by default.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Paul Jakma <[email protected]>
This shuts up make by default (can be reversed with "make V=1" or
--disable-silent-rules).  This is useful since warnings and error
messages become more visible with less noise.

Tested on Linux with GNU make and FreeBSD with system's BSD make.

Signed-off-by: David Lamparter <[email protected]>
Acked-by: Paul Jakma <[email protected]>
This extends the ip_mreq hack to DragonFlyBSD and SunOS. This has been
in pkgsrc for some time. I've cleaned up the pkgsrc patch a little and
am submitting it upstream. Credit is due to pkgsrc maintainers.

Tested on SmartOS (illumos).

Fixes: #819
Signed-off-by: Greg Troxel <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
Resolves an issue where quagga daemons restart in an infinite loop.
Quagga daemons declare a dependency on zebra that requires a restart
of the daemon when zebra restarts and they explicitly restart zebra,
which again triggers their own restart.

Restarting zebra when other daemons are started is explicitly removed,
leaving dependency management up to SMF rather than handling it in the
start method.

solaris/quagga.init.in: Remove calls to routeadm_zebra_enable, and the
    routeadm_zebra_enable function.
solaris/quagga.xml.in: Set dependency zebra grouping to require_all.

Fixes: #818
Signed-off-by: Greg Troxel <[email protected]>
Signed-off-by: David Lamparter <[email protected]>
The default for this is slated to change, so let's print the current
default value for preexisting configurations.

Signed-off-by: David Lamparter <[email protected]>
This crept in as part of the MRIB improvements and I missed the compiler
warning between other noise.  Unfortunately, printing an uninitialised
variable can in fact make zebra crash, so this is not trivial.

Fixes: 3b02fe8 ("zebra: add "show ip rpf" to get result of RPF lookup")
Signed-off-by: David Lamparter <[email protected]>
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.