[quagga-dev 14771] Re: FreeBSD Bug: Zebra not deleting routes
Thanks, I see it applied.
Most of the issues are fixed. I see a few ISIS & OSPF differences.
Could be the different base (the Ubuntu one was done before the last
rebase).
I’m currently re-running Ubuntu against the rebased tree to have
a base for comparison.
Updated test comparison is here:
https://drive.google.com/open?id=0B8W_T0dxQfwxN2ZXSnkweF94anc
- Martin
On 24 Feb 2016, at 4:42, Donald Sharp wrote:
Martin -
Thanks for this work.
I'll be applying the patch here in a few minutes.
donald
On Wed, Feb 24, 2016 at 6:02 AM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
On 22 Feb 2016, at 6:40, Donald Sharp wrote:
Martin -
Any word on this? I'd like to push this patch if it fixes your
issues.
Ok, finally more answers.
It seems something went wrong when I retested with Timo’s fix.
(Might be
related to the way I retested as I was unable to do a clean rebuild
and
applied patch [probably wrong] on top of the last commit. Rebase made
it
impossible to pull a new copy as I usually do for each run)
Anyway, re-running the tests and Timo’s patch (same fixed version
as
mailed earlier) fixes at least all BGP IPv4 bug difference between
Linux and FreeBSD.
So I suggest to go ahead an apply that patch on top of proposed/6
branch
Full results (including all other protocols) of the rerun will be
available
in approx another 12..18hrs.
- Martin
On Thu, Feb 18, 2016 at 10:30 PM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
On 18 Feb 2016, at 18:35, Donald Sharp wrote:
Martin -
rt_socket.c is not compiled on linux so no need for verification.
Once
you
have a run please let me know and I'll push your updated patch.
It’s running now. Will know soon. It definitely fixes some of the
errors
and I hope it actually fixes all of the FreeBSD errors.
Will know in approx one more day.
(BTW: Testing on a git commit which does no longer exist because of
rebase,
but I needed the same to compare. Lucky that I still had the VMs
with the
old git checkout running…)
- Martin
donald
On Thu, Feb 18, 2016 at 9:19 PM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
Timo,
On 17 Feb 2016, at 23:01, Timo Teras wrote:
On Wed, 17 Feb 2016 20:11:07 -0800
"Martin Winter" wrote:
This is based on Quagga proposed 6 branch of Feb 10 (git
f48d5b9957 -
which does no longer exist, [no] thanks to rebase on a public
git) on FreeBSD 10.2.
Zebra seems to fail delete any routes in FreeBSD RIB. It fails
with
updates (better routes to different interface) and
it fails to remove them when quagga is killed.
Thanks for the testing. Sounds like this is breakage caused by
my
atomic FIB patch which was pretty untested on *BSD.
Looks like the code talking to kernel does not handle RTM_CHANGE
correctly. As first test, perhaps just removing RTM_CHANGE usage
might
fix the issues and revert to how it used to work.
Using RTM_CHANGE on kernels where it works is better, but it's
left an
exercise for developer who has access and will to fix it on
*BSD.
Thanks for the patch. Seems like you never tested it (failed to
compile),
but was close enough to guess what you probably meant. See inline
on
patch
diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c
index 4d0a7db..9012280 100644
--- a/zebra/rt_socket.c
+++ b/zebra/rt_socket.c
@@ -68,7 +68,7 @@ sin_masklen (struct in_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib,
int
family)
+kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in *mask = NULL;
@@ -245,7 +245,7 @@ sin6_masklen (struct in6_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib,
int
family)
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in6 *mask;
struct sockaddr_in6 sin_dest, sin_mask, sin_gate;
@@ -363,33 +363,32 @@ kernel_rtm_ipv6 (int cmd, struct prefix
*p,
struct
rib *rib, int family)
#endif
+static int
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
I assume this should be
kernel_rtm - not kernel_rtm_ipv6
(otherwise you have a duplicate for kernel_rtm_ipv6() and a loop
in case of AF_INET6)
+{
+ switch (PREFIX_FAMILY(p))
+{
+case AF_INET:
+ return kernel_rtm_ipv4 (cmd, p, rib);
+case AF_INET6:
+ return kernel_rtm_ipv6 (cmd, p, rib);
+}
+ return 0;
+}
+
int
kernel_route_rib (struct prefix *p, struct rib *old, struct rib
*new)
{
- struct rib *rib;
- int route = 0, cmd;
-
- if (!old && new)
-cmd = RTM_ADD;
- else if (old && !new)
-cmd = RTM_DELETE;
- else
-cmd = RTM_CHANGE;
-
- rib = new ? new : old;
+ int route = 0;
if (zserv_privs.change(ZPRIVS_RAISE))
zlog (NULL, LOG_ERR, "Can't raise privileges");
- switch (PREFIX_FAMILY(p))
-{
-case AF_INET:
- route = kernel_rtm_ipv4 (cmd, p, rib, AF_INET);
-
[quagga-dev 14770] Re: [PATCH 04/10] lib: migrate to new memory-type handling
On Wed, Feb 24, 2016 at 06:25:06PM -0500, Donald Sharp wrote:
> I'm not a big fan of #if 0 or #if 1's introduced with this patch. Is there
> someway we can mitigate them?
This is what the comments from Paul & myself in the other mail referred to:
> Paul:
> > - The __builtin_types_{expect,compatible_p} stuff, would C11 _Generic
> >allow the same to be achieved?
>
> The __builtin_types_* stuff is actually in a commented-out section that
> I forgot to remove. It was there for temporary compatibility between
> the old and new schemes, so that the code works correctly even at every
> single step in the middle.
I'll resend a version (in a bit - eta a day or two) with
- this fixed (i.e. removed)
- whitespace cleaned
- it applying on the rebased git tree :S
-David
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14767] Re: [PATCH 08/10] lib: clean/restore memory debugging functions
Acked-by: Donald Sharp
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> This adapts the dump-at-exit handler and removes the old leftover code.
>
> (Note the text in log_memtype_stderr was actually incorrect as the only
> caller in bgpd cleans up configuration before calling it, i.e. any
> remaining allocations are missing-cleanup bugs.)
>
> Signed-off-by: David Lamparter
> ---
> lib/memory.c | 32
> lib/memory.h | 1 +
> lib/memory_vty.c | 89
> +---
> lib/memory_vty.h | 1 -
> 4 files changed, 34 insertions(+), 89 deletions(-)
>
> diff --git a/lib/memory.c b/lib/memory.c
> index 80914b8..8247b7b 100644
> --- a/lib/memory.c
> +++ b/lib/memory.c
> @@ -103,3 +103,35 @@ int qmem_walk (qmem_walk_fn *func, void *arg)
> }
>return 0;
> }
> +
> +struct exit_dump_args {
> + const char *prefix;
> + int error;
> +};
> +
> +static int qmem_exit_walker(void *arg, struct memgroup *mg, struct
> memtype *mt)
> +{
> + struct exit_dump_args *eda = arg;
> +
> + if (!mt)
> +{
> + fprintf (stderr, "%s: showing active allocations in memory group
> %s\n",
> + eda->prefix, mg->name);
> +}
> + else if (mt->n_alloc)
> +{
> + char size[32];
> + eda->error++;
> + snprintf (size, sizeof (size), "%10zu", mt->size);
> + fprintf (stderr, "%s: %-30s: %6zu * %s\n",
> + eda->prefix, mt->name, mt->n_alloc,
> + mt->size == SIZE_VAR ? "(variably sized)" : size);
> +}
> + return 0;
> +}
> +
> +void log_memstats_stderr (const char *prefix)
> +{
> + struct exit_dump_args eda = { .prefix = prefix, .error = 0 };
> + qmem_walk (qmem_exit_walker, &eda);
> +}
> diff --git a/lib/memory.h b/lib/memory.h
> index 8e9608a..cf184d4 100644
> --- a/lib/memory.h
> +++ b/lib/memory.h
> @@ -198,6 +198,7 @@ static inline size_t mtype_stats_alloc(struct memtype
> *mt)
> * last value from qmem_walk_fn. */
> typedef int qmem_walk_fn (void *arg, struct memgroup *mg, struct memtype
> *mt);
> extern int qmem_walk (qmem_walk_fn *func, void *arg);
> +extern void log_memstats_stderr (const char *);
>
> extern void memory_oom (size_t size, const char *name);
>
> diff --git a/lib/memory_vty.c b/lib/memory_vty.c
> index e1c08ce..0b702ed 100644
> --- a/lib/memory_vty.c
> +++ b/lib/memory_vty.c
> @@ -35,80 +35,6 @@
> #include "vty.h"
> #include "command.h"
>
> -void
> -log_memstats_stderr (const char *prefix)
> -{
> -#if 0
> - struct mlist *ml;
> - struct memory_list *m;
> - int i;
> - int j = 0;
> -
> - for (ml = mlists; ml->list; ml++)
> -{
> - i = 0;
> -
> - for (m = ml->list; m->index >= 0; m++)
> -if (m->index && mstat[m->index].alloc)
> - {
> -if (!i)
> - fprintf (stderr,
> - "%s: memstats: Current memory utilization in
> module %s:\n",
> - prefix,
> - ml->name);
> -fprintf (stderr,
> - "%s: memstats: %-30s: %10ld%s\n",
> - prefix,
> - m->format,
> - mstat[m->index].alloc,
> - mstat[m->index].alloc < 0 ? " (REPORT THIS BUG!)" :
> "");
> -i = j = 1;
> - }
> -}
> -
> - if (j)
> -fprintf (stderr,
> - "%s: memstats: NOTE: If configuration exists, utilization
> may be "
> - "expected.\n",
> - prefix);
> - else
> -fprintf (stderr,
> - "%s: memstats: No remaining tracked memory utilization.\n",
> - prefix);
> -#endif
> -}
> -
> -#if 0
> -static void
> -show_separator(struct vty *vty)
> -{
> - vty_out (vty, "-\r\n");
> -}
> -
> -static int
> -show_memory_vty (struct vty *vty, struct memory_list *list)
> -{
> - struct memory_list *m;
> - int needsep = 0;
> -
> - for (m = list; m->index >= 0; m++)
> -if (m->index == 0)
> - {
> - if (needsep)
> - {
> - show_separator (vty);
> - needsep = 0;
> - }
> - }
> -else if (mstat[m->index].alloc)
> - {
> - vty_out (vty, "%-30s: %10ld\r\n", m->format,
> mstat[m->index].alloc);
> - needsep = 1;
> - }
> - return needsep;
> -}
> -#endif
> -
> #ifdef HAVE_MALLINFO
> static int
> show_memory_mallinfo (struct vty *vty)
> @@ -174,23 +100,10 @@ DEFUN (show_memory,
> "Show running system information\n"
> "Memory statistics\n")
> {
> - int needsep = 0;
> -
> #ifdef HAVE_MALLINFO
> - needsep = show_memory_mallinfo (vty);
> + show_memory_mallinfo (vty);
> #endif /* HAVE_MALLINFO */
>
> - (void) needsep;
> -#if 0
> - struct mlist *ml;
> - for (ml = mlists; ml->list; ml++)
> -{
> - if (needsep)
> - show_separator (vty);
> - needsep = show_memory_vty (vty, ml->list);
> -}
> -#endif
> -
>qmem_walk(qmem_walker, vty);
>retu
[quagga-dev 14769] Re: [PATCH 10/10] build: goodbye, gawk
Acked-by: Donald Sharp
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> Signed-off-by: David Lamparter
> ---
> INSTALL.quagga.txt | 1 -
> configure.ac | 6 --
> 2 files changed, 7 deletions(-)
>
> diff --git a/INSTALL.quagga.txt b/INSTALL.quagga.txt
> index 11c85b1..b414d94 100644
> --- a/INSTALL.quagga.txt
> +++ b/INSTALL.quagga.txt
> @@ -69,7 +69,6 @@ deficient is made.
> autoconf: 2.59 (2.60 on 2006-06-26 is too recent to require)
> libtool:1.5.22 (released 2005-12-18)
> texinfo:4.7 (released 2004-04-10; 4.8 is not yet common)
> - GNU AWK:3.1.5 (released 2005-08-12)
>
> For running tests, one also needs:
>
> diff --git a/configure.ac b/configure.ac
> index 7aaacb9..79e5128 100755
> --- a/configure.ac
> +++ b/configure.ac
> @@ -25,12 +25,6 @@ AM_SILENT_RULES([yes])
> AC_CONFIG_HEADERS(config.h)
>
> AC_PATH_PROG(PERL, perl)
> -AC_CHECK_PROG([GAWK],[gawk],[gawk],[not-in-PATH])
> -if test "x$GAWK" = "xnot-in-PATH" ; then
> - AC_MSG_ERROR([GNU awk is required for lib/memtype.h made by
> memtypes.awk.
> -BSD awk complains: awk: gensub doesn't support backreferences (subst
> "\1") ])
> -fi
> -AC_ARG_VAR([GAWK],[GNU AWK])
>
> dnl default is to match previous behavior
> exampledir=${sysconfdir}
> --
> 2.3.6
>
>
> ___
> Quagga-dev mailing list
> Quagga-dev@lists.quagga.net
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14768] Re: [PATCH 09/10] build: goodbye, memtypes.c
Acked-by: Donald Sharp
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> Signed-off-by: David Lamparter
> ---
> lib/.gitignore | 1 -
> lib/Makefile.am | 10 +++---
> lib/memtypes.c | 16
> lib/memtypes.pl | 6 --
> 4 files changed, 3 insertions(+), 30 deletions(-)
> delete mode 100644 lib/memtypes.c
> delete mode 100755 lib/memtypes.pl
>
> diff --git a/lib/.gitignore b/lib/.gitignore
> index 02aa432..930d737 100644
> --- a/lib/.gitignore
> +++ b/lib/.gitignore
> @@ -14,5 +14,4 @@ gitversion.h.tmp
> .arch-ids
> *~
> *.loT
> -memtypes.h
> route_types.h
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 7456cc6..abd0e3e 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -13,11 +13,11 @@ libzebra_la_SOURCES = \
> sockunion.c prefix.c thread.c if.c buffer.c table.c hash.c \
> filter.c routemap.c distribute.c stream.c str.c log.c plist.c \
> zclient.c sockopt.c smux.c agentx.c snmp.c md5.c if_rmap.c
> keychain.c privs.c \
> - sigevent.c pqueue.c jhash.c memtypes.c workqueue.c vrf.c \
> + sigevent.c pqueue.c jhash.c workqueue.c vrf.c \
> memory.c \
> memory_vty.c
>
> -BUILT_SOURCES = memtypes.h route_types.h gitversion.h
> +BUILT_SOURCES = route_types.h gitversion.h
>
> libzebra_la_DEPENDENCIES = @LIB_REGEX@
>
> @@ -29,7 +29,7 @@ pkginclude_HEADERS = \
> memory.h network.h prefix.h routemap.h distribute.h sockunion.h \
> str.h stream.h table.h thread.h vector.h version.h vty.h zebra.h \
> plist.h zclient.h sockopt.h smux.h md5.h if_rmap.h keychain.h \
> - privs.h sigevent.h pqueue.h jhash.h zassert.h memtypes.h \
> + privs.h sigevent.h pqueue.h jhash.h zassert.h \
> workqueue.h route_types.h libospf.h vrf.h \
> memory_vty.h
>
> @@ -39,13 +39,9 @@ noinst_HEADERS = \
> EXTRA_DIST = \
> regex.c regex-gnu.h \
> queue.h \
> - memtypes.pl \
> route_types.pl route_types.txt \
> gitversion.pl
>
> -memtypes.h: $(srcdir)/memtypes.c $(srcdir)/memtypes.pl
> - @PERL@ $(srcdir)/memtypes.pl < $(srcdir)/memtypes.c > $@
> -
> route_types.h: $(srcdir)/route_types.txt $(srcdir)/route_types.pl
> @PERL@ $(srcdir)/route_types.pl < $(srcdir)/route_types.txt > $@
>
> diff --git a/lib/memtypes.c b/lib/memtypes.c
> deleted file mode 100644
> index fe78f50..000
> --- a/lib/memtypes.c
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -/*
> - * Memory type definitions. This file is parsed by memtypes.awk to extract
> - * MTYPE_ and memory_list_.. information in order to autogenerate
> - * memtypes.h.
> - *
> - * The script is sensitive to the format (though not whitespace), see
> - * the top of memtypes.awk for more details.
> - */
> -
> -#include "zebra.h"
> -#include "memory.h"
> -
> -DEFINE_MGROUP(BABELD, "babeld")
> -DEFINE_MTYPE(BABELD, BABEL,"Babel structure")
> -DEFINE_MTYPE(BABELD, BABEL_IF, "Babel interface")
> -
> diff --git a/lib/memtypes.pl b/lib/memtypes.pl
> deleted file mode 100755
> index 0955161..000
> --- a/lib/memtypes.pl
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#!/usr/bin/perl
> -while () {
> - $_ =~
> s/DEFINE_MTYPE\([^,]+,\s*([^,]+)\s*,.*\)/DECLARE_MTYPE\($1\)/;
> - $_ =~ s/DEFINE_MGROUP\(([^,]+),.*\)/DECLARE_MGROUP\($1\)/;
> - print $_;
> -}
> --
> 2.3.6
>
>
> ___
> Quagga-dev mailing list
> Quagga-dev@lists.quagga.net
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14766] Re: [PATCH 07/10] lib: distribute mtypes into files
Acked-by: Donald Sharp
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> This places MTYPE definitions into the files where they're actually
> used, making most of them static in the process.
>
> Signed-off-by: David Lamparter
> ---
> lib/buffer.c | 3 ++-
> lib/command.c | 4
> lib/command.h | 6 +
> lib/distribute.c | 3 +++
> lib/filter.c | 4
> lib/hash.c| 4
> lib/hash.h| 5 +
> lib/if.c | 4
> lib/if.h | 4
> lib/if_rmap.c | 3 +++
> lib/keychain.c| 3 +++
> lib/linklist.c| 3 +++
> lib/log.c | 2 ++
> lib/memory.c | 3 +++
> lib/memory.h | 5 +++--
> lib/memtypes.c| 65
> ---
> lib/plist.c | 8 +--
> lib/pqueue.c | 3 +++
> lib/prefix.c | 2 ++
> lib/privs.c | 3 +++
> lib/routemap.c| 7 ++
> lib/routemap.h| 4
> lib/sockunion.c | 2 ++
> lib/stream.c | 4
> lib/table.c | 3 +++
> lib/table.h | 3 +++
> lib/thread.c | 4
> lib/vector.c | 3 +++
> lib/vector.h | 3 +++
> lib/vrf.c | 4
> lib/vty.c | 4
> lib/workqueue.c | 4
> lib/workqueue.h | 3 +++
> lib/zclient.c | 2 ++
> zebra/zebra_vty.c | 1 +
> 35 files changed, 118 insertions(+), 70 deletions(-)
>
> diff --git a/lib/buffer.c b/lib/buffer.c
> index ee93101..1dfcdb4 100644
> --- a/lib/buffer.c
> +++ b/lib/buffer.c
> @@ -28,7 +28,8 @@
> #include "network.h"
> #include
>
> -
> +DEFINE_MTYPE_STATIC(LIB, BUFFER, "Buffer")
> +DEFINE_MTYPE_STATIC(LIB, BUFFER_DATA, "Buffer data")
>
> /* Buffer master. */
> struct buffer
> diff --git a/lib/command.c b/lib/command.c
> index 8089360..0cd2245 100644
> --- a/lib/command.c
> +++ b/lib/command.c
> @@ -33,6 +33,10 @@ Boston, MA 02111-1307, USA. */
> #include "command.h"
> #include "workqueue.h"
>
> +DEFINE_MTYPE( LIB, HOST, "Host config")
> +DEFINE_MTYPE( LIB, STRVEC, "String vector")
> +DEFINE_MTYPE_STATIC(LIB, CMD_TOKENS, "Command desc")
> +
> /* Command vector which includes some level of command lists. Normally
> each daemon maintains each own cmdvec. */
> vector cmdvec = NULL;
> diff --git a/lib/command.h b/lib/command.h
> index 6a20e23..16f1784 100644
> --- a/lib/command.h
> +++ b/lib/command.h
> @@ -26,6 +26,12 @@
> #include "vector.h"
> #include "vty.h"
> #include "lib/route_types.h"
> +#include "memory.h"
> +
> +DECLARE_MTYPE(HOST)
> +
> +/* for test-commands.c */
> +DECLARE_MTYPE(STRVEC)
>
> /* Host configuration variable */
> struct host
> diff --git a/lib/distribute.c b/lib/distribute.c
> index ba8043c..da6f68e 100644
> --- a/lib/distribute.c
> +++ b/lib/distribute.c
> @@ -28,6 +28,9 @@
> #include "distribute.h"
> #include "memory.h"
>
> +DEFINE_MTYPE_STATIC(LIB, DISTRIBUTE,"Distribute list")
> +DEFINE_MTYPE_STATIC(LIB, DISTRIBUTE_IFNAME, "Dist-list ifname")
> +
> /* Hash of distribute list. */
> struct hash *disthash;
>
> diff --git a/lib/filter.c b/lib/filter.c
> index a472941..7476d32 100644
> --- a/lib/filter.c
> +++ b/lib/filter.c
> @@ -29,6 +29,10 @@
> #include "buffer.h"
> #include "log.h"
>
> +DEFINE_MTYPE_STATIC(LIB, ACCESS_LIST, "Access List")
> +DEFINE_MTYPE_STATIC(LIB, ACCESS_LIST_STR, "Access List Str")
> +DEFINE_MTYPE_STATIC(LIB, ACCESS_FILTER, "Access Filter")
> +
> struct filter_cisco
> {
>/* Cisco access-list */
> diff --git a/lib/hash.c b/lib/hash.c
> index 56e41fa..570329a 100644
> --- a/lib/hash.c
> +++ b/lib/hash.c
> @@ -24,6 +24,10 @@
> #include "hash.h"
> #include "memory.h"
>
> +DEFINE_MTYPE( LIB, HASH,"Hash")
> +DEFINE_MTYPE( LIB, HASH_BACKET, "Hash Bucket")
> +DEFINE_MTYPE_STATIC(LIB, HASH_INDEX, "Hash Index")
> +
> /* Allocate a new hash. */
> struct hash *
> hash_create_size (unsigned int size, unsigned int (*hash_key) (void *),
> diff --git a/lib/hash.h b/lib/hash.h
> index 920c668..b640b71 100644
> --- a/lib/hash.h
> +++ b/lib/hash.h
> @@ -21,6 +21,11 @@ Boston, MA 02111-1307, USA. */
> #ifndef _ZEBRA_HASH_H
> #define _ZEBRA_HASH_H
>
> +#include "memory.h"
> +
> +DECLARE_MTYPE(HASH)
> +DECLARE_MTYPE(HASH_BACKET)
> +
> /* Default hash table size. */
> #define HASH_INITIAL_SIZE 256 /* initial number of backets. */
> #define HASH_THRESHOLD 10/* expand when backet. */
> diff --git a/lib/if.c b/lib/if.c
> index 44b8586..b6c8b61 100644
> --- a/lib/if.c
> +++ b/lib/if.c
> @@ -37,6 +37,10 @@
> #include "str.h"
> #include "log.h"
>
> +DEFINE_MTYPE( LIB, IF, "Interface")
> +DEFINE_MTYPE_STATIC(LIB, CONNECTED, "Connected")
> +DEFINE_MTYPE( LIB, CONNECTED_LABEL, "Connected interface label")
> +
> /* List of interfaces in only the default VRF */
> struct list *iflist;
>
> diff --git a/lib/if.h b/lib/if.h
> index b3d14ba..3832021 100644
> --- a/li
[quagga-dev 14764] Re: [PATCH 05/10] *: split & distribute memtypes
Acked-by: Donald Sharp On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter < equi...@opensourcerouting.org> wrote: > This is a rather large mechanical commit that splits up the memory types > defined in lib/memtypes.c and distributes them into *_memory.[ch] files > in the individual daemons. > > The zebra change is slightly annoying because there is no nice place to > put the #include "zebra_memory.h" statement. > > Signed-off-by: David Lamparter > --- > bgpd/Makefile.am | 2 + > bgpd/bgp_community.c | 1 + > bgpd/bgp_memory.c| 99 > bgpd/bgp_memory.h| 95 > bgpd/bgpd.h | 1 + > isisd/Makefile.am| 2 + > isisd/dict.c | 1 + > isisd/isis_memory.c | 45 +++ > isisd/isis_memory.h | 46 > isisd/isisd.h| 1 + > lib/memtypes.c | 190 > --- > ospf6d/Makefile.am | 2 + > ospf6d/ospf6_memory.c| 44 +++ > ospf6d/ospf6_memory.h| 45 +++ > ospf6d/ospf6d.h | 2 + > ospfd/Makefile.am| 6 +- > ospfd/ospf_memory.c | 54 ++ > ospfd/ospf_memory.h | 55 ++ > ospfd/ospfd.h| 2 + > pimd/Makefile.am | 2 + > pimd/pim_memory.c| 40 ++ > pimd/pim_memory.h| 41 ++ > pimd/pimd.h | 1 + > ripd/Makefile.am | 2 + > ripd/rip_memory.c| 35 + > ripd/rip_memory.h| 36 + > ripd/ripd.h | 2 + > ripngd/Makefile.am | 2 + > ripngd/ripng_memory.c| 35 + > ripngd/ripng_memory.h| 36 + > ripngd/ripngd.h | 2 + > vtysh/vtysh.h| 3 + > vtysh/vtysh_config.c | 4 + > zebra/Makefile.am| 6 +- > zebra/connected.c| 1 + > zebra/if_ioctl.c | 1 + > zebra/if_ioctl_solaris.c | 1 + > zebra/if_sysctl.c| 1 + > zebra/interface.c| 1 + > zebra/irdp_interface.c | 1 + > zebra/irdp_main.c| 1 + > zebra/irdp_packet.c | 1 + > zebra/kernel_socket.c| 1 + > zebra/main.c | 1 + > zebra/router-id.c| 1 + > zebra/rt_netlink.c | 1 + > zebra/rtadv.c| 1 + > zebra/rtread_sysctl.c| 1 + > zebra/test_main.c| 1 + > zebra/zebra_memory.c | 38 ++ > zebra/zebra_memory.h | 39 ++ > zebra/zebra_rib.c| 1 + > zebra/zebra_routemap.c | 1 + > zebra/zebra_vty.c| 1 + > zebra/zserv.c| 1 + > 55 files changed, 843 insertions(+), 193 deletions(-) > create mode 100644 bgpd/bgp_memory.c > create mode 100644 bgpd/bgp_memory.h > create mode 100644 isisd/isis_memory.c > create mode 100644 isisd/isis_memory.h > create mode 100644 ospf6d/ospf6_memory.c > create mode 100644 ospf6d/ospf6_memory.h > create mode 100644 ospfd/ospf_memory.c > create mode 100644 ospfd/ospf_memory.h > create mode 100644 pimd/pim_memory.c > create mode 100644 pimd/pim_memory.h > create mode 100644 ripd/rip_memory.c > create mode 100644 ripd/rip_memory.h > create mode 100644 ripngd/ripng_memory.c > create mode 100644 ripngd/ripng_memory.h > create mode 100644 zebra/zebra_memory.c > create mode 100644 zebra/zebra_memory.h > > diff --git a/bgpd/Makefile.am b/bgpd/Makefile.am > index d2775f3..608d018 100644 > --- a/bgpd/Makefile.am > +++ b/bgpd/Makefile.am > @@ -11,6 +11,7 @@ sbin_PROGRAMS = bgpd > bin_PROGRAMS = bgp_btoa > > libbgp_a_SOURCES = \ > + bgp_memory.c \ > bgpd.c bgp_fsm.c bgp_aspath.c bgp_community.c bgp_attr.c \ > bgp_debug.c bgp_route.c bgp_zebra.c bgp_open.c bgp_routemap.c \ > bgp_packet.c bgp_network.c bgp_filter.c bgp_regex.c bgp_clist.c \ > @@ -19,6 +20,7 @@ libbgp_a_SOURCES = \ > bgp_encap.c bgp_encap_tlv.c > > noinst_HEADERS = \ > + bgp_memory.h \ > bgp_aspath.h bgp_attr.h bgp_community.h bgp_debug.h bgp_fsm.h \ > bgp_network.h bgp_open.h bgp_packet.h bgp_regex.h bgp_route.h \ > bgpd.h bgp_filter.h bgp_clist.h bgp_dump.h bgp_zebra.h \ > diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c > index f1997bd..0d272c4 100644 > --- a/bgpd/bgp_community.c > +++ b/bgpd/bgp_community.c > @@ -23,6 +23,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, > Boston, MA > #include "hash.h" > #include "memory.h" > > +#include "bgpd/bgp_memory.h" > #include "bgpd/bgp_community.h" > > /* Hash of community attribute. */ > diff --git a/bgpd/bgp_memory.c b/bgpd/bgp_memory.c > new file mode 100644 > index 000..1a0b2c3 > --- /dev/null > +++ b/bgpd/bgp_memory.c > @@ -0,0 +1,99 @@ > +/* bgpd memory type definitions > + * > + * Copyright (C) 2015 David Lamparter > + * > + * This file is part of Quagga. > + * > + * Quagga is free software; you can redistribute it and/or modify it > + * under the terms of the GN
[quagga-dev 14765] Re: [PATCH 06/10] *: stop (re|ab)using lib/ MTYPEs
Acked-by: Donald Sharp
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> bgpd, ospf6d, isisd and some tests were reusing MTYPEs defined in the
> library for its own use. This is bad practice and will break with the
> later commit making the library's MTYPEs static.
>
> Signed-off-by: David Lamparter
> ---
> bgpd/bgp_nexthop.c | 8 +---
> isisd/isis_memory.c | 2 ++
> isisd/isis_memory.h | 2 ++
> isisd/isis_redist.c | 13 +++--
> ospf6d/ospf6_interface.c | 10 ++
> tests/heavy-wq.c | 14 +-
> tests/test-memory.c | 49
> +---
> 7 files changed, 57 insertions(+), 41 deletions(-)
>
> diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
> index 4375be9..158f1df 100644
> --- a/bgpd/bgp_nexthop.c
> +++ b/bgpd/bgp_nexthop.c
> @@ -42,6 +42,8 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA
> #include "zebra/rib.h"
> #include "zebra/zserv.h" /* For ZEBRA_SERV_PATH. */
>
> +DEFINE_MTYPE_STATIC(BGPD, BGP_NEXTHOP, "BGP nexthop")
> +
> struct bgp_nexthop_cache *zlookup_query (struct in_addr);
> struct bgp_nexthop_cache *zlookup_query_ipv6 (struct in6_addr *);
>
> @@ -92,7 +94,7 @@ bnc_nexthop_free (struct bgp_nexthop_cache *bnc)
>for (nexthop = bnc->nexthop; nexthop; nexthop = next)
> {
>next = nexthop->next;
> - XFREE (MTYPE_NEXTHOP, nexthop);
> + XFREE (MTYPE_BGP_NEXTHOP, nexthop);
> }
> }
>
> @@ -819,7 +821,7 @@ zlookup_read (void)
>
>for (i = 0; i < nexthop_num; i++)
> {
> - nexthop = XCALLOC (MTYPE_NEXTHOP, sizeof (struct nexthop));
> + nexthop = XCALLOC (MTYPE_BGP_NEXTHOP, sizeof (struct nexthop));
> nexthop->type = stream_getc (s);
> switch (nexthop->type)
> {
> @@ -922,7 +924,7 @@ zlookup_read_ipv6 (void)
>
>for (i = 0; i < nexthop_num; i++)
> {
> - nexthop = XCALLOC (MTYPE_NEXTHOP, sizeof (struct nexthop));
> + nexthop = XCALLOC (MTYPE_BGP_NEXTHOP, sizeof (struct nexthop));
> nexthop->type = stream_getc (s);
> switch (nexthop->type)
> {
> diff --git a/isisd/isis_memory.c b/isisd/isis_memory.c
> index 9bc506e..5c5fee7 100644
> --- a/isisd/isis_memory.c
> +++ b/isisd/isis_memory.c
> @@ -43,3 +43,5 @@ DEFINE_MTYPE(ISISD, ISIS_NEXTHOP, "ISIS nexthop")
> DEFINE_MTYPE(ISISD, ISIS_NEXTHOP6, "ISIS nexthop6")
> DEFINE_MTYPE(ISISD, ISIS_DICT, "ISIS dictionary")
> DEFINE_MTYPE(ISISD, ISIS_DICT_NODE, "ISIS dictionary node")
> +DEFINE_MTYPE(ISISD, ISIS_EXT_ROUTE, "ISIS redistributed route")
> +DEFINE_MTYPE(ISISD, ISIS_EXT_INFO, "ISIS redistributed route info")
> diff --git a/isisd/isis_memory.h b/isisd/isis_memory.h
> index 40bb88a..6ebce1f 100644
> --- a/isisd/isis_memory.h
> +++ b/isisd/isis_memory.h
> @@ -42,5 +42,7 @@ DECLARE_MTYPE(ISIS_NEXTHOP)
> DECLARE_MTYPE(ISIS_NEXTHOP6)
> DECLARE_MTYPE(ISIS_DICT)
> DECLARE_MTYPE(ISIS_DICT_NODE)
> +DECLARE_MTYPE(ISIS_EXT_ROUTE)
> +DECLARE_MTYPE(ISIS_EXT_INFO)
>
> #endif /* _QUAGGA_ISIS_MEMORY_H */
> diff --git a/isisd/isis_redist.c b/isisd/isis_redist.c
> index 4a84d51..dc82678 100644
> --- a/isisd/isis_redist.c
> +++ b/isisd/isis_redist.c
> @@ -24,6 +24,7 @@
> #include "if.h"
> #include "linklist.h"
> #include "memory.h"
> +#include "isis_memory.h"
> #include "prefix.h"
> #include "routemap.h"
> #include "stream.h"
> @@ -95,7 +96,7 @@ isis_redist_route_node_create(route_table_delegate_t
> *delegate,
>struct route_table *table)
> {
>struct route_node *node;
> - node = XCALLOC(MTYPE_ROUTE_NODE, sizeof(*node));
> + node = XCALLOC(MTYPE_ISIS_EXT_ROUTE, sizeof(*node));
>return node;
> }
>
> @@ -105,8 +106,8 @@ isis_redist_route_node_destroy(route_table_delegate_t
> *delegate,
> struct route_node *node)
> {
>if (node->info)
> -XFREE(MTYPE_ISIS, node->info);
> - XFREE (MTYPE_ROUTE_NODE, node);
> +XFREE(MTYPE_ISIS_EXT_INFO, node->info);
> + XFREE (MTYPE_ISIS_EXT_ROUTE, node);
> }
>
> static route_table_delegate_t isis_redist_rt_delegate = {
> @@ -143,7 +144,7 @@ isis_redist_install(struct isis_area *area, int level,
> }
>else
> {
> - er_node->info = XMALLOC(MTYPE_ISIS, sizeof(*info));
> + er_node->info = XMALLOC(MTYPE_ISIS_EXT_INFO, sizeof(*info));
> }
>
>memcpy(er_node->info, info, sizeof(*info));
> @@ -242,7 +243,7 @@ isis_redist_ensure_default(struct isis *isis, int
> family)
>return;
> }
>
> - ei_node->info = XCALLOC(MTYPE_ISIS, sizeof(struct isis_ext_info));
> + ei_node->info = XCALLOC(MTYPE_ISIS_EXT_INFO, sizeof(struct
> isis_ext_info));
>
>info = ei_node->info;
>info->origin = DEFAULT_ROUTE;
> @@ -280,7 +281,7 @@ isis_redist_add(int type, struct prefix *p, u_char
> distance, uint32_t metric)
>if (ei_node->info)
> route_unlock_node(e
[quagga-dev 14763] Re: [PATCH 04/10] lib: migrate to new memory-type handling
I'm not a big fan of #if 0 or #if 1's introduced with this patch. Is there
someway we can mitigate them?
thanks!
donald
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> Move over to the new allocation counting added in the previous commit.
>
> (This commit is mostly mechanical.)
>
> Signed-off-by: David Lamparter
> ---
> bgpd/bgp_main.c | 1 +
> bgpd/bgp_vty.c | 1 +
> isisd/isis_main.c | 1 +
> isisd/isis_redist.c | 1 -
> lib/Makefile.am | 6 +-
> lib/memory.h| 4 +-
> lib/memory_vty.c| 268 +-
> lib/memory_vty.h| 64 +-
> lib/memtypes.awk| 87 ---
> lib/memtypes.c | 542
> +---
> lib/memtypes.pl | 6 +
> ospf6d/ospf6_main.c | 1 +
> ospfclient/ospf_apiclient.c | 2 +
> ospfclient/ospf_apiclient.h | 2 -
> ospfd/ospf_main.c | 1 +
> ospfd/ospf_opaque.c | 9 +-
> ospfd/ospf_te.c | 5 +-
> pimd/pim_main.c | 1 +
> ripd/rip_main.c | 1 +
> ripngd/ripng_main.c | 1 +
> tests/common-cli.c | 1 +
> tests/main.c| 1 +
> tests/test-buffer.c | 1 +
> tests/test-privs.c | 1 +
> vtysh/vtysh_main.c | 1 +
> zebra/main.c| 1 +
> zebra/test_main.c | 1 +
> 27 files changed, 305 insertions(+), 706 deletions(-)
> delete mode 100644 lib/memtypes.awk
> create mode 100755 lib/memtypes.pl
>
> diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
> index 11c73ce..25669a0 100644
> --- a/bgpd/bgp_main.c
> +++ b/bgpd/bgp_main.c
> @@ -27,6 +27,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA
> #include "thread.h"
> #include
> #include "memory.h"
> +#include "memory_vty.h"
> #include "prefix.h"
> #include "log.h"
> #include "privs.h"
> diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
> index 6db3dcb..a2d72ff 100644
> --- a/bgpd/bgp_vty.c
> +++ b/bgpd/bgp_vty.c
> @@ -29,6 +29,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA
> #include "thread.h"
> #include "log.h"
> #include "memory.h"
> +#include "memory_vty.h"
> #include "hash.h"
> #include "filter.h"
>
> diff --git a/isisd/isis_main.c b/isisd/isis_main.c
> index 3a73087..1c3323e 100644
> --- a/isisd/isis_main.c
> +++ b/isisd/isis_main.c
> @@ -29,6 +29,7 @@
> #include "command.h"
> #include "vty.h"
> #include "memory.h"
> +#include "memory_vty.h"
> #include "stream.h"
> #include "if.h"
> #include "privs.h"
> diff --git a/isisd/isis_redist.c b/isisd/isis_redist.c
> index 552613a..4a84d51 100644
> --- a/isisd/isis_redist.c
> +++ b/isisd/isis_redist.c
> @@ -24,7 +24,6 @@
> #include "if.h"
> #include "linklist.h"
> #include "memory.h"
> -#include "memtypes.h"
> #include "prefix.h"
> #include "routemap.h"
> #include "stream.h"
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index f77b5a7..7456cc6 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -39,12 +39,12 @@ noinst_HEADERS = \
> EXTRA_DIST = \
> regex.c regex-gnu.h \
> queue.h \
> - memtypes.awk \
> + memtypes.pl \
> route_types.pl route_types.txt \
> gitversion.pl
>
> -memtypes.h: $(srcdir)/memtypes.c $(srcdir)/memtypes.awk
> - ($(GAWK) -f $(srcdir)/memtypes.awk $(srcdir)/memtypes.c > $@)
> +memtypes.h: $(srcdir)/memtypes.c $(srcdir)/memtypes.pl
> + @PERL@ $(srcdir)/memtypes.pl < $(srcdir)/memtypes.c > $@
>
> route_types.h: $(srcdir)/route_types.txt $(srcdir)/route_types.pl
> @PERL@ $(srcdir)/route_types.pl < $(srcdir)/route_types.txt > $@
> diff --git a/lib/memory.h b/lib/memory.h
> index 1ef411f..56af4a4 100644
> --- a/lib/memory.h
> +++ b/lib/memory.h
> @@ -147,7 +147,7 @@ extern void *qstrdup (struct memtype *mt, const char
> *str)
> __attribute__ ((malloc, nonnull (1) _RET_NONNULL));
> extern void qfree (struct memtype *mt, void *ptr);
>
> -#if 0
> +#if 1
> #define XMALLOC(mtype, size) qmalloc(mtype, size)
> #define XCALLOC(mtype, size) qcalloc(mtype, size)
> #define XREALLOC(mtype, ptr, size) qrealloc(mtype, ptr, size)
> @@ -198,4 +198,6 @@ extern int qmem_walk (qmem_walk_fn *func, void *arg);
>
> extern void memory_oom (size_t size, const char *name);
>
> +#include "memtypes.h"
> +
> #endif /* _QUAGGA_MEMORY_H */
> diff --git a/lib/memory_vty.c b/lib/memory_vty.c
> index 5e0b8af..e1c08ce 100644
> --- a/lib/memory_vty.c
> +++ b/lib/memory_vty.c
> @@ -28,265 +28,17 @@
>
> #include "log.h"
> #include "memory.h"
> -
> -static void alloc_inc (int);
> -static void alloc_dec (int);
> -static void log_memstats(int log_priority);
> -
> -static const struct message mstr [] =
> -{
> - { MTYPE_THREAD, "thread" },
> - { MTYPE_THREAD_MASTER, "thread_master" },
> - { MTYPE_VECTOR, "vector" },
> - { MTYPE_VECTOR_IND
[quagga-dev 14762] Re: [PATCH 03/10] lib: add new extensible memory-type handling
I think we are in agreement then. Acked-by: Donald Sharp wrote: > [btw, my previous response didn't make the loop back to me, is that just > me or is something stuck on the mailing list?] > > On Wed, Feb 24, 2016 at 04:26:58PM +, Paul Jakma wrote: > > Also, does this depend on using dynamic linking? We do have people who > > like to build Quagga with --disable-shared at least (though, that still > > results in an ELF binary dynamically linked). > > It's no problem, constructors work on statically linked binaries too. > > > > No -- moving memtypes into the files they're used in was also a major > > > design goal, thus the existence of DEFINE_MTYPE_STATIC(). A table does > > > not allow for that. > > > > You could have file specific groups. > > Looking at the changes in lib/, the average is 2.39 MTYPEs per file > (only counting files that have MTYPE definitions). There are 33 such > files. Every single file would not only need a group but also an > initialisation function that is called from somewhere, without using > constructors I guess. > > This sounds very finicky to me, but if you wanna cook up a patch I'm > happy to review it... > > > > Considering that the linker functionality for constructors is > > > neccessarily present on any architecture that supports C++ (this is > > > how global variable classes get their constructors called), > > > > Right. Not everything supports C++ though. Not that Quagga.net supports > > such platforms though. C++ is harder to bring up on weird platforms than > > C though. > > > > The thing I have on my mind is embedding Quagga into non-standard, > > non-Unix, relatively minimal runtimes, that might have their own > > compiler or else do not have special support from GCC for their exe > > format. More and more of these runtimes are springing into existence, > > thanks to IoT and ultra-minimal "virtualisation" efforts. > > > > I used to sit beside someone who worked on that stuff, and they used to > > have fun porting C code over. > > I don't see how this is relevant to the discussion on this patch set? > > If someone wants to start porting Quagga to IoT devices, they have much > bigger problems than this patchset... heck, it's problematic to even > get Quagga onto a low-flash (4 MB or 8 MB) OpenWRT device, and that's > still Linux+gcc. > > Btw, 8-bit AVR supports gcc and constructors. It's one of the > architectures that no-prologue-chains them. Same for Cortex-M ARMs, > it's part of the linker magic, you don't even need an RTOS. > > > > Also, I have put considerable thought into making this as hard as > > > possible to (accidentally) misuse, removing for example the > > > possibility to forget initialisation calls. If someone forgets the > > > DECLARE_MTYPE (or DEFINE_MTYPE_STATIC), they get a compiler error. > > > If someone forgets the DEFINE_MTYPE, they get a linker error. Pass > > > NULL as type and you get a warning from the "nonnull" attribute. I'm > > > not saying it's impossible to misuse but it needs a special kind of > > > stupid ;) > > > > Sure. > > > > It's cute. > > I must strongly object to the use of the diminutive word "cute" in this > context. > > We live in an age of ubiquitous security vulnerabilities in all kinds of > code. A lot of them are caused by improperly used interfaces, which in > a lot of cases again is caused by a lack of safety features in interface > (or language - yay C!) designs. > > Proper UX design, in this case from one developer providing an API to > another developer, is a critical matter that must be valued > appropriately. Even arguing that the memtypes API is probably not > security critical is fallacious -- proper UX design needs to be > pervasive, and to put it to the ridiculous extreme, even "just" a > crashing Quagga instance at some ISP can cost the life of a person > trying to make a VoIP 911 call. (Urgh, I hope not.) > > Using the word "cute" diminishes the value of the work I have done to > produce an interface that pushes ease of use, difficulty to misuse and > maintainability. It's an improvement in usability, and that's all it > needs to be to have considerable impact and importance. > > The fact that the first patch in the series goes and fixes existing > misuse of the old API illustrates this well enough. > > > -David > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14761] Mailing list snafu?
Apologies if you received (or will receive) duplicates of my messages. Something seems to be wrong with the mailing list; it lost 3 of my messages today... can somebody investigate? Message-ID: <20160224090742.GA873456@eidolon> Message-ID: <20160224120905.GB873456@eidolon> Message-ID: <20160224131806.GC873456@eidolon> It's not a Google problem though, my MTA claims to have delivered the original mails, like this one: 2016-02-24 14:18:26 1aYZKU-001xna-Fu => quagga-dev@lists.quagga.net R=quagga_direct T=remote_smtp H=master.quagga.net [2001:41c8:51:21b::10] X=TLSv1.2:AES256-GCM-SHA384:256 CV=no C="250 OK id=1aYZKk-00029k-4U" (timestamp is +01:00, note "250 OK" from master.quagga.net) (replies off-list pls, this is only on-list to apologize for possible duplicates.) -David ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14760] Re: CI Testresult: FAILED (Re: [quagga-dev, 14741, 10/10] build: goodbye, gawk)
On Wed, Feb 24, 2016 at 02:53:19AM -0800, Martin Winter wrote:
> On 24 Feb 2016, at 1:07, David Lamparter wrote:
>
> > On Wed, Feb 24, 2016 at 01:00:03AM -0800, cisys...@netdef.org wrote:
> >> Continous Integration Result: FAILED
> > [...]
> >> Tested on top of Git : eae18d1 (as of 20151209.135437 UTC)
> >
> > As mentioned in the original mail, this patchset applies on top of
> > "lib: fix vrf_bitmap leak in zclient_free()", not master. The patchset
> > has passed a manual run (though I did some style polishing afterwards, I
> > hope I didn't accidentally break anything there.)
>
> Sorry, my CI system doesn’t speak english.
>
> Resubmitted to be on top of the specified Git
>
> But patching seemed to fail. (01/10 worked, 02/10 failed)
> See CI email.
-- CI email:
# Tested on top of Git : f739363 (as of 20160218.155415 UTC)
# CI System Testrun URL: https://ci1.netdef.org/browse/QUAGGA-QPWORK-243/
commit f739363231b45d1085b0f243a43a846b465c4f94
Author: David Lamparter
AuthorDate: Wed Dec 16 19:38:23 2015 +0100
Commit: Paul Jakma
CommitDate: Thu Feb 18 15:54:15 2016 +
lib: fix vrf_bitmap leak in zclient_free()
-- My base commit was:
commit f48d5b99571d7ed956d4767047d07a245bec05c1
Author: David Lamparter
AuthorDate: Wed Dec 16 19:38:23 2015 +0100
Commit: Donald Sharp
CommitDate: Wed Feb 10 19:48:02 2016 -0500
lib: fix vrf_bitmap leak in zclient_free()
> Let me guess: The rebase changed things?
Well... see below. Not even gonna try and guess why these changes had
to be secretly snuck into a history rewrite...
$ git diff f739363..f48d5b9
diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 692d361..deb24a1 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -40,7 +40,6 @@ decode_rd_type (u_char *pnt)
v = ((u_int16_t) *pnt++ << 8);
v |= (u_int16_t) *pnt;
-
return v;
}
@@ -93,8 +92,8 @@ decode_rd_ip (u_char *pnt, struct rd_ip *rd_ip)
}
int
-bgp_nlri_parse_vpn (afi_t afi, struct peer *peer, struct attr *attr,
-struct bgp_nlri *packet, int withdraw)
+bgp_nlri_parse_vpn (struct peer *peer, struct attr *attr,
+struct bgp_nlri *packet)
{
u_char *pnt;
u_char *lim;
@@ -125,13 +124,7 @@ bgp_nlri_parse_vpn (afi_t afi, struct peer *peer, struct
attr *attr,
/* Fetch prefix length. */
prefixlen = *pnt++;
- p.family = afi2family(afi);
- if (p.family == 0)
-{
- /* bad afi, shouldn't happen */
- zlog_warn("%s: bad afi %d, dropping incoming route", __func__, afi);
- continue;
-}
+ p.family = AF_INET;
psize = PSIZE (prefixlen);
if (prefixlen < 88)
@@ -174,16 +167,12 @@ bgp_nlri_parse_vpn (afi_t afi, struct peer *peer, struct
attr *attr,
if (pnt + psize > lim)
return -1;
- if (!withdraw)
-{
- bgp_update (peer, &p, attr, afi, SAFI_MPLS_VPN,
- ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt, 0);
-}
+ if (attr)
+bgp_update (peer, &p, attr, packet->afi, SAFI_MPLS_VPN,
+ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt, 0);
else
-{
- bgp_withdraw (peer, &p, attr, afi, SAFI_MPLS_VPN,
-ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt);
-}
+bgp_withdraw (peer, &p, attr, packet->afi, SAFI_MPLS_VPN,
+ ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt);
}
/* Packet length consistency check. */
diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index b2a286c..3299b9c 100644
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -42,8 +42,7 @@ struct rd_ip
};
extern void bgp_mplsvpn_init (void);
-extern int bgp_nlri_parse_vpn (afi_t, struct peer *, struct attr *,
- struct bgp_nlri *, int withdraw);
+extern int bgp_nlri_parse_vpn (struct peer *, struct attr *, struct bgp_nlri
*);
extern u_int32_t decode_label (u_char *);
extern int str2prefix_rd (const char *, struct prefix_rd *);
extern int str2tag (const char *, u_char *);
diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
index 4375be9..2406814 100644
--- a/bgpd/bgp_nexthop.c
+++ b/bgpd/bgp_nexthop.c
@@ -488,7 +488,7 @@ bgp_scan (afi_t afi, safi_t safi)
}
}
if (rn->info)
- bgp_process (bgp, rn, afi, SAFI_UNICAST);
+bgp_process (bgp, rn, afi, SAFI_UNICAST);
}
/* Flash old cache. */
diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index ca38150..7b8b657 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -440,7 +440,7 @@ static const size_t cap_minsizes[] =
[CAPABILITY_CODE_MP] = CAPABILITY_CODE_MP_LEN,
[CAPABILITY_CODE_REFRESH]= CAPABILITY_CODE_REFRESH_LEN,
[CAPABILITY_CODE_ORF]= CAPABILITY_CODE_ORF_LEN,
- [CAPABILITY_CODE_RESTART]= 6,
+ [CAPABILITY_CODE_RESTART]= CAPABILITY_CODE_RESTART_LEN,
[CAPABILITY_CODE_AS4]= CAPABILITY_CODE_AS4_
[quagga-dev 14759] Re: [PATCH 03/10] lib: add new extensible memory-type handling
On Wed, Feb 24, 2016 at 11:58:42AM +, Paul Jakma wrote:
> - This seems to require various extensions to work, some seem to be GCC
>specific?
It's specific to the "GCC-like group". I have tested on:
Linux clang 3.6.1
Linux icc 14.0.3
Linux gcc 4.9.3
Linux gcc 4.7.4
Linux gcc 4.2.4
Linux gcc 3.4.6 (no, that's not a joke. I couldn't install 2.95.3 btw.)
The CI system has tested on Linux, FreeBSD 10 + 9, OpenBSD 5.8, NetBSD 7
and OmniOS (gcc). The OmniOS test actually discovered the issue where
Solaris doesn't support constructor priorities, that was fixed.
("Solaris doesn't do constructor priorities due to linker restrictions")
I have no Apple OSX system to test on. If someone could do that, that
would be appreciated. (OSX is listed as "work needed" anyway, though.)
It probably doesn't work on Visual Studio ;)
>Are there people using other compilers? (I used to regularly build
>with the Sun One Studio compiler, but I don't anymore; only clang
>and gcc).
The Sun One Studio compiler is not a supported compiler per the list in
doc/overview.texi. As far as I know, that stopped working a long time
ago. I looked at it when we set up the OmniOS CI and vaguely remember
it was infeasible to make the shipped compiler work. Since gcc is
readily available for Solaris, I don't see this as an issue.
> - The __builtin_types_{expect,compatible_p} stuff, would C11 _Generic
>allow the same to be achieved?
The __builtin_types_* stuff is actually in a commented-out section that
I forgot to remove. It was there for temporary compatibility between
the old and new schemes, so that the code works correctly even at every
single step in the middle.
> - Could the manipulation of the linked list global pointers (the
>addition of a group onto the global groups list, then additions of
>memtypes in a group onto the group list) be done behind a centralised
>function in lib/memory and go through that, rather than twiddled
>directly in each mtype specific constructor function?
Calling other functions from constructors is generally a bit of a bad
idea as the C runtime has not neccessarily [it should, but...] fully
initialized -- and any Quagga functions quite certainly won't have been
initialized. Having the code in a centralised function would make
neccessary a big fat warning on top of it so that no one accidentally
calls zlog_* or something similar. Having it right there in the header
with quite a bit of "magic" around it makes it very obvious that the
code is special.
It also provides no real gain as the struct layout is embedded into the
executable either way, i.e. there's no abstraction gain.
Lastly, on some architectures constructors are chained together as pure
function bodies without prologue and epilogue, i.e. they just run in
sequence. Moving bits to a function would nicely contravene this
"optimization" (which, honestly, I don't think makes much of a
difference, but whatever.)
> - Compiler and linker specific extensions to arrange to declare static
>stuff on by one and have a function called for each.
>
>The key thing here really is just to not have to update the library,
>right?
No -- moving memtypes into the files they're used in was also a major
design goal, thus the existence of DEFINE_MTYPE_STATIC(). A table does
not allow for that.
An alternate approach would be to use data section attributes, this
however depends on *linker* behaviour more than I'd be willing to rely
on.
[cut]
>I.e., I'm just wondering if non-standard construction extensions are
>enough of a win,
Considering that the linker functionality for constructors is
neccessarily present on any architecture that supports C++ (this is how
global variable classes get their constructors called), this boils down
to only compiler extensions. These seem to be a non-problem to the
point where I won't even call it non-standard anymore. Can you find a
compiler/environment that does NOT work (but worked before)?
>given they only eliminiate a couple of initialisation calls?
Localizing things to their point of use is in my opinion a significant
gain for both maintainability and usability; I guess the "couple of
initialisation calls" you mention refers to your suggested table-based
solution which does not provide that. So, this is apples vs. oranges.
Also, I have put considerable thought into making this as hard as
possible to (accidentally) misuse, removing for example the possibility
to forget initialisation calls. If someone forgets the DECLARE_MTYPE
(or DEFINE_MTYPE_STATIC), they get a compiler error. If someone forgets
the DEFINE_MTYPE, they get a linker error. Pass NULL as type and you
get a warning from the "nonnull" attribute. I'm not saying it's
impossible to misuse but it needs a special kind of stupid ;)
> - The formatting should follow the standard style in Quagga,
>particularly anything intended for core lib/ bits.
Sure, I can go over it again, I thought I f
[quagga-dev 14758] Re: CI Testresult: FAILED (Re: [quagga-dev, 14741, 10/10] build: goodbye, gawk)
On Wed, Feb 24, 2016 at 01:00:03AM -0800, cisys...@netdef.org wrote: > Continous Integration Result: FAILED [...] > Tested on top of Git : eae18d1 (as of 20151209.135437 UTC) As mentioned in the original mail, this patchset applies on top of "lib: fix vrf_bitmap leak in zclient_free()", not master. The patchset has passed a manual run (though I did some style polishing afterwards, I hope I didn't accidentally break anything there.) -David ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14757] Re: [PATCH 03/10] lib: add new extensible memory-type handling
[btw, my previous response didn't make the loop back to me, is that just me or is something stuck on the mailing list?] On Wed, Feb 24, 2016 at 04:26:58PM +, Paul Jakma wrote: > Also, does this depend on using dynamic linking? We do have people who > like to build Quagga with --disable-shared at least (though, that still > results in an ELF binary dynamically linked). It's no problem, constructors work on statically linked binaries too. > > No -- moving memtypes into the files they're used in was also a major > > design goal, thus the existence of DEFINE_MTYPE_STATIC(). A table does > > not allow for that. > > You could have file specific groups. Looking at the changes in lib/, the average is 2.39 MTYPEs per file (only counting files that have MTYPE definitions). There are 33 such files. Every single file would not only need a group but also an initialisation function that is called from somewhere, without using constructors I guess. This sounds very finicky to me, but if you wanna cook up a patch I'm happy to review it... > > Considering that the linker functionality for constructors is > > neccessarily present on any architecture that supports C++ (this is > > how global variable classes get their constructors called), > > Right. Not everything supports C++ though. Not that Quagga.net supports > such platforms though. C++ is harder to bring up on weird platforms than > C though. > > The thing I have on my mind is embedding Quagga into non-standard, > non-Unix, relatively minimal runtimes, that might have their own > compiler or else do not have special support from GCC for their exe > format. More and more of these runtimes are springing into existence, > thanks to IoT and ultra-minimal "virtualisation" efforts. > > I used to sit beside someone who worked on that stuff, and they used to > have fun porting C code over. I don't see how this is relevant to the discussion on this patch set? If someone wants to start porting Quagga to IoT devices, they have much bigger problems than this patchset... heck, it's problematic to even get Quagga onto a low-flash (4 MB or 8 MB) OpenWRT device, and that's still Linux+gcc. Btw, 8-bit AVR supports gcc and constructors. It's one of the architectures that no-prologue-chains them. Same for Cortex-M ARMs, it's part of the linker magic, you don't even need an RTOS. > > Also, I have put considerable thought into making this as hard as > > possible to (accidentally) misuse, removing for example the > > possibility to forget initialisation calls. If someone forgets the > > DECLARE_MTYPE (or DEFINE_MTYPE_STATIC), they get a compiler error. > > If someone forgets the DEFINE_MTYPE, they get a linker error. Pass > > NULL as type and you get a warning from the "nonnull" attribute. I'm > > not saying it's impossible to misuse but it needs a special kind of > > stupid ;) > > Sure. > > It's cute. I must strongly object to the use of the diminutive word "cute" in this context. We live in an age of ubiquitous security vulnerabilities in all kinds of code. A lot of them are caused by improperly used interfaces, which in a lot of cases again is caused by a lack of safety features in interface (or language - yay C!) designs. Proper UX design, in this case from one developer providing an API to another developer, is a critical matter that must be valued appropriately. Even arguing that the memtypes API is probably not security critical is fallacious -- proper UX design needs to be pervasive, and to put it to the ridiculous extreme, even "just" a crashing Quagga instance at some ISP can cost the life of a person trying to make a VoIP 911 call. (Urgh, I hope not.) Using the word "cute" diminishes the value of the work I have done to produce an interface that pushes ease of use, difficulty to misuse and maintainability. It's an improvement in usability, and that's all it needs to be to have considerable impact and importance. The fact that the first patch in the series goes and fixes existing misuse of the old API illustrates this well enough. -David ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14756] Re: [PATCH 03/10] lib: add new extensible memory-type handling
On Wed, 24 Feb 2016, David Lamparter wrote: The Sun One Studio compiler is not a supported compiler per the list in doc/overview.texi. As far as I know, that stopped working a long time ago. I looked at it when we set up the OmniOS CI and vaguely remember it was infeasible to make the shipped compiler work. Since gcc is readily available for Solaris, I don't see this as an issue. Well, indeed. I'm the last person I know of who regularly did non-GGC (or GCC-following) builds (and that one had '#pragma(init_func)', which GCC supports too, at least on that platform). I could go with going GCC specific. Be good to try flush out anyone who might be affected by that and give them a chance to object though. Also, does this depend on using dynamic linking? We do have people who like to build Quagga with --disable-shared at least (though, that still results in an ELF binary dynamically linked). Calling other functions from constructors is generally a bit of a bad idea as the C runtime has not neccessarily [it should, but...] fully initialized -- and any Quagga functions quite certainly won't have been initialized. Having the code in a centralised function would make neccessary a big fat warning on top of it so that no one accidentally calls zlog_* or something similar. Having it right there in the header with quite a bit of "magic" around it makes it very obvious that the code is special. A function can be documented special too. A function entry point in the library just seems a little cleaner than twiddling the list head directly, but... gut feeling more than anything. That said, on ordering, the ELF stuff that provides DT_INIT also provides a way of specifying dependencies via DT_NEEDED. Though, if that level of twiddling was needed I'd definitely be arguing to stick to idiomatic, standard C. :) Lastly, on some architectures constructors are chained together as pure function bodies without prologue and epilogue, i.e. they just run in sequence. That's... horrible. The key thing here really is just to not have to update the library, right? No -- moving memtypes into the files they're used in was also a major design goal, thus the existence of DEFINE_MTYPE_STATIC(). A table does not allow for that. You could have file specific groups. Considering that the linker functionality for constructors is neccessarily present on any architecture that supports C++ (this is how global variable classes get their constructors called), Right. Not everything supports C++ though. Not that Quagga.net supports such platforms though. C++ is harder to bring up on weird platforms than C though. The thing I have on my mind is embedding Quagga into non-standard, non-Unix, relatively minimal runtimes, that might have their own compiler or else do not have special support from GCC for their exe format. More and more of these runtimes are springing into existence, thanks to IoT and ultra-minimal "virtualisation" efforts. I used to sit beside someone who worked on that stuff, and they used to have fun porting C code over. Can you find a compiler/environment that does NOT work (but worked before)? For what we support, I don't know of any. I'm just wondering how necessary it is to deviate from C and add more runtime feature dependencies. If this can be done in C, even if a bit more verbosely, I'd kind of tend to that. If it can't be done in C, then the issue for me is that it will be harder (again) to embed Quagga into other things. Note, the table approach can still have per-file scope I think, by having a group per file (and some way of making the group implicit to the X... macros, if they're to be used as is). Also, I have put considerable thought into making this as hard as possible to (accidentally) misuse, removing for example the possibility to forget initialisation calls. If someone forgets the DECLARE_MTYPE (or DEFINE_MTYPE_STATIC), they get a compiler error. If someone forgets the DEFINE_MTYPE, they get a linker error. Pass NULL as type and you get a warning from the "nonnull" attribute. I'm not saying it's impossible to misuse but it needs a special kind of stupid ;) Sure. It's cute. regards, -- Paul Jakma p...@jakma.org @pjakma Key ID: 64A2FF6A Fortune: You can fool some of the people some of the time, and some of the people all of the time, and that is sufficient. ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14755] Re: [PATCH 02/10] lib: move memory.[ch] out of the way
Acked-by: Donald Sharp
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> The following commit will recreate memory.[ch].
>
> Signed-off-by: David Lamparter
> ---
> lib/Makefile.am | 8 +-
> lib/memory.c | 486
> ---
> lib/memory.h | 98 +--
> lib/memory_vty.c | 486
> +++
> lib/memory_vty.h | 96 +++
> 5 files changed, 589 insertions(+), 585 deletions(-)
> delete mode 100644 lib/memory.c
> create mode 100644 lib/memory_vty.c
> create mode 100644 lib/memory_vty.h
>
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 6b107f4..2093f30 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -10,10 +10,11 @@ libzebra_la_LDFLAGS = -version-info 0:0:0
> libzebra_la_SOURCES = \
> network.c pid_output.c getopt.c getopt1.c daemon.c \
> checksum.c vector.c linklist.c vty.c command.c \
> - sockunion.c prefix.c thread.c if.c memory.c buffer.c table.c
> hash.c \
> + sockunion.c prefix.c thread.c if.c buffer.c table.c hash.c \
> filter.c routemap.c distribute.c stream.c str.c log.c plist.c \
> zclient.c sockopt.c smux.c agentx.c snmp.c md5.c if_rmap.c
> keychain.c privs.c \
> - sigevent.c pqueue.c jhash.c memtypes.c workqueue.c vrf.c
> + sigevent.c pqueue.c jhash.c memtypes.c workqueue.c vrf.c \
> + memory_vty.c
>
> BUILT_SOURCES = memtypes.h route_types.h gitversion.h
>
> @@ -28,7 +29,8 @@ pkginclude_HEADERS = \
> str.h stream.h table.h thread.h vector.h version.h vty.h zebra.h \
> plist.h zclient.h sockopt.h smux.h md5.h if_rmap.h keychain.h \
> privs.h sigevent.h pqueue.h jhash.h zassert.h memtypes.h \
> - workqueue.h route_types.h libospf.h vrf.h
> + workqueue.h route_types.h libospf.h vrf.h \
> + memory_vty.h
>
> noinst_HEADERS = \
> plist_int.h
> diff --git a/lib/memory.c b/lib/memory.c
> deleted file mode 100644
> index 269520d..000
> --- a/lib/memory.c
> +++ /dev/null
> @@ -1,486 +0,0 @@
> -/*
> - * Memory management routine
> - * Copyright (C) 1998 Kunihiro Ishiguro
> - *
> - * This file is part of GNU Zebra.
> - *
> - * GNU Zebra is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License as published by the
> - * Free Software Foundation; either version 2, or (at your option) any
> - * later version.
> - *
> - * GNU Zebra is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - * General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with GNU Zebra; see the file COPYING. If not, write to the Free
> - * Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> - * 02111-1307, USA.
> - */
> -
> -#include
> -/* malloc.h is generally obsolete, however GNU Libc mallinfo wants it. */
> -#if !defined(HAVE_STDLIB_H) || (defined(GNU_LINUX) &&
> defined(HAVE_MALLINFO))
> -#include
> -#endif /* !HAVE_STDLIB_H || HAVE_MALLINFO */
> -
> -#include "log.h"
> -#include "memory.h"
> -
> -static void alloc_inc (int);
> -static void alloc_dec (int);
> -static void log_memstats(int log_priority);
> -
> -static const struct message mstr [] =
> -{
> - { MTYPE_THREAD, "thread" },
> - { MTYPE_THREAD_MASTER, "thread_master" },
> - { MTYPE_VECTOR, "vector" },
> - { MTYPE_VECTOR_INDEX, "vector_index" },
> - { MTYPE_IF, "interface" },
> - { 0, NULL },
> -};
> -
> -/* Fatal memory allocation error occured. */
> -static void __attribute__ ((noreturn))
> -zerror (const char *fname, int type, size_t size)
> -{
> - zlog_err ("%s : can't allocate memory for `%s' size %d: %s\n",
> - fname, lookup (mstr, type), (int) size, safe_strerror(errno));
> - log_memstats(LOG_WARNING);
> - /* N.B. It might be preferable to call zlog_backtrace_sigsafe here,
> since
> - that function should definitely be safe in an OOM condition. But
> - unfortunately zlog_backtrace_sigsafe does not support syslog logging
> at
> - this time... */
> - zlog_backtrace(LOG_WARNING);
> - abort();
> -}
> -
> -/*
> - * Allocate memory of a given size, to be tracked by a given type.
> - * Effects: Returns a pointer to usable memory. If memory cannot
> - * be allocated, aborts execution.
> - */
> -void *
> -zmalloc (int type, size_t size)
> -{
> - void *memory;
> -
> - memory = malloc (size);
> -
> - if (memory == NULL)
> -zerror ("malloc", type, size);
> -
> - alloc_inc (type);
> -
> - return memory;
> -}
> -
> -/*
> - * Allocate memory as in zmalloc, and also clear the memory.
> - */
> -void *
> -zcalloc (int type, size_t size)
> -{
> - void *memory;
> -
> - memory = calloc (1, size);
> -
> - if (memory == NULL)
> -zerror ("calloc", type, size);
>
[quagga-dev 14754] Re: [PATCH 01/10] *: get rid of "MTYPE 0"
Acked-by: Donald Sharp
On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
equi...@opensourcerouting.org> wrote:
> 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.
>
> Signed-off-by: David Lamparter
> ---
> lib/prefix.c| 2 +-
> ospfclient/ospf_apiclient.h | 2 +-
> ospfd/ospf_opaque.c | 6 +++---
> ospfd/ospf_snmp.c | 4 ++--
> ospfd/ospf_te.c | 2 +-
> vtysh/vtysh_main.c | 2 +-
> vtysh/vtysh_user.c | 2 +-
> zebra/zebra_rib.c | 10 +-
> zebra/zserv.c | 2 +-
> 9 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/lib/prefix.c b/lib/prefix.c
> index 3e4ca16..aeb627b 100644
> --- a/lib/prefix.c
> +++ b/lib/prefix.c
> @@ -566,7 +566,7 @@ str2prefix_ipv6 (const char *str, struct prefix_ipv6
> *p)
> {
>int plen;
>
> - cp = XMALLOC (0, (pnt - str) + 1);
> + cp = XMALLOC (MTYPE_TMP, (pnt - str) + 1);
>strncpy (cp, str, pnt - str);
>*(cp + (pnt - str)) = '\0';
>ret = inet_pton (AF_INET6, cp, &p->prefix);
> diff --git a/ospfclient/ospf_apiclient.h b/ospfclient/ospf_apiclient.h
> index 0e74787..8098619 100644
> --- a/ospfclient/ospf_apiclient.h
> +++ b/ospfclient/ospf_apiclient.h
> @@ -23,7 +23,7 @@
> #ifndef _OSPF_APICLIENT_H
> #define _OSPF_APICLIENT_H
>
> -#define MTYPE_OSPF_APICLIENT 0
> +#define MTYPE_OSPF_APICLIENT MTYPE_TMP
>
> /* Structure for the OSPF API client */
> struct ospf_apiclient
> diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c
> index 61e98f4..697655d 100644
> --- a/ospfd/ospf_opaque.c
> +++ b/ospfd/ospf_opaque.c
> @@ -22,9 +22,9 @@
> */
>
> /* MTYPE definitions are not reflected to "memory.h" yet. */
> -#define MTYPE_OSPF_OPAQUE_FUNCTAB 0
> -#define MTYPE_OPAQUE_INFO_PER_TYPE 0
> -#define MTYPE_OPAQUE_INFO_PER_ID 0
> +#define MTYPE_OSPF_OPAQUE_FUNCTAB MTYPE_TMP
> +#define MTYPE_OPAQUE_INFO_PER_TYPE MTYPE_TMP
> +#define MTYPE_OPAQUE_INFO_PER_ID MTYPE_TMP
>
> #include
>
> diff --git a/ospfd/ospf_snmp.c b/ospfd/ospf_snmp.c
> index ebeffa8..1f9b851 100644
> --- a/ospfd/ospf_snmp.c
> +++ b/ospfd/ospf_snmp.c
> @@ -1420,13 +1420,13 @@ struct ospf_snmp_if
> static struct ospf_snmp_if *
> ospf_snmp_if_new (void)
> {
> - return XCALLOC (0, sizeof (struct ospf_snmp_if));
> + return XCALLOC (MTYPE_TMP, sizeof (struct ospf_snmp_if));
> }
>
> static void
> ospf_snmp_if_free (struct ospf_snmp_if *osif)
> {
> - XFREE (0, osif);
> + XFREE (MTYPE_TMP, osif);
> }
>
> void
> diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
> index cd52866..03109bc 100644
> --- a/ospfd/ospf_te.c
> +++ b/ospfd/ospf_te.c
> @@ -22,7 +22,7 @@
> */
>
> /* MTYPE definition is not reflected to "memory.h" yet. */
> -#define MTYPE_OSPF_MPLS_TE_LINKPARAMS 0
> +#define MTYPE_OSPF_MPLS_TE_LINKPARAMS MTYPE_TMP
>
> #include
>
> diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c
> index 02a19b7..e82771b 100644
> --- a/vtysh/vtysh_main.c
> +++ b/vtysh/vtysh_main.c
> @@ -251,7 +251,7 @@ main (int argc, char **argv, char **env)
> case 'c':
> {
> struct cmd_rec *cr;
> - cr = XMALLOC(0, sizeof(*cr));
> + cr = XMALLOC(MTYPE_TMP, sizeof(*cr));
> cr->line = optarg;
> cr->next = NULL;
> if (tail)
> diff --git a/vtysh/vtysh_user.c b/vtysh/vtysh_user.c
> index 239a633..248b181 100644
> --- a/vtysh/vtysh_user.c
> +++ b/vtysh/vtysh_user.c
> @@ -102,7 +102,7 @@ struct list *userlist;
> static struct vtysh_user *
> user_new ()
> {
> - return XCALLOC (0, sizeof (struct vtysh_user));
> + return XCALLOC (MTYPE_TMP, sizeof (struct vtysh_user));
> }
>
> #if 0
> diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
> index 1650dab..15b7015 100644
> --- a/zebra/zebra_rib.c
> +++ b/zebra/zebra_rib.c
> @@ -218,7 +218,7 @@ nexthop_ifname_add (struct rib *rib, char *ifname)
>
>nexthop = XCALLOC (MTYPE_NEXTHOP, sizeof (struct nexthop));
>nexthop->type = NEXTHOP_TYPE_IFNAME;
> - nexthop->ifname = XSTRDUP (0, ifname);
> + nexthop->ifname = XSTRDUP (MTYPE_TMP, ifname);
>
>nexthop_add (rib, nexthop);
>
> @@ -282,7 +282,7 @@ nexthop_ipv6_ifname_add (struct rib *rib, struct
> in6_addr *ipv6,
>nexthop = XCALLOC (MTYPE_NEXTHOP, sizeof (struct nexthop));
>nexthop->type = NEXTHOP_TYPE_IPV6_IFNAME;
>nexthop->gate.ipv6 = *ipv6;
> - nexthop->ifname = XSTRDUP (0, ifname);
> + nexthop->ifname = XSTRDUP (MTYPE_TMP, ifname);
>
>nexthop_add (rib, nexthop);
>
> @@ -2460,7 +2460,7 @@ static_add_ipv4_safi (safi_t safi, struct prefix *p,
> struct in_addr *gate,
>if (gate)
> si->addr.ipv4 = *gate;
>if (ifname)
> -si->ifname = XSTRDUP (0, ifname);
> +si->ifname = XSTRDUP (MTYPE_TMP, ifname);
>
>/* Add new static route information to the tree with sort by
> distance value and gateway add
[quagga-dev 14753] Re: FreeBSD Bug: Zebra not deleting routes
On Wed, 24 Feb 2016, Martin Winter wrote: applied patch [probably wrong] on top of the last commit. Rebase made it impossible to pull a new copy as I usually do for each run) I think the lesson from round-6 is to use separate branches when it makes sense and merge, rather than have it all in a 'ff' branch that ends up being rebased and not fast-forwardable. Maybe we need to keep patches from each contributor in a separate branch under proposed/$R/ - each forking from the previous master. You could test each branch independently and pass/fail them. Any that pass can be pulled into the 'ff' branch either. Or whatever would make life easier for testing. Though, I'm not a huge fan of massive octupus-tentacled parallel branches in the main history either. So, perhaps a final rebase to linearlise 'ff' and final all-to-gether test before end of round would still be an idea. regards, -- Paul Jakma p...@jakma.org @pjakma Key ID: 64A2FF6A Fortune: "If you lived today as if it were your last, you'd buy up a box of rockets and fire them all off, wouldn't you?" -- Garrison Keillor ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14752] Re: FreeBSD Bug: Zebra not deleting routes
Martin -
Thanks for this work.
I'll be applying the patch here in a few minutes.
donald
On Wed, Feb 24, 2016 at 6:02 AM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
> On 22 Feb 2016, at 6:40, Donald Sharp wrote:
>
>> Martin -
>>
>> Any word on this? I'd like to push this patch if it fixes your issues.
>>
>
> Ok, finally more answers.
> It seems something went wrong when I retested with Timo’s fix. (Might be
> related to the way I retested as I was unable to do a clean rebuild and
> applied patch [probably wrong] on top of the last commit. Rebase made it
> impossible to pull a new copy as I usually do for each run)
>
> Anyway, re-running the tests and Timo’s patch (same fixed version as
> mailed earlier) fixes at least all BGP IPv4 bug difference between
> Linux and FreeBSD.
>
> So I suggest to go ahead an apply that patch on top of proposed/6 branch
>
> Full results (including all other protocols) of the rerun will be available
> in approx another 12..18hrs.
>
> - Martin
>
>
>
> On Thu, Feb 18, 2016 at 10:30 PM, Martin Winter <
>> mwin...@opensourcerouting.org> wrote:
>>
>> On 18 Feb 2016, at 18:35, Donald Sharp wrote:
>>>
>>> Martin -
rt_socket.c is not compiled on linux so no need for verification. Once
>>> you
>>>
have a run please let me know and I'll push your updated patch.
>>>
>>> It’s running now. Will know soon. It definitely fixes some of the errors
>>> and I hope it actually fixes all of the FreeBSD errors.
>>>
>>> Will know in approx one more day.
>>>
>>> (BTW: Testing on a git commit which does no longer exist because of
>>> rebase,
>>> but I needed the same to compare. Lucky that I still had the VMs with the
>>> old git checkout running…)
>>>
>>> - Martin
>>>
>>>
donald
On Thu, Feb 18, 2016 at 9:19 PM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
Timo,
>
> On 17 Feb 2016, at 23:01, Timo Teras wrote:
>
> On Wed, 17 Feb 2016 20:11:07 -0800
>
>> "Martin Winter" wrote:
>>
>> This is based on Quagga proposed 6 branch of Feb 10 (git f48d5b9957 -
>>
>>> which does no longer exist, [no] thanks to rebase on a public
>>> git) on FreeBSD 10.2.
>>>
>>> Zebra seems to fail delete any routes in FreeBSD RIB. It fails with
>>> updates (better routes to different interface) and
>>> it fails to remove them when quagga is killed.
>>>
>>>
>> Thanks for the testing. Sounds like this is breakage caused by my
>> atomic FIB patch which was pretty untested on *BSD.
>>
>> Looks like the code talking to kernel does not handle RTM_CHANGE
>> correctly. As first test, perhaps just removing RTM_CHANGE usage might
>> fix the issues and revert to how it used to work.
>>
>> Using RTM_CHANGE on kernels where it works is better, but it's left an
>> exercise for developer who has access and will to fix it on *BSD.
>>
>>
> Thanks for the patch. Seems like you never tested it (failed to
>
compile),
>>>
but was close enough to guess what you probably meant. See inline on
>
patch
>>>
> diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c
>
>> index 4d0a7db..9012280 100644
>> --- a/zebra/rt_socket.c
>> +++ b/zebra/rt_socket.c
>> @@ -68,7 +68,7 @@ sin_masklen (struct in_addr mask)
>>
>> /* Interface between zebra message and rtm message. */
>> static int
>> -kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib, int
>>
> family)
>>>
+kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib)
>>
>> {
>> struct sockaddr_in *mask = NULL;
>> @@ -245,7 +245,7 @@ sin6_masklen (struct in6_addr mask)
>>
>> /* Interface between zebra message and rtm message. */
>> static int
>> -kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib, int
>>
> family)
>>>
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
>> {
>> struct sockaddr_in6 *mask;
>> struct sockaddr_in6 sin_dest, sin_mask, sin_gate;
>> @@ -363,33 +363,32 @@ kernel_rtm_ipv6 (int cmd, struct prefix *p,
>>
> struct
>>>
rib *rib, int family)
>>
>> #endif
>>
>> +static int
>> +kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
>>
>>
> I assume this should be
> kernel_rtm - not kernel_rtm_ipv6
> (otherwise you have a duplicate for kernel_rtm_ipv6() and a loop
> in case of AF_INET6)
>
>
> +{
>
>> + switch (PREFIX_FAMILY(p))
>> +{
>> +case AF_INET:
>> + return kernel_rtm_ipv4 (cmd, p, rib);
>> +case AF_INET6:
>> + return kernel_rtm_ipv6 (cmd, p, rib);
>> +}
>> + return 0;
>> +}
>> +
>> int
>> kernel_route_rib (struct prefix *p, struct rib *old, struct rib *new)
>> {
>> - struct rib *rib;
>> - int route = 0, cmd;
>> -
>> - if (!old && new)
>> -cmd = RTM
[quagga-dev 14751] Re: [PATCH 03/10] lib: add new extensible memory-type handling
On Wed, 24 Feb 2016, David Lamparter wrote:
This rewrites Quagga's memory per-type allocation counting, without
using a fixed global list of types. Instead, source files can declare
memory types which get handled through constructor functions called by
the dynamic linker during startup.
Using the pre-processor to encode things into the symbol namespace, to
avoid runtime expenses++. :) Working on same myself.
Couple of initial comments:
- This seems to require various extensions to work, some seem to be GCC
specific?
Are there people using other compilers? (I used to regularly build
with the Sun One Studio compiler, but I don't anymore; only clang
and gcc).
How much do people value compiler portability?
- The __builtin_types_{expect,compatible_p} stuff, would C11 _Generic
allow the same to be achieved?
- Could the manipulation of the linked list global pointers (the
addition of a group onto the global groups list, then additions of
memtypes in a group onto the group list) be done behind a centralised
function in lib/memory and go through that, rather than twiddled
directly in each mtype specific constructor function?
- Compiler and linker specific extensions to arrange to declare static
stuff on by one and have a function called for each.
The key thing here really is just to not have to update the library,
right? I.e., the dynamic group registration is the more important bit?
And groups will hopefully be few (perhaps 1 per daemon - certainly in
Quagga)
If the types in a group were still kept as a table, they could be
handed to a lib/memory registration function, and standard C could be
used.
enum memtypes_bgp_c {
MEMTYPE_ENT_C(BGP_NEXTHOP),
MEMTYPE_ENT_C(BGP_ROUTE),
};
struct memtypes {
MEMTYPE_ENT(BGP_NEXTHOP, "BGP nexthop"),
MEMTYPE_ENT(BGP_ROUTE, "BGP route"),
MEMTYPE_ENT_END,
} memtypes_bgp;
somewhere in an early init func or main:
MEMTYPES_REG (BGP, memtypes_bgp);
where MEMTYPES_REG uses the appropriate func created when the BGP
group was declared to find the right group and then call a centralised
function to add it.
??
Explicitly registering the group, and then the table to the group, via
helper macros, doesn't seem like it should be onerous, given the
number of groups should be low? (with library memtypes init done
behind a library init).
I.e., I'm just wondering if non-standard construction extensions are
enough of a win, given they only eliminiate a couple of initialisation
calls?
- The formatting should follow the standard style in Quagga,
particularly anything intended for core lib/ bits.
regards,
--
Paul Jakma p...@jakma.org @pjakma Key ID: 64A2FF6A
Fortune:
"Life sucks, but death doesn't put out at all"
-- Thomas J. Kopp
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14750] Re: FreeBSD Bug: Zebra not deleting routes
On 22 Feb 2016, at 6:40, Donald Sharp wrote:
Martin -
Any word on this? I'd like to push this patch if it fixes your
issues.
Ok, finally more answers.
It seems something went wrong when I retested with Timo’s fix. (Might
be
related to the way I retested as I was unable to do a clean rebuild and
applied patch [probably wrong] on top of the last commit. Rebase made it
impossible to pull a new copy as I usually do for each run)
Anyway, re-running the tests and Timo’s patch (same fixed version as
mailed earlier) fixes at least all BGP IPv4 bug difference between
Linux and FreeBSD.
So I suggest to go ahead an apply that patch on top of proposed/6 branch
Full results (including all other protocols) of the rerun will be
available
in approx another 12..18hrs.
- Martin
On Thu, Feb 18, 2016 at 10:30 PM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
On 18 Feb 2016, at 18:35, Donald Sharp wrote:
Martin -
rt_socket.c is not compiled on linux so no need for verification.
Once
you
have a run please let me know and I'll push your updated patch.
It’s running now. Will know soon. It definitely fixes some of the
errors
and I hope it actually fixes all of the FreeBSD errors.
Will know in approx one more day.
(BTW: Testing on a git commit which does no longer exist because of
rebase,
but I needed the same to compare. Lucky that I still had the VMs with
the
old git checkout running…)
- Martin
donald
On Thu, Feb 18, 2016 at 9:19 PM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
Timo,
On 17 Feb 2016, at 23:01, Timo Teras wrote:
On Wed, 17 Feb 2016 20:11:07 -0800
"Martin Winter" wrote:
This is based on Quagga proposed 6 branch of Feb 10 (git
f48d5b9957 -
which does no longer exist, [no] thanks to rebase on a public
git) on FreeBSD 10.2.
Zebra seems to fail delete any routes in FreeBSD RIB. It fails
with
updates (better routes to different interface) and
it fails to remove them when quagga is killed.
Thanks for the testing. Sounds like this is breakage caused by my
atomic FIB patch which was pretty untested on *BSD.
Looks like the code talking to kernel does not handle RTM_CHANGE
correctly. As first test, perhaps just removing RTM_CHANGE usage
might
fix the issues and revert to how it used to work.
Using RTM_CHANGE on kernels where it works is better, but it's
left an
exercise for developer who has access and will to fix it on *BSD.
Thanks for the patch. Seems like you never tested it (failed to
compile),
but was close enough to guess what you probably meant. See inline
on
patch
diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c
index 4d0a7db..9012280 100644
--- a/zebra/rt_socket.c
+++ b/zebra/rt_socket.c
@@ -68,7 +68,7 @@ sin_masklen (struct in_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib, int
family)
+kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in *mask = NULL;
@@ -245,7 +245,7 @@ sin6_masklen (struct in6_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib, int
family)
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in6 *mask;
struct sockaddr_in6 sin_dest, sin_mask, sin_gate;
@@ -363,33 +363,32 @@ kernel_rtm_ipv6 (int cmd, struct prefix *p,
struct
rib *rib, int family)
#endif
+static int
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
I assume this should be
kernel_rtm - not kernel_rtm_ipv6
(otherwise you have a duplicate for kernel_rtm_ipv6() and a loop
in case of AF_INET6)
+{
+ switch (PREFIX_FAMILY(p))
+{
+case AF_INET:
+ return kernel_rtm_ipv4 (cmd, p, rib);
+case AF_INET6:
+ return kernel_rtm_ipv6 (cmd, p, rib);
+}
+ return 0;
+}
+
int
kernel_route_rib (struct prefix *p, struct rib *old, struct rib
*new)
{
- struct rib *rib;
- int route = 0, cmd;
-
- if (!old && new)
-cmd = RTM_ADD;
- else if (old && !new)
-cmd = RTM_DELETE;
- else
-cmd = RTM_CHANGE;
-
- rib = new ? new : old;
+ int route = 0;
if (zserv_privs.change(ZPRIVS_RAISE))
zlog (NULL, LOG_ERR, "Can't raise privileges");
- switch (PREFIX_FAMILY(p))
-{
-case AF_INET:
- route = kernel_rtm_ipv4 (cmd, p, rib, AF_INET);
- break;
-case AF_INET6:
- route = kernel_rtm_ipv6 (cmd, p, rib, AF_INET6);
- break;
-}
+ if (old)
+route |= kernel_rtm (RTM_DELETE, p, rib);
Changed to
route |= kernel_rtm (RTM_DELETE, p, old);
+
+ if (new)
+route |= kernel_rtm (RTM_ADD, p, rib);
and changed to
route |= kernel_rtm (RTM_ADD, p, new);
(You removed the declaration of “rib” above - so rib is
undefined)
if (zserv_privs.change(ZPRIVS_LOWER))
zlog (NULL, LOG_ERR, "Can't lower privileges");
Attached is a updated patch
With the changes it fixes the specific issue I’ve mentioned. I
have not
verified the
patch on Linux yet. Wil
[quagga-dev 14748] CI Testresult: FAILED (Re: [quagga-dev, 14741, 10/10] build: goodbye, gawk)
Continous Integration Result: FAILED See below for issues. This is an EXPERIMENTAL automated CI system. For questions and feedback, feel free to email Martin Winter . Patches applied : Patchwork 1822: http://patchwork.quagga.net/patch/1822 [quagga-dev,14734,01/10] *: get rid of "MTYPE 0" Patchwork 1823: http://patchwork.quagga.net/patch/1823 [quagga-dev,14735,02/10] lib: move memory.[ch] out of the way Patchwork 1824: http://patchwork.quagga.net/patch/1824 [quagga-dev,14736,03/10] lib: add new extensible memory-type handling Patchwork 1828: http://patchwork.quagga.net/patch/1828 [quagga-dev,14739,04/10] lib: migrate to new memory-type handling Patchwork 1831: http://patchwork.quagga.net/patch/1831 [quagga-dev,14743,05/10] *: split & distribute memtypes Patchwork 1826: http://patchwork.quagga.net/patch/1826 [quagga-dev,14738,06/10] *: stop (re|ab)using lib/ MTYPEs Patchwork 1830: http://patchwork.quagga.net/patch/1830 [quagga-dev,14742,07/10] lib: distribute mtypes into files Patchwork 1825: http://patchwork.quagga.net/patch/1825 [quagga-dev,14737,08/10] lib: clean/restore memory debugging functions Patchwork 1827: http://patchwork.quagga.net/patch/1827 [quagga-dev,14740,09/10] build: goodbye, memtypes.c Patchwork 1829: http://patchwork.quagga.net/patch/1829 [quagga-dev,14741,10/10] build: goodbye, gawk Tested on top of Git : f739363 (as of 20160218.155415 UTC) CI System Testrun URL: https://ci1.netdef.org/browse/QUAGGA-QPWORK-243/ Get source and apply patch from patchwork: Failed > Applying Patchwork patch 1822 > > patching file lib/prefix.c > patching file ospfclient/ospf_apiclient.h > patching file ospfd/ospf_opaque.c > patching file ospfd/ospf_snmp.c > patching file ospfd/ospf_te.c > patching file vtysh/vtysh_main.c > patching file vtysh/vtysh_user.c > patching file zebra/zebra_rib.c > patching file zebra/zserv.c > Applying Patchwork patch 1823 > > patching file lib/Makefile.am > Hunk #2 FAILED at 29. > 1 out of 2 hunks FAILED -- saving rejects to file lib/Makefile.am.rej > patching file lib/memory.c > patching file lib/memory.h > patching file lib/memory_vty.c > patching file lib/memory_vty.h Regards, NetDEF/OpenSourceRouting Continous Integration (CI) System --- OpenSourceRouting.org is a project of the Network Device Education Foundation, For more information, see www.netdef.org and www.opensourcerouting.org For questions in regards to this CI System, contact Martin Winter, mwin...@netdef.org ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14749] Re: CI Testresult: FAILED (Re: [quagga-dev, 14741, 10/10] build: goodbye, gawk)
On 24 Feb 2016, at 1:07, David Lamparter wrote: > On Wed, Feb 24, 2016 at 01:00:03AM -0800, cisys...@netdef.org wrote: >> Continous Integration Result: FAILED > [...] >> Tested on top of Git : eae18d1 (as of 20151209.135437 UTC) > > As mentioned in the original mail, this patchset applies on top of > "lib: fix vrf_bitmap leak in zclient_free()", not master. The patchset > has passed a manual run (though I did some style polishing afterwards, I > hope I didn't accidentally break anything there.) Sorry, my CI system doesn’t speak english. Resubmitted to be on top of the specified Git But patching seemed to fail. (01/10 worked, 02/10 failed) See CI email. Let me guess: The rebase changed things? - Martin ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14747] CI Testresult: FAILED (Re: [quagga-dev, 14746] Re: adding copyright lines when modifying a file)
Continous Integration Result: FAILED See below for issues. This is an EXPERIMENTAL automated CI system. For questions and feedback, feel free to email Martin Winter . Patches applied : Patchwork 1832: http://patchwork.quagga.net/patch/1832 [quagga-dev,14746] Re: adding copyright lines when modifying a file Tested on top of Git : eae18d1 (as of 20151209.135437 UTC) CI System Testrun URL: https://ci1.netdef.org/browse/QUAGGA-QPWORK-242/ Get source and apply patch from patchwork: Failed > Applying Patchwork patch 1832 > > patching file HACKING.md > Hunk #1 FAILED at 50. > 1 out of 1 hunk FAILED -- saving rejects to file HACKING.md.rej Regards, NetDEF/OpenSourceRouting Continous Integration (CI) System --- OpenSourceRouting.org is a project of the Network Device Education Foundation, For more information, see www.netdef.org and www.opensourcerouting.org For questions in regards to this CI System, contact Martin Winter, mwin...@netdef.org ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14746] Re: adding copyright lines when modifying a file
On Tue, 23 Feb 2016, Lou Berger wrote:
With my Quagga hat on, if someone contributes something and feels a need
to add "their" copyright claim, I usually'd have no reason to not put it
through.
Sure, but I'd hope you'd block someone adding a copyright statement on
others work.
Only if it was 100% clear they could have nothing to have copyright over
in the file they were adding the string to.
Remember, the "Copyright..." string of itself doesn't affect any
copyright claims one way or another. It's just documentation, allowing
someone to say:
"FWIW, just to help you, I happen to think I have a Copyright interest
in this file"
So if someone thinks that, I'd encourage them to document it by adding
the string. If it seemed they had to be mistaken I'd only /query/ it -
but if they still believed they had a claim, I'd still add it - it'd be
documenting the reality of what they believe after all.
Whether their belief in their claim is right or wrong isn't really
something I can decide. However, so long as they have they that belief
and it's not disproven, their "Copyright ..." string would be somewhat
consistent with reality?
We havn't though had much of a problem with people adding notices when
they shouldn't though. The problem is more the reverse.
Why is that a problem? As you say above, it's their choice.
Hmm, I'd rather err on the side of documenting copyright holders than
not, in a more durable way than SCM meta-data.
Once information is lost, it's lost for ever.
I see no reason to take a position one way or another. If someone
doesn't want to add a copyright, it's their choice. If they do, it's
also their choice as long as they don't copyright someone else's work.
Thinking about this on the way home yesterday, the original issue that
made you think about this was that I added a copyright string as part of
an update of a file, and the lack of other "Copyright" strings of other
authors might make it seem like the string I added was the only
copyright claim or exclusive, or whatever.
So the issue was that "Copyright" strings for modifications could cause
confusion about the scope, right? How about we focus on that and just
say for modifications, it should be prefaced with "Portions:\n"? Also
use "may" instead of "are encouraged". ??
From f2e489120173f14ba2f5e429c9414e973c2de87b Mon Sep 17 00:00:00 2001
From: Paul Jakma
Date: Tue, 26 Jan 2016 12:45:31 +
Subject: HACKING: Document how to add standard copyright claims to files
* (REQUIRED READING) Copyright claims may be documented in the standard way,
with a "Copyright ..." line near the beginning of the file.
diff --git a/HACKING.md b/HACKING.md
index 7a5d973..fe58b4f 100644
--- a/HACKING.md
+++ b/HACKING.md
@@ -50,6 +50,15 @@ be _explicitly_ stated with the contribution. A
"Signed-off-by" line is
_not_ sufficient. The "Signed-off-by" line is not used by the Quagga
project.
+You may document applicable copyright claims to files being modified or
+added. The standard way is to add a string in the following format near the
+beginning of the file:
+
+Copyright (C) [, optional contact details]
+
+When adding such claims for modifications to an existing file, please
+preface the claim with "Portions: " on a line before it.
+
GUIDELINES FOR HACKING ON QUAGGA {#sec:guidelines}
regards,
--
Paul Jakma p...@jakma.org @pjakma Key ID: 64A2FF6A
Fortune:
Artificial intelligence has the same relation to intelligence as
artificial flowers have to flowers.
-- David Parnas
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14745] CI Testresult: FAILED (Re: [quagga-dev, 14741, 10/10] build: goodbye, gawk)
Continous Integration Result: FAILED See below for issues. This is an EXPERIMENTAL automated CI system. For questions and feedback, feel free to email Martin Winter . Patches applied : Patchwork 1822: http://patchwork.quagga.net/patch/1822 [quagga-dev,14734,01/10] *: get rid of "MTYPE 0" Patchwork 1823: http://patchwork.quagga.net/patch/1823 [quagga-dev,14735,02/10] lib: move memory.[ch] out of the way Patchwork 1824: http://patchwork.quagga.net/patch/1824 [quagga-dev,14736,03/10] lib: add new extensible memory-type handling Patchwork 1828: http://patchwork.quagga.net/patch/1828 [quagga-dev,14739,04/10] lib: migrate to new memory-type handling Patchwork 1831: http://patchwork.quagga.net/patch/1831 [quagga-dev,14743,05/10] *: split & distribute memtypes Patchwork 1826: http://patchwork.quagga.net/patch/1826 [quagga-dev,14738,06/10] *: stop (re|ab)using lib/ MTYPEs Patchwork 1830: http://patchwork.quagga.net/patch/1830 [quagga-dev,14742,07/10] lib: distribute mtypes into files Patchwork 1825: http://patchwork.quagga.net/patch/1825 [quagga-dev,14737,08/10] lib: clean/restore memory debugging functions Patchwork 1827: http://patchwork.quagga.net/patch/1827 [quagga-dev,14740,09/10] build: goodbye, memtypes.c Patchwork 1829: http://patchwork.quagga.net/patch/1829 [quagga-dev,14741,10/10] build: goodbye, gawk Tested on top of Git : eae18d1 (as of 20151209.135437 UTC) CI System Testrun URL: https://ci1.netdef.org/browse/QUAGGA-QPWORK-241/ Get source and apply patch from patchwork: Failed > Applying Patchwork patch 1822 > > patching file lib/prefix.c > Hunk #1 succeeded at 550 (offset -16 lines). > patching file ospfclient/ospf_apiclient.h > patching file ospfd/ospf_opaque.c > Hunk #1 succeeded at 22 with fuzz 1. > patching file ospfd/ospf_snmp.c > patching file ospfd/ospf_te.c > patching file vtysh/vtysh_main.c > Hunk #1 succeeded at 250 (offset -1 lines). > patching file vtysh/vtysh_user.c > patching file zebra/zebra_rib.c > Hunk #3 succeeded at 2522 (offset 62 lines). > Hunk #4 succeeded at 2918 (offset 61 lines). > patching file zebra/zserv.c > Hunk #1 succeeded at 1317 (offset 5 lines). > Applying Patchwork patch 1823 > > patching file lib/Makefile.am > patching file lib/memory.c > Reversed (or previously applied) patch detected! Skipping patch. > 1 out of 1 hunk ignored -- saving rejects to file lib/memory.c.rej > patching file lib/memory.h > patching file lib/memory_vty.c > patching file lib/memory_vty.h Regards, NetDEF/OpenSourceRouting Continous Integration (CI) System --- OpenSourceRouting.org is a project of the Network Device Education Foundation, For more information, see www.netdef.org and www.opensourcerouting.org For questions in regards to this CI System, contact Martin Winter, mwin...@netdef.org ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 14744] Re: FreeBSD Bug: Zebra not deleting routes
Le 24/02/2016 01:10, Martin Winter a écrit : Ok, false alert. Lovely CI system picks random execution nodes during the git bisect and one of the node had a stuck route - so by random it happened to end up with this specific commit as “bad” Ok, thank you for the explanation, the relationship with that commit was not obvious ;-) Regards, Nicolas ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
