| Message ID | 1366886611-21666-2-git-send-email-fw@strlen.de |
|---|---|
| State | Superseded |
| Headers | show |
Hi Florian, On Thu, Apr 25, 2013 at 12:43:28PM +0200, Florian Westphal wrote: > This is a partial revert of a0c885ae5a79457aa592cb70c27a7dee619762a4 > > Specifically, it removes the header linux/netfilter/nfnetlink_queue.h > added in that commit. The idea for caching that file header is to make sure libnetfilter_queue compiles out of the box, without external linux kernel headers installed in the system. The idea is to make sure the libraries compilation does not break in case there's an old library header installed in the system. > 1), there is already a /usr/include/linux/netfilter/nfnetlink_queue.h, > which is part of the linux kernel API As said, that file may not be available or may be stale. > 2), we already have > include/libnetfilter_queue/linux_nfnetlink_queue.h That was added initially for the old libnetfilter_queue API, I'd like to get rid of it. It's currently being installed and it results in (very likely) two duplicated headers in the system (the one from the linux kernel headers and this one from libnetfilter_queue). Once the old API is deprecated (we should do that anytime soon), we can get finally rid of linux_nfnetlink_queue.h > which contains the same definitions/structures/macros, so it makes > little sense to have two headers in libnetfilter_queue that share > almost their entire content. > > [ worse, the nfnetlink_queue.h header reverted here actually is > incompatible with mainline kernels, since a few defines have the > wrong value ... ] Then, please refresh it. -- 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
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Thu, Apr 25, 2013 at 12:43:28PM +0200, Florian Westphal wrote: > > This is a partial revert of a0c885ae5a79457aa592cb70c27a7dee619762a4 > > > > Specifically, it removes the header linux/netfilter/nfnetlink_queue.h > > added in that commit. > > The idea for caching that file header is to make sure > libnetfilter_queue compiles out of the box, without external linux > kernel headers installed in the system. The idea is to make sure the > libraries compilation does not break in case there's an old library > header installed in the system. > > > 1), there is already a /usr/include/linux/netfilter/nfnetlink_queue.h, > > which is part of the linux kernel API > > As said, that file may not be available or may be stale. Yes, but at this time the system-wide header is used, and not this copy. Compile of libnetfilter_queue breaks with the other patches applied because the new attribute is missing from the system header. > > 2), we already have > > include/libnetfilter_queue/linux_nfnetlink_queue.h > > That was added initially for the old libnetfilter_queue API, I'd like > to get rid of it. It's currently being installed and it results in > (very likely) two duplicated headers in the system (the one from the > linux kernel headers and this one from libnetfilter_queue). Hrm, yes. I thought that was intentional. Or are you saying that you basically just want to rename linux_nfnetlink_queue.h and carry a copy of nfnetlink_queue.h instead? > Once the old API is deprecated (we should do that anytime soon), we > can get finally rid of linux_nfnetlink_queue.h I guess what confused me is that there is nothing related to the old api in that header, so we might as well keep it? > > which contains the same definitions/structures/macros, so it makes > > little sense to have two headers in libnetfilter_queue that share > > almost their entire content. > > > > [ worse, the nfnetlink_queue.h header reverted here actually is > > incompatible with mainline kernels, since a few defines have the > > wrong value ... ] > > Then, please refresh it. Alright, but its not enough; the build system would require additional treatment to give precedence to that header when compiling the library. -- 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
On Fri, Apr 26, 2013 at 09:32:39AM +0200, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Thu, Apr 25, 2013 at 12:43:28PM +0200, Florian Westphal wrote: > > > This is a partial revert of a0c885ae5a79457aa592cb70c27a7dee619762a4 > > > > > > Specifically, it removes the header linux/netfilter/nfnetlink_queue.h > > > added in that commit. > > > > The idea for caching that file header is to make sure > > libnetfilter_queue compiles out of the box, without external linux > > kernel headers installed in the system. The idea is to make sure the > > libraries compilation does not break in case there's an old library > > header installed in the system. > > > > > 1), there is already a /usr/include/linux/netfilter/nfnetlink_queue.h, > > > which is part of the linux kernel API > > > > As said, that file may not be available or may be stale. > > Yes, but at this time the system-wide header is used, and not this copy. > Compile of libnetfilter_queue breaks with the other patches applied > because the new attribute is missing from the system header. Hm, it's using the local cache copy here. I tested it by adding some new (random) attribute definition (NFQA_XYZ) to the locally cached header file, and using it from some of the source files (e.g. nlmsg.c). > > > 2), we already have > > > include/libnetfilter_queue/linux_nfnetlink_queue.h > > > > That was added initially for the old libnetfilter_queue API, I'd like > > to get rid of it. It's currently being installed and it results in > > (very likely) two duplicated headers in the system (the one from the > > linux kernel headers and this one from libnetfilter_queue). > > Hrm, yes. I thought that was intentional. > Or are you saying that you basically just want to rename > linux_nfnetlink_queue.h and carry a copy of nfnetlink_queue.h instead? We cannot rename that header. Some people are including it into it into their source code and that will break their compilation. We can add some #warning in that file to say that people should use cache some fresh kernel header in their source code tree, or simply tell them to use the system kernel headers. 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
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Yes, but at this time the system-wide header is used, and not this copy. > > Compile of libnetfilter_queue breaks with the other patches applied > > because the new attribute is missing from the system header. > > Hm, it's using the local cache copy here. You're right. > > Hrm, yes. I thought that was intentional. > > Or are you saying that you basically just want to rename > > linux_nfnetlink_queue.h and carry a copy of nfnetlink_queue.h instead? > > We cannot rename that header. Some people are including it into it > into their source code and that will break their compilation. Yes, I meant "in the long run, when old API is removed". I got rid of the revert and it works for me after refreshing with a copy from net-next. I hope the V2 i posted is in the direction you had in mind. -- 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
On Fri, Apr 26, 2013 at 12:02:47PM +0200, Florian Westphal wrote: [...] > > We cannot rename that header. Some people are including it into it > > into their source code and that will break their compilation. > > Yes, I meant "in the long run, when old API is removed". > I got rid of the revert and it works for me after refreshing > with a copy from net-next. Feel free to post a patch for that refresh. > I hope the V2 i posted is in the direction you had in mind. Looks good, only missing some explanation in the doxygen documentation on the "csum not ready" thing. Thanks. -- 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
diff --git a/Makefile.am b/Makefile.am index 6b4ef77..1230dc1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,6 +1,6 @@ ACLOCAL_AMFLAGS = -I m4 -EXTRA_DIST = $(man_MANS) include/linux +EXTRA_DIST = $(man_MANS) SUBDIRS = src utils include examples diff --git a/configure.ac b/configure.ac index 07747a6..649060d 100644 --- a/configure.ac +++ b/configure.ac @@ -32,6 +32,5 @@ PKG_CHECK_MODULES([LIBMNL], [libmnl >= 1.0.3]) dnl Output the makefiles AC_CONFIG_FILES([Makefile src/Makefile utils/Makefile examples/Makefile libnetfilter_queue.pc doxygen.cfg - include/Makefile include/libnetfilter_queue/Makefile - include/linux/Makefile include/linux/netfilter/Makefile]) + include/Makefile include/libnetfilter_queue/Makefile]) AC_OUTPUT diff --git a/include/Makefile.am b/include/Makefile.am index 54ea0b4..1e766d5 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -1 +1 @@ -SUBDIRS= libnetfilter_queue linux +SUBDIRS= libnetfilter_queue diff --git a/include/linux/Makefile.am b/include/linux/Makefile.am deleted file mode 100644 index 38eb109..0000000 --- a/include/linux/Makefile.am +++ /dev/null @@ -1 +0,0 @@ -SUBDIRS = netfilter diff --git a/include/linux/netfilter/Makefile.am b/include/linux/netfilter/Makefile.am deleted file mode 100644 index d0937cb..0000000 --- a/include/linux/netfilter/Makefile.am +++ /dev/null @@ -1 +0,0 @@ -noinst_HEADERS = nfnetlink_queue.h diff --git a/include/linux/netfilter/nfnetlink_queue.h b/include/linux/netfilter/nfnetlink_queue.h deleted file mode 100644 index da44b33..0000000 --- a/include/linux/netfilter/nfnetlink_queue.h +++ /dev/null @@ -1,98 +0,0 @@ -#ifndef _NFNETLINK_QUEUE_H -#define _NFNETLINK_QUEUE_H - -#include <linux/types.h> -#include <linux/netfilter/nfnetlink.h> - -enum nfqnl_msg_types { - NFQNL_MSG_PACKET, /* packet from kernel to userspace */ - NFQNL_MSG_VERDICT, /* verdict from userspace to kernel */ - NFQNL_MSG_CONFIG, /* connect to a particular queue */ - NFQNL_MSG_VERDICT_BATCH, /* batchv from userspace to kernel */ - - NFQNL_MSG_MAX -}; - -struct nfqnl_msg_packet_hdr { - __be32 packet_id; /* unique ID of packet in queue */ - __be16 hw_protocol; /* hw protocol (network order) */ - __u8 hook; /* netfilter hook */ -} __attribute__ ((packed)); - -struct nfqnl_msg_packet_hw { - __be16 hw_addrlen; - __u16 _pad; - __u8 hw_addr[8]; -}; - -struct nfqnl_msg_packet_timestamp { - __aligned_be64 sec; - __aligned_be64 usec; -}; - -enum nfqnl_attr_type { - NFQA_UNSPEC, - NFQA_PACKET_HDR, - NFQA_VERDICT_HDR, /* nfqnl_msg_verdict_hrd */ - NFQA_MARK, /* __u32 nfmark */ - NFQA_TIMESTAMP, /* nfqnl_msg_packet_timestamp */ - NFQA_IFINDEX_INDEV, /* __u32 ifindex */ - NFQA_IFINDEX_OUTDEV, /* __u32 ifindex */ - NFQA_IFINDEX_PHYSINDEV, /* __u32 ifindex */ - NFQA_IFINDEX_PHYSOUTDEV, /* __u32 ifindex */ - NFQA_HWADDR, /* nfqnl_msg_packet_hw */ - NFQA_PAYLOAD, /* opaque data payload */ - NFQA_CT, /* nf_conntrack_netlink.h */ - NFQA_CT_INFO, /* enum ip_conntrack_info */ - - __NFQA_MAX -}; -#define NFQA_MAX (__NFQA_MAX - 1) - -struct nfqnl_msg_verdict_hdr { - __be32 verdict; - __be32 id; -}; - - -enum nfqnl_msg_config_cmds { - NFQNL_CFG_CMD_NONE, - NFQNL_CFG_CMD_BIND, - NFQNL_CFG_CMD_UNBIND, - NFQNL_CFG_CMD_PF_BIND, - NFQNL_CFG_CMD_PF_UNBIND, -}; - -struct nfqnl_msg_config_cmd { - __u8 command; /* nfqnl_msg_config_cmds */ - __u8 _pad; - __be16 pf; /* AF_xxx for PF_[UN]BIND */ -}; - -enum nfqnl_config_mode { - NFQNL_COPY_NONE, - NFQNL_COPY_META, - NFQNL_COPY_PACKET, -}; - -struct nfqnl_msg_config_params { - __be32 copy_range; - __u8 copy_mode; /* enum nfqnl_config_mode */ -} __attribute__ ((packed)); - -enum nfqnl_flags { - NFQNL_F_NONE = 0, - NFQNL_F_CONNTRACK = (1 << 0), -}; - -enum nfqnl_attr_config { - NFQA_CFG_UNSPEC, - NFQA_CFG_CMD, /* nfqnl_msg_config_cmd */ - NFQA_CFG_PARAMS, /* nfqnl_msg_config_params */ - NFQA_CFG_QUEUE_MAXLEN, /* __u32 */ - NFQA_CFG_FLAGS, /* __u32 */ - __NFQA_CFG_MAX -}; -#define NFQA_CFG_MAX (__NFQA_CFG_MAX-1) - -#endif /* _NFNETLINK_QUEUE_H */ diff --git a/src/nlmsg.c b/src/nlmsg.c index 6c4a139..e592ebd 100644 --- a/src/nlmsg.c +++ b/src/nlmsg.c @@ -19,8 +19,6 @@ #define __aligned_le64 __le64 __attribute__((aligned(8))) #endif -#include <linux/netfilter/nfnetlink_queue.h> - #include <libnetfilter_queue/libnetfilter_queue.h> #include "internal.h"
This is a partial revert of a0c885ae5a79457aa592cb70c27a7dee619762a4 Specifically, it removes the header linux/netfilter/nfnetlink_queue.h added in that commit. 1), there is already a /usr/include/linux/netfilter/nfnetlink_queue.h, which is part of the linux kernel API 2), we already have include/libnetfilter_queue/linux_nfnetlink_queue.h which contains the same definitions/structures/macros, so it makes little sense to have two headers in libnetfilter_queue that share almost their entire content. [ worse, the nfnetlink_queue.h header reverted here actually is incompatible with mainline kernels, since a few defines have the wrong value ... ] Signed-off-by: Florian Westphal <fw@strlen.de> --- Makefile.am | 2 +- configure.ac | 3 +- include/Makefile.am | 2 +- include/linux/Makefile.am | 1 - include/linux/netfilter/Makefile.am | 1 - include/linux/netfilter/nfnetlink_queue.h | 98 ----------------------------- src/nlmsg.c | 2 - 7 files changed, 3 insertions(+), 106 deletions(-) delete mode 100644 include/linux/Makefile.am delete mode 100644 include/linux/netfilter/Makefile.am delete mode 100644 include/linux/netfilter/nfnetlink_queue.h