Paul (and everyone else),
On 17 Sep 2015, at 9:41, Paul Jakma wrote:
Hi,
Next round of outstanding patches being tracked, collected from
patchwork from the last couple of months (which should mean all recent
patches are now tracked):
Spent some time testing all (at least the ones in patchwork) of these
patches.
Please don’t take a PASS below as an ACK - it only means that it
passed my basic
CI system tests (haven’t run the full ones - Limited it to 1hr runtime
for a
“quick” test)
http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/3/proposed/ff
Not sure if these is a Git Newbie question. I find all the commits in
Savannah under this
URL, but I can’t check out any of them. All complaining to commit not
found and I can’t
checkout the branch either.
Do I miss something stupid here or is this some hidden/protected branch?
Anyway, I did a “quick” test of all the listed patchwork patches on
top
of master (git @ 52c0bc7) with (as a workaround for the BGP and OSPF
issue), then
patchwork 1341 and 1336 applied and then on top of them each of these
patchwork patches.
(Each Test is run separate - so only git master@52c0bc7 with PW 1341,
1336 and the
patchwork under test.)
The Tests are under the Patchwork CI plan at
https://ci1.netdef.org/browse/QUAGGA-QPWORK
This is what the test does:
1) Apply patch (“Get Sourcecode” phase if you look it up in my CI
system)
2) Compile, run DejaGNU tests, build native package on each platform
(Ubuntu 12.04, Ubuntu 14.04, CentOS 6, CentOS 7, Debian 8, NetBSD 6,
FreeBSD 8, FreeBSD 9, FreeBSD 10)
(“Building Stage” phase if you look it up in my CI system)
3) Run a few selected IPv4 and IPv6 routing protocol tests (Subset of
the full suite. I’ve tried
to pick a 2..3 meaningful tests from each protocol test suite). This is
only done with the
Ubuntu 14.04 debian package as built in 2)
NO -—Werror-enable
*********************
Please be aware that the configure option —-Werror-enable fails at
this time with a few errors
on some platforms. For all of these tests, I had to keep it to
—-Werror-disable (the old default)
-—Werror-enable works on the following platforms:
Ubuntu 12.04
Ubuntu 14.04
CentOS 7
Debian 8
—-Werror-enable fails on these platforms:
CentOS 6 (see
https://ci1.netdef.org/browse/TESTING-WERROR-CI006BUILD-1 )
cc1: error: unrecognized command line option
"-Wno-unused-result"
NetBSD 6 (see
https://ci1.netdef.org/browse/TESTING-WERROR-CI007BUILD-1 )
pim_cmd.c: In function 'show_mroute_count':
pim_cmd.c:2353:10: error: format '%d' expects type 'int', but argument
3 has type 'long unsigned int'
FreeBSD 8 (see
https://ci1.netdef.org/browse/TESTING-WERROR-CI009BUILD-1 )
same as NetBSD 6
FreeBSD 9 (see
https://ci1.netdef.org/browse/TESTING-WERROR-CI004BUILD-1 )
same as NetBSD 6
FreeBSD 10 (see
https://ci1.netdef.org/browse/TESTING-WERROR-CI003BUILD-1 )
same as NetBSD 6
[…]
Now to the individual list of the patches:
http://patchwork.quagga.net/patch/1311/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/012993.html
Adding redhat init/service files to start pimd
PASS.
(Fails in my CI system, but is a PASS. it fails as I had a manual
workaround on this missing service files which conflicts with this
patch)
Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-53
https://lists.quagga.net/pipermail/quagga-dev/2015-July/012930.html
http://patchwork.quagga.net/patch/1292/
bgpd: improve ebgp-multihop and ttl-security handling
Outstanding comment on the command being confusing by donald?
Some fuzzing and failures in applying patch.
FAILS. Patch fails to apply on current master.
Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-54
http://patchwork.quagga.net/patch/1294/
Enable vtysh and pimd as part of default build
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-55
http://patchwork.quagga.net/patch/1293/
https://lists.quagga.net/pipermail/quagga-dev/2015-July/012936.html
Switchover -Werror to default setting
FAILS. Patch fails to apply on current master.
The patchwork includes (in addition to this flag) a change in
zebra/rt_netlink.c
which fails to apply.
But even with this changed, please see above. We need to fix the
warnings
for the other OS first before pushing this change. I would prefer to
delay
this one for the next round to give enough time to clean up the
warnings.
Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-56
http://patchwork.quagga.net/patch/1295/
https://lists.quagga.net/pipermail/quagga-dev/2015-July/012939.html
Fix optional arguments with description interactions
(missing "topic:" prefix, as with a few other contributions in this
list)
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-57
http://patchwork.quagga.net/patch/1191/
https://lists.quagga.net/pipermail/quagga-dev/2015-July/012933.html
bgpd: Addition of "show ip bgp dampening" command tree Was Acked by
Donald?
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-58
http://patchwork.quagga.net/patch/1296/
bgpd: Fix 'struct peer' memory leaks
Should be split into 2 commits for clarity?
Done:
http://patchwork.quagga.net/patch/1345/
bgpd: Add some peer_lock/unlock debug code
http://patchwork.quagga.net/patch/1346/
bgpd: Fix 'struct peer' memory leaks
1296: PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-59
1345: PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-60
1346: PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-61
http://patchwork.quagga.net/patch/1300/
Fix useless call in bgpd/bgp_mplsvpn.c
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-62
http://patchwork.quagga.net/patch/1301/
Fixed rtadv_make_socket socket leak
Something went wrong preparing the patch? (And a number of others in
that series from Denil - Donald is respinning?)
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-63
http://patchwork.quagga.net/patch/1302/
Fix Free Pointer dereference in lib/filter.c
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-64
http://patchwork.quagga.net/patch/1306/
Variable reuse in bgpd/bgpd.c
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-65
http://patchwork.quagga.net/patch/1307/
Fix memory leak in bgpd/bgp_route.c
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-66
http://patchwork.quagga.net/patch/1304/
https://lists.quagga.net/pipermail/quagga-dev/2015-July/012957.html
Fast OSPF convergence
Git commit message is a bit vague. Took the 2nd version which
addressed some comments.
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-67
http://patchwork.quagga.net/patch/1305/
Arm compilation warning fix
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-68
http://patchwork.quagga.net/patch/1308/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/012988.html
Fixed if_add_update possible null dereference
Should this maybe fail a bit harder than a zlog_debug? (Donald to send
update)
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-69
http://patchwork.quagga.net/patch/1310/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/012992.html
Restrict Shell access from vtysh
Still a bit surprising that vtysh is opened to untrusted users. Do we
support that config?
ABORTED TEST. Compiles, but Compliance Test need changes in my test
script to
test without the shell access.
Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-70
http://patchwork.quagga.net/patch/1312/
ospfd: Fix for 'no' + 'debug command' does not disable 'debug command'
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-71
http://patchwork.quagga.net/patch/1313/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/013007.html
Remove unnecessary stream_dup calls
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-72
http://patchwork.quagga.net/patch/1314/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/013008.html
HACKING: Change format to MarkDown
Some concerns about the longevity of MarkDown and its ability to
handle anything non-trivial that might come along. Are these allayed
by 'pandoc', which can convert between TeX and MD, and indeed which
allows TeX in MD - note the submitted document actually contains TeX
commands. With 'pandoc' we can think of HACKING as a TeX document that
allows using the easier to type/read MarkDown for the common case.
Sufficient?
FAIL. Patchwork patch isn’t properly formatted, so fails to apply.
Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-74
http://patchwork.quagga.net/patch/1315/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/013023.html
Add missing show commands to vtysh
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-75
http://patchwork.quagga.net/patch/1316/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/013024.html
Fix small memory leak in str2prefix_rd
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-76
http://patchwork.quagga.net/patch/1322/
https://lists.quagga.net/pipermail/quagga-dev/2015-August/013050.html
See also:
https://lists.quagga.net/pipermail/quagga-dev/2015-August/013038.html
https://lists.quagga.net/pipermail/quagga-dev/2015-August/013046.html
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013081.html
The consensus unclear to me?
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-77
http://patchwork.quagga.net/patch/1327/
Recursive routes are not removed from kernel.
Rejected by Donald?
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-78
http://patchwork.quagga.net/patch/1331/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013083.html
vrf: add a runtime check before playing with netns
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-79
http://patchwork.quagga.net/patch/1343/
supersedes:
http://patchwork.quagga.net/patch/1341/
http://patchwork.quagga.net/patch/1334/
http://patchwork.quagga.net/patch/1332/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013094.html
see also:
https://lists.quagga.net/pipermail/quagga-dev/2015-July/012927.html
1343: PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-92
(Tested without 1341 applied - which I applied to all other tests)
http://patchwork.quagga.net/patch/1333/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013096.html
allow --with-libpam to build with --enable-werror
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-83
http://patchwork.quagga.net/patch/1335/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013105.html
zebra: use addr (Zserv sending the pointer address instaed of the IPv6
address)
FAIL. Malformed patch in Patchwork.
Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-CHECKOUT-84
http://patchwork.quagga.net/patch/1336/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/thread.html
ospfd: Fix bug in 94266fa822ba, nbr_self rebuild didn't add valid
nbr_self
PASS. Part of the baseline test (all tests included this one to make
OSPF working
http://patchwork.quagga.net/patch/1337/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013117.html
pimd: Fix leaked fd
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-85
http://patchwork.quagga.net/patch/1338/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013119.html
Pim: Remove stdout zlog changes
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-86
http://patchwork.quagga.net/patch/1340/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013133.html
Warn user that bgp is setting maximum-paths larger than MULTIPATH_NUM
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-87
http://patchwork.quagga.net/patch/1342/
rib->nexthop_num is double incremented
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-88
https://bugzilla.quagga.net/show_bug.cgi?id=833
ospf trap on state change seems to send incorrect value for
ospfNbrState
Not tested (my CI system isn’t setup for bugzilla patches)
http://patchwork.quagga.net/patch/1344/
zebra: Fix leaked sockets in rtadv.c
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-89
http://patchwork.quagga.net/patch/1348/
Consolidate error reporting for zclient_read_header
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-90
http://patchwork.quagga.net/patch/1347/
https://lists.quagga.net/pipermail/quagga-dev/2015-September/013169.html
bgpd: If route-map does not exist DENY for redistribute statements
Config compatibility / UI expectations - worth getting wider feedback
on?
PASS. Results: https://ci1.netdef.org/browse/QUAGGA-QPWORK-91
http://patchwork.quagga.net/patch/1349/
distro: fix redhat/quagga.spec.in
Not tested with CI. (I’m currently ignoring the SPEC file and use my
own)
However, would prefer to get more time on this. It kind of conflicts
with
the other patch (the one I submitted with pimd.init/service files) as it
doesn’t start pimd and doesn’t install these files.
A quick look at it also gives me the impression that it won’t work on
CentOS 7 / RedHat 7 as it uses the old init files for startup instead of
the services files.
Regards,
Martin Winter
mwin...@opensourcerouting.org
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
