xtables2 a8, netlink interface

xtables2 a8, netlink interface

[Jan Engelhardt]

So here is the first set of patches implementing part of the xt2 core
and nfnl interface. Please review, I am sure you will have something
to say :)

The userspace part is at git://dev.medozas.de/libnetfilter_xtables,
the xtnl-test program compiled as part of the default make target
gives access to the kernel functions implemented by the patches.

===
The following changes since commit c3b92c8787367a8bb53d57d9789b558f1295cc96:

  Linux 3.1 (2011-10-24 09:10:05 +0200)

are available in the git repository at:
  git://dev.medozas.de/linux xtables2_a8

Jan Engelhardt (7):
      netfilter: xtables2: initial table skeletal functions
      netfilter: xtables2: initial Netlink interface
      netfilter: xtables2: chain creation and deletion
      netfilter: xtables2: chain renaming support
      netfilter: xtables2: initial table replace support
      netfilter: xtables2: transaction abort support
      netfilter: xtables2: redirect writes into transaction buffer

 include/linux/netfilter/Kbuild              |    1 +
 include/linux/netfilter/nfnetlink.h         |    3 +-
 include/linux/netfilter/nfnetlink_xtables.h |   48 +++
 include/net/netfilter/x_tables2.h           |   50 +++
 net/netfilter/Kconfig                       |   16 +-
 net/netfilter/Makefile                      |    2 +
 net/netfilter/xt2_core.c                    |  199 +++++++++
 net/netfilter/xt2_nfnetlink.c               |  584 +++++++++++++++++++++++++++
 8 files changed, 901 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/netfilter/nfnetlink_xtables.h
 create mode 100644 include/net/netfilter/x_tables2.h
 create mode 100644 net/netfilter/xt2_core.c
 create mode 100644 net/netfilter/xt2_nfnetlink.c

[PATCH 1/7] netfilter: xtables2: initial table skeletal functions

[Jan Engelhardt]

This patch adds the xt2 table functions. Of course this does not do
anything useful yet, chain and rule support directly follow.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/net/netfilter/x_tables2.h |   17 +++++++
 net/netfilter/Kconfig             |    8 +++-
 net/netfilter/Makefile            |    1 +
 net/netfilter/xt2_core.c          |   85 +++++++++++++++++++++++++++++++++++++
 4 files changed, 110 insertions(+), 1 deletions(-)
 create mode 100644 include/net/netfilter/x_tables2.h
 create mode 100644 net/netfilter/xt2_core.c

diff --git a/include/net/netfilter/x_tables2.h b/include/net/netfilter/x_tables2.h
new file mode 100644
index 0000000..a219952
--- /dev/null
+++ b/include/net/netfilter/x_tables2.h
@@ -0,0 +1,17 @@
+#ifndef _NET_NETFILTER_XTABLES2_H
+#define _NET_NETFILTER_XTABLES2_H 1
+
+#define XTABLES2_VTAG "Xtables2 A8"
+
+/**
+ * @master:	the master table
+ */
+struct xt2_pernet_data {
+	struct xt2_table __rcu *master;
+};
+
+struct xt2_table {
+	int _dummy;
+};
+
+#endif /* _NET_NETFILTER_XTABLES2_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 32bff6d..5b3d9ca 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -321,7 +321,13 @@ config NETFILTER_XTABLES
 	  This is required if you intend to use any of ip_tables,
 	  ip6_tables or arp_tables.
 
-if NETFILTER_XTABLES
+config NETFILTER_XTABLES2
+	tristate "Netfilter Xtables2 packet filtering"
+	---help---
+	Xtables2 is a rework of the internal architecture of Xtables.
+	It supersedes iptables, ip6tables, arptables and ebtables.
+
+if NETFILTER_XTABLES || NETFILTER_XTABLES2
 
 comment "Xtables combined modules"
 
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 1a02853..8504ebd 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_NETFILTER_TPROXY) += nf_tproxy_core.o
 
 # generic X tables 
 obj-$(CONFIG_NETFILTER_XTABLES) += x_tables.o xt_tcpudp.o
+obj-$(CONFIG_NETFILTER_XTABLES2) += xt2_core.o
 
 # combos
 obj-$(CONFIG_NETFILTER_XT_MARK) += xt_mark.o
diff --git a/net/netfilter/xt2_core.c b/net/netfilter/xt2_core.c
new file mode 100644
index 0000000..ab73c4d
--- /dev/null
+++ b/net/netfilter/xt2_core.c
@@ -0,0 +1,85 @@
+/*
+ *	Xtables2 core
+ *	Copyright © Jan Engelhardt, 2009-2012
+ *
+ *	This program is free software: you can redistribute it and/or modify
+ *	it under the terms of the GNU General Public License as published by
+ *	the Free Software Foundation, either version 2 of the License, or
+ *	(at your option) any later version.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <net/net_namespace.h>
+#include <net/netns/generic.h>
+#include <net/netfilter/x_tables2.h>
+
+MODULE_DESCRIPTION("Netfilter Xtables2 packet filtering");
+MODULE_AUTHOR("Jan Engelhardt");
+MODULE_LICENSE("GPL");
+
+static int xtables2_net_id __read_mostly;
+
+static inline struct xt2_pernet_data *xtables2_pernet(struct net *net)
+{
+	return net_generic(net, xtables2_net_id);
+}
+
+/**
+ * Create a new table with no chains and no rules.
+ */
+static struct xt2_table *xt2_table_new(void)
+{
+	struct xt2_table *table;
+
+	table = kzalloc(sizeof(*table), GFP_KERNEL);
+	if (table == NULL)
+		return NULL;
+
+	return table;
+}
+
+static void xt2_table_free(struct xt2_table *table)
+{
+	kfree(table);
+}
+
+static int __net_init xtables2_net_init(struct net *net)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(net);
+
+	pnet->master = xt2_table_new();
+	if (IS_ERR(pnet->master))
+		return PTR_ERR(pnet->master);
+	return 0;
+}
+
+static void __net_exit xtables2_net_exit(struct net *net)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(net);
+
+	xt2_table_free(pnet->master);
+}
+
+static struct pernet_operations xtables2_pernet_ops = {
+	.init = xtables2_net_init,
+	.exit = xtables2_net_exit,
+	.id   = &xtables2_net_id,
+	.size = sizeof(struct xt2_pernet_data),
+};
+
+static int __init xtables2_init(void)
+{
+	pr_info(XTABLES2_VTAG ", (C) 2009-2012, J.Engelhardt\n");
+	return register_pernet_subsys(&xtables2_pernet_ops);
+}
+
+static void __exit xtables2_exit(void)
+{
+	unregister_pernet_subsys(&xtables2_pernet_ops);
+}
+
+module_init(xtables2_init);
+module_exit(xtables2_exit);
-- 
1.7.7

--
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 2/7] netfilter: xtables2: initial Netlink interface

[Jan Engelhardt]

This populates xt2_nfnetlink.c with support for NFXTM_IDENTIFY. Right
now that just returns a freeform string but eventually shall dump all
the match and target modules' info.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/linux/netfilter/Kbuild              |    1 +
 include/linux/netfilter/nfnetlink.h         |    3 +-
 include/linux/netfilter/nfnetlink_xtables.h |   20 ++++
 net/netfilter/Kconfig                       |    8 ++
 net/netfilter/Makefile                      |    1 +
 net/netfilter/xt2_nfnetlink.c               |  153 +++++++++++++++++++++++++++
 6 files changed, 185 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/netfilter/nfnetlink_xtables.h
 create mode 100644 net/netfilter/xt2_nfnetlink.c

diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
index a1b410c..6e568c9 100644
--- a/include/linux/netfilter/Kbuild
+++ b/include/linux/netfilter/Kbuild
@@ -10,6 +10,7 @@ header-y += nfnetlink_compat.h
 header-y += nfnetlink_conntrack.h
 header-y += nfnetlink_log.h
 header-y += nfnetlink_queue.h
+header-y += nfnetlink_xtables.h
 header-y += x_tables.h
 header-y += xt_AUDIT.h
 header-y += xt_CHECKSUM.h
diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index 74d3386..93228b4 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -48,7 +48,8 @@ struct nfgenmsg {
 #define NFNL_SUBSYS_ULOG		4
 #define NFNL_SUBSYS_OSF			5
 #define NFNL_SUBSYS_IPSET		6
-#define NFNL_SUBSYS_COUNT		7
+#define NFNL_SUBSYS_XTABLES		8
+#define NFNL_SUBSYS_COUNT		9
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/netfilter/nfnetlink_xtables.h b/include/linux/netfilter/nfnetlink_xtables.h
new file mode 100644
index 0000000..4c53042
--- /dev/null
+++ b/include/linux/netfilter/nfnetlink_xtables.h
@@ -0,0 +1,20 @@
+#ifndef _LINUX_NFNETLINK_XTABLES_H
+#define _LINUX_NFNETLINK_XTABLES_H 1
+
+enum nfxt_msg_type {
+	NFXTM_IDENTIFY = 1,
+};
+
+/**
+ * %NFXTA_NAME:			name of the object being operated on
+ */
+enum nfxt_attr_type {
+	NFXTA_UNSPEC = 0,
+	NFXTA_NAME,
+};
+
+enum nfxt_errno {
+	NFXTE_SUCCESS = 0,
+};
+
+#endif /* _LINUX_NFNETLINK_XTABLES_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 5b3d9ca..6237a30 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -327,6 +327,14 @@ config NETFILTER_XTABLES2
 	Xtables2 is a rework of the internal architecture of Xtables.
 	It supersedes iptables, ip6tables, arptables and ebtables.
 
+if NETFILTER_XTABLES2
+
+config NETFILTER_XTNFNETLINK
+	tristate "Configuration interface over nfnetlink"
+	default m if NETFILTER_ADVANCED=n
+
+endif # NETFILTER_XTABLES2
+
 if NETFILTER_XTABLES || NETFILTER_XTABLES2
 
 comment "Xtables combined modules"
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 8504ebd..634e6a8 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_NETFILTER_TPROXY) += nf_tproxy_core.o
 # generic X tables 
 obj-$(CONFIG_NETFILTER_XTABLES) += x_tables.o xt_tcpudp.o
 obj-$(CONFIG_NETFILTER_XTABLES2) += xt2_core.o
+obj-$(CONFIG_NETFILTER_XTNFNETLINK) += xt2_nfnetlink.o
 
 # combos
 obj-$(CONFIG_NETFILTER_XT_MARK) += xt_mark.o
diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
new file mode 100644
index 0000000..3dc241f
--- /dev/null
+++ b/net/netfilter/xt2_nfnetlink.c
@@ -0,0 +1,153 @@
+/*
+ *	Xtables2 nfnetlink interface
+ *	Copyright © Jan Engelhardt, 2010-2012
+ *
+ *	This program is free software: you can redistribute it and/or modify
+ *	it under the terms of the GNU General Public License as published by
+ *	the Free Software Foundation, either version 2 of the License, or
+ *	(at your option) any later version.
+ */
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/netlink.h>
+#include <linux/skbuff.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter/nfnetlink.h>
+#include <linux/netfilter/nfnetlink_xtables.h>
+#include <net/netlink.h>
+#include <net/netfilter/x_tables2.h>
+
+MODULE_DESCRIPTION("Xtables2 nfnetlink interface");
+MODULE_AUTHOR("Jan Engelhardt");
+MODULE_LICENSE("GPL");
+
+/**
+ * Something to keep all the pointers that NL gives us in one place.
+ */
+struct xtnetlink_pktref {
+	union {
+		struct sk_buff *skb;
+		const struct sk_buff *c_skb;
+	};
+	union {
+		struct nlmsghdr *msg;
+		const struct nlmsghdr *c_msg;
+	};
+};
+
+/**
+ * @skb:	outgoing skb
+ * @old:	pointers to the original incoming skb/nl headers
+ * @flags:	extra flags to set in nlmsg
+ *
+ * Fill an skb (for outgoing direction) with NL and NFNL headers, using data
+ * from the original skb for initialization.
+ */
+static struct nlmsghdr *
+xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
+	       unsigned int flags)
+{
+	uint16_t nlmsg_type = NFNL_MSG_TYPE(old->msg->nlmsg_type);
+	struct nlmsghdr *nlmsg;
+	struct nfgenmsg *hdr;
+
+	nlmsg_type |= NFNL_SUBSYS_XTABLES << 8;
+	nlmsg = nlmsg_put(skb, NETLINK_CB(old->skb).pid, old->msg->nlmsg_seq,
+			  nlmsg_type, sizeof(*hdr), flags);
+	if (nlmsg == NULL) {
+		nlmsg_cancel(skb, nlmsg);
+		return ERR_PTR(-ENOBUFS);
+	}
+
+	hdr               = nlmsg_data(nlmsg);
+	hdr->nfgen_family = NFPROTO_UNSPEC;
+	hdr->version      = NFNETLINK_V0;
+	hdr->res_id       = 0;
+	return nlmsg;
+}
+
+/**
+ * Ran too often into NULL derefs. Now there is a dummy function for unused
+ * message type 0.
+ */
+static int
+xtnetlink_ignore(struct sock *k, struct sk_buff *s,
+		 const struct nlmsghdr *n, const struct nlattr *const *a)
+{
+	return -ENXIO;
+}
+
+/**
+ * Respond to a %NFXTM_IDENTIFY inquiry. Reports the Xtables2 version (for
+ * now), and in future the extension info.
+ */
+static int
+xtnetlink_identify2(struct sk_buff *skb, struct netlink_callback *nl_cb)
+{
+	struct xtnetlink_pktref ref = {.c_skb = skb, .c_msg = nl_cb->nlh};
+	struct nlmsghdr *nlmsg = NULL;
+
+	switch (nl_cb->args[0]) {
+	case 0:
+		nlmsg = xtnetlink_fill(skb, &ref, NLM_F_MULTI);
+		if (IS_ERR(nlmsg))
+			return 0;
+		NLA_PUT_STRING(skb, NFXTA_NAME, XTABLES2_VTAG);
+		++nl_cb->args[0];
+		break;
+	}
+	if (nlmsg != NULL)
+		nlmsg_end(skb, nlmsg);
+	return skb->len;
+ nla_put_failure:
+	return 0;
+}
+
+static int
+xtnetlink_identify(struct sock *xtnl, struct sk_buff *iskb,
+		   const struct nlmsghdr *imsg, const struct nlattr *const *ad)
+{
+	return netlink_dump_start(xtnl, iskb, imsg, xtnetlink_identify2,
+				  NULL, 0);
+}
+
+static const struct nla_policy xtnetlink_policy[] = {
+	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
+};
+
+/*
+ * Use the same policy for all messages. I do not want to see EINVAL anytime
+ * soon again just because I forgot sending an attribute from userspace.
+ * (If such occurs, it will be dealt with %NFXTE_ATTRSET_INCOMPLETE, tbd.)
+ */
+#define pol \
+	.policy = xtnetlink_policy, \
+	.attr_count = ARRAY_SIZE(xtnetlink_policy)
+static const struct nfnl_callback xtnetlink_callback[] = {
+	[0] = {.call = xtnetlink_ignore},
+	[NFXTM_IDENTIFY] = {.call = xtnetlink_identify, pol},
+};
+#undef pol
+
+static const struct nfnetlink_subsystem xtnetlink_subsys = {
+	.name      = "xtables",
+	.subsys_id = NFNL_SUBSYS_XTABLES,
+	.cb        = xtnetlink_callback,
+	.cb_count  = ARRAY_SIZE(xtnetlink_callback),
+};
+
+static int __init xtnetlink_init(void)
+{
+	return nfnetlink_subsys_register(&xtnetlink_subsys);
+}
+
+static void __exit xtnetlink_exit(void)
+{
+	nfnetlink_subsys_unregister(&xtnetlink_subsys);
+}
+
+module_init(xtnetlink_init);
+module_exit(xtnetlink_exit);
+MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_XTABLES);
-- 
1.7.7

--
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 3/7] netfilter: xtables2: chain creation and deletion

[Jan Engelhardt]

Notes: xt2_core will have two independent users: xt2_nfnetlink and the
xt1 translator, and they shall be separate modules, thereby
necessiting the use of EXPORT_SYMBOLs.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/linux/netfilter/nfnetlink_xtables.h |   18 ++++
 include/net/netfilter/x_tables2.h           |   31 ++++++-
 net/netfilter/xt2_core.c                    |   81 +++++++++++++++-
 net/netfilter/xt2_nfnetlink.c               |  146 ++++++++++++++++++++++++++-
 4 files changed, 273 insertions(+), 3 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_xtables.h b/include/linux/netfilter/nfnetlink_xtables.h
index 4c53042..fe1b6ce 100644
--- a/include/linux/netfilter/nfnetlink_xtables.h
+++ b/include/linux/netfilter/nfnetlink_xtables.h
@@ -3,18 +3,36 @@
 
 enum nfxt_msg_type {
 	NFXTM_IDENTIFY = 1,
+	NFXTM_CHAIN_NEW,
+	NFXTM_CHAIN_DEL,
 };
 
 /**
  * %NFXTA_NAME:			name of the object being operated on
+ * %NFXTA_ERRNO:		system error code (%Exxx)
+ * %NFXTA_XTERRNO:		NFXT-specific error code (cf. enum nfxt_errno)
  */
 enum nfxt_attr_type {
 	NFXTA_UNSPEC = 0,
 	NFXTA_NAME,
+	NFXTA_ERRNO,
+	NFXTA_XTERRNO,
 };
 
+/**
+ * %NFXTE_ATTRSET_INCOMPLETE:	Not all required attributes are present in nlmsg
+ * %NFXTE_CHAIN_INVALID_NAME:	Chain name is not acceptable
+ * %NFXTE_CHAIN_EXIST:		Chain already exists
+ * %NFXTE_CHAIN_NOENT:		Chain does not exist
+ * %NFXTE_CHAIN_NAMETOOLONG:	New chain name is too long
+ */
 enum nfxt_errno {
 	NFXTE_SUCCESS = 0,
+	NFXTE_ATTRSET_INCOMPLETE,
+	NFXTE_CHAIN_INVALID_NAME,
+	NFXTE_CHAIN_EXISTS,
+	NFXTE_CHAIN_NOENT,
+	NFXTE_CHAIN_NAMETOOLONG,
 };
 
 #endif /* _LINUX_NFNETLINK_XTABLES_H */
diff --git a/include/net/netfilter/x_tables2.h b/include/net/netfilter/x_tables2.h
index a219952..b13eab7 100644
--- a/include/net/netfilter/x_tables2.h
+++ b/include/net/netfilter/x_tables2.h
@@ -1,17 +1,46 @@
 #ifndef _NET_NETFILTER_XTABLES2_H
 #define _NET_NETFILTER_XTABLES2_H 1
 
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/rcupdate.h>
+
 #define XTABLES2_VTAG "Xtables2 A8"
 
 /**
  * @master:	the master table
+ * @master_lock:	protecting changes to @master
  */
 struct xt2_pernet_data {
 	struct xt2_table __rcu *master;
+	struct mutex master_lock;
 };
 
+/**
+ * @chain_list:		list of chains (struct xt2_chain)
+ * @lock:		protecting changes to @chain_list
+ */
 struct xt2_table {
-	int _dummy;
+	struct list_head chain_list;
+	struct mutex lock;
 };
 
+/**
+ * @anchor:	list anchor for parent (struct xt2_table.chain_list)
+ * @name:	name of chain; its large size is for the xt1 translator
+ * @rcu:	rcu head for delayed deletion
+ */
+struct xt2_chain {
+	struct list_head anchor;
+	char name[48];
+	struct rcu_head rcu;
+};
+
+struct net;
+
+extern struct xt2_pernet_data *xtables2_pernet(struct net *);
+extern struct xt2_chain *xt2_chain_new(struct xt2_table *, const char *);
+extern struct xt2_chain *xt2_chain_lookup(struct xt2_table *, const char *);
+extern void xt2_chain_free(struct xt2_chain *);
+
 #endif /* _NET_NETFILTER_XTABLES2_H */
diff --git a/net/netfilter/xt2_core.c b/net/netfilter/xt2_core.c
index ab73c4d..5e7426d 100644
--- a/net/netfilter/xt2_core.c
+++ b/net/netfilter/xt2_core.c
@@ -9,9 +9,12 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/err.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/rculist.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 #include <net/netfilter/x_tables2.h>
@@ -22,10 +25,79 @@ MODULE_LICENSE("GPL");
 
 static int xtables2_net_id __read_mostly;
 
-static inline struct xt2_pernet_data *xtables2_pernet(struct net *net)
+struct xt2_pernet_data *xtables2_pernet(struct net *net)
 {
 	return net_generic(net, xtables2_net_id);
 }
+EXPORT_SYMBOL_GPL(xtables2_pernet);
+
+/**
+ * @table:	table to add the new chain to
+ * @name:	name for the chain; may be %NULL
+ *
+ * Creates a new chain for @table using the given @name. @name may be NULL or
+ * the empty string, in which case an anonymous chain is created.
+ *
+ * Caller should hold @table->lock and verify chain uniqueness.
+ */
+struct xt2_chain *xt2_chain_new(struct xt2_table *table, const char *name)
+{
+	struct xt2_chain *chain;
+
+	if (strlen(name) >= ARRAY_SIZE(chain->name))
+		return ERR_PTR(-ENAMETOOLONG);
+	chain = kmalloc(sizeof(*chain), GFP_KERNEL);
+	if (chain == NULL)
+		return ERR_PTR(-ENOMEM);
+	INIT_LIST_HEAD(&chain->anchor);
+	if (name != NULL)
+		strncpy(chain->name, name, sizeof(chain->name));
+	else
+		*chain->name = '\0';
+	chain->name[sizeof(chain->name)-1] = '\0';
+	list_add_tail_rcu(&chain->anchor, &table->chain_list);
+	return chain;
+}
+EXPORT_SYMBOL_GPL(xt2_chain_new);
+
+/**
+ * @table:	table to search chain in
+ * @name:	name of desired chain
+ *
+ * Looks for a chain by its name in the given table.
+ * Caller should hold RCU if the chain is supposed to not go away.
+ * (= Caller can ignore RCU if it just wants an existence test.)
+ */
+struct xt2_chain *xt2_chain_lookup(struct xt2_table *table, const char *name)
+{
+	/* Future patch: Use better-suited data structure. */
+	struct xt2_chain *chain;
+
+	list_for_each_entry_rcu(chain, &table->chain_list, anchor)
+		if (strcmp(chain->name, name) == 0)
+			return chain;
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(xt2_chain_lookup);
+
+/**
+ * Frees the chain and all its associated memory.
+ */
+static void xt2_chain_free_rcu(struct rcu_head *rcu)
+{
+	kfree(container_of(rcu, struct xt2_chain, rcu));
+}
+
+void xt2_chain_free(struct xt2_chain *chain)
+{
+	/*
+	 * Not using kfree_rcu here, as we may want to free more,
+	 * in xt2_chain_free_rcu, soon.
+	 */
+	list_del_rcu(&chain->anchor);
+	call_rcu(&chain->rcu, xt2_chain_free_rcu);
+}
+EXPORT_SYMBOL_GPL(xt2_chain_free);
 
 /**
  * Create a new table with no chains and no rules.
@@ -38,11 +110,17 @@ static struct xt2_table *xt2_table_new(void)
 	if (table == NULL)
 		return NULL;
 
+	mutex_init(&table->lock);
+	INIT_LIST_HEAD(&table->chain_list);
 	return table;
 }
 
 static void xt2_table_free(struct xt2_table *table)
 {
+	struct xt2_chain *chain, *next;
+
+	list_for_each_entry_safe(chain, next, &table->chain_list, anchor)
+		xt2_chain_free(chain);
 	kfree(table);
 }
 
@@ -50,6 +128,7 @@ static int __net_init xtables2_net_init(struct net *net)
 {
 	struct xt2_pernet_data *pnet = xtables2_pernet(net);
 
+	mutex_init(&pnet->master_lock);
 	pnet->master = xt2_table_new();
 	if (IS_ERR(pnet->master))
 		return PTR_ERR(pnet->master);
diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
index 3dc241f..b50e468d 100644
--- a/net/netfilter/xt2_nfnetlink.c
+++ b/net/netfilter/xt2_nfnetlink.c
@@ -17,6 +17,7 @@
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nfnetlink_xtables.h>
 #include <net/netlink.h>
+#include <net/sock.h>
 #include <net/netfilter/x_tables2.h>
 
 MODULE_DESCRIPTION("Xtables2 nfnetlink interface");
@@ -69,6 +70,66 @@ xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
 }
 
 /**
+ * @ref:	skb/nl pointers that will be filled in (secondary return values)
+ * @old:	pointers to the original incoming skb/nl headers
+ *
+ * xtnetlink_fill can be used when the outgoing skb already exists (e.g. in
+ * case of a dump operation), but for non-dump responses, we have to create it
+ * ourselves.
+ */
+static int
+xtnetlink_new_fill(struct xtnetlink_pktref *ref,
+		   const struct xtnetlink_pktref *old)
+{
+	ref->skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (ref->skb == NULL)
+		return -ENOMEM;
+	ref->msg = xtnetlink_fill(ref->skb, old, 0);
+	if (IS_ERR(ref->msg)) {
+		kfree_skb(ref->skb);
+		return PTR_ERR(ref->msg);
+	}
+	return 0;
+}
+
+/**
+ * @xtnl:	socket to send the error packet out on
+ * @old:	pointers to the original incoming skb/nl headers
+ * @errcode:	last error code
+ *
+ * Create and send out an NFXT error packet. If @errcode is < 0, it indicates
+ * a system-level error (such as %-ENOMEM), reported back using %NFXTA_ERRNO.
+ * If @errcode is >= 0, it indicates an NFXT-specific error codes (%NFXTE_*),
+ * which is more fine grained than the dreaded %EINVAL, and which is reported
+ * back using %NFXTA_XTERRNO.
+ */
+static int
+xtnetlink_error(struct sock *xtnl, const struct xtnetlink_pktref *old,
+		int errcode)
+{
+	struct xtnetlink_pktref ref;
+	int ret;
+
+	ret = xtnetlink_new_fill(&ref, old);
+	if (ret < 0)
+		return ret;
+	if (errcode < 0)
+		/* Prefer positive numbers on the wire */
+		NLA_PUT_U32(ref.skb, NFXTA_ERRNO, -errcode);
+	else
+		NLA_PUT_U32(ref.skb, NFXTA_XTERRNO, errcode);
+	nlmsg_end(ref.skb, ref.msg);
+	ret = netlink_unicast(xtnl, ref.skb, NETLINK_CB(old->skb).pid,
+			      MSG_DONTWAIT);
+	if (ret < 0)
+		return ret;
+	/* ret is skb->len, but values >0 mean error to the caller -.- */
+	return 0;
+ nla_put_failure:
+	return -ENOBUFS;
+}
+
+/**
  * Ran too often into NULL derefs. Now there is a dummy function for unused
  * message type 0.
  */
@@ -113,14 +174,95 @@ xtnetlink_identify(struct sock *xtnl, struct sk_buff *iskb,
 				  NULL, 0);
 }
 
+static int
+xtnetlink_chain_new(struct sock *xtnl, struct sk_buff *iskb,
+		    const struct nlmsghdr *imsg, const struct nlattr *const *ad)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
+	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	const struct nlattr *attr;
+	struct xt2_table *table;
+	struct xt2_chain *chain;
+	const char *name;
+	int ret = 0;
+
+	attr = nlmsg_find_attr(imsg, sizeof(struct nfgenmsg), NFXTA_NAME);
+	if (attr == NULL)
+		return xtnetlink_error(xtnl, &ref, NFXTE_ATTRSET_INCOMPLETE);
+	name = nla_data(attr);
+	if (*name == '\0')
+		/* Anonymous chains are internal. */
+		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_INVALID_NAME);
+	/*
+	 * The table needs to stay, but note that rcu_read_lock cannot be used,
+	 * since we might sleep.
+	 */
+	mutex_lock(&pnet->master_lock);
+	table = pnet->master;
+	mutex_lock(&table->lock);
+	if (xt2_chain_lookup(table, name) != NULL) {
+		ret = xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_EXISTS);
+	} else {
+		chain = xt2_chain_new(table, name);
+		if (IS_ERR(chain))
+			ret = PTR_ERR(chain);
+		/* Use NFXTE error codes whenever possible. */
+		if (ret == -ENAMETOOLONG)
+			ret = NFXTE_CHAIN_NAMETOOLONG;
+		ret = xtnetlink_error(xtnl, &ref, ret);
+	}
+	mutex_unlock(&table->lock);
+	mutex_unlock(&pnet->master_lock);
+	return ret;
+}
+
+/**
+ * Act on a %NFXTM_CHAIN_DEL message.
+ */
+static int
+xtnetlink_chain_del(struct sock *xtnl, struct sk_buff *iskb,
+		    const struct nlmsghdr *imsg,
+		    const struct nlattr *const *ad)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
+	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	const struct nlattr *name_attr;
+	struct xt2_table *table;
+	struct xt2_chain *chain;
+	const char *name;
+	int ret = 0;
+
+	name_attr = nlmsg_find_attr(imsg, sizeof(struct nfgenmsg), NFXTA_NAME);
+	if (name_attr == NULL)
+		return xtnetlink_error(xtnl, &ref, NFXTE_ATTRSET_INCOMPLETE);
+	name = nla_data(name_attr);
+	if (*name == '\0')
+		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_NOENT);
+
+	mutex_lock(&pnet->master_lock);
+	table = pnet->master;
+	mutex_lock(&table->lock);
+	chain = xt2_chain_lookup(table, name);
+	if (chain != NULL)
+		xt2_chain_free(chain);
+	else
+		ret = NFXTE_CHAIN_NOENT;
+	ret = xtnetlink_error(xtnl, &ref, ret);
+	mutex_unlock(&table->lock);
+	mutex_unlock(&pnet->master_lock);
+	return ret;
+}
+
 static const struct nla_policy xtnetlink_policy[] = {
 	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
+	[NFXTA_ERRNO] = {.type = NLA_U32},
+	[NFXTA_XTERRNO] = {.type = NLA_U32},
 };
 
 /*
  * Use the same policy for all messages. I do not want to see EINVAL anytime
  * soon again just because I forgot sending an attribute from userspace.
- * (If such occurs, it will be dealt with %NFXTE_ATTRSET_INCOMPLETE, tbd.)
+ * (If such occurs, it will be dealt with %NFXTE_ATTRSET_INCOMPLETE.)
  */
 #define pol \
 	.policy = xtnetlink_policy, \
@@ -128,6 +270,8 @@ static const struct nla_policy xtnetlink_policy[] = {
 static const struct nfnl_callback xtnetlink_callback[] = {
 	[0] = {.call = xtnetlink_ignore},
 	[NFXTM_IDENTIFY] = {.call = xtnetlink_identify, pol},
+	[NFXTM_CHAIN_NEW] = {.call = xtnetlink_chain_new, pol},
+	[NFXTM_CHAIN_DEL] = {.call = xtnetlink_chain_del, pol},
 };
 #undef pol
 
-- 
1.7.7

[PATCH 4/7] netfilter: xtables2: chain renaming support

[Jan Engelhardt]

To keep in mind for later patches: for a ruleset to use
pointer-following chain jumps that work with the create-delete cycle
of renames of the target chain, it needs to point to something that
remains during that. What I thought up:

struct xt2_chain { char name[48]; struct xt2_p_chain *persistent; };
struct xt2_p_chain { struct xt2_chain *back; void *packed_data; };

and the ruleset pointing to a struct xt2_p_chain.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/linux/netfilter/nfnetlink_xtables.h |    3 ++
 include/net/netfilter/x_tables2.h           |    2 +
 net/netfilter/xt2_core.c                    |   31 +++++++++++++++++
 net/netfilter/xt2_nfnetlink.c               |   47 +++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_xtables.h b/include/linux/netfilter/nfnetlink_xtables.h
index fe1b6ce..296012a 100644
--- a/include/linux/netfilter/nfnetlink_xtables.h
+++ b/include/linux/netfilter/nfnetlink_xtables.h
@@ -5,18 +5,21 @@ enum nfxt_msg_type {
 	NFXTM_IDENTIFY = 1,
 	NFXTM_CHAIN_NEW,
 	NFXTM_CHAIN_DEL,
+	NFXTM_CHAIN_MOVE,
 };
 
 /**
  * %NFXTA_NAME:			name of the object being operated on
  * %NFXTA_ERRNO:		system error code (%Exxx)
  * %NFXTA_XTERRNO:		NFXT-specific error code (cf. enum nfxt_errno)
+ * %NFXTA_NEW_NAME:		new name of object
  */
 enum nfxt_attr_type {
 	NFXTA_UNSPEC = 0,
 	NFXTA_NAME,
 	NFXTA_ERRNO,
 	NFXTA_XTERRNO,
+	NFXTA_NEW_NAME,
 };
 
 /**
diff --git a/include/net/netfilter/x_tables2.h b/include/net/netfilter/x_tables2.h
index b13eab7..198ec31 100644
--- a/include/net/netfilter/x_tables2.h
+++ b/include/net/netfilter/x_tables2.h
@@ -42,5 +42,7 @@ extern struct xt2_pernet_data *xtables2_pernet(struct net *);
 extern struct xt2_chain *xt2_chain_new(struct xt2_table *, const char *);
 extern struct xt2_chain *xt2_chain_lookup(struct xt2_table *, const char *);
 extern void xt2_chain_free(struct xt2_chain *);
+extern struct xt2_chain *xt2_chain_move(struct xt2_table *, const char *,
+					const char *);
 
 #endif /* _NET_NETFILTER_XTABLES2_H */
diff --git a/net/netfilter/xt2_core.c b/net/netfilter/xt2_core.c
index 5e7426d..5d8f155 100644
--- a/net/netfilter/xt2_core.c
+++ b/net/netfilter/xt2_core.c
@@ -100,6 +100,37 @@ void xt2_chain_free(struct xt2_chain *chain)
 EXPORT_SYMBOL_GPL(xt2_chain_free);
 
 /**
+ * @table:	table to add the new chain to
+ * @name:	current name for the chain; not %NULL
+ * @new_name:	new name for the chain; not %NULL
+ *
+ * Rename chain by means of a create-delete cycle. This is to avoid
+ * temporary invisiblity of the chain and/or duplicate chain names for readers.
+ *
+ * Caller should hold table lock.
+ */
+struct xt2_chain *xt2_chain_move(struct xt2_table *table, const char *old_name,
+				 const char *new_name)
+{
+	struct xt2_chain *old_chain, *new_chain;
+
+	if (strlen(new_name) >= ARRAY_SIZE(new_chain->name))
+		return ERR_PTR(-ENAMETOOLONG);
+	if (xt2_chain_lookup(table, new_name) != NULL)
+		return ERR_PTR(-EEXIST);
+	old_chain = xt2_chain_lookup(table, old_name);
+	if (old_chain == NULL)
+		return ERR_PTR(-ENOENT);
+	new_chain = xt2_chain_new(table, new_name);
+	if (IS_ERR(new_chain))
+		return new_chain;
+	/* - rule move magic here once that code appears */
+	xt2_chain_free(old_chain);
+	return new_chain;
+}
+EXPORT_SYMBOL_GPL(xt2_chain_move);
+
+/**
  * Create a new table with no chains and no rules.
  */
 static struct xt2_table *xt2_table_new(void)
diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
index b50e468d..9f29b34 100644
--- a/net/netfilter/xt2_nfnetlink.c
+++ b/net/netfilter/xt2_nfnetlink.c
@@ -253,10 +253,56 @@ xtnetlink_chain_del(struct sock *xtnl, struct sk_buff *iskb,
 	return ret;
 }
 
+static int
+xtnetlink_chain_move(struct sock *xtnl, struct sk_buff *iskb,
+		     const struct nlmsghdr *imsg,
+		     const struct nlattr *const *ad)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
+	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	const char *old_name, *new_name;
+	const struct xt2_chain *chain;
+	const struct nlattr *attr;
+	struct xt2_table *table;
+	int ret;
+
+	attr = nlmsg_find_attr(imsg, sizeof(struct nfgenmsg), NFXTA_NAME);
+	if (attr == NULL)
+		return xtnetlink_error(xtnl, &ref, NFXTE_ATTRSET_INCOMPLETE);
+	old_name = nla_data(attr);
+	if (*old_name == '\0')
+		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_NOENT);
+	attr = nlmsg_find_attr(imsg, sizeof(struct nfgenmsg), NFXTA_NEW_NAME);
+	if (attr == NULL)
+		return xtnetlink_error(xtnl, &ref, NFXTE_ATTRSET_INCOMPLETE);
+	new_name = nla_data(attr);
+	if (*new_name == '\0')
+		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_INVALID_NAME);
+
+	mutex_lock(&pnet->master_lock);
+	table = pnet->master;
+	mutex_lock(&table->lock);
+	chain = xt2_chain_move(table, old_name, new_name);
+	ret   = IS_ERR(chain) ? PTR_ERR(chain) : 0;
+	mutex_unlock(&table->lock);
+	mutex_unlock(&pnet->master_lock);
+	switch (ret) {
+	case -ENAMETOOLONG:
+		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_NAMETOOLONG);
+	case -EEXIST:
+		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_EXISTS);
+	case -ENOENT:
+		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_NOENT);
+	default:
+		return xtnetlink_error(xtnl, &ref, ret);
+	}
+}
+
 static const struct nla_policy xtnetlink_policy[] = {
 	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
 	[NFXTA_ERRNO] = {.type = NLA_U32},
 	[NFXTA_XTERRNO] = {.type = NLA_U32},
+	[NFXTA_NEW_NAME] = {.type = NLA_NUL_STRING},
 };
 
 /*
@@ -272,6 +318,7 @@ static const struct nfnl_callback xtnetlink_callback[] = {
 	[NFXTM_IDENTIFY] = {.call = xtnetlink_identify, pol},
 	[NFXTM_CHAIN_NEW] = {.call = xtnetlink_chain_new, pol},
 	[NFXTM_CHAIN_DEL] = {.call = xtnetlink_chain_del, pol},
+	[NFXTM_CHAIN_MOVE] = {.call = xtnetlink_chain_move, pol},
 };
 #undef pol
 
-- 
1.7.7

[PATCH 5/7] netfilter: xtables2: initial table replace support

[Jan Engelhardt]

In Xtables1/iptables, table replace was easy, since userspace
practically only had to do a single kernel call (SO_SET_REPLACE)
and the kernel got the entire ruleset at once.

With Netlink (and its wonderful limitations), the kernel module
instead will have to collect chain/rule modification messages first,
necessiting a temporary scratch area, implemented herein in struct
xtnetlink_transact.

This patch adds the skeletons for transactioning and the
NFXTM_TABLE_REPLACE handler, making it possible to nuke all chains
with it (that's all for this patch).

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/linux/netfilter/nfnetlink_xtables.h |    6 +
 include/net/netfilter/x_tables2.h           |    2 +
 net/netfilter/xt2_core.c                    |    8 +-
 net/netfilter/xt2_nfnetlink.c               |  180 +++++++++++++++++++++++++++
 4 files changed, 194 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_xtables.h b/include/linux/netfilter/nfnetlink_xtables.h
index 296012a..2399678 100644
--- a/include/linux/netfilter/nfnetlink_xtables.h
+++ b/include/linux/netfilter/nfnetlink_xtables.h
@@ -6,6 +6,8 @@ enum nfxt_msg_type {
 	NFXTM_CHAIN_NEW,
 	NFXTM_CHAIN_DEL,
 	NFXTM_CHAIN_MOVE,
+	NFXTM_TABLE_REPLACE,
+	NFXTM_COMMIT,
 };
 
 /**
@@ -28,6 +30,8 @@ enum nfxt_attr_type {
  * %NFXTE_CHAIN_EXIST:		Chain already exists
  * %NFXTE_CHAIN_NOENT:		Chain does not exist
  * %NFXTE_CHAIN_NAMETOOLONG:	New chain name is too long
+ * %NFXTE_TRANSACT_ACTIVE:	Attempted to start transact while already active
+ * %NFXTE_TRANSACT_INACTIVE:	Commit issued when no transaction active
  */
 enum nfxt_errno {
 	NFXTE_SUCCESS = 0,
@@ -36,6 +40,8 @@ enum nfxt_errno {
 	NFXTE_CHAIN_EXISTS,
 	NFXTE_CHAIN_NOENT,
 	NFXTE_CHAIN_NAMETOOLONG,
+	NFXTE_TRANSACT_ACTIVE,
+	NFXTE_TRANSACT_INACTIVE,
 };
 
 #endif /* _LINUX_NFNETLINK_XTABLES_H */
diff --git a/include/net/netfilter/x_tables2.h b/include/net/netfilter/x_tables2.h
index 198ec31..1744182 100644
--- a/include/net/netfilter/x_tables2.h
+++ b/include/net/netfilter/x_tables2.h
@@ -44,5 +44,7 @@ extern struct xt2_chain *xt2_chain_lookup(struct xt2_table *, const char *);
 extern void xt2_chain_free(struct xt2_chain *);
 extern struct xt2_chain *xt2_chain_move(struct xt2_table *, const char *,
 					const char *);
+extern struct xt2_table *xt2_table_new(void);
+extern void xt2_table_free(struct xt2_table *);
 
 #endif /* _NET_NETFILTER_XTABLES2_H */
diff --git a/net/netfilter/xt2_core.c b/net/netfilter/xt2_core.c
index 5d8f155..c13102d 100644
--- a/net/netfilter/xt2_core.c
+++ b/net/netfilter/xt2_core.c
@@ -133,7 +133,7 @@ EXPORT_SYMBOL_GPL(xt2_chain_move);
 /**
  * Create a new table with no chains and no rules.
  */
-static struct xt2_table *xt2_table_new(void)
+struct xt2_table *xt2_table_new(void)
 {
 	struct xt2_table *table;
 
@@ -145,15 +145,19 @@ static struct xt2_table *xt2_table_new(void)
 	INIT_LIST_HEAD(&table->chain_list);
 	return table;
 }
+EXPORT_SYMBOL_GPL(xt2_table_new);
 
-static void xt2_table_free(struct xt2_table *table)
+void xt2_table_free(struct xt2_table *table)
 {
 	struct xt2_chain *chain, *next;
 
+	if (table == NULL)
+		return;
 	list_for_each_entry_safe(chain, next, &table->chain_list, anchor)
 		xt2_chain_free(chain);
 	kfree(table);
 }
+EXPORT_SYMBOL_GPL(xt2_table_free);
 
 static int __net_init xtables2_net_init(struct net *net)
 {
diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
index 9f29b34..0ef6442 100644
--- a/net/netfilter/xt2_nfnetlink.c
+++ b/net/netfilter/xt2_nfnetlink.c
@@ -12,13 +12,16 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/netlink.h>
+#include <linux/notifier.h>
 #include <linux/skbuff.h>
 #include <linux/netfilter.h>
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nfnetlink_xtables.h>
 #include <net/netlink.h>
+#include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/netfilter/x_tables2.h>
+#include <asm-generic/bug.h>
 
 MODULE_DESCRIPTION("Xtables2 nfnetlink interface");
 MODULE_AUTHOR("Jan Engelhardt");
@@ -39,6 +42,79 @@ struct xtnetlink_pktref {
 };
 
 /**
+ * Per-client transaction state
+ * @netns:		part of the tuple to uniquely identify client
+ * @nladdr:		client address
+ * @table:		temporary new table
+ *
+ * Because Netlink attrs can only be so big (and certain uses of Netlink are
+ * opposed to by others), the kernel won't be seeing the entire ruleset at once
+ * from userspace, but has to collect it. Also, I [j.eng] think that passing a
+ * 150 MB data stream via a __user pointer through SO_SET_REPLACE is a bit
+ * different after all, compared to a 150 MB netlink message - limited
+ * (default) size of socket buffers probably would forbid it already.
+ */
+struct xtnetlink_transact {
+	struct list_head anchor;
+	const struct net *netns;
+	uint32_t nladdr;
+	struct xt2_table *table;
+};
+
+static struct mutex xtnetlink_transact_lock;
+static LIST_HEAD(xtnetlink_transact_list);
+
+/**
+ * Create a new transaction state for the client given by @nladdr.
+ * @nladdr:	client address (NETLINK_CB(skb).pid)
+ *
+ * The caller should hold appropriate locks.
+ */
+static struct xtnetlink_transact *
+xtnetlink_transact_new(const struct net *net, uint32_t nladdr)
+{
+	struct xtnetlink_transact *xa;
+
+	xa = kmalloc(sizeof(*xa), GFP_KERNEL);
+	if (xa == NULL)
+		return NULL;
+	INIT_LIST_HEAD(&xa->anchor);
+	xa->netns  = net;
+	xa->nladdr = nladdr;
+	list_add_tail(&xa->anchor, &xtnetlink_transact_list);
+	return xa;
+}
+
+/**
+ * Find and return the transaction state for client given by @nladdr.
+ * @nladdr:	client address (NETLINK_CB(skb).pid)
+ *
+ * The caller should hold appropriate locks here as well.
+ */
+static struct xtnetlink_transact *
+xtnetlink_transact_lookup(struct net *netns, uint32_t nladdr)
+{
+	struct xtnetlink_transact *e;
+
+	list_for_each_entry(e, &xtnetlink_transact_list, anchor)
+		if (net_eq(e->netns, netns) && e->nladdr == nladdr)
+			return e;
+	return NULL;
+}
+
+/**
+ * Tear down a client's transaction state.
+ * @xa:		transaction buildup space to be freed
+ *
+ */
+static void xtnetlink_transact_free(struct xtnetlink_transact *xa)
+{
+	list_del(&xa->anchor);
+	xt2_table_free(xa->table);
+	kfree(xa);
+}
+
+/**
  * @skb:	outgoing skb
  * @old:	pointers to the original incoming skb/nl headers
  * @flags:	extra flags to set in nlmsg
@@ -298,6 +374,70 @@ xtnetlink_chain_move(struct sock *xtnl, struct sk_buff *iskb,
 	}
 }
 
+/**
+ * This function initiates a new transaction, a scratch space of sorts.
+ * %NFXTM_TABLE_REPLACE starts with a clean table, and one is to issue
+ * %NFXTM_CHAIN_* to "edit" it. It will be switched for the live ruleset once
+ * %NFXTM_COMMIT is issued - the whole sequence is what makes up the atomic
+ * table replacement feature that was already in Xtables1.
+ */
+static int
+xtnetlink_table_replace(struct sock *xtnl, struct sk_buff *iskb,
+			const struct nlmsghdr *imsg,
+			const struct nlattr *const *ad)
+{
+	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	struct xtnetlink_transact *xa;
+
+	mutex_lock(&xtnetlink_transact_lock);
+	if (xtnetlink_transact_lookup(sock_net(xtnl),
+	    NETLINK_CB(iskb).pid) != NULL) {
+		mutex_unlock(&xtnetlink_transact_lock);
+		return xtnetlink_error(xtnl, &ref, NFXTE_TRANSACT_ACTIVE);
+	}
+	xa = xtnetlink_transact_new(sock_net(xtnl), NETLINK_CB(iskb).pid);
+	if (xa == NULL)
+		goto out;
+	xa->table = xt2_table_new();
+	if (xa->table == NULL)
+		goto out;
+	mutex_unlock(&xtnetlink_transact_lock);
+	return xtnetlink_error(xtnl, &ref, NFXTE_SUCCESS);
+ out:
+	xtnetlink_transact_free(xa);
+	mutex_unlock(&xtnetlink_transact_lock);
+	return -ENOMEM;
+}
+
+static int
+xtnetlink_commit(struct sock *xtnl, struct sk_buff *iskb,
+		 const struct nlmsghdr *imsg, const struct nlattr *const *ad)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
+	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	struct xtnetlink_transact *xa;
+	struct xt2_table *old_table;
+
+	mutex_lock(&xtnetlink_transact_lock);
+	xa = xtnetlink_transact_lookup(sock_net(xtnl), NETLINK_CB(iskb).pid);
+	if (xa == NULL) {
+		mutex_unlock(&xtnetlink_transact_lock);
+		return xtnetlink_error(xtnl, &ref, NFXTE_TRANSACT_INACTIVE);
+	}
+
+	/* <- ruleset verification/packing here */
+	mutex_lock(&pnet->master_lock);
+	old_table = pnet->master;
+	printk("Old table: %p, New table: %p\n", old_table, xa->table);
+	rcu_assign_pointer(pnet->master, xa->table);
+	mutex_unlock(&pnet->master_lock);
+	/* Just use transact_free to kill the old one off. */
+	xa->table = old_table;
+	xtnetlink_transact_free(xa);
+	mutex_unlock(&xtnetlink_transact_lock);
+	return xtnetlink_error(xtnl, &ref, NFXTE_SUCCESS);
+}
+
 static const struct nla_policy xtnetlink_policy[] = {
 	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
 	[NFXTA_ERRNO] = {.type = NLA_U32},
@@ -319,6 +459,8 @@ static const struct nfnl_callback xtnetlink_callback[] = {
 	[NFXTM_CHAIN_NEW] = {.call = xtnetlink_chain_new, pol},
 	[NFXTM_CHAIN_DEL] = {.call = xtnetlink_chain_del, pol},
 	[NFXTM_CHAIN_MOVE] = {.call = xtnetlink_chain_move, pol},
+	[NFXTM_TABLE_REPLACE] = {.call = xtnetlink_table_replace, pol},
+	[NFXTM_COMMIT] = {.call = xtnetlink_commit, pol},
 };
 #undef pol
 
@@ -329,14 +471,52 @@ static const struct nfnetlink_subsystem xtnetlink_subsys = {
 	.cb_count  = ARRAY_SIZE(xtnetlink_callback),
 };
 
+static int
+xtnetlink_nlevent(struct notifier_block *blk, unsigned long event, void *ptr)
+{
+	const struct netlink_notify *note = ptr;
+	struct xtnetlink_transact *xa;
+
+	if (event != NETLINK_URELEASE || note->protocol != NETLINK_NETFILTER)
+		return NOTIFY_DONE;
+	/*
+	 * Freeing is non-sleeping thanks to kfree_rcu in xt2_table_free.
+	 * Is this needed, or do we have a user context in this NL notifier?
+	 *
+	 * If notifiers are not executed right when they are issued, this
+	 * becomes as a race, as a new NL socket could be created with the
+	 * same nladdr value (.pid member).
+	 */
+	mutex_lock(&xtnetlink_transact_lock);
+	xa = xtnetlink_transact_lookup(note->net, note->pid);
+	if (xa != NULL)
+		xtnetlink_transact_free(xa);
+	mutex_unlock(&xtnetlink_transact_lock);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block xtnetlink_nlevent_notifier __read_mostly = {
+	.notifier_call = xtnetlink_nlevent,
+};
+
 static int __init xtnetlink_init(void)
 {
+	int ret;
+
+	INIT_LIST_HEAD(&xtnetlink_transact_list);
+	mutex_init(&xtnetlink_transact_lock);
+	ret = netlink_register_notifier(&xtnetlink_nlevent_notifier);
+	if (ret < 0)
+		return ret;
 	return nfnetlink_subsys_register(&xtnetlink_subsys);
 }
 
 static void __exit xtnetlink_exit(void)
 {
 	nfnetlink_subsys_unregister(&xtnetlink_subsys);
+	netlink_unregister_notifier(&xtnetlink_nlevent_notifier);
+	WARN_ON(!list_empty(&xtnetlink_transact_list));
+	mutex_destroy(&xtnetlink_transact_lock);
 }
 
 module_init(xtnetlink_init);
-- 
1.7.7

[PATCH 6/7] netfilter: xtables2: transaction abort support

[Jan Engelhardt]

This is rather straightforward.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/linux/netfilter/nfnetlink_xtables.h |    1 +
 net/netfilter/xt2_nfnetlink.c               |   27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_xtables.h b/include/linux/netfilter/nfnetlink_xtables.h
index 2399678..497200b 100644
--- a/include/linux/netfilter/nfnetlink_xtables.h
+++ b/include/linux/netfilter/nfnetlink_xtables.h
@@ -8,6 +8,7 @@ enum nfxt_msg_type {
 	NFXTM_CHAIN_MOVE,
 	NFXTM_TABLE_REPLACE,
 	NFXTM_COMMIT,
+	NFXTM_ABORT,
 };
 
 /**
diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
index 0ef6442..08bcc5e 100644
--- a/net/netfilter/xt2_nfnetlink.c
+++ b/net/netfilter/xt2_nfnetlink.c
@@ -438,6 +438,32 @@ xtnetlink_commit(struct sock *xtnl, struct sk_buff *iskb,
 	return xtnetlink_error(xtnl, &ref, NFXTE_SUCCESS);
 }
 
+/**
+ * NFXTM_ABORT: The client wants to explicitly abandon a transaction it itself
+ * had started earlier. Usually issued by the client when part of a
+ * transaction, for example a NFXTM_CHAIN_SPLICE within a
+ * NFXTM_TABLE_REPLACE..NFXTM_COMMIT window yielded ENOMEM.
+ */
+static int
+xtnetlink_abort(struct sock *xtnl, struct sk_buff *iskb,
+		const struct nlmsghdr *imsg, const struct nlattr *const *ad)
+{
+	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	struct xtnetlink_transact *xa;
+	int ret;
+
+	mutex_lock(&xtnetlink_transact_lock);
+	xa = xtnetlink_transact_lookup(sock_net(xtnl), NETLINK_CB(iskb).pid);
+	if (xa == NULL) {
+		ret = NFXTE_TRANSACT_INACTIVE;
+	} else {
+		xtnetlink_transact_free(xa);
+		ret = NFXTE_SUCCESS;
+	}
+	mutex_unlock(&xtnetlink_transact_lock);
+	return xtnetlink_error(xtnl, &ref, ret);
+}
+
 static const struct nla_policy xtnetlink_policy[] = {
 	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
 	[NFXTA_ERRNO] = {.type = NLA_U32},
@@ -461,6 +487,7 @@ static const struct nfnl_callback xtnetlink_callback[] = {
 	[NFXTM_CHAIN_MOVE] = {.call = xtnetlink_chain_move, pol},
 	[NFXTM_TABLE_REPLACE] = {.call = xtnetlink_table_replace, pol},
 	[NFXTM_COMMIT] = {.call = xtnetlink_commit, pol},
+	[NFXTM_ABORT] = {.call = xtnetlink_abort, pol},
 };
 #undef pol
 
-- 
1.7.7

[PATCH 7/7] netfilter: xtables2: redirect writes into transaction buffer

[Jan Engelhardt]

While a transaction is active, operations like chain add/del/move
should be done against the temporary table of course.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/netfilter/xt2_nfnetlink.c |   77 +++++++++++++++++++++++++++++------------
 1 files changed, 55 insertions(+), 22 deletions(-)

diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
index 08bcc5e..3138f8c 100644
--- a/net/netfilter/xt2_nfnetlink.c
+++ b/net/netfilter/xt2_nfnetlink.c
@@ -206,6 +206,51 @@ xtnetlink_error(struct sock *xtnl, const struct xtnetlink_pktref *old,
 }
 
 /**
+ * @xa:		place for returning transaction, if any
+ * @net:	network namespace of the client
+ * @nladdr:	Netlink address of the same
+ *
+ * Return a pointer to either the live table, or to the transaction's temporary
+ * buffer (if there is a transaction open by the client identified by the
+ * {@net,@nladdr} tuple). All appropriate data structures are locked against
+ * modification by other instances. A symmetric call to xtnetlink_table_put is
+ * required afterwards.
+ */
+static struct xt2_table *
+xtnetlink_table_get(struct xtnetlink_transact **xa, struct net *net,
+		    int nladdr)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(net);
+	struct xt2_table *table;
+
+	mutex_lock(&xtnetlink_transact_lock);
+	*xa = xtnetlink_transact_lookup(net, nladdr);
+	if (*xa == NULL) {
+		mutex_unlock(&xtnetlink_transact_lock);
+		mutex_lock(&pnet->master_lock);
+		table = pnet->master;
+	} else {
+		table = (*xa)->table;
+	}
+	mutex_lock(&table->lock);
+	return table;
+}
+
+static void
+xtnetlink_table_put(struct xtnetlink_transact *xa, struct net *net,
+		    struct xt2_table *table)
+{
+	struct xt2_pernet_data *pnet = xtables2_pernet(net);
+
+	mutex_unlock(&table->lock);
+	if (xa == NULL) {
+		mutex_unlock(&pnet->master_lock);
+		return;
+	}
+	mutex_unlock(&xtnetlink_transact_lock);
+}
+
+/**
  * Ran too often into NULL derefs. Now there is a dummy function for unused
  * message type 0.
  */
@@ -254,8 +299,8 @@ static int
 xtnetlink_chain_new(struct sock *xtnl, struct sk_buff *iskb,
 		    const struct nlmsghdr *imsg, const struct nlattr *const *ad)
 {
-	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
 	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	struct xtnetlink_transact *xa;
 	const struct nlattr *attr;
 	struct xt2_table *table;
 	struct xt2_chain *chain;
@@ -269,13 +314,8 @@ xtnetlink_chain_new(struct sock *xtnl, struct sk_buff *iskb,
 	if (*name == '\0')
 		/* Anonymous chains are internal. */
 		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_INVALID_NAME);
-	/*
-	 * The table needs to stay, but note that rcu_read_lock cannot be used,
-	 * since we might sleep.
-	 */
-	mutex_lock(&pnet->master_lock);
-	table = pnet->master;
-	mutex_lock(&table->lock);
+
+	table = xtnetlink_table_get(&xa, sock_net(xtnl), NETLINK_CB(iskb).pid);
 	if (xt2_chain_lookup(table, name) != NULL) {
 		ret = xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_EXISTS);
 	} else {
@@ -287,8 +327,7 @@ xtnetlink_chain_new(struct sock *xtnl, struct sk_buff *iskb,
 			ret = NFXTE_CHAIN_NAMETOOLONG;
 		ret = xtnetlink_error(xtnl, &ref, ret);
 	}
-	mutex_unlock(&table->lock);
-	mutex_unlock(&pnet->master_lock);
+	xtnetlink_table_put(xa, sock_net(xtnl), table);
 	return ret;
 }
 
@@ -300,8 +339,8 @@ xtnetlink_chain_del(struct sock *xtnl, struct sk_buff *iskb,
 		    const struct nlmsghdr *imsg,
 		    const struct nlattr *const *ad)
 {
-	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
 	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	struct xtnetlink_transact *xa;
 	const struct nlattr *name_attr;
 	struct xt2_table *table;
 	struct xt2_chain *chain;
@@ -315,17 +354,14 @@ xtnetlink_chain_del(struct sock *xtnl, struct sk_buff *iskb,
 	if (*name == '\0')
 		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_NOENT);
 
-	mutex_lock(&pnet->master_lock);
-	table = pnet->master;
-	mutex_lock(&table->lock);
+	table = xtnetlink_table_get(&xa, sock_net(xtnl), NETLINK_CB(iskb).pid);
 	chain = xt2_chain_lookup(table, name);
 	if (chain != NULL)
 		xt2_chain_free(chain);
 	else
 		ret = NFXTE_CHAIN_NOENT;
 	ret = xtnetlink_error(xtnl, &ref, ret);
-	mutex_unlock(&table->lock);
-	mutex_unlock(&pnet->master_lock);
+	xtnetlink_table_put(xa, sock_net(xtnl), table);
 	return ret;
 }
 
@@ -334,8 +370,8 @@ xtnetlink_chain_move(struct sock *xtnl, struct sk_buff *iskb,
 		     const struct nlmsghdr *imsg,
 		     const struct nlattr *const *ad)
 {
-	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
 	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
+	struct xtnetlink_transact *xa;
 	const char *old_name, *new_name;
 	const struct xt2_chain *chain;
 	const struct nlattr *attr;
@@ -355,13 +391,10 @@ xtnetlink_chain_move(struct sock *xtnl, struct sk_buff *iskb,
 	if (*new_name == '\0')
 		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_INVALID_NAME);
 
-	mutex_lock(&pnet->master_lock);
-	table = pnet->master;
-	mutex_lock(&table->lock);
+	table = xtnetlink_table_get(&xa, sock_net(xtnl), NETLINK_CB(iskb).pid);
 	chain = xt2_chain_move(table, old_name, new_name);
 	ret   = IS_ERR(chain) ? PTR_ERR(chain) : 0;
-	mutex_unlock(&table->lock);
-	mutex_unlock(&pnet->master_lock);
+	xtnetlink_table_put(xa, sock_net(xtnl), table);
 	switch (ret) {
 	case -ENAMETOOLONG:
 		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_NAMETOOLONG);
-- 
1.7.7

Re: [PATCH 1/7] netfilter: xtables2: initial table skeletal functions

[Pablo Neira Ayuso]

Hi Jan,

On Thu, Jan 19, 2012 at 05:26:15PM +0100, Jan Engelhardt wrote:
> This patch adds the xt2 table functions. Of course this does not do
> anything useful yet, chain and rule support directly follow.

First off, I'm happy to see these patches.

The netlink interface for iptables has been a missing feature for long
time.

> ---
>  include/net/netfilter/x_tables2.h |   17 +++++++
>  net/netfilter/Kconfig             |    8 +++-
>  net/netfilter/Makefile            |    1 +
>  net/netfilter/xt2_core.c          |   85 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 110 insertions(+), 1 deletions(-)
>  create mode 100644 include/net/netfilter/x_tables2.h
>  create mode 100644 net/netfilter/xt2_core.c
> 
> diff --git a/include/net/netfilter/x_tables2.h b/include/net/netfilter/x_tables2.h
> new file mode 100644
> index 0000000..a219952
> --- /dev/null
> +++ b/include/net/netfilter/x_tables2.h
> @@ -0,0 +1,17 @@
> +#ifndef _NET_NETFILTER_XTABLES2_H
> +#define _NET_NETFILTER_XTABLES2_H 1
> +
> +#define XTABLES2_VTAG "Xtables2 A8"

I don't want to center the discussion on naming, but I'd prefer if we
stick to xtables without version 2 and A8 (what does it mean A8, btw?).

At some point the old xtables infrastructure will be removed, then
we'll have lots of references to xt2 in the tree.

And, if we provide the netlink interface for xtables, we should
be able to remove the old setsockopt/getsockopt interface quite fast
(as soon as we provide user-space tools that can speak both netlink and
setsockopt/getsockopt interface for some time).

> +/**
> + * @master:	the master table
> + */
> +struct xt2_pernet_data {
> +	struct xt2_table __rcu *master;
> +};
> +
> +struct xt2_table {
> +	int _dummy;
> +};
> +
> +#endif /* _NET_NETFILTER_XTABLES2_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 32bff6d..5b3d9ca 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -321,7 +321,13 @@ config NETFILTER_XTABLES
>  	  This is required if you intend to use any of ip_tables,
>  	  ip6_tables or arp_tables.
>  
> -if NETFILTER_XTABLES
> +config NETFILTER_XTABLES2
> +	tristate "Netfilter Xtables2 packet filtering"
> +	---help---
> +	Xtables2 is a rework of the internal architecture of Xtables.
> +	It supersedes iptables, ip6tables, arptables and ebtables.

My idea is that this does not supersede any of these tools.

Instead, these tools should be ported to the netlink interface.
I prefer if users don't notice any change regarding tools in the
short term.

I still think there's valuable work in Patrick's nftables. IMO, the
scope of this work should be limited to providing the netlink
interface for iptables (ip6tables, arptables, and so on), not modifying
the command line tool syntax (which is a different discussion, don't
get me wrong I'm not telling that revisiting the syntax is bad, but
it's a different discussion and I don't want to mix things).

Re: xtables2 a8, netlink interface

[Stephen Hemminger]

On Thu, 19 Jan 2012 17:26:14 +0100
Jan Engelhardt <jengelh@medozas.de> wrote:

> 
> So here is the first set of patches implementing part of the xt2 core
> and nfnl interface. Please review, I am sure you will have something
> to say :)
> 
> The userspace part is at git://dev.medozas.de/libnetfilter_xtables,
> the xtnl-test program compiled as part of the default make target
> gives access to the kernel functions implemented by the patches.
> 

Does it allow for watching for iptables changes (notifications),
similar to what is done for link, address, and qdisc changes?

Re: xtables2 a8, netlink interface

[Jan Engelhardt]

On Friday 2012-01-20 01:56, Stephen Hemminger wrote:
>> 
>> So here is the first set of patches implementing part of the xt2 core
>> and nfnl interface. Please review, I am sure you will have something
>> to say :)
>> 
>> The userspace part is at git://dev.medozas.de/libnetfilter_xtables,
>> the xtnl-test program compiled as part of the default make target
>> gives access to the kernel functions implemented by the patches.
>
>Does it allow for watching for iptables changes (notifications),
>similar to what is done for link, address, and qdisc changes?

It certainly does not look like a big deal to write it, so yes, I 
had that on my list of things to add. (Even though I could not come up 
with a usecase for that feature myself yet.)

Re: [PATCH 1/7] netfilter: xtables2: initial table skeletal functions

[Jan Engelhardt]

On Friday 2012-01-20 01:23, Pablo Neira Ayuso wrote:
>> +#ifndef _NET_NETFILTER_XTABLES2_H
>> +#define _NET_NETFILTER_XTABLES2_H 1
>> +
>> +#define XTABLES2_VTAG "Xtables2 A8"
>
>I don't want to center the discussion on naming, but I'd prefer if we
>stick to xtables without version 2 and A8 (what does it mean A8, btw?).

Architecture no. 8 (it's in the technical PDFs posted last time).
I just had to enumerate the different implementation proposals that
we have had over time.

>At some point the old xtables infrastructure will be removed, then
>we'll have lots of references to xt2 in the tree.

That can be renamed afterwards when xt1 is gone.
Right now I feel it is a reasonable separation method,
and the '2' is not present in any static part of the kernel-user 
interface.

>> index 32bff6d..5b3d9ca 100644
>> --- a/net/netfilter/Kconfig
>> +++ b/net/netfilter/Kconfig
>> @@ -321,7 +321,13 @@ config NETFILTER_XTABLES
>>  	  This is required if you intend to use any of ip_tables,
>>  	  ip6_tables or arp_tables.
>>  
>> -if NETFILTER_XTABLES
>> +config NETFILTER_XTABLES2
>> +	tristate "Netfilter Xtables2 packet filtering"
>> +	---help---
>> +	Xtables2 is a rework of the internal architecture of Xtables.
>> +	It supersedes iptables, ip6tables, arptables and ebtables.
>
>My idea is that this does not supersede any of these tools.

Right, the wording is chosen little weirdly, since no kernel
interface (except maybe sysfs) can replace a userspace tool to do the 
talking.

>Instead, these tools should be ported to the netlink interface.
>I prefer if users don't notice any change regarding tools in the
>short term.

I have it all worked out :) they won't notice.

>I still think there's valuable work in Patrick's nftables. IMO, the
>scope of this work should be limited to providing the netlink
>interface for iptables (ip6tables, arptables, and so on), not modifying
>the command line tool syntax (which is a different discussion, don't
>get me wrong I'm not telling that revisiting the syntax is bad, but
>it's a different discussion and I don't want to mix things).

I concur, this is already underway in exactly the way you say it.

Re: xtables2 a8, netlink interface

[Dave Taht]

On Fri, Jan 20, 2012 at 9:33 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
> On Friday 2012-01-20 01:56, Stephen Hemminger wrote:
>>>
>>> So here is the first set of patches implementing part of the xt2 core
>>> and nfnl interface. Please review, I am sure you will have something
>>> to say :)
>>>
>>> The userspace part is at git://dev.medozas.de/libnetfilter_xtables,
>>> the xtnl-test program compiled as part of the default make target
>>> gives access to the kernel functions implemented by the patches.
>>
>>Does it allow for watching for iptables changes (notifications),
>>similar to what is done for link, address, and qdisc changes?
>
> It certainly does not look like a big deal to write it, so yes, I
> had that on my list of things to add. (Even though I could not come up
> with a usecase for that feature myself yet.)

My use case would be for a routing daemon to detect when nat was present on
an interface, so as to not advertise invalid routes.

> --
> 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



-- 
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
--
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

Re: xtables2 a8, netlink interface

[Stephen Hemminger]

On Fri, 20 Jan 2012 10:23:54 +0100
Dave Taht <dave.taht@gmail.com> wrote:

> On Fri, Jan 20, 2012 at 9:33 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
> > On Friday 2012-01-20 01:56, Stephen Hemminger wrote:
> >>>
> >>> So here is the first set of patches implementing part of the xt2 core
> >>> and nfnl interface. Please review, I am sure you will have something
> >>> to say :)
> >>>
> >>> The userspace part is at git://dev.medozas.de/libnetfilter_xtables,
> >>> the xtnl-test program compiled as part of the default make target
> >>> gives access to the kernel functions implemented by the patches.
> >>
> >>Does it allow for watching for iptables changes (notifications),
> >>similar to what is done for link, address, and qdisc changes?
> >
> > It certainly does not look like a big deal to write it, so yes, I
> > had that on my list of things to add. (Even though I could not come up
> > with a usecase for that feature myself yet.)
> 
> My use case would be for a routing daemon to detect when nat was present on
> an interface, so as to not advertise invalid routes.

Our use case is to control an external dataplane.

Re: xtables2 a8, netlink interface

[Jozsef Kadlecsik]

Hi Jan,

On Thu, 19 Jan 2012, Jan Engelhardt wrote:

> So here is the first set of patches implementing part of the xt2 core
> and nfnl interface. Please review, I am sure you will have something
> to say :)

At a first glance I must say it looks very promising. Good job!
What seems to be missing is some kind of protocol version negotiation 
support between kernel and userspace. Or it'll come in the next batch of 
patches? :-)

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: xtables2 a8, netlink interface

[Jan Engelhardt]

On Saturday 2012-01-21 15:10, Jozsef Kadlecsik wrote:

>Hi Jan,
>
>On Thu, 19 Jan 2012, Jan Engelhardt wrote:
>
>> So here is the first set of patches implementing part of the xt2 core
>> and nfnl interface. Please review, I am sure you will have something
>> to say :)
>
>At a first glance I must say it looks very promising. Good job!
>What seems to be missing is some kind of protocol version negotiation 
>support between kernel and userspace. Or it'll come in the next batch of 
>patches? :-)

Can be an attribute returned with NFXTM_IDENTIFY messages.
But I had not really given thought to that (ipset is also still at protocol 
6, so..)

Re: xtables2 a8, netlink interface

[Jozsef Kadlecsik]

On Sat, 21 Jan 2012, Jan Engelhardt wrote:

> On Saturday 2012-01-21 15:10, Jozsef Kadlecsik wrote:
> 
> >On Thu, 19 Jan 2012, Jan Engelhardt wrote:
> >
> >> So here is the first set of patches implementing part of the xt2 core
> >> and nfnl interface. Please review, I am sure you will have something
> >> to say :)
> >
> >At a first glance I must say it looks very promising. Good job!
> >What seems to be missing is some kind of protocol version negotiation 
> >support between kernel and userspace. Or it'll come in the next batch of 
> >patches? :-)
> 
> Can be an attribute returned with NFXTM_IDENTIFY messages.

That attribute should be there from the very beginning.

> But I had not really given thought to that (ipset is also still at protocol 
> 6, so..)

Yes, but anytime there would be a need for a protocol extension/new 
protocol, that'd be added easily, with full transition support.

Netlink makes too easy to add new attributes - and ignores unknown ones. 
However, in our case both communicating parties must know the exact 
capabilities of the other one: it's unacceptable that say a new flag is 
introduced and sent from userspace and got silently discarded by the 
(older) kernel.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: xtables2 a8, netlink interface

[Jan Engelhardt]

On Saturday 2012-01-21 21:21, Jozsef Kadlecsik wrote:
>
>Netlink makes too easy to add new attributes - and ignores unknown ones. 
>However, in our case both communicating parties must know the exact 
>capabilities of the other one: it's unacceptable that say a new flag is 
>introduced and sent from userspace and got silently discarded by the 
>(older) kernel.

I thought about exposing the revision window here, much like
libtool's CURRENT and (CURRENT-AGE), rather than just a single rev
like ipset currently does. Does that make sense to you?

1. kern: 600--611, prog:    , 599 => prog too old
2. kern: 600--611, prog: 600, 610 => can use some/all features
3. kern: 600--611, prog: 611      => can use all features
4. kern: 600--611, prog: 612,     => kernel too old

Re: xtables2 a8, netlink interface

[Jozsef Kadlecsik]

On Mon, 23 Jan 2012, Jan Engelhardt wrote:

> On Saturday 2012-01-21 21:21, Jozsef Kadlecsik wrote:
> >
> >Netlink makes too easy to add new attributes - and ignores unknown ones. 
> >However, in our case both communicating parties must know the exact 
> >capabilities of the other one: it's unacceptable that say a new flag is 
> >introduced and sent from userspace and got silently discarded by the 
> >(older) kernel.
> 
> I thought about exposing the revision window here, much like
> libtool's CURRENT and (CURRENT-AGE), rather than just a single rev
> like ipset currently does. Does that make sense to you?
> 
> 1. kern: 600--611, prog:    , 599 => prog too old
> 2. kern: 600--611, prog: 600, 610 => can use some/all features
> 3. kern: 600--611, prog: 611      => can use all features
> 4. kern: 600--611, prog: 612,     => kernel too old

That's perfect (ipset does the same, just it doesn't send a version range 
when the window size is zero).

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 2/7] netfilter: xtables2: initial Netlink interface

[Pablo Neira Ayuso]

On Thu, Jan 19, 2012 at 05:26:16PM +0100, Jan Engelhardt wrote:
> This populates xt2_nfnetlink.c with support for NFXTM_IDENTIFY. Right
> now that just returns a freeform string but eventually shall dump all
> the match and target modules' info.
>
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  include/linux/netfilter/Kbuild              |    1 +
>  include/linux/netfilter/nfnetlink.h         |    3 +-
>  include/linux/netfilter/nfnetlink_xtables.h |   20 ++++
>  net/netfilter/Kconfig                       |    8 ++
>  net/netfilter/Makefile                      |    1 +
>  net/netfilter/xt2_nfnetlink.c               |  153 +++++++++++++++++++++++++++

I prefer if you call this nfnetlink_xtables.c following the same
naming convention.

nf_conntrack_netlink may seems one exception but I have a patch here
to integrate ctnetlink with nf_queue (to allow delivering conntrack
events over netlink unicast using iptables) and, then, we'll have
nfnetlink_conntrack.c as well.

>  6 files changed, 185 insertions(+), 1 deletions(-)
>  create mode 100644 include/linux/netfilter/nfnetlink_xtables.h
>  create mode 100644 net/netfilter/xt2_nfnetlink.c
> 
> diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
> index a1b410c..6e568c9 100644
> --- a/include/linux/netfilter/Kbuild
> +++ b/include/linux/netfilter/Kbuild
> @@ -10,6 +10,7 @@ header-y += nfnetlink_compat.h
>  header-y += nfnetlink_conntrack.h
>  header-y += nfnetlink_log.h
>  header-y += nfnetlink_queue.h
> +header-y += nfnetlink_xtables.h
>  header-y += x_tables.h
>  header-y += xt_AUDIT.h
>  header-y += xt_CHECKSUM.h
> diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
> index 74d3386..93228b4 100644
> --- a/include/linux/netfilter/nfnetlink.h
> +++ b/include/linux/netfilter/nfnetlink.h
> @@ -48,7 +48,8 @@ struct nfgenmsg {
>  #define NFNL_SUBSYS_ULOG		4
>  #define NFNL_SUBSYS_OSF			5
>  #define NFNL_SUBSYS_IPSET		6
> -#define NFNL_SUBSYS_COUNT		7
> +#define NFNL_SUBSYS_XTABLES		8
> +#define NFNL_SUBSYS_COUNT		9
>  
>  #ifdef __KERNEL__
>  
> diff --git a/include/linux/netfilter/nfnetlink_xtables.h b/include/linux/netfilter/nfnetlink_xtables.h
> new file mode 100644
> index 0000000..4c53042
> --- /dev/null
> +++ b/include/linux/netfilter/nfnetlink_xtables.h
> @@ -0,0 +1,20 @@
> +#ifndef _LINUX_NFNETLINK_XTABLES_H
> +#define _LINUX_NFNETLINK_XTABLES_H 1
> +
> +enum nfxt_msg_type {
> +	NFXTM_IDENTIFY = 1,

I'd suggest:

NFXT_MSG_NEW,
NFXT_MSG_GET,
...

and so on.

> +};
> +
> +/**
> + * %NFXTA_NAME:			name of the object being operated on
> + */
> +enum nfxt_attr_type {
> +	NFXTA_UNSPEC = 0,
> +	NFXTA_NAME,
> +};
> +
> +enum nfxt_errno {
> +	NFXTE_SUCCESS = 0,
> +};
> +
> +#endif /* _LINUX_NFNETLINK_XTABLES_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 5b3d9ca..6237a30 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -327,6 +327,14 @@ config NETFILTER_XTABLES2
>  	Xtables2 is a rework of the internal architecture of Xtables.
>  	It supersedes iptables, ip6tables, arptables and ebtables.
>  
> +if NETFILTER_XTABLES2
> +
> +config NETFILTER_XTNFNETLINK
> +	tristate "Configuration interface over nfnetlink"
> +	default m if NETFILTER_ADVANCED=n
> +
> +endif # NETFILTER_XTABLES2
> +
>  if NETFILTER_XTABLES || NETFILTER_XTABLES2
>  
>  comment "Xtables combined modules"
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 8504ebd..634e6a8 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_NETFILTER_TPROXY) += nf_tproxy_core.o
>  # generic X tables 
>  obj-$(CONFIG_NETFILTER_XTABLES) += x_tables.o xt_tcpudp.o
>  obj-$(CONFIG_NETFILTER_XTABLES2) += xt2_core.o
> +obj-$(CONFIG_NETFILTER_XTNFNETLINK) += xt2_nfnetlink.o
>  
>  # combos
>  obj-$(CONFIG_NETFILTER_XT_MARK) += xt_mark.o
> diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
> new file mode 100644
> index 0000000..3dc241f
> --- /dev/null
> +++ b/net/netfilter/xt2_nfnetlink.c
> @@ -0,0 +1,153 @@
> +/*
> + *	Xtables2 nfnetlink interface
> + *	Copyright © Jan Engelhardt, 2010-2012
> + *
> + *	This program is free software: you can redistribute it and/or modify
> + *	it under the terms of the GNU General Public License as published by
> + *	the Free Software Foundation, either version 2 of the License, or
> + *	(at your option) any later version.
> + */
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/netlink.h>
> +#include <linux/skbuff.h>
> +#include <linux/netfilter.h>
> +#include <linux/netfilter/nfnetlink.h>
> +#include <linux/netfilter/nfnetlink_xtables.h>
> +#include <net/netlink.h>
> +#include <net/netfilter/x_tables2.h>
> +
> +MODULE_DESCRIPTION("Xtables2 nfnetlink interface");
> +MODULE_AUTHOR("Jan Engelhardt");
> +MODULE_LICENSE("GPL");
> +
> +/**
> + * Something to keep all the pointers that NL gives us in one place.
> + */
> +struct xtnetlink_pktref {
> +	union {
> +		struct sk_buff *skb;
> +		const struct sk_buff *c_skb;
> +	};
> +	union {
> +		struct nlmsghdr *msg;
> +		const struct nlmsghdr *c_msg;
> +	};
> +};
> +
> +/**
> + * @skb:	outgoing skb
> + * @old:	pointers to the original incoming skb/nl headers
> + * @flags:	extra flags to set in nlmsg
> + *
> + * Fill an skb (for outgoing direction) with NL and NFNL headers, using data
> + * from the original skb for initialization.
> + */
> +static struct nlmsghdr *
> +xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
> +	       unsigned int flags)
> +{
> +	uint16_t nlmsg_type = NFNL_MSG_TYPE(old->msg->nlmsg_type);
> +	struct nlmsghdr *nlmsg;
> +	struct nfgenmsg *hdr;
> +
> +	nlmsg_type |= NFNL_SUBSYS_XTABLES << 8;
> +	nlmsg = nlmsg_put(skb, NETLINK_CB(old->skb).pid, old->msg->nlmsg_seq,
> +			  nlmsg_type, sizeof(*hdr), flags);
> +	if (nlmsg == NULL) {
> +		nlmsg_cancel(skb, nlmsg);
> +		return ERR_PTR(-ENOBUFS);

my experience is that it's better to use goto to handle errors.

> +	}
> +
> +	hdr               = nlmsg_data(nlmsg);
> +	hdr->nfgen_family = NFPROTO_UNSPEC;
> +	hdr->version      = NFNETLINK_V0;
> +	hdr->res_id       = 0;
> +	return nlmsg;
> +}
> +
> +/**
> + * Ran too often into NULL derefs. Now there is a dummy function for unused
> + * message type 0.
> + */
> +static int
> +xtnetlink_ignore(struct sock *k, struct sk_buff *s,
> +		 const struct nlmsghdr *n, const struct nlattr *const *a)
> +{
> +	return -ENXIO;
> +}
> +
> +/**
> + * Respond to a %NFXTM_IDENTIFY inquiry. Reports the Xtables2 version (for
> + * now), and in future the extension info.
> + */
> +static int
> +xtnetlink_identify2(struct sk_buff *skb, struct netlink_callback *nl_cb)

I'd add the infix _dump_ so you can notice this is the callback that
netlink_dump_start uses.

> +{
> +	struct xtnetlink_pktref ref = {.c_skb = skb, .c_msg = nl_cb->nlh};
> +	struct nlmsghdr *nlmsg = NULL;
> +
> +	switch (nl_cb->args[0]) {
> +	case 0:
> +		nlmsg = xtnetlink_fill(skb, &ref, NLM_F_MULTI);
> +		if (IS_ERR(nlmsg))
> +			return 0;
> +		NLA_PUT_STRING(skb, NFXTA_NAME, XTABLES2_VTAG);
> +		++nl_cb->args[0];
> +		break;
> +	}
> +	if (nlmsg != NULL)
> +		nlmsg_end(skb, nlmsg);
> +	return skb->len;
> + nla_put_failure:
> +	return 0;
> +}
> +
> +static int
> +xtnetlink_identify(struct sock *xtnl, struct sk_buff *iskb,
> +		   const struct nlmsghdr *imsg, const struct nlattr *const *ad)
> +{
> +	return netlink_dump_start(xtnl, iskb, imsg, xtnetlink_identify2,
> +				  NULL, 0);

you have to check for NLM_F_DUMP. Otherwise, you are requesting one
single table. Please, see ctnetlink or nfnetlink_acct for reference.

> +}
> +
> +static const struct nla_policy xtnetlink_policy[] = {
> +	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
> +};
> +
> +/*
> + * Use the same policy for all messages. I do not want to see EINVAL anytime
> + * soon again just because I forgot sending an attribute from userspace.
> + * (If such occurs, it will be dealt with %NFXTE_ATTRSET_INCOMPLETE, tbd.)
> + */
> +#define pol \
> +	.policy = xtnetlink_policy, \
> +	.attr_count = ARRAY_SIZE(xtnetlink_policy)
> +static const struct nfnl_callback xtnetlink_callback[] = {
> +	[0] = {.call = xtnetlink_ignore},
> +	[NFXTM_IDENTIFY] = {.call = xtnetlink_identify, pol},
> +};
> +#undef pol
> +
> +static const struct nfnetlink_subsystem xtnetlink_subsys = {
> +	.name      = "xtables",
> +	.subsys_id = NFNL_SUBSYS_XTABLES,
> +	.cb        = xtnetlink_callback,
> +	.cb_count  = ARRAY_SIZE(xtnetlink_callback),
> +};
> +
> +static int __init xtnetlink_init(void)
> +{
> +	return nfnetlink_subsys_register(&xtnetlink_subsys);
> +}
> +
> +static void __exit xtnetlink_exit(void)
> +{
> +	nfnetlink_subsys_unregister(&xtnetlink_subsys);
> +}
> +
> +module_init(xtnetlink_init);
> +module_exit(xtnetlink_exit);
> +MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_XTABLES);
> -- 
> 1.7.7
> 
--
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

Re: [PATCH 3/7] netfilter: xtables2: chain creation and deletion

[Pablo Neira Ayuso]

On Thu, Jan 19, 2012 at 05:26:17PM +0100, Jan Engelhardt wrote:
[...]
> diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
> index 3dc241f..b50e468d 100644
> --- a/net/netfilter/xt2_nfnetlink.c
> +++ b/net/netfilter/xt2_nfnetlink.c
> @@ -17,6 +17,7 @@
>  #include <linux/netfilter/nfnetlink.h>
>  #include <linux/netfilter/nfnetlink_xtables.h>
>  #include <net/netlink.h>
> +#include <net/sock.h>
>  #include <net/netfilter/x_tables2.h>
>  
>  MODULE_DESCRIPTION("Xtables2 nfnetlink interface");
> @@ -69,6 +70,66 @@ xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
>  }
>  
>  /**
> + * @ref:	skb/nl pointers that will be filled in (secondary return values)
> + * @old:	pointers to the original incoming skb/nl headers
> + *
> + * xtnetlink_fill can be used when the outgoing skb already exists (e.g. in
> + * case of a dump operation), but for non-dump responses, we have to create it
> + * ourselves.
> + */
> +static int
> +xtnetlink_new_fill(struct xtnetlink_pktref *ref,
> +		   const struct xtnetlink_pktref *old)
> +{
> +	ref->skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (ref->skb == NULL)
> +		return -ENOMEM;
> +	ref->msg = xtnetlink_fill(ref->skb, old, 0);
> +	if (IS_ERR(ref->msg)) {
> +		kfree_skb(ref->skb);
> +		return PTR_ERR(ref->msg);
> +	}
> +	return 0;
> +}
> +
> +/**
> + * @xtnl:	socket to send the error packet out on
> + * @old:	pointers to the original incoming skb/nl headers
> + * @errcode:	last error code
> + *
> + * Create and send out an NFXT error packet. If @errcode is < 0, it indicates
> + * a system-level error (such as %-ENOMEM), reported back using %NFXTA_ERRNO.
> + * If @errcode is >= 0, it indicates an NFXT-specific error codes (%NFXTE_*),
> + * which is more fine grained than the dreaded %EINVAL, and which is reported
> + * back using %NFXTA_XTERRNO.
> + */
> +static int
> +xtnetlink_error(struct sock *xtnl, const struct xtnetlink_pktref *old,
> +		int errcode)
> +{
> +	struct xtnetlink_pktref ref;
> +	int ret;
> +
> +	ret = xtnetlink_new_fill(&ref, old);
> +	if (ret < 0)
> +		return ret;
> +	if (errcode < 0)
> +		/* Prefer positive numbers on the wire */
> +		NLA_PUT_U32(ref.skb, NFXTA_ERRNO, -errcode);
> +	else
> +		NLA_PUT_U32(ref.skb, NFXTA_XTERRNO, errcode);
> +	nlmsg_end(ref.skb, ref.msg);
> +	ret = netlink_unicast(xtnl, ref.skb, NETLINK_CB(old->skb).pid,
> +			      MSG_DONTWAIT);
> +	if (ret < 0)
> +		return ret;
> +	/* ret is skb->len, but values >0 mean error to the caller -.- */
> +	return 0;
> + nla_put_failure:
> +	return -ENOBUFS;
> +}

You seem to be using some similar approach that ipset uses for error
handling. I like the idea of having fine grain error handling indeed,
but I think we can extend the Netlink framework to integrate as part
of NLMSG_ERR message types.

My idea is to attach the specific error at the end of the NLMSG_ERR,
IIRC we need to allow passing one callback to netlink_ack to add the
specific error message after the generic netlink error message.

I think this will be also backward compatible with existing netlink
library since they will ignore the trailing part of the error message
that they don't know how to interpret.

> +static int
> +xtnetlink_chain_new(struct sock *xtnl, struct sk_buff *iskb,
> +		    const struct nlmsghdr *imsg, const struct nlattr *const *ad)
> +{
> +	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
> +	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
> +	const struct nlattr *attr;
> +	struct xt2_table *table;
> +	struct xt2_chain *chain;
> +	const char *name;
> +	int ret = 0;
> +
> +	attr = nlmsg_find_attr(imsg, sizeof(struct nfgenmsg), NFXTA_NAME);
> +	if (attr == NULL)
> +		return xtnetlink_error(xtnl, &ref, NFXTE_ATTRSET_INCOMPLETE);
> +	name = nla_data(attr);
> +	if (*name == '\0')

I think you may better identify this special chains with some flags?
You end up defining flags for objects sooner or later instead of using
this special name.

> +		/* Anonymous chains are internal. */
> +		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_INVALID_NAME);
> +	/*
> +	 * The table needs to stay, but note that rcu_read_lock cannot be used,
> +	 * since we might sleep.
> +	 */
> +	mutex_lock(&pnet->master_lock);
> +	table = pnet->master;
> +	mutex_lock(&table->lock);
> +	if (xt2_chain_lookup(table, name) != NULL) {
> +		ret = xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_EXISTS);
> +	} else {
> +		chain = xt2_chain_new(table, name);
> +		if (IS_ERR(chain))
> +			ret = PTR_ERR(chain);
> +		/* Use NFXTE error codes whenever possible. */
> +		if (ret == -ENAMETOOLONG)
> +			ret = NFXTE_CHAIN_NAMETOOLONG;
> +		ret = xtnetlink_error(xtnl, &ref, ret);
> +	}
> +	mutex_unlock(&table->lock);
> +	mutex_unlock(&pnet->master_lock);
> +	return ret;
> +}
> +
> +/**
> + * Act on a %NFXTM_CHAIN_DEL message.
> + */
> +static int
> +xtnetlink_chain_del(struct sock *xtnl, struct sk_buff *iskb,
> +		    const struct nlmsghdr *imsg,
> +		    const struct nlattr *const *ad)
> +{
> +	struct xt2_pernet_data *pnet = xtables2_pernet(sock_net(xtnl));
> +	struct xtnetlink_pktref ref = {.c_skb = iskb, .c_msg = imsg};
> +	const struct nlattr *name_attr;
> +	struct xt2_table *table;
> +	struct xt2_chain *chain;
> +	const char *name;
> +	int ret = 0;
> +
> +	name_attr = nlmsg_find_attr(imsg, sizeof(struct nfgenmsg), NFXTA_NAME);
> +	if (name_attr == NULL)
> +		return xtnetlink_error(xtnl, &ref, NFXTE_ATTRSET_INCOMPLETE);
> +	name = nla_data(name_attr);
> +	if (*name == '\0')
> +		return xtnetlink_error(xtnl, &ref, NFXTE_CHAIN_NOENT);
> +
> +	mutex_lock(&pnet->master_lock);
> +	table = pnet->master;
> +	mutex_lock(&table->lock);
> +	chain = xt2_chain_lookup(table, name);
> +	if (chain != NULL)
> +		xt2_chain_free(chain);
> +	else
> +		ret = NFXTE_CHAIN_NOENT;
> +	ret = xtnetlink_error(xtnl, &ref, ret);
> +	mutex_unlock(&table->lock);
> +	mutex_unlock(&pnet->master_lock);
> +	return ret;
> +}
> +
>  static const struct nla_policy xtnetlink_policy[] = {
>  	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
> +	[NFXTA_ERRNO] = {.type = NLA_U32},
> +	[NFXTA_XTERRNO] = {.type = NLA_U32},
>  };
>  
>  /*
>   * Use the same policy for all messages. I do not want to see EINVAL anytime
>   * soon again just because I forgot sending an attribute from userspace.
> - * (If such occurs, it will be dealt with %NFXTE_ATTRSET_INCOMPLETE, tbd.)
> + * (If such occurs, it will be dealt with %NFXTE_ATTRSET_INCOMPLETE.)
>   */
>  #define pol \
>  	.policy = xtnetlink_policy, \
> @@ -128,6 +270,8 @@ static const struct nla_policy xtnetlink_policy[] = {
>  static const struct nfnl_callback xtnetlink_callback[] = {
>  	[0] = {.call = xtnetlink_ignore},
>  	[NFXTM_IDENTIFY] = {.call = xtnetlink_identify, pol},
> +	[NFXTM_CHAIN_NEW] = {.call = xtnetlink_chain_new, pol},
> +	[NFXTM_CHAIN_DEL] = {.call = xtnetlink_chain_del, pol},
>  };
>  #undef pol
>  
> -- 
> 1.7.7
> 

Re: [PATCH 2/7] netfilter: xtables2: initial Netlink interface

[Jan Engelhardt]

On Tuesday 2012-02-14 11:47, Pablo Neira Ayuso wrote:
>>  include/linux/netfilter/nfnetlink_xtables.h |   20 ++++
>>  net/netfilter/Kconfig                       |    8 ++
>>  net/netfilter/Makefile                      |    1 +
>>  net/netfilter/xt2_nfnetlink.c               |  153 +++++++++++++++++++++++++++
>
>I prefer if you call this nfnetlink_xtables.c following the same
>naming convention.

(I dropped the '2' meanwhile) Hm I chose so as I planned to have
at least another file, xt_compat.c. In that regard, it's more like
what ipset/ has (also not nfnetlink_ipset). Should I pick a
different header name then?

>> +enum nfxt_msg_type {
>> +	NFXTM_IDENTIFY = 1,
>
>I'd suggest: NFXT_MSG_NEW, NFXT_MSG_GET,...and so on.

The identifiers are already long enough IMO. Hence the choice
to use NFXTM_ (msg), NFXTA_ (attr) and NFXTE_ (error codes).

>> +static struct nlmsghdr *
>> +xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
>> +	       unsigned int flags)
>> +{ [...]
>> +	if (nlmsg == NULL) {
>> +		nlmsg_cancel(skb, nlmsg);
>> +		return ERR_PTR(-ENOBUFS);
>
>my experience is that it's better to use goto to handle errors.

There is just one error case here in this function,
so goto was nowhere to be desired :)

>> +static int
>> +xtnetlink_identify(struct sock *xtnl, struct sk_buff *iskb,
>> +		   const struct nlmsghdr *imsg, const struct nlattr *const *ad)
>> +{
>> +	return netlink_dump_start(xtnl, iskb, imsg, xtnetlink_identify2,
>> +				  NULL, 0);
>
>you have to check for NLM_F_DUMP. Otherwise, you are requesting one
>single table. Please, see ctnetlink or nfnetlink_acct for reference.

identify returns auxiliary information about the implementation.
For tables there is(will be) NFXTM_TABLE_DUMP.

Re: [PATCH 2/7] netfilter: xtables2: initial Netlink interface

[Pablo Neira Ayuso]

On Tue, Feb 14, 2012 at 04:56:01PM +0100, Jan Engelhardt wrote:
> 
> On Tuesday 2012-02-14 11:47, Pablo Neira Ayuso wrote:
> >>  include/linux/netfilter/nfnetlink_xtables.h |   20 ++++
> >>  net/netfilter/Kconfig                       |    8 ++
> >>  net/netfilter/Makefile                      |    1 +
> >>  net/netfilter/xt2_nfnetlink.c               |  153 +++++++++++++++++++++++++++
> >
> >I prefer if you call this nfnetlink_xtables.c following the same
> >naming convention.
> 
> (I dropped the '2' meanwhile) Hm I chose so as I planned to have
> at least another file, xt_compat.c. In that regard, it's more like
> what ipset/ has (also not nfnetlink_ipset). Should I pick a
> different header name then?

As said, I'd prefer to stick to nfnetlink_* naming. But I don't mind.

> >> +enum nfxt_msg_type {
> >> +	NFXTM_IDENTIFY = 1,
> >
> >I'd suggest: NFXT_MSG_NEW, NFXT_MSG_GET,...and so on.
> 
> The identifiers are already long enough IMO. Hence the choice
> to use NFXTM_ (msg), NFXTA_ (attr) and NFXTE_ (error codes).

OK

> >> +static struct nlmsghdr *
> >> +xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
> >> +	       unsigned int flags)
> >> +{ [...]
> >> +	if (nlmsg == NULL) {
> >> +		nlmsg_cancel(skb, nlmsg);
> >> +		return ERR_PTR(-ENOBUFS);
> >
> >my experience is that it's better to use goto to handle errors.
> 
> There is just one error case here in this function,
> so goto was nowhere to be desired :)

OK

> >> +static int
> >> +xtnetlink_identify(struct sock *xtnl, struct sk_buff *iskb,
> >> +		   const struct nlmsghdr *imsg, const struct nlattr *const *ad)
> >> +{
> >> +	return netlink_dump_start(xtnl, iskb, imsg, xtnetlink_identify2,
> >> +				  NULL, 0);
> >
> >you have to check for NLM_F_DUMP. Otherwise, you are requesting one
> >single table. Please, see ctnetlink or nfnetlink_acct for reference.
> 
> identify returns auxiliary information about the implementation.
> For tables there is(will be) NFXTM_TABLE_DUMP.

Better use standard netlink flags like NLM_F_DUMP, NLM_F_REPLACE and
so on.