[iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Alexander Kanavin]

From: "Maxin B. John" <maxin.john@intel.com>

This changes the configure behaviour from autodetecting
for libnfnetlink to having an option to disable it explicitly.

Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Maxin B. John <maxin.john@intel.com>
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 configure.ac | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d99fa3b9..d6077723 100644
--- a/configure.ac
+++ b/configure.ac
@@ -63,6 +63,9 @@ AC_ARG_WITH([pkgconfigdir], AS_HELP_STRING([--with-pkgconfigdir=PATH],
 AC_ARG_ENABLE([nftables],
 	AS_HELP_STRING([--disable-nftables], [Do not build nftables compat]),
 	[enable_nftables="$enableval"], [enable_nftables="yes"])
+AC_ARG_ENABLE([libnfnetlink],
+    AS_HELP_STRING([--disable-libnfnetlink], [Do not use netfilter netlink library]),
+    [enable_libnfnetlink="$enableval"], [enable_libnfnetlink="yes"])
 AC_ARG_ENABLE([connlabel],
 	AS_HELP_STRING([--disable-connlabel],
 	[Do not build libnetfilter_conntrack]),
@@ -113,9 +116,10 @@ AM_CONDITIONAL([ENABLE_SYNCONF], [test "$enable_nfsynproxy" = "yes"])
 AM_CONDITIONAL([ENABLE_NFTABLES], [test "$enable_nftables" = "yes"])
 AM_CONDITIONAL([ENABLE_CONNLABEL], [test "$enable_connlabel" = "yes"])
 
-PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0],
-	[nfnetlink=1], [nfnetlink=0])
-AM_CONDITIONAL([HAVE_LIBNFNETLINK], [test "$nfnetlink" = 1])
+AS_IF([test "x$enable_libnfnetlink" = "xyes"], [
+    PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0])
+    ])
+AM_CONDITIONAL([HAVE_LIBNFNETLINK], [test "x$enable_libnfnetlink" = "xyes"])
 
 if test "x$enable_bpfc" = "xyes" || test "x$enable_nfsynproxy" = "xyes"; then
 	PKG_CHECK_MODULES([libpcap], [libpcap], [], [
-- 
2.39.2

Re: [iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Alexander Kanavin]

On 4/24/24 16:58, Phil Sutter wrote:
> Thanks for the explanation. I don't quite get how a build is
> deterministic if libnfnetlink presence is not, but OK.

If you specify either command line option, the outcome of the build (if 
it is successful) depends only on that option:

- if the option disables support, it will be disabled regardless of 
whether the needed library is present

- if the option enables support, either it will be enabled, or the build 
will error out with a missing library message, avoiding the situation 
where support was requested, but quietly disabled because library wasn't 
found.

>
> The problem I see with the patch is the changed default behaviour. Could
> you please retain the conditional build if neither --enable-libnfnetlink
> nor --disable-libnfnetlink was specified?

I sent a v2 that retains autodetection, can you please check that?


-- 

Alexander Kanavin
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 39; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier (Informations on data privacy
can be found here): https://linutronix.de/legal/data-protection.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen |
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700
806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner
Tiffany Silva, Sean Fennelly, Jeffrey Schneiderman

Re: [iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Pablo Neira Ayuso]

On Wed, Apr 24, 2024 at 08:35:12PM +0200, Phil Sutter wrote:
> On Wed, Apr 24, 2024 at 05:20:15PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Apr 24, 2024 at 04:58:40PM +0200, Phil Sutter wrote:
> > > On Wed, Apr 24, 2024 at 04:11:59PM +0200, Alexander Kanavin wrote:
> > > > On 4/24/24 14:53, Phil Sutter wrote:
> > > > > Hi,
> > > > >
> > > > > On Wed, Apr 24, 2024 at 02:28:04PM +0200, Alexander Kanavin wrote:
> > > > >> From: "Maxin B. John" <maxin.john@intel.com>
> > > > >>
> > > > >> This changes the configure behaviour from autodetecting
> > > > >> for libnfnetlink to having an option to disable it explicitly.
> > > > >>
> > > > >> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > > >> Signed-off-by: Maxin B. John <maxin.john@intel.com>
> > > > >> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> > > > > The patch looks fine as-is, I wonder though what's the goal: Does the
> > > > > build system have an incompatible libnfnetlink which breaks the build?
> > > > > It is used by nfnl_osf only, right? So maybe introduce
> > > > > | AC_ARG_ENABLE([nfnl_osf], ...)
> > > > > instead?
> > > > 
> > > > The patch is very old, and I didn't write it (I'm only cleaning up the 
> > > > custom patches that yocto project is currently carrying). It was 
> > > > introduced for the purposes of ensuring build determinism and 
> > > > reproducibility: so that libnfnetlink support doesn't get quietly 
> > > > enabled or disabled depending on what is available in the build system, 
> > > > but can be reliably turned off or on.
> > > 
> > > Thanks for the explanation. I don't quite get how a build is
> > > deterministic if libnfnetlink presence is not, but OK.
> > 
> > IIRC, there are also dependencies on utils with libnfnetlink that
> > would need to be disabled too.
> 
> Within iptables, we only have nfnl_osf (in utils/) which depends on it,
> but missing HAVE_LIBNFNETLINK effectively disables it from being built.
> So unless you have something else in mind, that's fine with and without
> this patch.

That's fine then, thanks.

Re: [iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Phil Sutter]

On Wed, Apr 24, 2024 at 05:20:15PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Apr 24, 2024 at 04:58:40PM +0200, Phil Sutter wrote:
> > On Wed, Apr 24, 2024 at 04:11:59PM +0200, Alexander Kanavin wrote:
> > > On 4/24/24 14:53, Phil Sutter wrote:
> > > > Hi,
> > > >
> > > > On Wed, Apr 24, 2024 at 02:28:04PM +0200, Alexander Kanavin wrote:
> > > >> From: "Maxin B. John" <maxin.john@intel.com>
> > > >>
> > > >> This changes the configure behaviour from autodetecting
> > > >> for libnfnetlink to having an option to disable it explicitly.
> > > >>
> > > >> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > >> Signed-off-by: Maxin B. John <maxin.john@intel.com>
> > > >> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> > > > The patch looks fine as-is, I wonder though what's the goal: Does the
> > > > build system have an incompatible libnfnetlink which breaks the build?
> > > > It is used by nfnl_osf only, right? So maybe introduce
> > > > | AC_ARG_ENABLE([nfnl_osf], ...)
> > > > instead?
> > > 
> > > The patch is very old, and I didn't write it (I'm only cleaning up the 
> > > custom patches that yocto project is currently carrying). It was 
> > > introduced for the purposes of ensuring build determinism and 
> > > reproducibility: so that libnfnetlink support doesn't get quietly 
> > > enabled or disabled depending on what is available in the build system, 
> > > but can be reliably turned off or on.
> > 
> > Thanks for the explanation. I don't quite get how a build is
> > deterministic if libnfnetlink presence is not, but OK.
> 
> IIRC, there are also dependencies on utils with libnfnetlink that
> would need to be disabled too.

Within iptables, we only have nfnl_osf (in utils/) which depends on it,
but missing HAVE_LIBNFNETLINK effectively disables it from being built.
So unless you have something else in mind, that's fine with and without
this patch.

Cheers, Phil

Re: [iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Pablo Neira Ayuso]

On Wed, Apr 24, 2024 at 04:58:40PM +0200, Phil Sutter wrote:
> On Wed, Apr 24, 2024 at 04:11:59PM +0200, Alexander Kanavin wrote:
> > On 4/24/24 14:53, Phil Sutter wrote:
> > > Hi,
> > >
> > > On Wed, Apr 24, 2024 at 02:28:04PM +0200, Alexander Kanavin wrote:
> > >> From: "Maxin B. John" <maxin.john@intel.com>
> > >>
> > >> This changes the configure behaviour from autodetecting
> > >> for libnfnetlink to having an option to disable it explicitly.
> > >>
> > >> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > >> Signed-off-by: Maxin B. John <maxin.john@intel.com>
> > >> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> > > The patch looks fine as-is, I wonder though what's the goal: Does the
> > > build system have an incompatible libnfnetlink which breaks the build?
> > > It is used by nfnl_osf only, right? So maybe introduce
> > > | AC_ARG_ENABLE([nfnl_osf], ...)
> > > instead?
> > 
> > The patch is very old, and I didn't write it (I'm only cleaning up the 
> > custom patches that yocto project is currently carrying). It was 
> > introduced for the purposes of ensuring build determinism and 
> > reproducibility: so that libnfnetlink support doesn't get quietly 
> > enabled or disabled depending on what is available in the build system, 
> > but can be reliably turned off or on.
> 
> Thanks for the explanation. I don't quite get how a build is
> deterministic if libnfnetlink presence is not, but OK.

IIRC, there are also dependencies on utils with libnfnetlink that
would need to be disabled too.

Re: [iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Phil Sutter]

On Wed, Apr 24, 2024 at 04:11:59PM +0200, Alexander Kanavin wrote:
> On 4/24/24 14:53, Phil Sutter wrote:
> > Hi,
> >
> > On Wed, Apr 24, 2024 at 02:28:04PM +0200, Alexander Kanavin wrote:
> >> From: "Maxin B. John" <maxin.john@intel.com>
> >>
> >> This changes the configure behaviour from autodetecting
> >> for libnfnetlink to having an option to disable it explicitly.
> >>
> >> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> >> Signed-off-by: Maxin B. John <maxin.john@intel.com>
> >> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> > The patch looks fine as-is, I wonder though what's the goal: Does the
> > build system have an incompatible libnfnetlink which breaks the build?
> > It is used by nfnl_osf only, right? So maybe introduce
> > | AC_ARG_ENABLE([nfnl_osf], ...)
> > instead?
> 
> The patch is very old, and I didn't write it (I'm only cleaning up the 
> custom patches that yocto project is currently carrying). It was 
> introduced for the purposes of ensuring build determinism and 
> reproducibility: so that libnfnetlink support doesn't get quietly 
> enabled or disabled depending on what is available in the build system, 
> but can be reliably turned off or on.

Thanks for the explanation. I don't quite get how a build is
deterministic if libnfnetlink presence is not, but OK.

The problem I see with the patch is the changed default behaviour. Could
you please retain the conditional build if neither --enable-libnfnetlink
nor --disable-libnfnetlink was specified?

> Note that we also carry a related patch which I didn't look at properly 
> yet, but can submit as well:
> 
> https://git.yoctoproject.org/poky/tree/meta/recipes-extended/iptables/iptables/0004-configure.ac-only-check-conntrack-when-libnfnetlink-.patch

Implementing the above might require adjustments in this one, so you
might want to hold back a bit.

Cheers, Phil

Re: [iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Alexander Kanavin]

On 4/24/24 14:53, Phil Sutter wrote:
> Hi,
>
> On Wed, Apr 24, 2024 at 02:28:04PM +0200, Alexander Kanavin wrote:
>> From: "Maxin B. John" <maxin.john@intel.com>
>>
>> This changes the configure behaviour from autodetecting
>> for libnfnetlink to having an option to disable it explicitly.
>>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> Signed-off-by: Maxin B. John <maxin.john@intel.com>
>> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> The patch looks fine as-is, I wonder though what's the goal: Does the
> build system have an incompatible libnfnetlink which breaks the build?
> It is used by nfnl_osf only, right? So maybe introduce
> | AC_ARG_ENABLE([nfnl_osf], ...)
> instead?

The patch is very old, and I didn't write it (I'm only cleaning up the 
custom patches that yocto project is currently carrying). It was 
introduced for the purposes of ensuring build determinism and 
reproducibility: so that libnfnetlink support doesn't get quietly 
enabled or disabled depending on what is available in the build system, 
but can be reliably turned off or on.

Note that we also carry a related patch which I didn't look at properly 
yet, but can submit as well:

https://git.yoctoproject.org/poky/tree/meta/recipes-extended/iptables/iptables/0004-configure.ac-only-check-conntrack-when-libnfnetlink-.patch

-- 

Alexander Kanavin
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 39; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier (Informations on data privacy
can be found here): https://linutronix.de/legal/data-protection.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen |
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700
806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner
Tiffany Silva, Sean Fennelly, Jeffrey Schneiderman

Re: [iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Phil Sutter]

Hi,

On Wed, Apr 24, 2024 at 02:28:04PM +0200, Alexander Kanavin wrote:
> From: "Maxin B. John" <maxin.john@intel.com>
> 
> This changes the configure behaviour from autodetecting
> for libnfnetlink to having an option to disable it explicitly.
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> Signed-off-by: Maxin B. John <maxin.john@intel.com>
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>

The patch looks fine as-is, I wonder though what's the goal: Does the
build system have an incompatible libnfnetlink which breaks the build?
It is used by nfnl_osf only, right? So maybe introduce
| AC_ARG_ENABLE([nfnl_osf], ...)
instead?

Thanks, Phil

[iptables][PATCH] configure: Add option to enable/disable libnfnetlink

[Alexander Kanavin]

From: "Maxin B. John" <maxin.john@intel.com>

This changes the configure behaviour from autodetecting
for libnfnetlink to having an option to disable it explicitly.

Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Maxin B. John <maxin.john@intel.com>
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 configure.ac | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d99fa3b9..d6077723 100644
--- a/configure.ac
+++ b/configure.ac
@@ -63,6 +63,9 @@ AC_ARG_WITH([pkgconfigdir], AS_HELP_STRING([--with-pkgconfigdir=PATH],
 AC_ARG_ENABLE([nftables],
 	AS_HELP_STRING([--disable-nftables], [Do not build nftables compat]),
 	[enable_nftables="$enableval"], [enable_nftables="yes"])
+AC_ARG_ENABLE([libnfnetlink],
+    AS_HELP_STRING([--disable-libnfnetlink], [Do not use netfilter netlink library]),
+    [enable_libnfnetlink="$enableval"], [enable_libnfnetlink="yes"])
 AC_ARG_ENABLE([connlabel],
 	AS_HELP_STRING([--disable-connlabel],
 	[Do not build libnetfilter_conntrack]),
@@ -113,9 +116,10 @@ AM_CONDITIONAL([ENABLE_SYNCONF], [test "$enable_nfsynproxy" = "yes"])
 AM_CONDITIONAL([ENABLE_NFTABLES], [test "$enable_nftables" = "yes"])
 AM_CONDITIONAL([ENABLE_CONNLABEL], [test "$enable_connlabel" = "yes"])
 
-PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0],
-	[nfnetlink=1], [nfnetlink=0])
-AM_CONDITIONAL([HAVE_LIBNFNETLINK], [test "$nfnetlink" = 1])
+AS_IF([test "x$enable_libnfnetlink" = "xyes"], [
+    PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0])
+    ])
+AM_CONDITIONAL([HAVE_LIBNFNETLINK], [test "x$enable_libnfnetlink" = "xyes"])
 
 if test "x$enable_bpfc" = "xyes" || test "x$enable_nfsynproxy" = "xyes"; then
 	PKG_CHECK_MODULES([libpcap], [libpcap], [], [
-- 
2.39.2