[SSSD] [PATCH] Provide libnl3 support

Jakub Hrozek jhrozek at redhat.com
Thu Mar 21 16:46:31 UTC 2013


On Thu, Mar 21, 2013 at 05:21:09PM +0100, Jakub Hrozek wrote:
> On Thu, Mar 21, 2013 at 03:25:16PM +0100, 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.
> > 
> 
> OK, but please open a ticket.
> 
> > >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.
> > 
> 
> Ah, I see, they used a global errno in libnl1. Terrible :)
> 
> > >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?
> > 
> > 
> 
> I don't really mind.
> 
> This is better, almost there. See a couple of comments inline.

Also please amend the specfile so that for F-18 and higher, libnl3 is
BuildRequired.



More information about the sssd-devel mailing list