On (21/03/13 15:25), Ondrej Kos wrote:
On 03/07/2013 08:39 PM, Jakub Hrozek wrote:
>On Thu, Mar 07, 2013 at 02:17:19PM +0100, Ondrej Kos wrote:
>>Hi,
>>
>>Attached find patch introducing support of the libnl3 netlink
>>library. The SSSD now builds with libnl3 by default, but is still
>>configurable to use the libnl in version 1 by adding configure
>>parameter
>>--with-netlink-lib=libnl
>>
>
>The attached patch "only" converts to being able to use the new API.
>There is also one more enhancement I wanted to do when we move to libnl3
>which is being able to detect when IPv6 address lifetime has been bumped
>but the address has not changed really.
>
>If you run sssd in any environment that also has IPv6 addresses with
>libnl1 you would receive RTM_NEWADDR notification even when nothing has
>seemingly changed. Can you check if it's still the case with libnl3
>and if it is, then also check if there is any way to filter these out?
>
This is still happening and I couldn't find any workaround.tgr isn't
responding now, so I'd put it in another patch, when I'll have some
pointers from him.
>That would be separate patch (and even a ticket if you like). I vaguely
>remember that tgr told me this was possible with libnl3 but I can't find
>the details right now.
>
>
>I'm thinking we might want to keep the same options (so that 3rd party
>build that use --without-libnl would continue working). Defaulting to
>another version of library would be OK in a new upstream release (if
>called out in the release notes). If libnl3 is not available but libnl1
>is, we should just fall back to libnl1 I think.
>
>So maybe we should keep the --with-libnl switch that could have these
>values:
> * auto (default) - try libnl3, then fall back to libnl1, if neither
> is found, warn
> * libnl1 - explicitly request libnl1, if not available, fail
> * libnl3 - explicitly request libnl3, if not available, fail
> * --without-libnl - disable libnl support explicitly, no checks
>
The options are now as you proposed:
- nothing specified:
tries the libnl3
if fails
tries the libnl(1)
if fails
prints warning
--with-libnl=libnl3
tries the libnl3
if fails
prints error and fails
--with-libnl=libnl1
tries the libnl(1)
if fails
prints error and fails
--with-libnl=no / --without-libnl
libnl disabled by default
>Why do you check for libnl-cli here? That's the command line tools..
>
>Also is there any reason to require 3.2 as the minimum version? Was
>there any feature present in 3.2 upstream that would prevent us from
>using 3.0 or 3.1 ?
This was because the check for libnl-cli exports the correct _LIBS
and _CFLAGS, replaced with only the needed components
>>+#define nlw_disable_seq_check nl_socket_disable_seq_check
>>+
>>+#define nlw_geterror nl_geterror
>>+
>>+#define HAVE_NL_SET_PASSCRED 1
>>+#define HAVE_NL_SOCKET_ADD_MEMBERSHIP 1
>>+#define HAVE_NL_SOCKET_MODIFY_CB 1
>>+
>>+#else /* LIBNL1 */
>>+
>
>Maybe we should be paranoid here, check for HAVE_LIBNL1 in this branch
>and simply #error out in the #else branch.
>
>>+#define nlw_destroy_handle nl_handle_destroy
>>+#define nlw_alloc nl_handle_alloc
>>+#define nlw_handle nl_handle
>>+#define nlw_disable_seq_check nl_disable_sequence_check
>>+
>>+#define nlw_geterror(error) nl_geterror()
>
>This seems strange, the error variable is not passed on to the
>nl_geterror. Also nlw_geterror expands to nl_geterror for both libnl1
>and libnl3.
>
This is because the nl_geterror changet between versions. In libnl3
it takes an argument representing error message number, while in
libnl1 it had no argument at all.
>Why did you rename the handle to nlp? It's not wrong, I just don't see
>the point, sometimes the variable name change is the only change in
>certain hunks.
It seemed better than keep it nlh (h for handle) while we are using
the nl_handle or nl_socket depending or netlink version. So i set it
to nlp - netlink pointer, it seemed appropriate. Should i change it
back?
New patch is attached.
Ondra
diff --git a/src/external/libnl.m4 b/src/external/libnl.m4
index 36619ff87490fa94651255224b9fb57e3194ecbc..1818065c8ebcf222bd4558d1437faf25d0e9fc8e
100644
--- a/src/external/libnl.m4
+++ b/src/external/libnl.m4
@@ -1,11 +1,19 @@
dnl A macro to check the availability and version of libnetlink
-AC_DEFUN([AM_CHECK_LIBNL],
+AC_DEFUN([AM_CHECK_LIBNL1],
[
- PKG_CHECK_MODULES(libnl, libnl-1 >= 1.1,[
+ PKG_CHECK_MODULES(LIBNL, libnl-1 >= 1.1,[
+
HAVE_LIBNL=1
+ HAVE_LIBNL1=1
I think that previous two lines should be renamed. It works,
but it may be
confusing. It seems that previous variables have something common with
AC_DEFINE_UNQUTOED few lines below, but they are two different things. Previous
two lines define (bash) variables, which are used later in src/conf_macros.m4
after using this macro (AM_CHECK_LIBNL1) and AC_DEFINE define C preprocesor
variable in config.h
+
AC_SUBST(HAVE_LIBNL)
+ AC_SUBST(HAVE_LIBNL1)
I think that both previous lines with AC_SUBST could
be removed, because there
are no substitution @HAVE_LIBNL@ anywhere in the files listed in
AC_CONFIG_FILES.
+
AC_DEFINE_UNQUOTED(HAVE_LIBNL, 1, [Build with libnetlink support])
+ AC_DEFINE_UNQUOTED(HAVE_LIBNL1, 1, [Libnetlink version = 1])
Similar issues are also in src/external/libnl3.m4, maybe because it was
inspired by previous file.
But someone more experienced with autotools should confirm my statements.
If I am misunderstood, please explain me it.
LS