[SSSD] [PATCH] Provide libnl3 support

Lukas Slebodnik lslebodn at redhat.com
Thu Mar 21 17:03:35 UTC 2013


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



More information about the sssd-devel mailing list