[SSSD] [PATCH] Provide libnl3 support

Jakub Hrozek jhrozek at redhat.com
Thu Mar 7 19:39:02 UTC 2013


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?

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.

> Ondra
> -- 
> Ondrej Kos
> Associate Software Engineer
> Identity Management
> Red Hat Czech
> 
> phone: +420-532-294-558
> cell:  +420-736-417-909
> ext:   82-62558
> loc:   1013 Brno 1 office
> irc:   okos @ #sssd #brno

> From 5a29554b77bcd426e6fa6fbe1422d087d4cf4cd4 Mon Sep 17 00:00:00 2001
> From: Ondrej Kos <okos at redhat.com>
> Date: Mon, 18 Feb 2013 14:43:19 +0100
> Subject: [PATCH] Provide libnl3 support
> 
> https://fedorahosted.org/sssd/ticket/812
> 
> Update the monitor code to be using the new libnl3 API.
> 
> Added configure option
> --with-netlink-lib=<library>
> which uses "libnl3" by default, or optionally "libnl" to build with
> libnl v1.
> ---
>  BUILD.txt                     |   2 +-
>  Makefile.am                   |   4 +-
>  configure.ac                  |  19 +++++--
>  src/conf_macros.m4            |  28 ++++++----
>  src/external/libnl.m4         |  11 ++--
>  src/external/libnl3.m4        |  18 ++++++
>  src/monitor/monitor_netlink.c | 124 ++++++++++++++++++++++++++++++------------
>  7 files changed, 151 insertions(+), 55 deletions(-)
>  create mode 100644 src/external/libnl3.m4
> 
> diff --git a/BUILD.txt b/BUILD.txt
> index 0eb3f468c14eb213a97a1522334f43653c80ff75..263129215d6af43869a9d97bbd291436b2cc914d 100644
> --- a/BUILD.txt
> +++ b/BUILD.txt
> @@ -26,7 +26,7 @@ yum install openldap-devel gettext libtool pcre-devel c-ares-devel \
>      libxml2 pam-devel nss-devel libtevent python-devel \
>      libtevent-devel libtdb libtdb-devel libtalloc libtalloc-devel \
>      libldb libldb-devel popt-devel c-ares-devel check-devel \
> -    doxygen libselinux-devel libsemanage-devel bind-utils libnl-devel \
> +    doxygen libselinux-devel libsemanage-devel bind-utils libnl3-devel \
>      gettext-devel glib2-devel
>  
>  ding-libs are available in Fedora 14 and later version:
> diff --git a/Makefile.am b/Makefile.am
> index 79e6e571cc12fd38f78ef0b031bb867b0b650eb0..7c70dec55e192bcf6476d0b52d3333d442ef2cef 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -257,7 +257,7 @@ AM_CPPFLAGS = \
>      $(COLLECTION_CFLAGS) \
>      $(INI_CONFIG_CFLAGS) \
>      $(DHASH_CFLAGS) \
> -    $(LIBNL_CFLAGS) \
> +    $(NETLINK_CFLAGS) \
>      $(OPENLDAP_CFLAGS) \
>      $(GLIB2_CFLAGS) \
>      -DLIBDIR=\"$(libdir)\" \
> @@ -573,7 +573,7 @@ sssd_SOURCES = \
>      src/confdb/confdb_setup.c
>  sssd_LDADD = \
>      $(SSSD_LIBS) \
> -    $(LIBNL_LIBS) \
> +    $(NETLINK_LIBS) \
>      $(KEYUTILS_LIBS) \
>      libsss_util.la
>  
> diff --git a/configure.ac b/configure.ac
> index af976151ad3c90bfcb056dd7d8d24bbc8b884ca5..0d5c0d8570b76528d03ed61a30f505392929bcab 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -114,7 +114,6 @@ WITH_PYTHON_BINDINGS
>  WITH_SELINUX
>  WITH_NSCD
>  WITH_SEMANAGE
> -WITH_LIBNL
>  WITH_NOLOGIN_SHELL
>  WITH_APP_LIBS
>  WITH_SUDO
> @@ -145,7 +144,6 @@ m4_include([src/external/crypto.m4])
>  m4_include([src/external/nscd.m4])
>  m4_include([src/external/nsupdate.m4])
>  m4_include([src/external/libkeyutils.m4])
> -m4_include([src/external/libnl.m4])
>  m4_include([src/external/systemd.m4])
>  m4_include([src/external/pac_responder.m4])
>  m4_include([src/util/signal.m4])
> @@ -168,6 +166,15 @@ if test x$initscript = xsystemd; then
>      WITH_SYSTEMD_UNIT_DIR
>  fi
>  
> +WITH_NETLINK_LIB
> +if test x$netlink_lib = xlibnl3; then
> +    m4_include([src/external/libnl3.m4])

I think you can safely always include the libnl external m4 files, there
are only AC_DEFUN directives. Probably there is also no reason to keep
them separate, just one libnl.m4 would do.

> +    AC_DEFINE_UNQUOTED(HAVE_LIBNL3, 1, [Libnetlink version 3])

Why do you define HAVE_LIBNL3 unconditionally if the branch is selected?
The WITH_NETLINK_LIB macro would give you BUILD_LIBNL1 or LIBNL3 which
is the only variable checked later in the code.

> +else
> +    m4_include([src/external/libnl.m4])
> +    AC_DEFINE_UNQUOTED(HAVE_LIBNL1, 1, [Libnetlink version 1])
> +fi
> +
>  PKG_CHECK_MODULES([DBUS],[dbus-1])
>  dnl if test -n "`$PKG_CONFIG --modversion dbus-1 | grep '^0\.'`" ; then
>  if ! $PKG_CONFIG --atleast-version 1.0.0 dbus-1; then
> @@ -227,8 +234,12 @@ if test x$HAVE_SEMANAGE != x -a x$HAVE_SELINUX != x; then
>      AM_CHECK_SEMANAGE
>  fi
>  
> -if test x$BUILD_LIBNL != x; then
> -    AM_CHECK_LIBNL
> +if test x$BUILD_LIBNL1 != x; then
> +    AM_CHECK_LIBNL1
> +fi
> +
> +if test x$BUILD_LIBNL3 != x; then
> +    AM_CHECK_LIBNL3
>  fi
>  

Because the values are exclusive maybe you could use elif here? But
please check whether elif is portable (it should be, we use it elsewhere
in our macros but better safe than sorry).

>  if test x$HAVE_SYSTEMD_UNIT != x; then
> diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
> index 65f4d6ce9abf3f20d93f08cb9f18cd4f077dfa77..b51d32972f3003dbee398d7590afa38e906f671c 100644
> --- a/src/conf_macros.m4
> +++ b/src/conf_macros.m4
> @@ -368,18 +368,26 @@ AC_DEFUN([WITH_SEMANAGE],
>      AM_CONDITIONAL([BUILD_SEMANAGE], [test x"$with_semanage" = xyes])
>    ])
>  
> -AC_DEFUN([WITH_LIBNL],
> -  [ AC_ARG_WITH([libnl],
> -                [AC_HELP_STRING([--with-libnl],
> -                                [Whether to build with libnetlink support [AUTO]]
> +AC_DEFUN([WITH_NETLINK_LIB],
> +  [ AC_ARG_WITH([netlink_lib],
> +                [AC_HELP_STRING([--with-netlink-lib=<library>],
> +                                [Which library to use for netlink layer (libnl, libnl3) [libnl3]]
>                                 )
> -                ],
> -                [],
> -                with_libnl=yes
> +                ]
>                 )

I'm wondering about the default here. Previously the default was that if
libnl-devel was available and libnl was not explicitly disabled, then
libnl would have been used. You could opt out using --without-libnl
switch.

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

> -    if test x"$with_libnl" = xyes; then
> -        BUILD_LIBNL=1
> -        AC_SUBST(BUILD_LIBNL)
> +    netlink_lib="libnl3"
> +    if test x"$with_netlink_lib" != x; then
> +        netlink_lib=$with_netlink_lib
> +    fi
> +
> +    if test x"$netlink_lib" == x"libnl3"; then
> +        BUILD_LIBNL3=1
> +        AC_SUBST(BUILD_LIBNL3)
> +    else
> +        if test x"$netlink_lib" == x"libnl"; then

This again calls for elif.

> +            BUILD_LIBNL1=1
> +            AC_SUBST(BUILD_LIBNL1)
> +        fi
>      fi
>    ])
>  
> diff --git a/src/external/libnl.m4 b/src/external/libnl.m4
> index 36619ff87490fa94651255224b9fb57e3194ecbc..3e902a8f0587f67933170154adf4a48fb796bab0 100644
> --- a/src/external/libnl.m4
> +++ b/src/external/libnl.m4
> @@ -1,11 +1,14 @@
>  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(NETLINK, libnl-1 >= 1.1,[
>          HAVE_LIBNL=1
> +        HAVE_LIBNL1=1
>          AC_SUBST(HAVE_LIBNL)
> +        AC_SUBST(HAVE_LIBNL1)
>          AC_DEFINE_UNQUOTED(HAVE_LIBNL, 1, [Build with libnetlink support])
> +        AC_DEFINE_UNQUOTED(HAVE_LIBNL1, 1, [Libnetlink version 1])
>  
>  	    AC_CHECK_HEADERS(netlink.h)
>  	    AC_CHECK_LIB(nl, nl_connect, [ LIBNL_LIBS="-lnl" ], [AC_MSG_ERROR([libnl is required])])
> @@ -30,6 +33,6 @@ AC_DEFUN([AM_CHECK_LIBNL],
>  	    )
>      ],[AC_MSG_WARN([Netlink support unavailable or too old])])
>  
> -    AC_SUBST(LIBNL_CFLAGS)
> -    AC_SUBST(LIBNL_LIBS)
> +    AC_SUBST(NETLINK_CFLAGS)
> +    AC_SUBST(NETLINK_LIBS)
>  ])
> diff --git a/src/external/libnl3.m4 b/src/external/libnl3.m4
> new file mode 100644
> index 0000000000000000000000000000000000000000..c712daca2a3d0acbdd3986bb75e2a2242eab69a9
> --- /dev/null
> +++ b/src/external/libnl3.m4
> @@ -0,0 +1,18 @@
> +dnl A macro to check the availability of libnetlink version 3
> +
> +AC_DEFUN([AM_CHECK_LIBNL3],
> +[
> +    PKG_CHECK_MODULES(NETLINK, libnl-cli-3.0 >= 3.2, [

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 ?

> +        HAVE_LIBNL=1
> +        HAVE_LIBNL3=1
> +        AC_SUBST(HAVE_LIBNL)
> +        AC_SUBST(HAVE_LIBNL3)
> +        AC_DEFINE_UNQUOTED(HAVE_LIBNL, 1, [Build with libnetlink support])
> +        AC_DEFINE_UNQUOTED(HAVE_LIBNL3, 1, [Libnetlink version 3])
> +
> +        AC_CHECK_HEADERS(netlink/netlink.h)
> +    ],[AC_MSG_WARN([Netlink v3 support unavailable])])
> +
> +    AC_SUBST(NETLINK_CFLAGS)
> +    AC_SUBST(NETLINK_LIBS)
> +])
> diff --git a/src/monitor/monitor_netlink.c b/src/monitor/monitor_netlink.c
> index 3842c4f1612367ab2d33a1000d81fd29506fbf4d..4850a22b48040f45e949cbbbd3ff6452fc8e7434 100644
> --- a/src/monitor/monitor_netlink.c
> +++ b/src/monitor/monitor_netlink.c

There is a compilation warning:
../src/monitor/monitor_netlink.c: In function ‘rtnl_route_get_oif’:
../src/monitor/monitor_netlink.c:146:9: warning: return makes integer
from pointer without a cast [enabled by default]

> @@ -47,6 +47,7 @@
>  #include <netlink/route/rtnl.h>
>  #include <netlink/route/route.h>
>  #include <netlink/handlers.h>
> +#include <netlink/socket.h>
>  #endif
>  
>  /* Linux header file confusion causes this to be undefined. */
> @@ -54,8 +55,6 @@
>  #define SOL_NETLINK 270
>  #endif
>  
> -#define nlw_get_fd nl_socket_get_fd
> -#define nlw_recvmsgs_default nl_recvmsgs_default
>  #define nlw_get_pid nl_socket_get_local_port
>  #define nlw_object_match nl_object_match_filter
>  #define NLW_OK NL_OK
> @@ -73,6 +72,36 @@
>  
>  #define BUFSIZE 8
>  
> +#ifdef HAVE_LIBNL
> +
> +/* Wrappers determining use of libnl version 1 or 3 */
> +#ifdef HAVE_LIBNL3
> +
> +#define nlw_destroy_handle      nl_socket_free
> +#define nlw_alloc               nl_socket_alloc
> +#define nlw_handle              nl_sock

nl_sock and nl_handle are types, I think we should use typeded instead
of #define. nlw stands for "netlink wrapper".

> +#define nl_set_passcred         nl_socket_set_passcred

I don't think we should define anything in the nl_ namespace ourselves.
If this is a function that was present in libnl1 as nl_set_passcred then
you should define nlw_set_passcred that expands to nl_set_passcred for
libnl1 and to nl_socket_set_passcred for libnl3.

> +#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.

> +
> +#endif /* HAVE_LIBNL3 */
> +
> +#endif /* HAVE_LIBNL */
> +
>  enum nlw_msg_type {
>      NLW_LINK,
>      NLW_ROUTE,
> @@ -82,7 +111,7 @@ enum nlw_msg_type {
>  
>  struct netlink_ctx {
>  #ifdef HAVE_LIBNL
> -    struct nl_handle *nlh;
> +    struct nlw_handle *nlp;
>  #endif
>      struct tevent_fd *tefd;
>  
> @@ -96,7 +125,7 @@ static int netlink_ctx_destructor(void *ptr)
>      struct netlink_ctx *nlctx;
>      nlctx = talloc_get_type(ptr, struct netlink_ctx);
>  
> -    nl_handle_destroy(nlctx->nlh);
> +    nlw_destroy_handle(nlctx->nlp);
>      return 0;
>  }
>  
> @@ -104,6 +133,25 @@ static int netlink_ctx_destructor(void *ptr)
>   *                      Utility functions
>   *******************************************************************/
>  
> +#ifdef HAVE_LIBNL3
> +/* rtnl_route_get_oif removed from libnl3 */
> +int
> +rtnl_route_get_oif (struct rtnl_route * route)
> +{
> +	struct rtnl_nexthop * nh;
> +	int hops;
> +
> +	hops = rtnl_route_get_nnexthops (route);
> +    if (hops <= 0) {
> +        return NULL;
> +    }
> +
> +	nh = rtnl_route_nexthop_n(route, 0);
> +
> +	return rtnl_route_nh_get_ifindex (nh);
> +}
> +#endif
> +

Please provide a nlw_route_get_oif wrapper that expands to
rtnl_route_get_oif if libnl1 is used and to this alternative
implementation if libnl3 is used.

In general we should never implement a function that overrides anything in
namespace of another C library. We should rather use something like nlw
or sss_nl_. (Had I implemented the netlink support from scratch, I would
probably use sss_nl or simply snl_ but I'm not sure changing the prefix
would bring us anything)

>  static bool has_wireless_extension(const char *ifname)
>  {
>      int s;
> @@ -270,7 +318,7 @@ static void nladdr_to_string(struct nl_addr *nl, char *buf, size_t bufsize)
>   * Wrappers for different capabilities of different libnl versions
>   *******************************************************************/
>  
> -static bool nlw_accept_message(struct nl_handle *nlh,
> +static bool nlw_accept_message(struct nlw_handle *nlp,
>                                 const struct sockaddr_nl *snl,
>                                 struct nlmsghdr *hdr)
>  {
> @@ -290,7 +338,7 @@ static bool nlw_accept_message(struct nl_handle *nlh,
>      /* And any multicast message directed to our netlink PID, since multicast
>       * currently requires CAP_ADMIN to use.
>       */
> -    local_port = nlw_get_pid(nlh);
> +    local_port = nl_socket_get_local_port(nlp);

Correct, nlw_get_pid used to be a wrapper when we supported also libnl older
than 1.1 but it only expands to nl_socket_get_local_port now. Can you
also remove nlw_object_match and NLW_OK ?

>      if ((hdr->nlmsg_pid == local_port) && snl->nl_groups) {
>          accept_msg = true;
>      }
> @@ -366,27 +414,28 @@ static bool nlw_is_link_object(struct nl_object *obj)
>      return is_link_object;
>  }
>  
> -static int nlw_enable_passcred(struct nl_handle *nlh)
> +static int nlw_enable_passcred(struct nlw_handle *nlp)

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.

>  {
>  #ifndef HAVE_NL_SET_PASSCRED
>      return EOK;                      /* not available in this version */
>  #else
> -    return nl_set_passcred(nlh, 1);  /* 1 = enabled */
> +    return nl_set_passcred(nlp, 1);  /* 1 = enabled */
>  #endif
>  }
>  
> -static int nlw_group_subscribe(struct nl_handle *nlh, int group)
> +static int nlw_group_subscribe(struct nlw_handle *nlp, int group)
>  {
>      int ret;
>  
>  #ifdef HAVE_NL_SOCKET_ADD_MEMBERSHIP
> -    ret = nl_socket_add_membership(nlh, group);
> +    ret = nl_socket_add_membership(nlp, group);
>      if (ret != 0) {
> -        DEBUG(1, ("Unable to add membership: %s\n", nl_geterror()));
> +        DEBUG(SSSDBG_CRIT_FAILURE,
> +              ("Unable to add membership: %s\n", nlw_geterror(ret)));
>          return ret;
>      }
>  #else
> -     int nlfd = nlw_get_fd(nlh);
> +     int nlfd = nl_socket_get_fd(nlp);
>  
>       errno = 0;
>       ret = setsockopt(nlfd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP,
> @@ -401,13 +450,13 @@ static int nlw_group_subscribe(struct nl_handle *nlh, int group)
>       return 0;
>  }
>  
> -static int nlw_groups_subscribe(struct nl_handle *nlh, int *groups)
> +static int nlw_groups_subscribe(struct nlw_handle *nlp, int *groups)
>  {
>      int ret;
>      int i;
>  
>      for (i=0; groups[i]; i++) {
> -        ret = nlw_group_subscribe(nlh, groups[i]);
> +        ret = nlw_group_subscribe(nlp, groups[i]);
>          if (ret != EOK) return ret;
>      }
>  
> @@ -435,7 +484,7 @@ static int event_msg_recv(struct nl_msg *msg, void *arg)
>      hdr = nlmsg_hdr(msg);
>      snl = nlmsg_get_src(msg);
>  
> -    if (!nlw_accept_message(ctx->nlh, snl, hdr)) {
> +    if (!nlw_accept_message(ctx->nlp, snl, hdr)) {
>          return NL_SKIP;
>      }
>  
> @@ -490,15 +539,16 @@ static int event_msg_ready(struct nl_msg *msg, void *arg)
>      return NLW_OK;
>  }
>  
> -static int nlw_set_callbacks(struct nl_handle *nlh, void *data)
> +static int nlw_set_callbacks(struct nlw_handle *nlp, void *data)
>  {
>      int ret = EIO;
>  
>  #ifndef HAVE_NL_SOCKET_MODIFY_CB
> -    struct nl_cb *cb = nl_handle_get_cb(nlh);
> +    struct nl_cb *cb = nl_handle_get_cb(nlp);
>      ret = nl_cb_set(cb, NL_CB_MSG_IN, NL_CB_CUSTOM, event_msg_recv, data);
>  #else
> -    ret = nl_socket_modify_cb(nlh, NL_CB_MSG_IN, NL_CB_CUSTOM, event_msg_recv, data);
> +    ret = nl_socket_modify_cb(nlp, NL_CB_MSG_IN, NL_CB_CUSTOM, event_msg_recv,
> +                              data);
>  #endif
>      if (ret != 0) {
>          DEBUG(1, ("Unable to set validation callback\n"));
> @@ -508,7 +558,8 @@ static int nlw_set_callbacks(struct nl_handle *nlh, void *data)
>  #ifndef HAVE_NL_SOCKET_MODIFY_CB
>      ret = nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, event_msg_ready, data);
>  #else
> -    ret = nl_socket_modify_cb(nlh, NL_CB_VALID, NL_CB_CUSTOM, event_msg_ready, data);
> +    ret = nl_socket_modify_cb(nlp, NL_CB_VALID, NL_CB_CUSTOM, event_msg_ready,
> +                              data);
>  #endif
>      if (ret != 0) {
>          DEBUG(1, ("Unable to set receive callback\n"));
> @@ -536,6 +587,7 @@ static void route_msg_debug_print(struct rtnl_route *route_obj)
>      DEBUG(SSSDBG_TRACE_LIBS, ("route idx %d flags %#X family %d addr %s/%d\n",
>            rtnl_route_get_oif(route_obj), rtnl_route_get_flags(route_obj),
>            rtnl_route_get_family(route_obj), buf, prefixlen));
> +
>  }
>  
>  /*
> @@ -665,12 +717,12 @@ static void netlink_fd_handler(struct tevent_context *ev, struct tevent_fd *fde,
>      struct netlink_ctx *nlctx = talloc_get_type(data, struct netlink_ctx);
>      int ret;
>  
> -    if (!nlctx || !nlctx->nlh) {
> +    if (!nlctx || !nlctx->nlp) {
>          DEBUG(1, ("Invalid netlink handle, this is most likely a bug!\n"));
>          return;
>      }
>  
> -    ret = nlw_recvmsgs_default(nlctx->nlh);
> +    ret = nl_recvmsgs_default(nlctx->nlp);
>      if (ret != EOK) {
>          DEBUG(1, ("Error while reading from netlink fd\n"));
>          return;
> @@ -699,17 +751,21 @@ int setup_netlink(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
>      nlctx->change_cb = change_cb;
>      nlctx->cb_data   = cb_data;
>  
> -    /* allocate the libnl handle and register the default filter set */
> -    nlctx->nlh = nl_handle_alloc();
> -    if (!nlctx->nlh) {
> -        DEBUG(1, (("unable to allocate netlink handle: %s"),
> -                   nl_geterror()));
> +    /* allocate the libnl handle/socket and register the default filter set */
> +    nlctx->nlp = nlw_alloc();
> +    if (!nlctx->nlp) {
> +#ifdef HAVE_LIBNL1
> +        DEBUG(SSSDBG_CRIT_FAILURE, (("unable to allocate netlink handle: %s"),
> +                                     nl_geterror()));
> +#elif HAVE_LIBNL3
> +        DEBUG(SSSDBG_CRIT_FAILURE, ("unable to allocate netlink socket\n"));
> +#endif

Can't the error handler be the same for both versions and use
nlw_geterror()?

>          ret = ENOMEM;
>          goto fail;
>      }
>  
>      /* Register our custom message validation filter */
> -    ret = nlw_set_callbacks(nlctx->nlh, nlctx);
> +    ret = nlw_set_callbacks(nlctx->nlp, nlctx);
>      if (ret != 0) {
>          DEBUG(1, ("Unable to set callbacks\n"));
>          ret = EIO;
> @@ -717,31 +773,31 @@ int setup_netlink(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
>      }
>  
>      /* Try to start talking to netlink */
> -    ret = nl_connect(nlctx->nlh, NETLINK_ROUTE);
> +    ret = nl_connect(nlctx->nlp, NETLINK_ROUTE);
>      if (ret != 0) {
> -        DEBUG(1, ("Unable to connect to netlink: %s\n", nl_geterror()));
> +        DEBUG(1, ("Unable to connect to netlink: %s\n", nlw_geterror(ret)));

Please also convert DEBUG levels when changing the messages.

>          ret = EIO;
>          goto fail;
>      }
>  
> -    ret = nlw_enable_passcred(nlctx->nlh);
> +    ret = nlw_enable_passcred(nlctx->nlp);
>      if (ret != 0) {
> -        DEBUG(1, ("Cannot enable credential passing: %s\n", nl_geterror()));
> +        DEBUG(1, ("Cannot enable credential passing: %s\n", nlw_geterror(ret)));
>          ret = EIO;
>          goto fail;
>      }
>  
>      /* Subscribe to the LINK group for internal carrier signals */
> -    ret = nlw_groups_subscribe(nlctx->nlh, groups);
> +    ret = nlw_groups_subscribe(nlctx->nlp, groups);
>      if (ret != 0) {
>          DEBUG(1, ("Unable to subscribe to netlink monitor\n"));
>          ret = EIO;
>          goto fail;
>      }
>  
> -    nl_disable_sequence_check(nlctx->nlh);
> +    nlw_disable_seq_check(nlctx->nlp);
>  
> -    nlfd = nlw_get_fd(nlctx->nlh);
> +    nlfd = nl_socket_get_fd(nlctx->nlp);
>      flags = fcntl(nlfd, F_GETFL, 0);
>  
>      errno = 0;
> -- 
> 1.8.1.2
> 

> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list