[PATCHv2] iptables: link against libnetfilter_conntrack

[PATCHv2] iptables: link against libnetfilter_conntrack

[Gustavo Zacarias]

When libnetfilter_conntrack is available iptables needs to link
against it otherwise we get build failures.

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 iptables/Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/iptables/Makefile.am b/iptables/Makefile.am
index a4246eb..6390b7c 100644
--- a/iptables/Makefile.am
+++ b/iptables/Makefile.am
@@ -21,6 +21,9 @@ xtables_multi_SOURCES += ip6tables-save.c ip6tables-restore.c \
 xtables_multi_CFLAGS  += -DENABLE_IPV6
 xtables_multi_LDADD   += ../libiptc/libip6tc.la ../extensions/libext6.a
 endif
+if HAVE_LIBNETFILTER_CONNTRACK
+xtables_multi_LDADD += @libnetfilter_conntrack_LIBS@
+endif
 xtables_multi_SOURCES += xshared.c
 xtables_multi_LDADD   += ../libxtables/libxtables.la -lm

--
1.8.1.5

Re: [PATCHv2] iptables: link against libnetfilter_conntrack

[Florian Westphal]

Gustavo Zacarias <gustavo@zacarias.com.ar> wrote:
> When libnetfilter_conntrack is available iptables needs to link
> against it otherwise we get build failures.

Again, I doubt that this is the case.

The only caller of libnetfilter_conntrack functions is libxt_connlabel,
and that is not tied to the xtables-multi binary, UNLESS you
build with --enable-static (which puts the extensions into the
executable).  So, afaiu, we should not need -lnetfilter_conntrack in the
!ENABLE_STATIC case.

Re: [PATCHv2] iptables: link against libnetfilter_conntrack

[Jan Engelhardt]

On Tuesday 2013-08-13 18:37, Gustavo Zacarias wrote:
>diff --git a/iptables/Makefile.am b/iptables/Makefile.am
>index a4246eb..6390b7c 100644
>--- a/iptables/Makefile.am
>+++ b/iptables/Makefile.am
>@@ -21,6 +21,9 @@ xtables_multi_SOURCES += ip6tables-save.c ip6tables-restore.c \
> xtables_multi_CFLAGS  += -DENABLE_IPV6
> xtables_multi_LDADD   += ../libiptc/libip6tc.la ../extensions/libext6.a
> endif
>+if HAVE_LIBNETFILTER_CONNTRACK
>+xtables_multi_LDADD += @libnetfilter_conntrack_LIBS@
>+endif
> xtables_multi_SOURCES += xshared.c
> xtables_multi_LDADD   += ../libxtables/libxtables.la -lm

The righteous patch would be something like the following.
It's libxtables.la(libxt_connlabel.o) uses the lib, not the
main program. As for -lm, *both* the main program and
libxtables.la(static-plugins) use math funcs.

If libnetfilter_conntrack is not found, @libnetfilter_conntrack_CFLAGS@
and @libnetfilter_conntrack_LIBS@ (and their ${} ones) should be empty,
therefore producing no harm to include unconditionally.


diff --git a/extensions/GNUmakefile.in b/extensions/GNUmakefile.in
index 14e7c57..780e715 100644
--- a/extensions/GNUmakefile.in
+++ b/extensions/GNUmakefile.in
@@ -21,7 +21,7 @@ regular_CPPFLAGS   = @regular_CPPFLAGS@
 kinclude_CPPFLAGS  = @kinclude_CPPFLAGS@
 
 AM_CFLAGS       = ${regular_CFLAGS}
-AM_CPPFLAGS     = ${regular_CPPFLAGS} -I${top_builddir}/include -I${top_builddir} -I${top_srcdir}/include ${kinclude_CPPFLAGS}
+AM_CPPFLAGS     = ${regular_CPPFLAGS} -I${top_builddir}/include -I${top_builddir} -I${top_srcdir}/include ${kinclude_CPPFLAGS} @libnetfilter_conntrack_CFLAGS@
 AM_DEPFLAGS     = -Wp,-MMD,$(@D)/.$(@F).d,-MT,$@
 AM_LDFLAGS      = @noundef_LDFLAGS@
 
@@ -93,7 +93,7 @@ lib%.so: lib%.oo
 	${AM_VERBOSE_CCLD} ${CCLD} ${AM_LDFLAGS} -shared ${LDFLAGS} -o $@ $< -L../libxtables/.libs -lxtables ${$*_LIBADD};
 
 lib%.oo: ${srcdir}/lib%.c
-	${AM_VERBOSE_CC} ${CC} ${AM_CPPFLAGS} ${AM_DEPFLAGS} ${AM_CFLAGS} -D_INIT=lib$*_init -DPIC -fPIC ${CFLAGS} ${$*_CFLAGADD} -o $@ -c $<;
+	${AM_VERBOSE_CC} ${CC} ${AM_CPPFLAGS} ${AM_DEPFLAGS} ${AM_CFLAGS} -D_INIT=lib$*_init -DPIC -fPIC ${CFLAGS} -o $@ -c $<;
 
 libxt_NOTRACK.so: libxt_CT.so
 	ln -fs $< $@
@@ -103,9 +103,7 @@ libxt_state.so: libxt_conntrack.so
 # Need the LIBADDs in iptables/Makefile.am too for libxtables_la_LIBADD
 xt_RATEEST_LIBADD   = -lm
 xt_statistic_LIBADD = -lm
-@HAVE_LIBNETFILTER_CONNTRACK_TRUE@xt_connlabel_LIBADD = @libnetfilter_conntrack_LIBS@
-
-@HAVE_LIBNETFILTER_CONNTRACK_TRUE@xt_connlabel_CFLAGADD = @libnetfilter_conntrack_CFLAGS@
+xt_connlabel_LIBADD = @libnetfilter_conntrack_LIBS@
 
 #
 #	Static bits
diff --git a/libxtables/Makefile.am b/libxtables/Makefile.am
index c5795fe..4267cb5 100644
--- a/libxtables/Makefile.am
+++ b/libxtables/Makefile.am
@@ -10,7 +10,7 @@ libxtables_la_LIBADD  =
 if ENABLE_STATIC
 # With --enable-static, shipped extensions are linked into the main executable,
 # so we need all the LIBADDs here too
-libxtables_la_LIBADD += -lm
+libxtables_la_LIBADD += -lm ${libnetfilter_conntrack_LIBS}
 endif
 if ENABLE_SHARED
 libxtables_la_CFLAGS  = ${AM_CFLAGS}

Re: [PATCHv2] iptables: link against libnetfilter_conntrack

[Florian Westphal]

Jan Engelhardt <jengelh@inai.de> wrote:
> The righteous patch would be something like the following.

Gustavo, could you please test Jans patch and report back
if that fixes the build for you?

[..]

> If libnetfilter_conntrack is not found, @libnetfilter_conntrack_CFLAGS@
> and @libnetfilter_conntrack_LIBS@ (and their ${} ones) should be empty,
> therefore producing no harm to include unconditionally.

Good point...

Re: [PATCHv2] iptables: link against libnetfilter_conntrack

[Gustavo Zacarias]

On 08/13/2013 05:42 PM, Florian Westphal wrote:

> Gustavo, could you please test Jans patch and report back
> if that fixes the build for you?

Yes, works for the three combinations (shared, static, shared+static).
But shouldn't @libnetfilter_conntrack_CFLAGS@ be added to AM_CFLAGS
rather than AM_CPPFLAGS for correctness?
Regards.

Re: [PATCHv2] iptables: link against libnetfilter_conntrack

[Jan Engelhardt]

On Wednesday 2013-08-14 03:13, Gustavo Zacarias wrote:

>On 08/13/2013 05:42 PM, Florian Westphal wrote:
>
>> Gustavo, could you please test Jans patch and report back
>> if that fixes the build for you?
>
>Yes, works for the three combinations (shared, static, shared+static).
>But shouldn't @libnetfilter_conntrack_CFLAGS@ be added to AM_CFLAGS
>rather than AM_CPPFLAGS for correctness?

pkgconfig does not distinguish between preprocessor and compiler
flags. pkgconfig's Cflags field is most often used for preprocessor
directives, and most .pc files do not contain any compiler flags
either for good reason, because handing out those (particularly -f
and -m) could conflict with one's own program.

Re: [PATCHv2] iptables: link against libnetfilter_conntrack

[Florian Westphal]

Jan Engelhardt <jengelh@inai.de> wrote:
> On Tuesday 2013-08-13 18:37, Gustavo Zacarias wrote:
> >diff --git a/iptables/Makefile.am b/iptables/Makefile.am
> >index a4246eb..6390b7c 100644
> >--- a/iptables/Makefile.am
> >+++ b/iptables/Makefile.am
> >@@ -21,6 +21,9 @@ xtables_multi_SOURCES += ip6tables-save.c ip6tables-restore.c \
> > xtables_multi_CFLAGS  += -DENABLE_IPV6
> > xtables_multi_LDADD   += ../libiptc/libip6tc.la ../extensions/libext6.a
> > endif
> >+if HAVE_LIBNETFILTER_CONNTRACK
> >+xtables_multi_LDADD += @libnetfilter_conntrack_LIBS@
> >+endif
> > xtables_multi_SOURCES += xshared.c
> > xtables_multi_LDADD   += ../libxtables/libxtables.la -lm
> 
> The righteous patch would be something like the following.
[..]

Applied to stable-1.4.20, thanks everyone.

Re: [PATCHv2] iptables: link against libnetfilter_conntrack

[Ed W]

On 23/08/2013 21:03, Florian Westphal wrote:
> Jan Engelhardt <jengelh@inai.de> wrote:
>> On Tuesday 2013-08-13 18:37, Gustavo Zacarias wrote:
>>> diff --git a/iptables/Makefile.am b/iptables/Makefile.am
>>> index a4246eb..6390b7c 100644
>>> --- a/iptables/Makefile.am
>>> +++ b/iptables/Makefile.am
>>> @@ -21,6 +21,9 @@ xtables_multi_SOURCES += ip6tables-save.c ip6tables-restore.c \
>>> xtables_multi_CFLAGS  += -DENABLE_IPV6
>>> xtables_multi_LDADD   += ../libiptc/libip6tc.la ../extensions/libext6.a
>>> endif
>>> +if HAVE_LIBNETFILTER_CONNTRACK
>>> +xtables_multi_LDADD += @libnetfilter_conntrack_LIBS@
>>> +endif
>>> xtables_multi_SOURCES += xshared.c
>>> xtables_multi_LDADD   += ../libxtables/libxtables.la -lm
>> The righteous patch would be something like the following.
> [..]
>
> Applied to stable-1.4.20, thanks everyone.

I don't see this patch in release 1.4.21 ?  Can you please confirm (and 
pull code to master if necessary)

Thanks

Ed W