nf-next: removals of old extensions/revs

nf-next: removals of old extensions/revs

[Jan Engelhardt]

Posting this freestanding set of 4 for review (only), as b05.. has
not been merged yet.


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

are available in the git repository at:

  git://dev.medozas.de/linux master3-obs

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: schedule xt_state for removal

 Documentation/Changes                      |    2 +-
 Documentation/feature-removal-schedule.txt |    8 +
 net/netfilter/xt_hashlimit.c               |  221 ----------------------------
 net/netfilter/xt_multiport.c               |   77 ----------
 net/netfilter/xt_string.c                  |   53 +++-----
 5 files changed, 27 insertions(+), 334 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: schedule xt_state for removal

[Jan Engelhardt]

xt_conntrack has been provided since v2.5.32.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 Documentation/feature-removal-schedule.txt |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index ea227e3..0749a8f 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -587,3 +587,11 @@ Files:	net/netfilter/xt_NOTRACK.c
 When:	April 2011
 Why:	Superseded by xt_CT
 Who:	Netfilter developer team <netfilter-devel@vger.kernel.org>
+
+---------------------------
+
+What:	xt_state
+Files:	net/netfilter/xt_state.c
+When:	April 2011
+Why:	Superseded by xt_conntrack
+Who:	Jan Engelhardt <jengelh@medozas.de>
-- 
1.7.0.2

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Patrick McHardy]

Jan Engelhardt wrote:
> xt_conntrack has been provided since v2.5.32.
>   

I'm fine with the removal of old revisions, but how are you planning on
informing users about removal of this module? Most people don't read
feature-removal-schedule, and distributions are unable to help with
user written scripts.

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Jan Engelhardt]

On Wednesday 2010-03-24 16:02, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> xt_conntrack has been provided since v2.5.32.
>>   
>
>I'm fine with the removal of old revisions, but how are you planning on
>informing users about removal of this module? Most people don't read
>feature-removal-schedule, and distributions are unable to help with
>user written scripts.

I would suggest to do the same as we did with disallowing DROP in the 
nat table:

 - a message printed by iptables whenever -m state is used

 - a kernel message whenever whenever a rule with xt_state is created

We did not actually do the kernel side with nat-prohibit-DROP, but I
regard it as very useful, as the community was very much able to help
itself if only they got the word - and it turned out that dmesg is
_the_ place people look in especially when they don't supervise
iptables output directly, as with, for example, boot splash where
messages are hidden, or server/router devices that one tends to
forget about.

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2010-03-24 16:02, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> xt_conntrack has been provided since v2.5.32.
>>>   
>> I'm fine with the removal of old revisions, but how are you planning on
>> informing users about removal of this module? Most people don't read
>> feature-removal-schedule, and distributions are unable to help with
>> user written scripts.
> 
> I would suggest to do the same as we did with disallowing DROP in the 
> nat table:
> 
>  - a message printed by iptables whenever -m state is used
> 
>  - a kernel message whenever whenever a rule with xt_state is created
> 
> We did not actually do the kernel side with nat-prohibit-DROP, but I
> regard it as very useful, as the community was very much able to help
> itself if only they got the word - and it turned out that dmesg is
> _the_ place people look in especially when they don't supervise
> iptables output directly, as with, for example, boot splash where
> messages are hidden, or server/router devices that one tends to
> forget about.

Yes, a kernel message sounds fine and less annoying than an
iptables message since we can limit it to print only once.

I'm not really convinced of removing state though, I has never
caused any maintenance overhead, it requires a lot less memory
than xt_conntrack and it seems more intuitive to write "-m state"
than "-m conntrack --ctstate" to me.

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Jozsef Kadlecsik]

On Thu, 25 Mar 2010, Patrick McHardy wrote:

> Jan Engelhardt wrote:
> > On Wednesday 2010-03-24 16:02, Patrick McHardy wrote:
> >> Jan Engelhardt wrote:
> >>> xt_conntrack has been provided since v2.5.32.
> >>>   
> >> I'm fine with the removal of old revisions, but how are you planning on
> >> informing users about removal of this module? Most people don't read
> >> feature-removal-schedule, and distributions are unable to help with
> >> user written scripts.
> > 
> > I would suggest to do the same as we did with disallowing DROP in the 
> > nat table:
> > 
> >  - a message printed by iptables whenever -m state is used
> > 
> >  - a kernel message whenever whenever a rule with xt_state is created
> > 
> > We did not actually do the kernel side with nat-prohibit-DROP, but I
> > regard it as very useful, as the community was very much able to help
> > itself if only they got the word - and it turned out that dmesg is
> > _the_ place people look in especially when they don't supervise
> > iptables output directly, as with, for example, boot splash where
> > messages are hidden, or server/router devices that one tends to
> > forget about.
> 
> Yes, a kernel message sounds fine and less annoying than an
> iptables message since we can limit it to print only once.
> 
> I'm not really convinced of removing state though, I has never
> caused any maintenance overhead, it requires a lot less memory
> than xt_conntrack and it seems more intuitive to write "-m state"
> than "-m conntrack --ctstate" to me.

I oppose the removal of xt_state, *unless* the userspace "-m state" is 
kept working and the conntrack module automatically supports it.

It's such a basic match that it's simply overkill to remove it.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> On Thu, 25 Mar 2010, Patrick McHardy wrote:
> 
>> Jan Engelhardt wrote:
>>> On Wednesday 2010-03-24 16:02, Patrick McHardy wrote:
>>>> Jan Engelhardt wrote:
>>>>> xt_conntrack has been provided since v2.5.32.
>>>>>   
>>>> I'm fine with the removal of old revisions, but how are you planning on
>>>> informing users about removal of this module? Most people don't read
>>>> feature-removal-schedule, and distributions are unable to help with
>>>> user written scripts.
>>> I would suggest to do the same as we did with disallowing DROP in the 
>>> nat table:
>>>
>>>  - a message printed by iptables whenever -m state is used
>>>
>>>  - a kernel message whenever whenever a rule with xt_state is created
>>>
>>> We did not actually do the kernel side with nat-prohibit-DROP, but I
>>> regard it as very useful, as the community was very much able to help
>>> itself if only they got the word - and it turned out that dmesg is
>>> _the_ place people look in especially when they don't supervise
>>> iptables output directly, as with, for example, boot splash where
>>> messages are hidden, or server/router devices that one tends to
>>> forget about.
>> Yes, a kernel message sounds fine and less annoying than an
>> iptables message since we can limit it to print only once.
>>
>> I'm not really convinced of removing state though, I has never
>> caused any maintenance overhead, it requires a lot less memory
>> than xt_conntrack and it seems more intuitive to write "-m state"
>> than "-m conntrack --ctstate" to me.
> 
> I oppose the removal of xt_state, *unless* the userspace "-m state" is 
> kept working and the conntrack module automatically supports it.

Yes, that would be acceptable.

> It's such a basic match that it's simply overkill to remove it.

Agreed.

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Jan Engelhardt]

On Thursday 2010-03-25 11:26, Patrick McHardy wrote:

>>> Yes, a kernel message sounds fine and less annoying than an
>>> iptables message since we can limit it to print only once.
>>>
>>> I'm not really convinced of removing state though, I has never
>>> caused any maintenance overhead, it requires a lot less memory
>>> than xt_conntrack

And yet, you proposed removing xt_NOTRACK in favor of xt_CT where the
same argument about memory tradeoff would hold.

>>> and it seems more intuitive to write "-m state"
>>> than "-m conntrack --ctstate" to me.
>> 
>> I oppose the removal of xt_state, *unless* the userspace "-m state" is 
>> kept working and the conntrack module automatically supports it.
>
>Yes, that would be acceptable.
>
>> It's such a basic match that it's simply overkill to remove it.
>
>Agreed.

So what now? Should xt_conntrack be perhaps rebranded as a new
xt_state rev and let's obsolete xt_conntrack.c instead?

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Jozsef Kadlecsik]

On Thu, 25 Mar 2010, Jan Engelhardt wrote:

> On Thursday 2010-03-25 11:26, Patrick McHardy wrote:
> 
> >>> Yes, a kernel message sounds fine and less annoying than an
> >>> iptables message since we can limit it to print only once.
> >>>
> >>> I'm not really convinced of removing state though, I has never
> >>> caused any maintenance overhead, it requires a lot less memory
> >>> than xt_conntrack
> 
> And yet, you proposed removing xt_NOTRACK in favor of xt_CT where the
> same argument about memory tradeoff would hold.

Just my thoughts: xt_NOTRACK is not so widely used than xt_state. So 
it'll bite less (and hopefully more experienced) users.
 
> >>> and it seems more intuitive to write "-m state"
> >>> than "-m conntrack --ctstate" to me.
> >> 
> >> I oppose the removal of xt_state, *unless* the userspace "-m state" is 
> >> kept working and the conntrack module automatically supports it.
> >
> >Yes, that would be acceptable.
> >
> >> It's such a basic match that it's simply overkill to remove it.
> >
> >Agreed.
> 
> So what now? Should xt_conntrack be perhaps rebranded as a new
> xt_state rev and let's obsolete xt_conntrack.c instead?

That's much more acceptable, also because of the usage patterns. And the 
migration can be made easier with module aliasing.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> On Thu, 25 Mar 2010, Jan Engelhardt wrote:
> 
>>>>> and it seems more intuitive to write "-m state"
>>>>> than "-m conntrack --ctstate" to me.
>>>> I oppose the removal of xt_state, *unless* the userspace "-m state" is 
>>>> kept working and the conntrack module automatically supports it.
>>> Yes, that would be acceptable.
>>>
>>>> It's such a basic match that it's simply overkill to remove it.
>>> Agreed.
>> So what now? Should xt_conntrack be perhaps rebranded as a new
>> xt_state rev and let's obsolete xt_conntrack.c instead?
> 
> That's much more acceptable, also because of the usage patterns. And the 
> migration can be made easier with module aliasing.

Yes, I prefer that way as well. Both state and conntrack should
continue to work in userspace.

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Jan Engelhardt]

On Thursday 2010-03-25 13:49, Patrick McHardy wrote:
>> 
>>>>>> and it seems more intuitive to write "-m state"
>>>>>> than "-m conntrack --ctstate" to me.

But "-m state --ctorigsrc fe80::/64" does not seem as intuitive
as "-m conntrack --ctorigsrc fe80::/64".

>>>>> It's such a basic match that it's simply overkill to remove it.
>>>> Agreed.
>>> So what now? Should xt_conntrack be perhaps rebranded as a new
>>> xt_state rev and let's obsolete xt_conntrack.c instead?
>> 
>> That's much more acceptable, also because of the usage patterns. And the 
>> migration can be made easier with module aliasing.
>
>Yes, I prefer that way as well. Both state and conntrack should
>continue to work in userspace.

We should be settling for one name in the long run.

Re: [PATCH 4/4] netfilter: xtables: schedule xt_state for removal

[Jan Engelhardt]

On Thursday 2010-03-25 13:43, Jozsef Kadlecsik wrote:
>> >>>
>> >>> I'm not really convinced of removing state though, I has never
>> >>> caused any maintenance overhead, it requires a lot less memory
>> >>> than xt_conntrack
>> 
>> And yet, you proposed removing xt_NOTRACK in favor of xt_CT where the
>> same argument about memory tradeoff would hold.
>
>Just my thoughts: xt_NOTRACK is not so widely used than xt_state. So 
>it'll bite less (and hopefully more experienced) users.

Well that's why we would have a warning messages. It is not rocket
science to switch "-m state --state" to "-m conntrack --ctstate"
(well, a necessary prerequisite is to know how to use a text editor).
And users who _do write_ their own surely are of the more experienced
category than those who don't do it at all but are fed by the
distro's generator.

>> >>> and it seems more intuitive to write "-m state"
>> >>> than "-m conntrack --ctstate" to me.
>> >> 
>> >> I oppose the removal of xt_state, *unless* the userspace "-m state" is 
>> >> kept working and the conntrack module automatically supports it.
>> >
>> >Yes, that would be acceptable.
>> >
>> >> It's such a basic match that it's simply overkill to remove it.
>> >
>> >Agreed.
>> 
>> So what now? Should xt_conntrack be perhaps rebranded as a new
>> xt_state rev and let's obsolete xt_conntrack.c instead?
>
>That's much more acceptable, also because of the usage patterns. And the 
>migration can be made easier with module aliasing.
>
>Best regards,
>Jozsef
>-
>E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
>PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
>Address : KFKI Research Institute for Particle and Nuclear Physics
>          H-1525 Budapest 114, POB. 49, Hungary
>