| Message ID | 20230331223601.315215-1-hi@alyssa.is |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [iptables] build: use pkg-config for libpcap | expand |
On 2023-03-31, at 22:36:01 +0000, Alyssa Ross wrote: > If building statically, with libpcap built with libnl support, linking > will fail, as the compiler won't be able to find the libnl symbols > since static libraries don't contain dependency information. To fix > this, use pkg-config to find the flags for linking libpcap, since the > pkg-config files contain the neccesary dependency information. ^^^^^^^^^ "necessary" > Signed-off-by: Alyssa Ross <hi@alyssa.is> LGTM. The only thing I would say is that pkg-config support was added to libpcap comparatively recently (2018). When I made similar changes to ulogd2 last year, I added a fall-back to pcap-config: https://git.netfilter.org/ulogd2/commit/?id=be4df8f66eb843dc19c7d1fed7c33fd7a40c2e21 > --- > configure.ac | 3 ++- > utils/Makefile.am | 6 +++--- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/configure.ac b/configure.ac > index bc2ed47b..e0bb26aa 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -114,7 +114,8 @@ AM_CONDITIONAL([ENABLE_NFTABLES], [test "$enable_nftables" = "yes"]) > AM_CONDITIONAL([ENABLE_CONNLABEL], [test "$enable_connlabel" = "yes"]) > > if test "x$enable_bpfc" = "xyes" || test "x$enable_nfsynproxy" = "xyes"; then > - AC_CHECK_LIB(pcap, pcap_compile,, AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)) > + PKG_CHECK_MODULES([libpcap], [libpcap], [], [ > + AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)]) > fi > > PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0], > diff --git a/utils/Makefile.am b/utils/Makefile.am > index e9eec48f..34056514 100644 > --- a/utils/Makefile.am > +++ b/utils/Makefile.am > @@ -2,7 +2,7 @@ > > AM_CFLAGS = ${regular_CFLAGS} > AM_CPPFLAGS = ${regular_CPPFLAGS} -I${top_builddir}/include \ > - -I${top_srcdir}/include ${libnfnetlink_CFLAGS} > + -I${top_srcdir}/include ${libnfnetlink_CFLAGS} ${libpcap_CFLAGS} > AM_LDFLAGS = ${regular_LDFLAGS} > > sbin_PROGRAMS = > @@ -25,12 +25,12 @@ endif > if ENABLE_BPFC > man_MANS += nfbpf_compile.8 > sbin_PROGRAMS += nfbpf_compile > -nfbpf_compile_LDADD = -lpcap > +nfbpf_compile_LDADD = ${libpcap_LIBS} > endif > > if ENABLE_SYNCONF > sbin_PROGRAMS += nfsynproxy > -nfsynproxy_LDADD = -lpcap > +nfsynproxy_LDADD = ${libpcap_LIBS} > endif > > CLEANFILES = nfnl_osf.8 nfbpf_compile.8 > > base-commit: 09f0bfe2032454d21e3650e7ac75c4dc53f3c881 > -- > 2.37.1 > > J.
On 2023-04-01, at 12:43:04 +0100, Jeremy Sowden wrote: > On 2023-03-31, at 22:36:01 +0000, Alyssa Ross wrote: > > If building statically, with libpcap built with libnl support, linking > > will fail, as the compiler won't be able to find the libnl symbols > > since static libraries don't contain dependency information. To fix > > this, use pkg-config to find the flags for linking libpcap, since the > > pkg-config files contain the neccesary dependency information. > ^^^^^^^^^ > "necessary" > > > Signed-off-by: Alyssa Ross <hi@alyssa.is> > > LGTM. Actually, there is a problem. See below. > The only thing I would say is that pkg-config support was added > to libpcap comparatively recently (2018). When I made similar changes > to ulogd2 last year, I added a fall-back to pcap-config: > > https://git.netfilter.org/ulogd2/commit/?id=be4df8f66eb843dc19c7d1fed7c33fd7a40c2e21 > > > --- > > configure.ac | 3 ++- > > utils/Makefile.am | 6 +++--- > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/configure.ac b/configure.ac > > index bc2ed47b..e0bb26aa 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -114,7 +114,8 @@ AM_CONDITIONAL([ENABLE_NFTABLES], [test "$enable_nftables" = "yes"]) > > AM_CONDITIONAL([ENABLE_CONNLABEL], [test "$enable_connlabel" = "yes"]) > > > > if test "x$enable_bpfc" = "xyes" || test "x$enable_nfsynproxy" = "xyes"; then > > - AC_CHECK_LIB(pcap, pcap_compile,, AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)) > > + PKG_CHECK_MODULES([libpcap], [libpcap], [], [ > > + AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)]) > > fi When autoconf first encounters `PKG_CHECK_MODULES`, if `$PKG_CONFIG` is not already defined it will execute `PKG_PROG_PKG_CONFIG` to find it. However, because you are calling `PKG_CHECK_MODULES` in a conditional, if `$enable_bpfc` and `$enable_nfsynproxy` are not `yes`, the expansion of `PKG_PROG_PKG_CONFIG` will not be executed and so the following pkg-config checks will fail: checking for libnfnetlink >= 1.0... no checking for libmnl >= 1.0... no *** Error: No suitable libmnl found. *** Please install the 'libmnl' package Or consider --disable-nftables to skip iptables-compat over nftables support. Something like this will fix it: @@ -14,6 +14,7 @@ AC_PROG_CC AM_PROG_CC_C_O m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) LT_INIT([disable-static]) +PKG_PROG_PKG_CONFIG AC_ARG_WITH([kernel], AS_HELP_STRING([--with-kernel=PATH], > > PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0], > > diff --git a/utils/Makefile.am b/utils/Makefile.am > > index e9eec48f..34056514 100644 > > --- a/utils/Makefile.am > > +++ b/utils/Makefile.am > > @@ -2,7 +2,7 @@ > > > > AM_CFLAGS = ${regular_CFLAGS} > > AM_CPPFLAGS = ${regular_CPPFLAGS} -I${top_builddir}/include \ > > - -I${top_srcdir}/include ${libnfnetlink_CFLAGS} > > + -I${top_srcdir}/include ${libnfnetlink_CFLAGS} ${libpcap_CFLAGS} > > AM_LDFLAGS = ${regular_LDFLAGS} > > > > sbin_PROGRAMS = > > @@ -25,12 +25,12 @@ endif > > if ENABLE_BPFC > > man_MANS += nfbpf_compile.8 > > sbin_PROGRAMS += nfbpf_compile > > -nfbpf_compile_LDADD = -lpcap > > +nfbpf_compile_LDADD = ${libpcap_LIBS} > > endif > > > > if ENABLE_SYNCONF > > sbin_PROGRAMS += nfsynproxy > > -nfsynproxy_LDADD = -lpcap > > +nfsynproxy_LDADD = ${libpcap_LIBS} > > endif > > > > CLEANFILES = nfnl_osf.8 nfbpf_compile.8 > > > > base-commit: 09f0bfe2032454d21e3650e7ac75c4dc53f3c881 > > -- > > 2.37.1 > > > > J.
On Sat, Apr 01, 2023 at 12:43:04PM +0100, Jeremy Sowden wrote: > On 2023-03-31, at 22:36:01 +0000, Alyssa Ross wrote: > > If building statically, with libpcap built with libnl support, linking > > will fail, as the compiler won't be able to find the libnl symbols > > since static libraries don't contain dependency information. To fix > > this, use pkg-config to find the flags for linking libpcap, since the > > pkg-config files contain the neccesary dependency information. > ^^^^^^^^^ > "necessary" > > > Signed-off-by: Alyssa Ross <hi@alyssa.is> > > LGTM. The only thing I would say is that pkg-config support was added > to libpcap comparatively recently (2018). When I made similar changes > to ulogd2 last year, I added a fall-back to pcap-config: > > https://git.netfilter.org/ulogd2/commit/?id=be4df8f66eb843dc19c7d1fed7c33fd7a40c2e21 That's quite a lot of extra code. Is it likely that people will want to build a version of iptables that is five years newer than their libpcap?
On Sat, Apr 01, 2023 at 01:30:18PM +0100, Jeremy Sowden wrote: > > > diff --git a/configure.ac b/configure.ac > > > index bc2ed47b..e0bb26aa 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -114,7 +114,8 @@ AM_CONDITIONAL([ENABLE_NFTABLES], [test "$enable_nftables" = "yes"]) > > > AM_CONDITIONAL([ENABLE_CONNLABEL], [test "$enable_connlabel" = "yes"]) > > > > > > if test "x$enable_bpfc" = "xyes" || test "x$enable_nfsynproxy" = "xyes"; then > > > - AC_CHECK_LIB(pcap, pcap_compile,, AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)) > > > + PKG_CHECK_MODULES([libpcap], [libpcap], [], [ > > > + AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)]) > > > fi > > When autoconf first encounters `PKG_CHECK_MODULES`, if `$PKG_CONFIG` is > not already defined it will execute `PKG_PROG_PKG_CONFIG` to find it. > However, because you are calling `PKG_CHECK_MODULES` in a conditional, > if `$enable_bpfc` and `$enable_nfsynproxy` are not `yes`, the expansion > of `PKG_PROG_PKG_CONFIG` will not be executed and so the following > pkg-config checks will fail: > > checking for libnfnetlink >= 1.0... no > checking for libmnl >= 1.0... no > *** Error: No suitable libmnl found. *** > Please install the 'libmnl' package > Or consider --disable-nftables to skip > iptables-compat over nftables support. > > Something like this will fix it: > > @@ -14,6 +14,7 @@ AC_PROG_CC > AM_PROG_CC_C_O > m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) > LT_INIT([disable-static]) > +PKG_PROG_PKG_CONFIG > > AC_ARG_WITH([kernel], > AS_HELP_STRING([--with-kernel=PATH], > > > > PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0], Another option would be to just move the libpcap change after this unconditional one, avoiding the manual initialization. Do you have a preference?
On 2023-04-01, at 19:25:45 +0000, Alyssa Ross wrote: > On Sat, Apr 01, 2023 at 12:43:04PM +0100, Jeremy Sowden wrote: > > On 2023-03-31, at 22:36:01 +0000, Alyssa Ross wrote: > > > If building statically, with libpcap built with libnl support, linking > > > will fail, as the compiler won't be able to find the libnl symbols > > > since static libraries don't contain dependency information. To fix > > > this, use pkg-config to find the flags for linking libpcap, since the > > > pkg-config files contain the neccesary dependency information. > > ^^^^^^^^^ > > "necessary" > > > > > Signed-off-by: Alyssa Ross <hi@alyssa.is> > > > > LGTM. The only thing I would say is that pkg-config support was added > > to libpcap comparatively recently (2018). When I made similar changes > > to ulogd2 last year, I added a fall-back to pcap-config: > > > > https://git.netfilter.org/ulogd2/commit/?id=be4df8f66eb843dc19c7d1fed7c33fd7a40c2e21 > > That's quite a lot of extra code. Is it likely that people will want > to build a version of iptables that is five years newer than their > libpcap? Not sure I'd call it a lot of code, but I agree that there probably aren't very many users who would benefit. J.
On 2023-04-01, at 19:54:12 +0000, Alyssa Ross wrote: > On Sat, Apr 01, 2023 at 01:30:18PM +0100, Jeremy Sowden wrote: > > > > diff --git a/configure.ac b/configure.ac > > > > index bc2ed47b..e0bb26aa 100644 > > > > --- a/configure.ac > > > > +++ b/configure.ac > > > > @@ -114,7 +114,8 @@ AM_CONDITIONAL([ENABLE_NFTABLES], [test "$enable_nftables" = "yes"]) > > > > AM_CONDITIONAL([ENABLE_CONNLABEL], [test "$enable_connlabel" = "yes"]) > > > > > > > > if test "x$enable_bpfc" = "xyes" || test "x$enable_nfsynproxy" = "xyes"; then > > > > - AC_CHECK_LIB(pcap, pcap_compile,, AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)) > > > > + PKG_CHECK_MODULES([libpcap], [libpcap], [], [ > > > > + AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)]) > > > > fi > > > > When autoconf first encounters `PKG_CHECK_MODULES`, if `$PKG_CONFIG` is > > not already defined it will execute `PKG_PROG_PKG_CONFIG` to find it. > > However, because you are calling `PKG_CHECK_MODULES` in a conditional, > > if `$enable_bpfc` and `$enable_nfsynproxy` are not `yes`, the expansion > > of `PKG_PROG_PKG_CONFIG` will not be executed and so the following > > pkg-config checks will fail: > > > > checking for libnfnetlink >= 1.0... no > > checking for libmnl >= 1.0... no > > *** Error: No suitable libmnl found. *** > > Please install the 'libmnl' package > > Or consider --disable-nftables to skip > > iptables-compat over nftables support. > > > > Something like this will fix it: > > > > @@ -14,6 +14,7 @@ AC_PROG_CC > > AM_PROG_CC_C_O > > m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) > > LT_INIT([disable-static]) > > +PKG_PROG_PKG_CONFIG > > > > AC_ARG_WITH([kernel], > > AS_HELP_STRING([--with-kernel=PATH], > > > > > > PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0], > > Another option would be to just move the libpcap change after this > unconditional one, avoiding the manual initialization. Do you have a > preference? Not really. Atm, I am marginally inclined to think that the explicit initialization is more robust, but I don't remember ever having done it myself in the past, so if you prefer to move the conditional that's fine by me. J.
diff --git a/configure.ac b/configure.ac index bc2ed47b..e0bb26aa 100644 --- a/configure.ac +++ b/configure.ac @@ -114,7 +114,8 @@ AM_CONDITIONAL([ENABLE_NFTABLES], [test "$enable_nftables" = "yes"]) AM_CONDITIONAL([ENABLE_CONNLABEL], [test "$enable_connlabel" = "yes"]) if test "x$enable_bpfc" = "xyes" || test "x$enable_nfsynproxy" = "xyes"; then - AC_CHECK_LIB(pcap, pcap_compile,, AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)) + PKG_CHECK_MODULES([libpcap], [libpcap], [], [ + AC_MSG_ERROR(missing libpcap library required by bpf compiler or nfsynproxy tool)]) fi PKG_CHECK_MODULES([libnfnetlink], [libnfnetlink >= 1.0], diff --git a/utils/Makefile.am b/utils/Makefile.am index e9eec48f..34056514 100644 --- a/utils/Makefile.am +++ b/utils/Makefile.am @@ -2,7 +2,7 @@ AM_CFLAGS = ${regular_CFLAGS} AM_CPPFLAGS = ${regular_CPPFLAGS} -I${top_builddir}/include \ - -I${top_srcdir}/include ${libnfnetlink_CFLAGS} + -I${top_srcdir}/include ${libnfnetlink_CFLAGS} ${libpcap_CFLAGS} AM_LDFLAGS = ${regular_LDFLAGS} sbin_PROGRAMS = @@ -25,12 +25,12 @@ endif if ENABLE_BPFC man_MANS += nfbpf_compile.8 sbin_PROGRAMS += nfbpf_compile -nfbpf_compile_LDADD = -lpcap +nfbpf_compile_LDADD = ${libpcap_LIBS} endif if ENABLE_SYNCONF sbin_PROGRAMS += nfsynproxy -nfsynproxy_LDADD = -lpcap +nfsynproxy_LDADD = ${libpcap_LIBS} endif CLEANFILES = nfnl_osf.8 nfbpf_compile.8
If building statically, with libpcap built with libnl support, linking will fail, as the compiler won't be able to find the libnl symbols since static libraries don't contain dependency information. To fix this, use pkg-config to find the flags for linking libpcap, since the pkg-config files contain the neccesary dependency information. Signed-off-by: Alyssa Ross <hi@alyssa.is> --- configure.ac | 3 ++- utils/Makefile.am | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) base-commit: 09f0bfe2032454d21e3650e7ac75c4dc53f3c881