Rework nfnetlink_queue conntrack support

net/netfilter/nf_conntrack_netlink.c +8 -4

index 94a6654..5fd3d48 100644
@@ -596,11 +596,9 @@ ctnetlink_timestamp_size(const struct nf_conn *ct)
 #endif
 }
 
-static inline size_t
-ctnetlink_nlmsg_size(const struct nf_conn *ct)
+static size_t ctnetlink_nlmsg_payload_size(const struct nf_conn *ct)
 {
-	return NLMSG_ALIGN(sizeof(struct nfgenmsg))
-	       + 3 * nla_total_size(0) /* CTA_TUPLE_ORIG|REPL|MASTER */
+	return   3 * nla_total_size(0) /* CTA_TUPLE_ORIG|REPL|MASTER */
 	       + 3 * nla_total_size(0) /* CTA_TUPLE_IP */
 	       + 3 * nla_total_size(0) /* CTA_TUPLE_PROTO */
 	       + 3 * nla_total_size(sizeof(u_int8_t)) /* CTA_PROTO_NUM */

@@ -628,6 +626,12 @@ ctnetlink_nlmsg_size(const struct nf_conn *ct)
 	       ;
 }
 
+static size_t ctnetlink_nlmsg_size(const struct nf_conn *ct)
+{
+	return NLMSG_ALIGN(sizeof(struct nfgenmsg)) +
+	       ctnetlink_nlmsg_payload_size(ct);
+}
+
 #ifdef CONFIG_NF_CONNTRACK_EVENTS
 static int
 ctnetlink_conntrack_event(unsigned int events, struct nf_ct_event *item)

On Tue, Sep 08, 2015 at 06:54:49PM +0200, Pablo Neira Ayuso wrote:
[...]

Quoted 32 lines

net/netfilter/nf_conntrack_netlink.c +8 -4

index 94a6654..5fd3d48 100644
@@ -596,11 +596,9 @@ ctnetlink_timestamp_size(const struct nf_conn *ct)
 #endif
 }
 
-static inline size_t
-ctnetlink_nlmsg_size(const struct nf_conn *ct)
+static size_t ctnetlink_nlmsg_payload_size(const struct nf_conn *ct)
 {
-	return NLMSG_ALIGN(sizeof(struct nfgenmsg))
-	       + 3 * nla_total_size(0) /* CTA_TUPLE_ORIG|REPL|MASTER */
+	return   3 * nla_total_size(0) /* CTA_TUPLE_ORIG|REPL|MASTER */
 	       + 3 * nla_total_size(0) /* CTA_TUPLE_IP */
 	       + 3 * nla_total_size(0) /* CTA_TUPLE_PROTO */
 	       + 3 * nla_total_size(sizeof(u_int8_t)) /* CTA_PROTO_NUM */

@@ -628,6 +626,12 @@ ctnetlink_nlmsg_size(const struct nf_conn *ct)
 	       ;
 }
 
+static size_t ctnetlink_nlmsg_size(const struct nf_conn *ct)
+{
+	return NLMSG_ALIGN(sizeof(struct nfgenmsg)) +
+	       ctnetlink_nlmsg_payload_size(ct);
+}
+
 #ifdef CONFIG_NF_CONNTRACK_EVENTS
 static int
 ctnetlink_conntrack_event(unsigned int events, struct nf_ct_event *item)

net/netfilter/nf_conntrack_netlink.c ctnetlink_alloc_expect(const struct nlattr *const cda[], struct nf_conn *ct, · line 2138

Quoted 5 lines

net/netfilter/nf_conntrack_netlink.c +0 -2

@@ -2134,34 +2138,6 @@ ctnetlink_alloc_expect(const struct nlattr *const cda[], struct nf_conn *ct,
 		       struct nf_conntrack_tuple *mask);
 
 #ifdef CONFIG_NETFILTER_NETLINK_QUEUE_CT
-static size_t
-ctnetlink_nfqueue_build_size(const struct nf_conn *ct)

A little apart from this issue, it seems that original ctnetlink_nlmsg_size() lacks CTA_USE size.

A conntrack information attached to nfqueue does not have CTA_COUNTERS_ and also CTA_USE attrs, but this change allocates a space for it. If we reuse ctnetlink_nlmsg_size() to avoid a code duplication, I think it would be better to comment it.

rename from net/netfilter/nfnetlink_queue_core.c
rename to net/netfilter/nfnetlink_queue.c

On Tue, Sep 08, 2015 at 06:54:51PM +0200, Pablo Neira Ayuso wrote:
[...]

Quoted 2 lines

rename from net/netfilter/nfnetlink_queue_core.c
rename to net/netfilter/nfnetlink_queue.c

So you know though, we can delete the nfnetlink_queue_core.o line from Makefile.

May I suggest one thing? How about returning -EOPNOTSUPP at nfqnl_recv_config() if derefering nfq_ct_hook is NULL?

BTW, I think it would be good to add the missing code on nfqnl_recv_verdict_batch() to support conntrack there too. Would you

Quoted 4 lines · Pablo Neira Ayuso

[...]
May I suggest one thing? How about returning -EOPNOTSUPP at nfqnl_recv_config() if derefering nfq_ct_hook is NULL?

Makes sense. Thus, the user knows that it is not supported, please send a patch for that.

They are totally untested, so please carefully review them.

This patch enables nf-queue in examples directory to show conntrack information if --enable-nfct configure option is specified.

examples/nf-queue.c nfq_send_verdict(int queue_num, uint32_t id) · line 63

Quoted 3 lines · Ken-ichirou MATSUZAWA

examples/nf-queue.c +3 -0

+	if (attr == NULL)
+		return MNL_CB_OK;
+	attr->nla_len = 12;

What this?

Quoted 6 lines · Florian Westphal

examples/nf-queue.c nfq_send_verdict(int queue_num, uint32_t id) · line 63

Quoted 3 lines · Ken-ichirou MATSUZAWA

examples/nf-queue.c +3 -0

+	if (attr == NULL)
+		return MNL_CB_OK;
+	attr->nla_len = 12;

What this?

I just wanted to test const qualifier... sorry.

Would you mind deleting the line? or should I send it again?

Quoted 2 lines · Ken-ichirou MATSUZAWA

This patch enables nf-queue in examples directory to show conntrack information if --enable-nfct configure option is specified.

Thanks for doing this!

Quoted 3 lines · Florian Westphal

Quoted 2 lines · Ken-ichirou MATSUZAWA

This patch enables nf-queue in examples directory to show conntrack information if --enable-nfct configure option is specified.

Any chance we can get rid of --enable-nfct configure option?

Hi Pablo,

Thank you for taking your time.

On Tue, Sep 08, 2015 at 06:54:48PM +0200, Pablo Neira Ayuso wrote:

Quoted 1 line

They are totally untested, so please carefully review them.

I'm sorry I could not to meet your expectations since I might not understand your intention correctly, please let me ask three questions:

* We will not modularize nfqueue attaching conntrack information part? (was nfnetlink_queue_ct.c)

* May I update nfqnl_ct prefix to nfnl_ct after this patch series applied to attach nfct info to nflog?

* We do not add another Kconfig for nflog attaching ct info?

I tested your patch a little with following lnf-queue patch and it works fine.

Quoted 1 line · Ken-ichirou MATSUZAWA

* We do not add another Kconfig for nflog attaching ct info?

I think you can add it too, yes.

So will you send a follow up patchset on top of those three to add what you need?

The idea of this series of patch is to attach conntrack information to nflog like nfqueue has already done. nfqueue conntrack info attaching basis is generic, rename those names to generic one, glue.

Signed-off-by Ken-ichirou MATSUZAWA <[email protected]>

This patch validate NFULA_CT and NFULA_CT_INFO attributes,

This patch enables to include the conntrack information together with the packet that is sent to user-space via NFLOG, then a user-space program can acquire NATed information by this NFULA_CT attribute.

Including the conntrack information is optional, you can set it via NFULNL_CFG_F_CONNTRACK flag with the NFULA_CFG_FLAGS attribute like NFQUEUE.

Signed-off-by Ken-ichirou MATSUZAWA <[email protected]>

---
 include/uapi/linux/netfilter/nfnetlink_log.h |    3 ++
 net/netfilter/Kconfig                        |    8 +++++
 net/netfilter/nf_conntrack_netlink.c         |    8 ++---
 net/netfilter/nfnetlink_log.c                |   42 ++++++++++++++++++++++----
 4 files changed, 51 insertions(+), 10 deletions(-)

include/uapi/linux/netfilter/nfnetlink_log.h +3 -0

index 90c2c95..081e7f9 100644
@@ -51,6 +51,8 @@ enum nfulnl_attr_type {
 	NFULA_HWTYPE,			/* hardware type */
 	NFULA_HWHEADER,			/* hardware header */
 	NFULA_HWLEN,			/* hardware header length */
+	NFULA_CT,			/* nf_conntrack_netlink.h */
+	NFULA_CT_INFO,			/* enum ip_conntrack_info */
 
 	__NFULA_MAX
 };

@@ -93,5 +95,6 @@ enum nfulnl_attr_config {
 
 #define NFULNL_CFG_F_SEQ	0x0001
 #define NFULNL_CFG_F_SEQ_GLOBAL	0x0002
+#define NFULNL_CFG_F_CONNTRACK	0x0004
 
 #endif /* _NFNETLINK_LOG_H */

net/netfilter/Kconfig +0 -0

index 3e1b4ab..a8853c8 100644

net/netfilter/Kconfig +5 -0

+        depends on NETFILTER_NETLINK_LOG
+	help
+	  If this option is enabled, NFLOG can include Connection Tracking
+	  information.
+
 config NF_NAT
 	tristate
 

net/netfilter/nf_conntrack_netlink.c +4 -4

index 155b2d0..ccdce3a 100644
@@ -2138,7 +2138,7 @@ ctnetlink_alloc_expect(const struct nlattr *const cda[], struct nf_conn *ct,
 		       struct nf_conntrack_tuple *tuple,
 		       struct nf_conntrack_tuple *mask);
 
-#ifdef CONFIG_NETFILTER_NETLINK_QUEUE_CT
+#if defined(CONFIG_NETFILTER_NETLINK_QUEUE_CT) || defined(NETFILTER_NETLINK_LOG_CT)
 static struct nf_conn *ctnetlink_glue_get_ct(const struct sk_buff *skb,
 					     enum ip_conntrack_info *ctinfo)
 {

@@ -2382,7 +2382,7 @@ static struct nfnl_ct_hook ctnetlink_glue_hook = {
 	.attach_expect	= ctnetlink_glue_attach_expect,
 	.seq_adjust	= ctnetlink_glue_seqadj,
 };
-#endif /* CONFIG_NETFILTER_NETLINK_QUEUE_CT */
+#endif /* CONFIG_NETFILTER_NETLINK_QUEUE_CT || NETFILTER_NETLINK_LOG_CT */
 
 /***********************************************************************
  * EXPECT

@@ -3366,7 +3366,7 @@ static int __init ctnetlink_init(void)
 		pr_err("ctnetlink_init: cannot register pernet operations\n");
 		goto err_unreg_exp_subsys;
 	}
-#ifdef CONFIG_NETFILTER_NETLINK_QUEUE_CT
+#if defined(CONFIG_NETFILTER_NETLINK_QUEUE_CT) || defined(NETFILTER_NETLINK_LOG_CT)
 	/* setup interaction between nf_queue and nf_conntrack_netlink. */
 	RCU_INIT_POINTER(nfnl_ct_hook, &ctnetlink_glue_hook);
 #endif

@@ -3387,7 +3387,7 @@ static void __exit ctnetlink_exit(void)
 	unregister_pernet_subsys(&ctnetlink_net_ops);
 	nfnetlink_subsys_unregister(&ctnl_exp_subsys);
 	nfnetlink_subsys_unregister(&ctnl_subsys);
-#ifdef CONFIG_NETFILTER_NETLINK_QUEUE_CT
+#if defined(CONFIG_NETFILTER_NETLINK_QUEUE_CT) || defined(NETFILTER_NETLINK_LOG_CT)
 	RCU_INIT_POINTER(nfnl_ct_hook, NULL);
 #endif
 }

net/netfilter/nfnetlink_log.c +5 -1

index 4670821..9d04c21 100644
@@ -27,6 +27,7 @@
 #include <net/netlink.h>
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nfnetlink_log.h>
+#include <linux/netfilter/nf_conntrack_common.h>
 #include <linux/spinlock.h>
 #include <linux/sysctl.h>
 #include <linux/proc_fs.h>

@@ -401,7 +402,9 @@ __build_packet_message(struct nfnl_log_net *log,
 			unsigned int hooknum,
 			const struct net_device *indev,
 			const struct net_device *outdev,
-			const char *prefix, unsigned int plen)
+			const char *prefix, unsigned int plen,
+			struct nf_conn *ct, enum ip_conntrack_info ctinfo)
+
 {
 	struct nfulnl_msg_packet_hdr pmsg;
 	struct nlmsghdr *nlh;

@@ -409,6 +412,7 @@ __build_packet_message(struct nfnl_log_net *log,
 	sk_buff_data_t old_tail = inst->skb->tail;
 	struct sock *sk;
 	const unsigned char *hwhdrp;
+	struct nfnl_ct_hook *nflog_ct;
 
 	nlh = nlmsg_put(inst->skb, 0, 0,
 			NFNL_SUBSYS_ULOG << 8 | NFULNL_MSG_PACKET,

net/netfilter/nfnetlink_log.c +21 -3

+	if (ct) {
+		nflog_ct = rcu_dereference(nfnl_ct_hook);
+		if (nflog_ct->build(inst->skb, ct, ctinfo,
+				    NFULA_CT, NFULA_CT_INFO) < 0)
+			goto nla_put_failure;
+	}
+
 	if (data_len) {
 		struct nlattr *nla;
 		int size = nla_attr_size(data_len);

@@ -620,12 +632,16 @@ nfulnl_log_packet(struct net *net,
 		  const struct nf_loginfo *li_user,
 		  const char *prefix)
 {
-	unsigned int size, data_len;
+	size_t size;
+	unsigned int data_len;
 	struct nfulnl_instance *inst;
 	const struct nf_loginfo *li;
 	unsigned int qthreshold;
 	unsigned int plen;
 	struct nfnl_log_net *log = nfnl_log_pernet(net);
+	struct nfnl_ct_hook *nflog_ct;
+	struct nf_conn *ct = NULL;
+	enum ip_conntrack_info uninitialized_var(ctinfo);
 
 	if (li_user && li_user->type == NF_LOG_TYPE_ULOG)
 		li = li_user;

@@ -671,7 +687,14 @@ nfulnl_log_packet(struct net *net,
 		size += nla_total_size(sizeof(u_int32_t));
 	if (inst->flags & NFULNL_CFG_F_SEQ_GLOBAL)
 		size += nla_total_size(sizeof(u_int32_t));
-
+	if (inst->flags & NFULNL_CFG_F_CONNTRACK) {
+		nflog_ct = rcu_dereference(nfnl_ct_hook);
+		if (nflog_ct != NULL) {
+			ct = nflog_ct->get_ct(skb, &ctinfo);
+			if (ct != NULL)
+				size += nflog_ct->build_size(ct);
+		}
+	}
 	qthreshold = inst->qthreshold;
 	/* per-rule qthreshold overrides per-instance */
 	if (li->u.ulog.qthreshold)

@@ -715,7 +738,7 @@ nfulnl_log_packet(struct net *net,
 	inst->qlen++;
 
 	__build_packet_message(log, inst, skb, data_len, pf,
-				hooknum, in, out, prefix, plen);
+				hooknum, in, out, prefix, plen, ct, ctinfo);
 
 	if (inst->qlen >= qthreshold)
 		__nfulnl_flush(inst);

net/netfilter/nfnetlink_log.c +8 -1

 		if (!inst) {
 			ret = -ENODEV;
 			goto out;
 		}
-		nfulnl_set_flags(inst, ntohs(flags));
+
+		if (flags & NFULNL_CFG_F_CONNTRACK &&
+		    rcu_dereference(nfnl_ct_hook) == NULL) {
+			ret = -EOPNOTSUPP;
+			goto out_put;
+		}
+
+		nfulnl_set_flags(inst, flags);
 	}
 
 out_put:

Signature · 1 line

1.7.10.4

utils/nf-log.c +8 -0

+#include <libnetfilter_log/linux_nfnetlink_log.h>
 
 #include <libmnl/libmnl.h>
 #include <libnetfilter_log/libnetfilter_log.h>
 
+#include "../config.h"
+#ifdef BUILD_NFCT
+#include <linux/netfilter/nf_conntrack_common.h>
+#include <libnetfilter_conntrack/libnetfilter_conntrack.h>
+#endif
+
+#ifdef BUILD_NFCT

utils/nf-log.c +4 -0

+	/* case IP_CT_ESTABLISHED_REPLY: == IP_CT_IS_REPLY
+	 *	s = "IP_CT_ESTABLISHED_REPLY";
+	 *	break;
+	 */

utils/nf-log.c line 6

Quoted 2 lines · Ken-ichirou MATSUZAWA

utils/nf-log.c +1 -1

-#include <linux/netfilter/nfnetlink_log.h>
+/* #include <linux/netfilter/nfnetlink_log.h> */

Do you remember why you had to comment out this line?

utils/nf-log.c line 28

Quoted 3 lines

utils/nf-log.c +3 -0

+	switch (ntohl(mnl_attr_get_u32(attr))) {
+	case IP_CT_ESTABLISHED:
+		s = "IP_CT_ESTABLISHED";

For this example, you can probably simplify this with.

        case IP_CT_ESTABLISHED:
        case IP_CT_ESTABLISHED_REPLY:
                s = "ESTABLISHED";

IP_CT_IS_REPLY is never used alone, it's is always combined with NEW, ESTABLISHED, RELATED states.

IP_CT_NUMBER should never happen. Please remove this code.

utils/nf-log.c +0 -0

index 5f2a192..006d4b0 100644

I think you can update configure.ac to update CFLAGS, like this:

CFLAGS="$CFLAGS -DBUILD_NFCT"

so we can skip this include.

gcc -lnetfilter_log -lnetfilter_conntrack -lmnl -DBUILD_NFCT nf-log.c -o nf-log

This patch enables nf-log in utils directory to show conntrack information if libnetfilter_conntrack exists.

Hi, sorry for so much late.

Quoted 5 lines · Pablo Neira Ayuso

I think you can update configure.ac to update CFLAGS, like this:

CFLAGS="$CFLAGS -DBUILD_NFCT"

so we can skip this include.

We can avoid to (re)run autogen.sh without lnf-conntrack at first then install it after, but

Quoted 1 line

gcc -lnetfilter_log -lnetfilter_conntrack -lmnl -DBUILD_NFCT nf-log.c -o nf-log

this make a sense, I follow your advice.

I have met another issue related to lnf-conntrack, could you give me some advice? The following patch is compile error:

In file included from nf-log.c:14:0:
/usr/include/libnetfilter_conntrack/libnetfilter_conntrack.h:729:6: error: nested redefinition of ‘enum ip_conntrack_status’
enum ip_conntrack_status {
^
/usr/include/libnetfilter_conntrack/libnetfilter_conntrack.h:729:6: error: redeclaration of ‘enum ip_conntrack_status’ In file included from nf-log.c:12:0:
/usr/include/linux/netfilter/nf_conntrack_common.h:37:6: note: originally defined here
enum ip_conntrack_status {

since both libnetfilter_conntrack.h and nf_conntrack_common.h defines ip_conntrack_status. Could you tell me how do I avoid such a error when using ip_conntrack_status and ip_conntrack_info?