[SSSD] [PATCH] Provide libnl3 support

Jakub Hrozek jhrozek at redhat.com
Fri Mar 22 13:19:06 UTC 2013


On Fri, Mar 22, 2013 at 10:57:18AM +0100, Ondrej Kos wrote:
> Hi,
> 
> Thanks for the review, new patch is attached.

Almost there. See inline for a couple of last requests.

> --- a/contrib/sssd.spec.in
> +++ b/contrib/sssd.spec.in
> @@ -122,14 +122,17 @@ BuildRequires: libselinux-devel
>  BuildRequires: libsemanage-devel
>  BuildRequires: bind-utils
>  BuildRequires: keyutils-libs-devel
> -BuildRequires: libnl-devel
>  BuildRequires: gettext-devel
>  BuildRequires: pkgconfig
>  BuildRequires: findutils
>  BuildRequires: glib2-devel
>  BuildRequires: selinux-policy-targeted
> +%if (0%{?fedora} < 18)
> +BuildRequires: libnl-devel
> +%endif
>  %if (0%{?fedora} >= 18)
>  BuildRequires: libcmocka-devel
> +BuildRequires: libnl3-devel
>  %endif
>  

I think this would mean that RHEL would be build without libnl
completely because it would match neither if?

> index 36619ff87490fa94651255224b9fb57e3194ecbc..1ccc4308a6f776f0f2241a9e54c472a193493c2a 100644
> --- a/src/external/libnl.m4
> +++ b/src/external/libnl.m4
> @@ -1,34 +1,88 @@
> +dnl A macro to check if this particular version of libnl supports particular common libnl functions
> +AC_DEFUN([AM_CHECK_LIBNL_FCS],
> +[
> +    if test x$LNL_CHECK_LIB != x; then
> +

Please pass function parameter instead of passing the info via a global
variable:
http://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Macro-Definitions.html

> +        AC_CHECK_LIB($LNL_CHECK_LIB,
> +                     [nl_socket_add_membership],
> +                     [AC_DEFINE([HAVE_NL_SOCKET_ADD_MEMBERSHIP], 1, [Does libnl have nl_socket_add_membership?])
> +                     ],
> +        )
> +
> +        AC_CHECK_LIB($LNL_CHECK_LIB,
> +                     [nl_socket_modify_cb],
> +                     [AC_DEFINE([HAVE_NL_SOCKET_MODIFY_CB], 1, [Does libnl have nl_socket_modify_cb?])
> +                     ],
> +        )
> +
> +        AC_CHECK_LIB($LNL_CHECK_LIB,
> +                     [rtnl_route_get_oif],
> +                     [AC_DEFINE([HAVE_RTNL_ROUTE_GET_OIF], 1, [Does libnl have rtnl_route_get_oif?])
> +                     ],
> +        )
> +
> +    fi
> +])
> +
>  dnl A macro to check the availability and version of libnetlink
> +AC_DEFUN([AM_CHECK_LIBNL1],
> +[
> +    PKG_CHECK_MODULES(LIBNL, libnl-1 >= 1.1,[
> +
> +        HAVE_LIBNL=1
> +        HAVE_LIBNL1=1
> +
> +        LNL_CHECK_LIB=nl

The value of this variable would be just passed to the
AM_CHECK_LIBNL_FCS macro as parameter.

>      AC_SUBST(LIBNL_CFLAGS)
>      AC_SUBST(LIBNL_LIBS)
> diff --git a/src/monitor/monitor_netlink.c b/src/monitor/monitor_netlink.c
> index 3842c4f1612367ab2d33a1000d81fd29506fbf4d..d7d1a341435852e14553421f7b2f48e513a93671 100644
> --- a/src/monitor/monitor_netlink.c
> +++ b/src/monitor/monitor_netlink.c
> @@ -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,12 +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
> -
>  #define SYSFS_IFACE_TEMPLATE "/sys/class/net/%s"
>  #define SYSFS_IFACE_PATH_MAX (16+IFNAMSIZ)
>  
> @@ -73,6 +68,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_set_passcred        nl_socket_set_passcred
> +#define nlw_disable_seq_check   nl_socket_disable_seq_check
> +#define nlw_route_get_oif       rtnlw_route_get_oif

No need for the indirect wrapper see below.

> @@ -104,6 +129,25 @@ static int netlink_ctx_destructor(void *ptr)
>   *                      Utility functions
>   *******************************************************************/
>  
> +#ifndef HAVE_RTNL_ROUTE_GET_OIF
> +/* rtnl_route_get_oif removed from libnl3 */
> +int
> +rtnlw_route_get_oif(struct rtnl_route * route)
> +{
> +    struct rtnl_nexthop * nh;
> +    int hops;
> +
> +    hops = rtnl_route_get_nnexthops(route);
> +    if (hops <= 0) {
> +        return 0;
> +    }
> +
> +    nh = rtnl_route_nexthop_n(route, 0);
> +
> +    return rtnl_route_nh_get_ifindex(nh);
> +}

You can always call this wrapper in the code, move the #ifdef inside the
function and call rtnl_route_get_oif() directly in the #else case. That
would get rid of the double indirection via wrapper and then with macro.

The rest looks good to me.



More information about the sssd-devel mailing list