Patchwork [PATCHv2] iptables: link against libnetfilter_conntrack

login
register
mail settings
Submitter Jan Engelhardt
Date Aug. 13, 2013, 7:02 p.m.
Message ID <alpine.LSU.2.10.9.1308132051420.22862@nerf07.vanv.qr>
Download mbox | patch
Permalink /patch/266907/
State Accepted
Headers show

Comments

Jan Engelhardt - Aug. 13, 2013, 7:02 p.m.
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.


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Westphal - Aug. 13, 2013, 8:42 p.m.
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...
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gustavo Zacarias - Aug. 14, 2013, 1:13 a.m.
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.

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt - Aug. 14, 2013, 2:46 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Westphal - Aug. 23, 2013, 8:03 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ed W - Dec. 24, 2013, 4 p.m.
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

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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}