nf-next: obsolete old extension revisions

nf-next: obsolete old extension revisions

[Jan Engelhardt]

The following changes since commit f95c74e33eff5e3fe9798e2dc0a7749150ea3f80:
  Jan Engelhardt (1):
        netfilter: xtables: shorten up return clause

are available in the git repository at:

  git://dev.medozas.de/linux master

Jan Engelhardt (4):
      netfilter: xtables: remove xt_hashlimit revision 0
      netfilter: xtables: remove xt_multiport revision 0
      netfilter: xtables: remove xt_string revision 0
      netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

 Documentation/Changes        |    2 +-
 net/netfilter/xt_hashlimit.c |  221 ------------------------------------------
 net/netfilter/xt_multiport.c |   77 ---------------
 net/netfilter/xt_state.c     |   31 ++----
 net/netfilter/xt_string.c    |   53 ++++-------
 5 files changed, 29 insertions(+), 355 deletions(-)

[PATCH 1/4] netfilter: xtables: remove xt_hashlimit revision 0

[Jan Engelhardt]

Superseded by xt_hashlimit revision 1 (linux v2.6.24-6212-g09e410d,
iptables 1.4.1-rc1).

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

diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index c89fde7..5470bb0 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -193,76 +193,6 @@ dsthash_free(struct xt_hashlimit_htable *ht, struct dsthash_ent *ent)
 }
 static void htable_gc(unsigned long htlong);
 
-static int htable_create_v0(struct net *net, struct xt_hashlimit_info *minfo, u_int8_t family)
-{
-	struct hashlimit_net *hashlimit_net = hashlimit_pernet(net);
-	struct xt_hashlimit_htable *hinfo;
-	unsigned int size;
-	unsigned int i;
-
-	if (minfo->cfg.size)
-		size = minfo->cfg.size;
-	else {
-		size = ((totalram_pages << PAGE_SHIFT) / 16384) /
-		       sizeof(struct list_head);
-		if (totalram_pages > (1024 * 1024 * 1024 / PAGE_SIZE))
-			size = 8192;
-		if (size < 16)
-			size = 16;
-	}
-	/* FIXME: don't use vmalloc() here or anywhere else -HW */
-	hinfo = vmalloc(sizeof(struct xt_hashlimit_htable) +
-			sizeof(struct list_head) * size);
-	if (!hinfo)
-		return -ENOMEM;
-	minfo->hinfo = hinfo;
-
-	/* copy match config into hashtable config */
-	hinfo->cfg.mode        = minfo->cfg.mode;
-	hinfo->cfg.avg         = minfo->cfg.avg;
-	hinfo->cfg.burst       = minfo->cfg.burst;
-	hinfo->cfg.max         = minfo->cfg.max;
-	hinfo->cfg.gc_interval = minfo->cfg.gc_interval;
-	hinfo->cfg.expire      = minfo->cfg.expire;
-
-	if (family == NFPROTO_IPV4)
-		hinfo->cfg.srcmask = hinfo->cfg.dstmask = 32;
-	else
-		hinfo->cfg.srcmask = hinfo->cfg.dstmask = 128;
-
-	hinfo->cfg.size = size;
-	if (!hinfo->cfg.max)
-		hinfo->cfg.max = 8 * hinfo->cfg.size;
-	else if (hinfo->cfg.max < hinfo->cfg.size)
-		hinfo->cfg.max = hinfo->cfg.size;
-
-	for (i = 0; i < hinfo->cfg.size; i++)
-		INIT_HLIST_HEAD(&hinfo->hash[i]);
-
-	hinfo->use = 1;
-	hinfo->count = 0;
-	hinfo->family = family;
-	hinfo->rnd_initialized = false;
-	spin_lock_init(&hinfo->lock);
-	hinfo->pde = proc_create_data(minfo->name, 0,
-		(family == NFPROTO_IPV4) ?
-		hashlimit_net->ipt_hashlimit : hashlimit_net->ip6t_hashlimit,
-		&dl_file_ops, hinfo);
-	if (!hinfo->pde) {
-		vfree(hinfo);
-		return -ENOMEM;
-	}
-	hinfo->net = net;
-
-	setup_timer(&hinfo->timer, htable_gc, (unsigned long )hinfo);
-	hinfo->timer.expires = jiffies + msecs_to_jiffies(hinfo->cfg.gc_interval);
-	add_timer(&hinfo->timer);
-
-	hlist_add_head(&hinfo->node, &hashlimit_net->htables);
-
-	return 0;
-}
-
 static int htable_create(struct net *net, struct xt_hashlimit_mtinfo1 *minfo,
 			 u_int8_t family)
 {
@@ -571,57 +501,6 @@ hashlimit_init_dst(const struct xt_hashlimit_htable *hinfo,
 }
 
 static bool
-hashlimit_mt_v0(const struct sk_buff *skb, const struct xt_match_param *par)
-{
-	const struct xt_hashlimit_info *r = par->matchinfo;
-	struct xt_hashlimit_htable *hinfo = r->hinfo;
-	unsigned long now = jiffies;
-	struct dsthash_ent *dh;
-	struct dsthash_dst dst;
-
-	if (hashlimit_init_dst(hinfo, &dst, skb, par->thoff) < 0)
-		goto hotdrop;
-
-	spin_lock_bh(&hinfo->lock);
-	dh = dsthash_find(hinfo, &dst);
-	if (!dh) {
-		dh = dsthash_alloc_init(hinfo, &dst);
-		if (!dh) {
-			spin_unlock_bh(&hinfo->lock);
-			goto hotdrop;
-		}
-
-		dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire);
-		dh->rateinfo.prev = jiffies;
-		dh->rateinfo.credit = user2credits(hinfo->cfg.avg *
-						   hinfo->cfg.burst);
-		dh->rateinfo.credit_cap = user2credits(hinfo->cfg.avg *
-						       hinfo->cfg.burst);
-		dh->rateinfo.cost = user2credits(hinfo->cfg.avg);
-	} else {
-		/* update expiration timeout */
-		dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
-		rateinfo_recalc(dh, now);
-	}
-
-	if (dh->rateinfo.credit >= dh->rateinfo.cost) {
-		/* We're underlimit. */
-		dh->rateinfo.credit -= dh->rateinfo.cost;
-		spin_unlock_bh(&hinfo->lock);
-		return true;
-	}
-
-	spin_unlock_bh(&hinfo->lock);
-
-	/* default case: we're overlimit, thus don't match */
-	return false;
-
-hotdrop:
-	*par->hotdrop = true;
-	return false;
-}
-
-static bool
 hashlimit_mt(const struct sk_buff *skb, const struct xt_match_param *par)
 {
 	const struct xt_hashlimit_mtinfo1 *info = par->matchinfo;
@@ -671,45 +550,6 @@ hashlimit_mt(const struct sk_buff *skb, const struct xt_match_param *par)
 	return false;
 }
 
-static int hashlimit_mt_check_v0(const struct xt_mtchk_param *par)
-{
-	struct net *net = par->net;
-	struct xt_hashlimit_info *r = par->matchinfo;
-	int ret;
-
-	/* Check for overflow. */
-	if (r->cfg.burst == 0 ||
-	    user2credits(r->cfg.avg * r->cfg.burst) < user2credits(r->cfg.avg)) {
-		pr_info("overflow, try lower: %u/%u\n",
-			r->cfg.avg, r->cfg.burst);
-		return -ERANGE;
-	}
-	if (r->cfg.mode == 0 ||
-	    r->cfg.mode > (XT_HASHLIMIT_HASH_DPT |
-			   XT_HASHLIMIT_HASH_DIP |
-			   XT_HASHLIMIT_HASH_SIP |
-			   XT_HASHLIMIT_HASH_SPT))
-		return -EINVAL;
-	if (!r->cfg.gc_interval)
-		return -EINVAL;
-	if (!r->cfg.expire)
-		return -EINVAL;
-	if (r->name[sizeof(r->name) - 1] != '\0')
-		return -EINVAL;
-
-	mutex_lock(&hashlimit_mutex);
-	r->hinfo = htable_find_get(net, r->name, par->family);
-	if (r->hinfo == NULL) {
-		ret = htable_create_v0(net, r, par->family);
-		if (ret < 0) {
-			mutex_unlock(&hashlimit_mutex);
-			return ret;
-		}
-	}
-	mutex_unlock(&hashlimit_mutex);
-	return 0;
-}
-
 static int hashlimit_mt_check(const struct xt_mtchk_param *par)
 {
 	struct net *net = par->net;
@@ -749,14 +589,6 @@ static int hashlimit_mt_check(const struct xt_mtchk_param *par)
 	return 0;
 }
 
-static void
-hashlimit_mt_destroy_v0(const struct xt_mtdtor_param *par)
-{
-	const struct xt_hashlimit_info *r = par->matchinfo;
-
-	htable_put(r->hinfo);
-}
-
 static void hashlimit_mt_destroy(const struct xt_mtdtor_param *par)
 {
 	const struct xt_hashlimit_mtinfo1 *info = par->matchinfo;
@@ -764,47 +596,8 @@ static void hashlimit_mt_destroy(const struct xt_mtdtor_param *par)
 	htable_put(info->hinfo);
 }
 
-#ifdef CONFIG_COMPAT
-struct compat_xt_hashlimit_info {
-	char name[IFNAMSIZ];
-	struct hashlimit_cfg cfg;
-	compat_uptr_t hinfo;
-	compat_uptr_t master;
-};
-
-static void hashlimit_mt_compat_from_user(void *dst, const void *src)
-{
-	int off = offsetof(struct compat_xt_hashlimit_info, hinfo);
-
-	memcpy(dst, src, off);
-	memset(dst + off, 0, sizeof(struct compat_xt_hashlimit_info) - off);
-}
-
-static int hashlimit_mt_compat_to_user(void __user *dst, const void *src)
-{
-	int off = offsetof(struct compat_xt_hashlimit_info, hinfo);
-
-	return copy_to_user(dst, src, off) ? -EFAULT : 0;
-}
-#endif
-
 static struct xt_match hashlimit_mt_reg[] __read_mostly = {
 	{
-		.name		= "hashlimit",
-		.revision	= 0,
-		.family		= NFPROTO_IPV4,
-		.match		= hashlimit_mt_v0,
-		.matchsize	= sizeof(struct xt_hashlimit_info),
-#ifdef CONFIG_COMPAT
-		.compatsize	= sizeof(struct compat_xt_hashlimit_info),
-		.compat_from_user = hashlimit_mt_compat_from_user,
-		.compat_to_user	= hashlimit_mt_compat_to_user,
-#endif
-		.checkentry	= hashlimit_mt_check_v0,
-		.destroy	= hashlimit_mt_destroy_v0,
-		.me		= THIS_MODULE
-	},
-	{
 		.name           = "hashlimit",
 		.revision       = 1,
 		.family         = NFPROTO_IPV4,
@@ -816,20 +609,6 @@ static struct xt_match hashlimit_mt_reg[] __read_mostly = {
 	},
 #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
 	{
-		.name		= "hashlimit",
-		.family		= NFPROTO_IPV6,
-		.match		= hashlimit_mt_v0,
-		.matchsize	= sizeof(struct xt_hashlimit_info),
-#ifdef CONFIG_COMPAT
-		.compatsize	= sizeof(struct compat_xt_hashlimit_info),
-		.compat_from_user = hashlimit_mt_compat_from_user,
-		.compat_to_user	= hashlimit_mt_compat_to_user,
-#endif
-		.checkentry	= hashlimit_mt_check_v0,
-		.destroy	= hashlimit_mt_destroy_v0,
-		.me		= THIS_MODULE
-	},
-	{
 		.name           = "hashlimit",
 		.revision       = 1,
 		.family         = NFPROTO_IPV6,
-- 
1.7.0.2

[PATCH 2/4] netfilter: xtables: remove xt_multiport revision 0

[Jan Engelhardt]

Superseded by xt_multiport revision 1 (introduction already predates
linux.git).

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

diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
index b446738..83b77ce 100644
--- a/net/netfilter/xt_multiport.c
+++ b/net/netfilter/xt_multiport.c
@@ -28,23 +28,6 @@ MODULE_ALIAS("ip6t_multiport");
 
 /* Returns 1 if the port is matched by the test, 0 otherwise. */
 static inline bool
-ports_match_v0(const u_int16_t *portlist, enum xt_multiport_flags flags,
-	       u_int8_t count, u_int16_t src, u_int16_t dst)
-{
-	unsigned int i;
-	for (i = 0; i < count; i++) {
-		if (flags != XT_MULTIPORT_DESTINATION && portlist[i] == src)
-			return true;
-
-		if (flags != XT_MULTIPORT_SOURCE && portlist[i] == dst)
-			return true;
-	}
-
-	return false;
-}
-
-/* Returns 1 if the port is matched by the test, 0 otherwise. */
-static inline bool
 ports_match_v1(const struct xt_multiport_v1 *minfo,
 	       u_int16_t src, u_int16_t dst)
 {
@@ -89,30 +72,6 @@ ports_match_v1(const struct xt_multiport_v1 *minfo,
 }
 
 static bool
-multiport_mt_v0(const struct sk_buff *skb, const struct xt_match_param *par)
-{
-	const __be16 *pptr;
-	__be16 _ports[2];
-	const struct xt_multiport *multiinfo = par->matchinfo;
-
-	if (par->fragoff != 0)
-		return false;
-
-	pptr = skb_header_pointer(skb, par->thoff, sizeof(_ports), _ports);
-	if (pptr == NULL) {
-		/* We've been asked to examine this packet, and we
-		 * can't.  Hence, no choice but to drop.
-		 */
-		pr_debug("Dropping evil offset=0 tinygram.\n");
-		*par->hotdrop = true;
-		return false;
-	}
-
-	return ports_match_v0(multiinfo->ports, multiinfo->flags,
-	       multiinfo->count, ntohs(pptr[0]), ntohs(pptr[1]));
-}
-
-static bool
 multiport_mt(const struct sk_buff *skb, const struct xt_match_param *par)
 {
 	const __be16 *pptr;
@@ -152,15 +111,6 @@ check(u_int16_t proto,
 		&& count <= XT_MULTI_PORTS;
 }
 
-static int multiport_mt_check_v0(const struct xt_mtchk_param *par)
-{
-	const struct ipt_ip *ip = par->entryinfo;
-	const struct xt_multiport *multiinfo = par->matchinfo;
-
-	return check(ip->proto, ip->invflags, multiinfo->flags,
-		     multiinfo->count);
-}
-
 static int multiport_mt_check(const struct xt_mtchk_param *par)
 {
 	const struct ipt_ip *ip = par->entryinfo;
@@ -170,15 +120,6 @@ static int multiport_mt_check(const struct xt_mtchk_param *par)
 		     multiinfo->count);
 }
 
-static int multiport_mt6_check_v0(const struct xt_mtchk_param *par)
-{
-	const struct ip6t_ip6 *ip = par->entryinfo;
-	const struct xt_multiport *multiinfo = par->matchinfo;
-
-	return check(ip->proto, ip->invflags, multiinfo->flags,
-		     multiinfo->count);
-}
-
 static int multiport_mt6_check(const struct xt_mtchk_param *par)
 {
 	const struct ip6t_ip6 *ip = par->entryinfo;
@@ -192,15 +133,6 @@ static struct xt_match multiport_mt_reg[] __read_mostly = {
 	{
 		.name		= "multiport",
 		.family		= NFPROTO_IPV4,
-		.revision	= 0,
-		.checkentry	= multiport_mt_check_v0,
-		.match		= multiport_mt_v0,
-		.matchsize	= sizeof(struct xt_multiport),
-		.me		= THIS_MODULE,
-	},
-	{
-		.name		= "multiport",
-		.family		= NFPROTO_IPV4,
 		.revision	= 1,
 		.checkentry	= multiport_mt_check,
 		.match		= multiport_mt,
@@ -210,15 +142,6 @@ static struct xt_match multiport_mt_reg[] __read_mostly = {
 	{
 		.name		= "multiport",
 		.family		= NFPROTO_IPV6,
-		.revision	= 0,
-		.checkentry	= multiport_mt6_check_v0,
-		.match		= multiport_mt_v0,
-		.matchsize	= sizeof(struct xt_multiport),
-		.me		= THIS_MODULE,
-	},
-	{
-		.name		= "multiport",
-		.family		= NFPROTO_IPV6,
 		.revision	= 1,
 		.checkentry	= multiport_mt6_check,
 		.match		= multiport_mt,
-- 
1.7.0.2

[PATCH 3/4] netfilter: xtables: remove xt_string revision 0

[Jan Engelhardt]

Superseded by xt_string revision 1 (linux v2.6.26-rc8-1127-g4ad3f26,
iptables 1.4.2-rc1).

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 Documentation/Changes     |    2 +-
 net/netfilter/xt_string.c |   53 +++++++++++++++-----------------------------
 2 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/Documentation/Changes b/Documentation/Changes
index f08b313..eca9f6e 100644
--- a/Documentation/Changes
+++ b/Documentation/Changes
@@ -49,7 +49,7 @@ o  oprofile               0.9                     # oprofiled --version
 o  udev                   081                     # udevinfo -V
 o  grub                   0.93                    # grub --version
 o  mcelog		  0.6
-o  iptables               1.4.1                   # iptables -V
+o  iptables               1.4.2                   # iptables -V
 
 
 Kernel compilation
diff --git a/net/netfilter/xt_string.c b/net/netfilter/xt_string.c
index b0f8292..488e368 100644
--- a/net/netfilter/xt_string.c
+++ b/net/netfilter/xt_string.c
@@ -26,12 +26,10 @@ string_mt(const struct sk_buff *skb, const struct xt_match_param *par)
 {
 	const struct xt_string_info *conf = par->matchinfo;
 	struct ts_state state;
-	int invert;
+	bool invert;
 
 	memset(&state, 0, sizeof(struct ts_state));
-
-	invert = (par->match->revision == 0 ? conf->u.v0.invert :
-				    conf->u.v1.flags & XT_STRING_FLAG_INVERT);
+	invert = conf->u.v1.flags & XT_STRING_FLAG_INVERT;
 
 	return (skb_find_text((struct sk_buff *)skb, conf->from_offset,
 			     conf->to_offset, conf->config, &state)
@@ -53,13 +51,11 @@ static int string_mt_check(const struct xt_mtchk_param *par)
 		return -EINVAL;
 	if (conf->patlen > XT_STRING_MAX_PATTERN_SIZE)
 		return -EINVAL;
-	if (par->match->revision == 1) {
-		if (conf->u.v1.flags &
-		    ~(XT_STRING_FLAG_IGNORECASE | XT_STRING_FLAG_INVERT))
-			return -EINVAL;
-		if (conf->u.v1.flags & XT_STRING_FLAG_IGNORECASE)
-			flags |= TS_IGNORECASE;
-	}
+	if (conf->u.v1.flags &
+	    ~(XT_STRING_FLAG_IGNORECASE | XT_STRING_FLAG_INVERT))
+		return -EINVAL;
+	if (conf->u.v1.flags & XT_STRING_FLAG_IGNORECASE)
+		flags |= TS_IGNORECASE;
 	ts_conf = textsearch_prepare(conf->algo, conf->pattern, conf->patlen,
 				     GFP_KERNEL, flags);
 	if (IS_ERR(ts_conf))
@@ -74,38 +70,25 @@ static void string_mt_destroy(const struct xt_mtdtor_param *par)
 	textsearch_destroy(STRING_TEXT_PRIV(par->matchinfo)->config);
 }
 
-static struct xt_match xt_string_mt_reg[] __read_mostly = {
-	{
-		.name 		= "string",
-		.revision	= 0,
-		.family		= NFPROTO_UNSPEC,
-		.checkentry	= string_mt_check,
-		.match 		= string_mt,
-		.destroy 	= string_mt_destroy,
-		.matchsize	= sizeof(struct xt_string_info),
-		.me 		= THIS_MODULE
-	},
-	{
-		.name 		= "string",
-		.revision	= 1,
-		.family		= NFPROTO_UNSPEC,
-		.checkentry	= string_mt_check,
-		.match 		= string_mt,
-		.destroy 	= string_mt_destroy,
-		.matchsize	= sizeof(struct xt_string_info),
-		.me 		= THIS_MODULE
-	},
+static struct xt_match xt_string_mt_reg __read_mostly = {
+	.name       = "string",
+	.revision   = 1,
+	.family     = NFPROTO_UNSPEC,
+	.checkentry = string_mt_check,
+	.match      = string_mt,
+	.destroy    = string_mt_destroy,
+	.matchsize  = sizeof(struct xt_string_info),
+	.me         = THIS_MODULE,
 };
 
 static int __init string_mt_init(void)
 {
-	return xt_register_matches(xt_string_mt_reg,
-				   ARRAY_SIZE(xt_string_mt_reg));
+	return xt_register_match(&xt_string_mt_reg);
 }
 
 static void __exit string_mt_exit(void)
 {
-	xt_unregister_matches(xt_string_mt_reg, ARRAY_SIZE(xt_string_mt_reg));
+	xt_unregister_match(&xt_string_mt_reg);
 }
 
 module_init(string_mt_init);
-- 
1.7.0.2

[PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/netfilter/xt_state.c |   31 ++++++++++---------------------
 1 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/net/netfilter/xt_state.c b/net/netfilter/xt_state.c
index be00d7b..bb12718 100644
--- a/net/netfilter/xt_state.c
+++ b/net/netfilter/xt_state.c
@@ -53,35 +53,24 @@ static void state_mt_destroy(const struct xt_mtdtor_param *par)
 	nf_ct_l3proto_module_put(par->family);
 }
 
-static struct xt_match state_mt_reg[] __read_mostly = {
-	{
-		.name		= "state",
-		.family		= NFPROTO_IPV4,
-		.checkentry	= state_mt_check,
-		.match		= state_mt,
-		.destroy	= state_mt_destroy,
-		.matchsize	= sizeof(struct xt_state_info),
-		.me		= THIS_MODULE,
-	},
-	{
-		.name		= "state",
-		.family		= NFPROTO_IPV6,
-		.checkentry	= state_mt_check,
-		.match		= state_mt,
-		.destroy	= state_mt_destroy,
-		.matchsize	= sizeof(struct xt_state_info),
-		.me		= THIS_MODULE,
-	},
+static struct xt_match state_mt_reg __read_mostly = {
+	.name       = "state",
+	.family     = NFPROTO_UNSPEC,
+	.checkentry = state_mt_check,
+	.match      = state_mt,
+	.destroy    = state_mt_destroy,
+	.matchsize  = sizeof(struct xt_state_info),
+	.me         = THIS_MODULE,
 };
 
 static int __init state_mt_init(void)
 {
-	return xt_register_matches(state_mt_reg, ARRAY_SIZE(state_mt_reg));
+	return xt_register_match(&state_mt_reg);
 }
 
 static void __exit state_mt_exit(void)
 {
-	xt_unregister_matches(state_mt_reg, ARRAY_SIZE(state_mt_reg));
+	xt_unregister_match(&state_mt_reg);
 }
 
 module_init(state_mt_init);
-- 
1.7.0.2

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Patrick McHardy]

Jan Engelhardt wrote:
> +static struct xt_match state_mt_reg __read_mostly = {
> +	.name       = "state",
> +	.family     = NFPROTO_UNSPEC,
> +	.checkentry = state_mt_check,
> +	.match      = state_mt,
> +	.destroy    = state_mt_destroy,
> +	.matchsize  = sizeof(struct xt_state_info),
> +	.me         = THIS_MODULE,
>  };

This is the one I actually wanted to see since the subject didn't
state which module is changes.

The patch looks fine, however I'd prefer if in the future you'd add
a changelog entry for these conversions stating why this will work
properly in all cases even with arptables and ebtables using the
xtables infrastructure.

Re: nf-next: obsolete old extension revisions

[Patrick McHardy]

Jan Engelhardt wrote:
> The following changes since commit f95c74e33eff5e3fe9798e2dc0a7749150ea3f80:
>   Jan Engelhardt (1):
>         netfilter: xtables: shorten up return clause
> 
> are available in the git repository at:
> 
>   git://dev.medozas.de/linux master
> 
> Jan Engelhardt (4):
>       netfilter: xtables: remove xt_hashlimit revision 0
>       netfilter: xtables: remove xt_multiport revision 0
>       netfilter: xtables: remove xt_string revision 0
>       netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

Pulled, thanks Jan.

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Jan Engelhardt]

On Wednesday 2010-03-31 10:31, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> +static struct xt_match state_mt_reg __read_mostly = {
>> +	.name       = "state",
>> +	.family     = NFPROTO_UNSPEC,
>> +	.checkentry = state_mt_check,
>> +	.match      = state_mt,
>> +	.destroy    = state_mt_destroy,
>> +	.matchsize  = sizeof(struct xt_state_info),
>> +	.me         = THIS_MODULE,
>>  };
>
>This is the one I actually wanted to see since the subject didn't
>state which module is changes.
>
>The patch looks fine, however I'd prefer if in the future you'd add
>a changelog entry for these conversions stating why this will work
>properly in all cases even with arptables and ebtables using the
>xtables infrastructure.

This will work because x_tables scans for NFPROTO_UNSPEC,
and arp/ebtables just using x_tables :-)

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2010-03-31 10:31, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> +static struct xt_match state_mt_reg __read_mostly = {
>>> +	.name       = "state",
>>> +	.family     = NFPROTO_UNSPEC,
>>> +	.checkentry = state_mt_check,
>>> +	.match      = state_mt,
>>> +	.destroy    = state_mt_destroy,
>>> +	.matchsize  = sizeof(struct xt_state_info),
>>> +	.me         = THIS_MODULE,
>>>  };
>> This is the one I actually wanted to see since the subject didn't
>> state which module is changes.
>>
>> The patch looks fine, however I'd prefer if in the future you'd add
>> a changelog entry for these conversions stating why this will work
>> properly in all cases even with arptables and ebtables using the
>> xtables infrastructure.
> 
> This will work because x_tables scans for NFPROTO_UNSPEC,
> and arp/ebtables just using x_tables :-)

I'm not sure I'm parsing this correctly. Both will find the match,
however the nf_ct_l3proto_try_module_get() call will fail, so they
can't actually use this (which is correct). Anyways, this should
be stated in the changelog.

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Jan Engelhardt]

On Wednesday 2010-03-31 10:41, Patrick McHardy wrote:
>>>> +static struct xt_match state_mt_reg __read_mostly = {
>>>> +	.name       = "state",
>>>> +	.family     = NFPROTO_UNSPEC,
>>>> +	.checkentry = state_mt_check,
>>>> +	.match      = state_mt,
>>>> +	.destroy    = state_mt_destroy,
>>>> +	.matchsize  = sizeof(struct xt_state_info),
>>>> +	.me         = THIS_MODULE,
>>>>  };
>>>
>>> The patch looks fine, however I'd prefer if in the future you'd add
>>> a changelog entry for these conversions stating why this will work
>>> properly in all cases even with arptables and ebtables using the
>>> xtables infrastructure.
>> 
>> This will work because x_tables scans for NFPROTO_UNSPEC,
>> and arp/ebtables just using x_tables :-)
>
>I'm not sure I'm parsing this correctly. Both will find the match,
>however the nf_ct_l3proto_try_module_get() call will fail

It won't fail - it is using par->family, not par->match->family.

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2010-03-31 10:41, Patrick McHardy wrote:
>>>>> +static struct xt_match state_mt_reg __read_mostly = {
>>>>> +	.name       = "state",
>>>>> +	.family     = NFPROTO_UNSPEC,
>>>>> +	.checkentry = state_mt_check,
>>>>> +	.match      = state_mt,
>>>>> +	.destroy    = state_mt_destroy,
>>>>> +	.matchsize  = sizeof(struct xt_state_info),
>>>>> +	.me         = THIS_MODULE,
>>>>>  };
>>>> The patch looks fine, however I'd prefer if in the future you'd add
>>>> a changelog entry for these conversions stating why this will work
>>>> properly in all cases even with arptables and ebtables using the
>>>> xtables infrastructure.
>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>> and arp/ebtables just using x_tables :-)
>> I'm not sure I'm parsing this correctly. Both will find the match,
>> however the nf_ct_l3proto_try_module_get() call will fail
> 
> It won't fail - it is using par->family, not par->match->family.

That's broken then.

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Jan Engelhardt]

On Wednesday 2010-03-31 11:01, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Wednesday 2010-03-31 10:41, Patrick McHardy wrote:
>>>>>> +static struct xt_match state_mt_reg __read_mostly = {
>>>>>> +	.name       = "state",
>>>>>> +	.family     = NFPROTO_UNSPEC,
>>>>>> +	.checkentry = state_mt_check,
>>>>>> +	.match      = state_mt,
>>>>>> +	.destroy    = state_mt_destroy,
>>>>>> +	.matchsize  = sizeof(struct xt_state_info),
>>>>>> +	.me         = THIS_MODULE,
>>>>>>  };
>>>>> The patch looks fine, however I'd prefer if in the future you'd add
>>>>> a changelog entry for these conversions stating why this will work
>>>>> properly in all cases even with arptables and ebtables using the
>>>>> xtables infrastructure.
>>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>>> and arp/ebtables just using x_tables :-)
>>> I'm not sure I'm parsing this correctly. Both will find the match,
>>> however the nf_ct_l3proto_try_module_get() call will fail
>> 
>> It won't fail - it is using par->family, not par->match->family.
>
>That's broken then.

How so?

(Mind, `iptables -m state --state NEW -j ACCEPT` still works,
and so is xt_state.)

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2010-03-31 11:01, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>>>> and arp/ebtables just using x_tables :-)
>>>> I'm not sure I'm parsing this correctly. Both will find the match,
>>>> however the nf_ct_l3proto_try_module_get() call will fail
>>> It won't fail - it is using par->family, not par->match->family.
>> That's broken then.
> 
> How so?

Because arptables and ebtables shouldn't be able to use this module
directly. Even less so after a patch stating "merge registration
structure".

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Jan Engelhardt]

On Wednesday 2010-03-31 11:08, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Wednesday 2010-03-31 11:01, Patrick McHardy wrote:
>>> Jan Engelhardt wrote:
>>>>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>>>>> and arp/ebtables just using x_tables :-)
>>>>> I'm not sure I'm parsing this correctly. Both will find the match,
>>>>> however the nf_ct_l3proto_try_module_get() call will fail
>>>> It won't fail - it is using par->family, not par->match->family.
>>> That's broken then.
>> 
>> How so?
>
>Because arptables and ebtables shouldn't be able to use this module
>directly. Even less so after a patch stating "merge registration
>structure".

arp/ebtables _couldn't_ even use this module. The simple showstopper:
arp/ebtables simply don't have a corresponding userspace portion for
it. Indeed nf_ct_l3proto_try_module_get(NFPROTO_BRIDGE) does not make
much sense, but, in all honesty, xt_state *is* testing for a
protocol-independent feature, so NFPROTO_UNSPEC is justified IMO.

Also, NFPROTO_BRIDGE is special anyway - it does not refer to an L3
protocol actually, but to L2 - so, well, it's kinda moot to muse
about the possibility of calling nf_ct_get(NFPROTO_BRIDGE). If you
_really_ wanted to support state matching at the ARP/EB level, you
would anyhow have to add a separate ->check function that loads all
possible L3 trackers. Which is not a big problem per se
(see patch - no touching of NFPROTO_UNSPEC was needed).
Or, as an alternate possibility, someone adds a nf_conntrack-proto-bridge
module that is just an empty module depending on nf_conntrack_ipv4 and
_ipv6.

diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index 3348706..ac817b1 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -210,51 +210,85 @@ static int conntrack_mt_check(const struct xt_mtchk_param *par)
 {
 	int ret;
 
 	ret = nf_ct_l3proto_try_module_get(par->family);
 	if (ret < 0)
 		pr_info("cannot load conntrack support for proto=%u\n",
 			par->family);
 	return ret;
 }
 
 static void conntrack_mt_destroy(const struct xt_mtdtor_param *par)
 {
 	nf_ct_l3proto_module_put(par->family);
 }
 
+static int conntrack_br_check(const struct xt_mtchk_param *par)
+{
+	int ret;
+
+	ret = nf_ct_l3proto_try_module_get(NFPROTO_IPV4);
+	if (ret < 0) {
+		pr_info("cannot load conntrack support for NFPROTO_IPV4\n");
+		return ret;
+	}
+	ret = nf_ct_l3proto_try_module_get(NFPROTO_IPV6);
+	if (ret < 0) {
+		nf_ct_l3proto_module_put(NFPROTO_IPV4);
+		pr_info("cannot load conntrack support for NFPROTO_IPV6\n");
+		return ret;
+	}
+	return 0;
+}
+
+static void conntrack_br_destroy(const struct xt_mtdtor_param *par)
+{
+	nf_ct_l3proto_module_put(NFPROTO_IPV4);
+	nf_ct_l3proto_module_put(NFPROTO_IPV6);
+}
+
 static struct xt_match conntrack_mt_reg[] __read_mostly = {
 	{
 		.name       = "conntrack",
 		.revision   = 1,
 		.family     = NFPROTO_UNSPEC,
 		.matchsize  = sizeof(struct xt_conntrack_mtinfo1),
 		.match      = conntrack_mt_v1,
 		.checkentry = conntrack_mt_check,
 		.destroy    = conntrack_mt_destroy,
 		.me         = THIS_MODULE,
 	},
 	{
 		.name       = "conntrack",
 		.revision   = 2,
 		.family     = NFPROTO_UNSPEC,
 		.matchsize  = sizeof(struct xt_conntrack_mtinfo2),
 		.match      = conntrack_mt_v2,
 		.checkentry = conntrack_mt_check,
 		.destroy    = conntrack_mt_destroy,
 		.me         = THIS_MODULE,
 	},
+	{
+		.name       = "conntrack",
+		.revision   = 2,
+		.family     = NFPROTO_BRIDGE,
+		.matchsize  = sizeof(struct xt_conntrack_mtinfo2),
+		.match      = conntrack_mt_v2,
+		.checkentry = conntrack_br_check,
+		.destroy    = conntrack_br_destroy,
+		.me         = THIS_MODULE,
+	},
 };
 
 static int __init conntrack_mt_init(void)
 {
 	return xt_register_matches(conntrack_mt_reg,
 	       ARRAY_SIZE(conntrack_mt_reg));
 }
 
 static void __exit conntrack_mt_exit(void)
 {
 	xt_unregister_matches(conntrack_mt_reg, ARRAY_SIZE(conntrack_mt_reg));
 }
 
 module_init(conntrack_mt_init);
 module_exit(conntrack_mt_exit);

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2010-03-31 11:08, Patrick McHardy wrote:
>   
>> Jan Engelhardt wrote:
>>     
>>> On Wednesday 2010-03-31 11:01, Patrick McHardy wrote:
>>>       
>>>> Jan Engelhardt wrote:
>>>>         
>>>>>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>>>>>> and arp/ebtables just using x_tables :-)
>>>>>>>               
>>>>>> I'm not sure I'm parsing this correctly. Both will find the match,
>>>>>> however the nf_ct_l3proto_try_module_get() call will fail
>>>>>>             
>>>>> It won't fail - it is using par->family, not par->match->family.
>>>>>           
>>>> That's broken then.
>>>>         
>>> How so?
>>>       
>> Because arptables and ebtables shouldn't be able to use this module
>> directly. Even less so after a patch stating "merge registration
>> structure".
>>     
>
> arp/ebtables _couldn't_ even use this module. The simple showstopper:
> arp/ebtables simply don't have a corresponding userspace portion for
> it.

That's a really bad argument.

>  Indeed nf_ct_l3proto_try_module_get(NFPROTO_BRIDGE) does not make
> much sense, but, in all honesty, xt_state *is* testing for a
> protocol-independent feature, so NFPROTO_UNSPEC is justified IMO.
>   
Agreed.
> Also, NFPROTO_BRIDGE is special anyway - it does not refer to an L3
> protocol actually, but to L2 - so, well, it's kinda moot to muse
> about the possibility of calling nf_ct_get(NFPROTO_BRIDGE).

I assume you mean nf_ct_l3proto_try_module_get(). Just as I was saying,
it *will* fail for NFPROTO_BRIDGE/ARP, so everything should be fine. You
disputed this however.

>  If you
> _really_ wanted to support state matching at the ARP/EB level, you
> would anyhow have to add a separate ->check function that loads all
> possible L3 trackers. Which is not a big problem per se
> (see patch - no touching of NFPROTO_UNSPEC was needed).
>   

That doesn't really work since bridge netfilter is (partially) invoked
before conntrack.

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Jan Engelhardt]

On Wednesday 2010-03-31 11:45, Patrick McHardy wrote:
>>>>>         
>>>>>>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>>>>>>> and arp/ebtables just using x_tables :-)
>>>>>>>>               
>>>>>>> I'm not sure I'm parsing this correctly. Both will find the match,
>>>>>>> however the nf_ct_l3proto_try_module_get() call will fail
>>>>>>>             
>>>>>> It won't fail - it is using par->family, not par->match->family.
>>>>>>           
>>>>> That's broken then.
>>
>> Also, NFPROTO_BRIDGE is special anyway - it does not refer to an L3
>> protocol actually, but to L2 - so, well, it's kinda moot to muse
>> about the possibility of calling nf_ct_get(NFPROTO_BRIDGE).
>
>I assume you mean nf_ct_l3proto_try_module_get(). Just as I was saying,
>it *will* fail for NFPROTO_BRIDGE/ARP, so everything should be fine. You
>disputed this however.

Ah... genuine mixup. I took the "both" in "Both will find the match"
as iptables and ip6tables because they used to find it before.

>>  If you
>> _really_ wanted to support state matching at the ARP/EB level, you
>> would anyhow have to add a separate ->check function that loads all
>> possible L3 trackers. Which is not a big problem per se
>> (see patch - no touching of NFPROTO_UNSPEC was needed).
>>   
>
>That doesn't really work since bridge netfilter is (partially) invoked
>before conntrack.

Not everywhere, indeed. But there are three theoretically usable blue boxes
(input, forward, output) in http://jengelh.medozas.de/images/nf-packet-flow.png
that come after conntrack. :-)

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2010-03-31 11:45, Patrick McHardy wrote:
>   
>>>>>>         
>>>>>>             
>>>>>>>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>>>>>>>> and arp/ebtables just using x_tables :-)
>>>>>>>>>               
>>>>>>>>>                   
>>>>>>>> I'm not sure I'm parsing this correctly. Both will find the match,
>>>>>>>> however the nf_ct_l3proto_try_module_get() call will fail
>>>>>>>>             
>>>>>>>>                 
>>>>>>> It won't fail - it is using par->family, not par->match->family.
>>>>>>>           
>>>>>>>               
>>>>>> That's broken then.
>>>>>>             
>>> Also, NFPROTO_BRIDGE is special anyway - it does not refer to an L3
>>> protocol actually, but to L2 - so, well, it's kinda moot to muse
>>> about the possibility of calling nf_ct_get(NFPROTO_BRIDGE).
>>>       
>> I assume you mean nf_ct_l3proto_try_module_get(). Just as I was saying,
>> it *will* fail for NFPROTO_BRIDGE/ARP, so everything should be fine. You
>> disputed this however.
>>     
>
> Ah... genuine mixup. I took the "both" in "Both will find the match"
> as iptables and ip6tables because they used to find it before.
>   

OK, so we're fine.
>>>  If you
>>> _really_ wanted to support state matching at the ARP/EB level, you
>>> would anyhow have to add a separate ->check function that loads all
>>> possible L3 trackers. Which is not a big problem per se
>>> (see patch - no touching of NFPROTO_UNSPEC was needed).
>>>   
>>>       
>> That doesn't really work since bridge netfilter is (partially) invoked
>> before conntrack.
>>     
>
> Not everywhere, indeed. But there are three theoretically usable blue boxes
> (input, forward, output) in http://jengelh.medozas.de/images/nf-packet-flow.png
> that come after conntrack. :-)
>   
Maybe, but since bridge netfilter would have to invoke the IPv4/IPv6 hooks
anyways for conntrack, it doesn't seem to be very useful. What I'd like
a lot more would be if ebtables could run conntrack/NAT and other useful
modules directly so we could get rid of most of "integration" mess.
Not sure if that's really possible though.

Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Jan Engelhardt]

On Wednesday 2010-03-31 11:56, Patrick McHardy wrote:
>
>What I'd like a lot more would be if ebtables could run
>conntrack/NAT and other useful modules directly so we could get rid
>of most of "integration" mess. Not sure if that's really possible
>though.

I do belive that it is possible. To that end, I had already posted a
patchset last October (and that is now slowly being poured into
nf-next) that would provide us with protocol-agnostic tables, a
welcome (and perhaps even necessary) prerequisite. From there on, it
should only be a small step in ensuring that same hooks don't get
called multiple times anymore.