| Message ID | 20200308070741.42542-1-james.hilliard1@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] package/connman: enable wireguard | expand |
Hello, On Sun, 8 Mar 2020 00:07:41 -0700 James Hilliard <james.hilliard1@gmail.com> wrote: > We also need to select libmnl when building with wireguard support or > with nftables. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > package/connman/Config.in | 5 +++++ > package/connman/connman.mk | 9 ++++++++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/package/connman/Config.in b/package/connman/Config.in > index ac012dda54..614b826f96 100644 > --- a/package/connman/Config.in > +++ b/package/connman/Config.in > @@ -33,6 +33,7 @@ config BR2_PACKAGE_CONNMAN_NFTABLES > bool "nftables" > depends on BR2_USE_WCHAR > depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12 > + select BR2_PACKAGE_LIBMNL How is this related to wireguard support ? > select BR2_PACKAGE_NFTABLES > help > Use nftables as firewall. > @@ -51,6 +52,10 @@ config BR2_PACKAGE_CONNMAN_WIFI > setup). ConnMan detects the start of wpa_supplicant > automatically. > > +config BR2_PACKAGE_CONNMAN_WIREGUARD > + bool "enable wireguard support" > + select BR2_PACKAGE_LIBMNL We already had a patch from Petr at http://patchwork.ozlabs.org/patch/1246181/ to add Wireguard support, but he was also selecting BR2_PACKAGE_WIREGUARD_TOOLS. Could you or Petr clarify this ? Best regards, Thomas
Hi Thomas, James, > On Sun, 8 Mar 2020 00:07:41 -0700 > James Hilliard <james.hilliard1@gmail.com> wrote: > > We also need to select libmnl when building with wireguard support or > > with nftables. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > package/connman/Config.in | 5 +++++ > > package/connman/connman.mk | 9 ++++++++- > > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/package/connman/Config.in b/package/connman/Config.in > > index ac012dda54..614b826f96 100644 > > --- a/package/connman/Config.in > > +++ b/package/connman/Config.in > > @@ -33,6 +33,7 @@ config BR2_PACKAGE_CONNMAN_NFTABLES > > bool "nftables" > > depends on BR2_USE_WCHAR > > depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12 > > + select BR2_PACKAGE_LIBMNL > How is this related to wireguard support ? Yes. > > select BR2_PACKAGE_NFTABLES > > help > > Use nftables as firewall. > > @@ -51,6 +52,10 @@ config BR2_PACKAGE_CONNMAN_WIFI > > setup). ConnMan detects the start of wpa_supplicant > > automatically. > > +config BR2_PACKAGE_CONNMAN_WIREGUARD > > + bool "enable wireguard support" > > + select BR2_PACKAGE_LIBMNL > We already had a patch from Petr at > http://patchwork.ozlabs.org/patch/1246181/ to add Wireguard support, > but he was also selecting BR2_PACKAGE_WIREGUARD_TOOLS. > Could you or Petr clarify this ? configure.ac found_libmnl="no" if (test "${firewall_type}" = "nftables" -o \ "${enable_wireguard}" != "no"); then PKG_CHECK_MODULES(LIBMNL, [libmnl >= 1.0.0], [found_libmnl="yes"], AC_MSG_ERROR([libmnl >= 1.0.0 not found])) AC_SUBST(LIBMNL_CFLAGS) AC_SUBST(LIBMNL_LIBS) fi AM_CONDITIONAL(LIBMNL, test "${found_libmnl}" != "no") IMHO this means that libmnl is required for nftables and wireguard. Kind regards, Petr
>>>>> "Petr" == Petr Vorel <petr.vorel@gmail.com> writes: Hi, >> > +config BR2_PACKAGE_CONNMAN_WIREGUARD >> > + bool "enable wireguard support" >> > + select BR2_PACKAGE_LIBMNL >> We already had a patch from Petr at >> http://patchwork.ozlabs.org/patch/1246181/ to add Wireguard support, >> but he was also selecting BR2_PACKAGE_WIREGUARD_TOOLS. >> Could you or Petr clarify this ? > configure.ac > found_libmnl="no" > if (test "${firewall_type}" = "nftables" -o \ > "${enable_wireguard}" != "no"); then > PKG_CHECK_MODULES(LIBMNL, [libmnl >= 1.0.0], [found_libmnl="yes"], > AC_MSG_ERROR([libmnl >= 1.0.0 not found])) > AC_SUBST(LIBMNL_CFLAGS) > AC_SUBST(LIBMNL_LIBS) > fi > AM_CONDITIONAL(LIBMNL, test "${found_libmnl}" != "no") > IMHO this means that libmnl is required for nftables and wireguard. Yes. What about wireguard-tools? Are they used by connman or does it directly talk the netlink protocol? I guess the latter given the libmnl dependency?
On Sun, Mar 8, 2020 at 12:17 PM Peter Korsgaard <peter@korsgaard.com> wrote: > > >>>>> "Petr" == Petr Vorel <petr.vorel@gmail.com> writes: > > Hi, > > >> > +config BR2_PACKAGE_CONNMAN_WIREGUARD > >> > + bool "enable wireguard support" > >> > + select BR2_PACKAGE_LIBMNL > > >> We already had a patch from Petr at > >> http://patchwork.ozlabs.org/patch/1246181/ to add Wireguard support, > >> but he was also selecting BR2_PACKAGE_WIREGUARD_TOOLS. > > >> Could you or Petr clarify this ? > > configure.ac > > found_libmnl="no" > > if (test "${firewall_type}" = "nftables" -o \ > > "${enable_wireguard}" != "no"); then > > PKG_CHECK_MODULES(LIBMNL, [libmnl >= 1.0.0], [found_libmnl="yes"], > > AC_MSG_ERROR([libmnl >= 1.0.0 not found])) > > AC_SUBST(LIBMNL_CFLAGS) > > AC_SUBST(LIBMNL_LIBS) > > fi > > AM_CONDITIONAL(LIBMNL, test "${found_libmnl}" != "no") > > > IMHO this means that libmnl is required for nftables and wireguard. > > Yes. What about wireguard-tools? Are they used by connman or does it > directly talk the netlink protocol? I guess the latter given the libmnl > dependency? Yeah, connman seemed to build fine with wireguard support without wireguard-tools. > > -- > Bye, Peter Korsgaard
>>>>> "James" == James Hilliard <james.hilliard1@gmail.com> writes: > On Sun, Mar 8, 2020 at 12:17 PM Peter Korsgaard <peter@korsgaard.com> wrote: >> >> >>>>> "Petr" == Petr Vorel <petr.vorel@gmail.com> writes: >> >> Hi, >> >> >> > +config BR2_PACKAGE_CONNMAN_WIREGUARD >> >> > + bool "enable wireguard support" >> >> > + select BR2_PACKAGE_LIBMNL >> >> >> We already had a patch from Petr at >> >> http://patchwork.ozlabs.org/patch/1246181/ to add Wireguard support, >> >> but he was also selecting BR2_PACKAGE_WIREGUARD_TOOLS. >> >> >> Could you or Petr clarify this ? >> > configure.ac >> > found_libmnl="no" >> > if (test "${firewall_type}" = "nftables" -o \ >> > "${enable_wireguard}" != "no"); then >> > PKG_CHECK_MODULES(LIBMNL, [libmnl >= 1.0.0], [found_libmnl="yes"], >> > AC_MSG_ERROR([libmnl >= 1.0.0 not found])) >> > AC_SUBST(LIBMNL_CFLAGS) >> > AC_SUBST(LIBMNL_LIBS) >> > fi >> > AM_CONDITIONAL(LIBMNL, test "${found_libmnl}" != "no") >> >> > IMHO this means that libmnl is required for nftables and wireguard. >> >> Yes. What about wireguard-tools? Are they used by connman or does it >> directly talk the netlink protocol? I guess the latter given the libmnl >> dependency? > Yeah, connman seemed to build fine with wireguard support without > wireguard-tools. Well, a wireguard-tools dependency issue would probably be visible at runtime as it would then just call out to those binaries. Looking at the code, I see that is not the case and connman directly talks the netlink interface of the kernel driver: https://git.kernel.org/pub/scm/network/connman/connman.git/tree/vpn/plugins/libwireguard.c
Hello, On Sun, 8 Mar 2020 18:37:18 +0100 Petr Vorel <petr.vorel@gmail.com> wrote: > > > diff --git a/package/connman/Config.in b/package/connman/Config.in > > > index ac012dda54..614b826f96 100644 > > > --- a/package/connman/Config.in > > > +++ b/package/connman/Config.in > > > @@ -33,6 +33,7 @@ config BR2_PACKAGE_CONNMAN_NFTABLES > > > bool "nftables" > > > depends on BR2_USE_WCHAR > > > depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12 > > > + select BR2_PACKAGE_LIBMNL > > > How is this related to wireguard support ? > Yes. > > > > select BR2_PACKAGE_NFTABLES > > > help > > > Use nftables as firewall. > > > @@ -51,6 +52,10 @@ config BR2_PACKAGE_CONNMAN_WIFI > > > setup). ConnMan detects the start of wpa_supplicant > > > automatically. > > > > +config BR2_PACKAGE_CONNMAN_WIREGUARD > > > + bool "enable wireguard support" > > > + select BR2_PACKAGE_LIBMNL > > > We already had a patch from Petr at > > http://patchwork.ozlabs.org/patch/1246181/ to add Wireguard support, > > but he was also selecting BR2_PACKAGE_WIREGUARD_TOOLS. > > > Could you or Petr clarify this ? > configure.ac > found_libmnl="no" > if (test "${firewall_type}" = "nftables" -o \ > "${enable_wireguard}" != "no"); then > PKG_CHECK_MODULES(LIBMNL, [libmnl >= 1.0.0], [found_libmnl="yes"], > AC_MSG_ERROR([libmnl >= 1.0.0 not found])) > AC_SUBST(LIBMNL_CFLAGS) > AC_SUBST(LIBMNL_LIBS) > fi > AM_CONDITIONAL(LIBMNL, test "${found_libmnl}" != "no") Then the "select BR2_PACKAGE_LIBMNL" added to BR2_PACKAGE_CONNMAN_NFTABLES is completely unrelated to wireguard support. connman needs libmnl if nftables is used *or* if wireguard is used. So the missing select BR2_PACKAGE_LIBMNL in BR2_PACKAGE_CONNMAN_NFTABLES should be added as a separate patch. Thanks! Thomas
On Sun, Mar 8, 2020 at 3:30 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Sun, 8 Mar 2020 18:37:18 +0100 > Petr Vorel <petr.vorel@gmail.com> wrote: > > > > > diff --git a/package/connman/Config.in b/package/connman/Config.in > > > > index ac012dda54..614b826f96 100644 > > > > --- a/package/connman/Config.in > > > > +++ b/package/connman/Config.in > > > > @@ -33,6 +33,7 @@ config BR2_PACKAGE_CONNMAN_NFTABLES > > > > bool "nftables" > > > > depends on BR2_USE_WCHAR > > > > depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12 > > > > + select BR2_PACKAGE_LIBMNL > > > > > How is this related to wireguard support ? > > Yes. > > > > > > select BR2_PACKAGE_NFTABLES > > > > help > > > > Use nftables as firewall. > > > > @@ -51,6 +52,10 @@ config BR2_PACKAGE_CONNMAN_WIFI > > > > setup). ConnMan detects the start of wpa_supplicant > > > > automatically. > > > > > > +config BR2_PACKAGE_CONNMAN_WIREGUARD > > > > + bool "enable wireguard support" > > > > + select BR2_PACKAGE_LIBMNL > > > > > We already had a patch from Petr at > > > http://patchwork.ozlabs.org/patch/1246181/ to add Wireguard support, > > > but he was also selecting BR2_PACKAGE_WIREGUARD_TOOLS. > > > > > Could you or Petr clarify this ? > > configure.ac > > found_libmnl="no" > > if (test "${firewall_type}" = "nftables" -o \ > > "${enable_wireguard}" != "no"); then > > PKG_CHECK_MODULES(LIBMNL, [libmnl >= 1.0.0], [found_libmnl="yes"], > > AC_MSG_ERROR([libmnl >= 1.0.0 not found])) > > AC_SUBST(LIBMNL_CFLAGS) > > AC_SUBST(LIBMNL_LIBS) > > fi > > AM_CONDITIONAL(LIBMNL, test "${found_libmnl}" != "no") > > Then the "select BR2_PACKAGE_LIBMNL" added to > BR2_PACKAGE_CONNMAN_NFTABLES is completely unrelated to wireguard > support. connman needs libmnl if nftables is used *or* if wireguard is > used. > > So the missing select BR2_PACKAGE_LIBMNL in > BR2_PACKAGE_CONNMAN_NFTABLES should be added as a separate patch. Sent here already: https://patchwork.ozlabs.org/patch/1251106/ > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Hi, > > Then the "select BR2_PACKAGE_LIBMNL" added to > > BR2_PACKAGE_CONNMAN_NFTABLES is completely unrelated to wireguard > > support. connman needs libmnl if nftables is used *or* if wireguard is > > used. > > So the missing select BR2_PACKAGE_LIBMNL in > > BR2_PACKAGE_CONNMAN_NFTABLES should be added as a separate patch. > Sent here already: https://patchwork.ozlabs.org/patch/1251106/ James, thanks for your fix! (add my Reviewed-by: tag to it). Sorry for wrong patch. Kind regards, Petr
On Sun, 8 Mar 2020 00:07:41 -0700 James Hilliard <james.hilliard1@gmail.com> wrote: > We also need to select libmnl when building with wireguard support or > with nftables. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > package/connman/Config.in | 5 +++++ > package/connman/connman.mk | 9 ++++++++- > 2 files changed, 13 insertions(+), 1 deletion(-) I've applied to master, but only the parts related to wireguard support, as the rest was applied separately. Thanks, Thomas
diff --git a/package/connman/Config.in b/package/connman/Config.in index ac012dda54..614b826f96 100644 --- a/package/connman/Config.in +++ b/package/connman/Config.in @@ -33,6 +33,7 @@ config BR2_PACKAGE_CONNMAN_NFTABLES bool "nftables" depends on BR2_USE_WCHAR depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12 + select BR2_PACKAGE_LIBMNL select BR2_PACKAGE_NFTABLES help Use nftables as firewall. @@ -51,6 +52,10 @@ config BR2_PACKAGE_CONNMAN_WIFI setup). ConnMan detects the start of wpa_supplicant automatically. +config BR2_PACKAGE_CONNMAN_WIREGUARD + bool "enable wireguard support" + select BR2_PACKAGE_LIBMNL + config BR2_PACKAGE_CONNMAN_WISPR bool "enable WISPr support" depends on !BR2_STATIC_LIBS # gnutls diff --git a/package/connman/connman.mk b/package/connman/connman.mk index 701be8b59d..23e7cb7797 100644 --- a/package/connman/connman.mk +++ b/package/connman/connman.mk @@ -41,7 +41,7 @@ CONNMAN_CONF_OPTS += --with-firewall=iptables CONNMAN_DEPENDENCIES += iptables else ifeq ($(BR2_PACKAGE_CONNMAN_NFTABLES),y) CONNMAN_CONF_OPTS += --with-firewall=nftables -CONNMAN_DEPENDENCIES += nftables +CONNMAN_DEPENDENCIES += libmnl nftables endif ifeq ($(BR2_PACKAGE_CONNMAN_LOOPBACK),y) @@ -70,6 +70,13 @@ else CONNMAN_CONF_OPTS += --disable-wifi endif +ifeq ($(BR2_PACKAGE_CONNMAN_WIREGUARD),y) +CONNMAN_CONF_OPTS += --enable-wireguard +CONNMAN_DEPENDENCIES += libmnl +else +CONNMAN_CONF_OPTS += --disable-wireguard +endif + ifeq ($(BR2_PACKAGE_CONNMAN_WISPR),y) CONNMAN_CONF_OPTS += --enable-wispr CONNMAN_DEPENDENCIES += gnutls
We also need to select libmnl when building with wireguard support or with nftables. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- package/connman/Config.in | 5 +++++ package/connman/connman.mk | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-)