[SSSD] [PATCH] Provide libnl3 support
Ondrej Kos
okos at redhat.com
Tue Mar 26 14:12:24 UTC 2013
On 03/22/2013 02:19 PM, Jakub Hrozek wrote:
> 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.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
Thanks for the review, new patch is attached
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Provide-libnl3-support.patch
Type: text/x-patch
Size: 19594 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130326/b9ae9ef8/attachment.bin>
More information about the sssd-devel
mailing list