[kea-dev] OpenBSD: Kea 1.2.0

[Marcin Siodelski]

Patrik,

Apologies for slightly delayed answer to this.

On 03.07.2017 21:05, Patrik Lundin wrote:
> Hello,
> 
> As mentioned in
> https://lists.isc.org/pipermail/kea-users/2017-July/001050.html I am
> currently updating the Kea port in OpenBSD to 1.2.0.
> 
> First of all I notice that while the majority of the shared libraries
> have an explicit version configured using the "-version-info" flag in
> the Makefiles, there are a few that do not:
> ===
> kea-asiodns (src/lib/asiodns/Makefile.am)
> kea-exceptions (src/lib/exceptions/Makefile.am)
> kea-util-io (src/lib/util/io/Makefile.am)
> ===
> 

We leave them in their defaults which is 0:0:0 and that's what we
consider is their current version. None of these libraries have been
recently updated and they will rarely be.

> Is there a reason for this or is it just an oversight? In general it
> seems the other ones have settings along these lines:
> ===
> libkea_cfgclient_la_LDFLAGS = -no-undefined -version-info 3:0:0
> ===
> 

The other libraries have explicit versions added because they are
updated from the release to release and for each library which code have
been updated we bump up version numbers prior to a Kea release. If a
library is at version 0:0:0 and its code haven't been updated we just
leave the implicit version. If we happen to update the code in any of
those libs in the future it will be followed by a version bump, so it
will be given an explicit version-info, e.g. 1:0:0.

> The reason I ask about this is that port Makefiles in OpenBSD keep track
> of SHARED_LIBS version numbers. It helps me out a lot when there is clear
> indication that a library version needs to be bumped because of changes by
> upstream.
> 

Is it possible for the port Makefiles to simply assume that no
version-info means version 0?

> I also noticed one of the version numbers went backwards in 1.2.0 in
> comparision to 1.1.0. kea-dns++ went from 2.0 to 1.1. It can be seen in
> src/lib/dns/Makefile.am, git commit 88d32454640a1f5f80fc4cd75339647be0210426.
> Was this a mistake or am I missing something?
> 

Yes, we had made some mistakes in the versioning of this library in the
past, i.e. the version number had been bumped too high. In one of the
recent releases we amended this version number to what it should be. I
realize it is strange to go backwards but the current version is what it
should be.

> As Mike noted in the above mentioned thread Kea 1.2.0 requires a more
> modern compiler and because of that I am using clang. The build fails in
> the following way without patches:
> ===
> c++ --std=c++11 -DHAVE_CONFIG_H -I. -I../../.. -I../../../src/lib -I../../../src/lib -I/usr/local/include -DOS_BSD -DBOOST_DISABLE_THREADS=1 -I../../../ext/coroutine -DBOOST_ASIO_HEADER_ONLY -DBOOST_ASIO_DISABLE_THREADS=1 -Qunused-arguments -Wall -Wextra -Wnon-virtual-dtor -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -pthread -Wno-missing-field-initializers -Wno-unused-variable -Wno-unused-parameter -fPIC -O2 -pipe -MT watch_socket.lo -MD -MP -MF .deps/watch_socket.Tpo -c watch_socket.cc -fPIC -DPIC -o .libs/watch_socket.o
> watch_socket.cc:29:9: error: use of undeclared identifier 'pipe'
>     if (pipe(fds)) {
>         ^
> watch_socket.cc:72:22: error: use of undeclared identifier 'write'
>         int nbytes = write (source_, &MARKER, sizeof(MARKER));
>                      ^
> watch_socket.cc:111:22: error: use of undeclared identifier 'read'
>         int nbytes = read (sink_, &buf, sizeof(buf));
>                      ^
> watch_socket.cc:133:13: error: use of undeclared identifier 'close'
>         if (close(source_)) {
>             ^
> watch_socket.cc:142:13: error: use of undeclared identifier 'close'
>         if (close(sink_)) {
>             ^
> 5 errors generated.
> Error while executing c++ --std=c++11 -DHAVE_CONFIG_H -I. -I../../.. -I../../../src/lib -I../../../src/lib -I/usr/local/include -DOS_BSD -DBOOST_DISABLE_THREADS=1 -I../../../ext/coroutine -DBOOST_ASIO_HEADER_ONLY -DBOOST_ASIO_DISABLE_THREADS=1 -Qunused-arguments -Wall -Wextra -Wnon-virtual-dtor -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -pthread -Wno-missing-field-initializers -Wno-unused-variable -Wno-unused-parameter -fPIC -O2 -pipe -MT watch_socket.lo -MD -MP -MF .deps/watch_socket.Tpo -c watch_socket.cc -fPIC -DPIC -o .libs/watch_socket.o
> *** Error 1 in src/lib/util (Makefile:617 'watch_socket.lo')
> *** Error 1 in src/lib/util (Makefile:683 'all-recursive')
> *** Error 1 in src/lib (Makefile:431 'all-recursive')
> *** Error 1 in src (Makefile:432 'all-recursive')
> *** Error 1 in . (Makefile:600 'all-recursive')
> *** Error 1 in /home/ports/pobj/kea-1.2.0/kea-1.2.0 (Makefile:441 'all')
> *** Error 1 in . (/usr/ports/infrastructure/mk/bsd.port.mk:2695 '/usr/ports/pobj/kea-1.2.0/.build_done')
> *** Error 1 in /usr/ports/net/kea (/usr/ports/infrastructure/mk/bsd.port.mk:2398 'build')
> ===
> 
> This was fixed by Marc Espie (espie@) prior to me starting to look at
> 1.2.0 here:
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/net/kea/patches/patch-src_lib_util_watch_socket_cc
> 
> I'm thinking it is a good candidate for being merged upstream.
> 

I agree. I don't see any problem with porting this fix into Kea. In fact
I have just done it.

> I also noticed that while a ticket i previously filed
> (https://kea.isc.org/ticket/5072) has been solved, and the lfc-interval
> has been added to a bunch of places in the repo, it has not been added
> to the src/bin/keactrl/kea.conf.pre file which ends up as the default
> kea.conf. Is this intentional? I also notice "persist" is not configured
> by default, even if this seems to be strongly encouraged in the
> documentation.
> 

Kea configuration allows for omission of some of the configuration
parameters causing the servers to fallback to the defaults. For the sake
of config readability we purposely omit some of the parameters in the
default configuration file.

As for "persist", we don't specify it explicitly in the default config
file and let be the default true (enabled). Note that if we put the
"persist": true into the default config we'd give an impression that
this is a parameter that can be controlled just like other parameters,
while we rather want people to not disable it.

> Finally, here are the warnings thrown by the build. I'm guessing none of
> them are critical, so I am mostly mentioning them here for completeness
> (I have done this before as well):
> ===
> .libs/libkea-asiolink.so.2.0: warning: warning: strcat() is almost always misused, please use strlcat()
> .libs/libkea-asiolink.so.2.0: warning: warning: sprintf() is often misused, please use snprintf()
> .libs/libkea-dhcp++.so.2.0: warning: warning: random() may return deterministic values, is that what you want?
> .libs/libkea-asiolink.so.2.0: warning: warning: strcat() is almost always misused, please use strlcat()
> .libs/libkea-asiolink.so.2.0: warning: warning: sprintf() is often misused, please use snprintf()
> .libs/libkea-dhcp++.so.2.0: warning: warning: random() may return deterministic values, is that what you want?
> .libs/libkea-asiolink.so.2.0: warning: warning: strcat() is almost always misused, please use strlcat()
> .libs/libkea-asiolink.so.2.0: warning: warning: sprintf() is often misused, please use snprintf()
> .libs/libkea-dhcp++.so.2.0: warning: warning: random() may return deterministic values, is that what you want?
> .libs/libkea-asiolink.so.2.0: warning: warning: strcat() is almost always misused, please use strlcat()
> .libs/libkea-asiolink.so.2.0: warning: warning: sprintf() is often misused, please use snprintf()
> .libs/libkea-dhcp++.so.2.0: warning: warning: random() may return deterministic values, is that what you want?
> test_control.cc:(.text._ZN3isc8perfdhcp13PacketStorageINS_4dhcp4Pkt4EE9getRandomEv[_ZN3isc8perfdhcp13PacketStorageINS_4dhcp4Pkt4EE9getRandomEv]+0x45): warning: warning: rand() may return deterministic values, is that what you want?
> .libs/libkea-asiolink.so.2.0: warning: warning: strcat() is almost always misused, please use strlcat()
> .libs/libkea-asiolink.so.2.0: warning: warning: sprintf() is often misused, please use snprintf()
> .libs/libkea-dhcp++.so.2.0: warning: warning: random() may return deterministic values, is that what you want?
> .libs/libkea-asiolink.so.2.0: warning: warning: strcat() is almost always misused, please use strlcat()
> .libs/libkea-asiolink.so.2.0: warning: warning: sprintf() is often misused, please use snprintf()
> .libs/libkea-dhcp++.so.2.0: warning: warning: random() may return deterministic values, is that what you want?
> ===
> 

Thanks for those. They could be fixed but as you say they aren't
critical so they tend to end up with low priority.

Marcin Siodelski
ISC