Re: state match is obsolete 1.4.17

Re: state match is obsolete 1.4.17

[Jozsef Kadlecsik]

On Tue, 15 Jan 2013, Victor Julien wrote:

> On 01/15/2013 10:11 AM, Jan Engelhardt wrote:
> > 
> > On Tuesday 2013-01-15 06:09, Nick Edwards wrote:
> > 
> >> WARNING: The state match is obsolete. Use conntrack instead.
> >>
> >> Getting these errors since upgrading to 1.4.17
> > 
> > It is a warning, not an error. (An error would not let use you
> > the command at all.)
> > 
> >> Am I right in assuming that :
> >> iptables -A INPUT -m state --state ESTABLISHED,RELATED -j ACCEPT
> >> must now become :
> >> iptables -A INPUT -m conntrack --ctstate ESTABLISHED,RELATED -j ACCEPT
> >> or does that not do the same thing?
> > 
> > state is a redundant subset of conntrack (the latter was introduced around
> > Linux 2.5.32) and shall go away.
> 
> I think removing it is a bad idea. For years and years all docs, books,
> tutorials and frontends (like my own) have worked with "state". The
> change seems so trivial "s/-m state --state/-m conntrack --ctstate/g"
> that it would appear keeping "state" around as an alias or compatibility
> layer would require minimal effort. Why not keep it around?

Actually, I have to agree. Why don't we keep "state" as an alias and 
accept the old syntax in "conntrack"?

What's the compelling reason to break countless scripts?

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: state match is obsolete 1.4.17

[Born Without]

On 15.01.2013 11:06, Jozsef Kadlecsik wrote:
> On Tue, 15 Jan 2013, Victor Julien wrote:
>
>> On 01/15/2013 10:11 AM, Jan Engelhardt wrote:
>>>
>>> On Tuesday 2013-01-15 06:09, Nick Edwards wrote:
>>>
>>>> WARNING: The state match is obsolete. Use conntrack instead.
>>>>
>>>> Getting these errors since upgrading to 1.4.17
>>>
>>> It is a warning, not an error. (An error would not let use you
>>> the command at all.)
>>>
>>>> Am I right in assuming that :
>>>> iptables -A INPUT -m state --state ESTABLISHED,RELATED -j ACCEPT
>>>> must now become :
>>>> iptables -A INPUT -m conntrack --ctstate ESTABLISHED,RELATED -j ACCEPT
>>>> or does that not do the same thing?
>>>
>>> state is a redundant subset of conntrack (the latter was introduced around
>>> Linux 2.5.32) and shall go away.
>>
>> I think removing it is a bad idea. For years and years all docs, books,
>> tutorials and frontends (like my own) have worked with "state". The
>> change seems so trivial "s/-m state --state/-m conntrack --ctstate/g"
>> that it would appear keeping "state" around as an alias or compatibility
>> layer would require minimal effort. Why not keep it around?
>
> Actually, I have to agree. Why don't we keep "state" as an alias and
> accept the old syntax in "conntrack"?
>
> What's the compelling reason to break countless scripts?
>

Yes please, bump +1

I never understood why 'state' wasn't simply extended.
Not doing a smooth transition, is just very unfriendly to users, for 
actually no good reason.

Re: state match is obsolete 1.4.17

[Jan Engelhardt]

On Tuesday 2013-01-15 11:06, Jozsef Kadlecsik wrote:
>> > 
>> > state is a redundant subset of conntrack (the latter was introduced around
>> > Linux 2.5.32) and shall go away.
>> 
>> I think removing it is a bad idea. For years and years all docs, books,
>> tutorials and frontends (like my own) have worked with "state". The
>> change seems so trivial "s/-m state --state/-m conntrack --ctstate/g"
>> that it would appear keeping "state" around as an alias or compatibility
>> layer would require minimal effort. Why not keep it around?
>
>Actually, I have to agree. Why don't we keep "state" as an alias and 
>accept the old syntax in "conntrack"?

state is currently aliased and translated to conntrack in iptables
if the kernel has it. No scripts are broken.

If the aliasing is done in userspace, the kernel part can be removed -
someday maybe.

Re: state match is obsolete 1.4.17

[Jozsef Kadlecsik]

On Tue, 15 Jan 2013, Jan Engelhardt wrote:

> On Tuesday 2013-01-15 11:06, Jozsef Kadlecsik wrote:
> >> > 
> >> > state is a redundant subset of conntrack (the latter was introduced around
> >> > Linux 2.5.32) and shall go away.
> >> 
> >> I think removing it is a bad idea. For years and years all docs, books,
> >> tutorials and frontends (like my own) have worked with "state". The
> >> change seems so trivial "s/-m state --state/-m conntrack --ctstate/g"
> >> that it would appear keeping "state" around as an alias or compatibility
> >> layer would require minimal effort. Why not keep it around?
> >
> >Actually, I have to agree. Why don't we keep "state" as an alias and 
> >accept the old syntax in "conntrack"?
> 
> state is currently aliased and translated to conntrack in iptables
> if the kernel has it. No scripts are broken.
> 
> If the aliasing is done in userspace, the kernel part can be removed -
> someday maybe.

The aliasing is already done in userspace. One types in "state" and it's 
converted into "conntrack" and that is then sent to the kernel. (So as far 
as I see if the ipt_state, etc module aliases were added to the conntrack 
module, even the state kernel module could be removed.)

However I suggest to delete the obsolete warnings completely from iptables 
and let these cases silently be handled as aliases.

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: state match is obsolete 1.4.17

[Jan Engelhardt]

On Tuesday 2013-01-15 14:22, Jozsef Kadlecsik wrote:
>> 
>> state is currently aliased and translated to conntrack in iptables
>> if the kernel has it. No scripts are broken.
>> 
>> If the aliasing is done in userspace, the kernel part can be removed -
>> someday maybe.
>
>The aliasing is already done in userspace. One types in "state" and it's 
>converted into "conntrack" and that is then sent to the kernel. (So as far 
>as I see if the ipt_state, etc module aliases were added to the conntrack 
>module, even the state kernel module could be removed.)

The module aliases were added because the module in fact (still) supports
the "state" extension by that name.

>However I suggest to delete the obsolete warnings completely from iptables 
>and let these cases silently be handled as aliases.

Then users will complain about spooky action at a distance.
(silent changing of rules) - not a great perspective either.
The obsolescence warning is an important part of documenting
changed behavior, and you really really do not want to take
that away from users.

Re: state match is obsolete 1.4.17

[Jozsef Kadlecsik]

On Tue, 15 Jan 2013, Jan Engelhardt wrote:

> On Tuesday 2013-01-15 14:22, Jozsef Kadlecsik wrote:
> >> 
> >> state is currently aliased and translated to conntrack in iptables
> >> if the kernel has it. No scripts are broken.
> >> 
> >> If the aliasing is done in userspace, the kernel part can be removed -
> >> someday maybe.
> >
> >The aliasing is already done in userspace. One types in "state" and it's 
> >converted into "conntrack" and that is then sent to the kernel. (So as far 
> >as I see if the ipt_state, etc module aliases were added to the conntrack 
> >module, even the state kernel module could be removed.)
> 
> The module aliases were added because the module in fact (still) supports
> the "state" extension by that name.

No, please don't "still". It implies "for a while, then it won't".
 
> >However I suggest to delete the obsolete warnings completely from iptables 
> >and let these cases silently be handled as aliases.
> 
> Then users will complain about spooky action at a distance.
> (silent changing of rules) - not a great perspective either.
> The obsolescence warning is an important part of documenting
> changed behavior, and you really really do not want to take
> that away from users.

With passing one more internal flag, indicating that the "state" alias is 
used, the "conntrack" module can remain completely hidden and the user can 
list/save exactly the same command as the issued one.

I do not want to take away the "state" match at all from the users, and 
don't want it to be tainted by a warning either. This is a basic match 
used everywhere and there's no point in forcing everyone to use a new 
syntax.

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

[PATCH]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Jozsef Kadlecsik]

On Tue, 15 Jan 2013, Jozsef Kadlecsik wrote:

> With passing one more internal flag, indicating that the "state" alias is 
> used, the "conntrack" module can remain completely hidden and the user can 
> list/save exactly the same command as the issued one.

Here follows the patch which introduces match module aliases (the same can 
be done for targets as well). The alias is handled as it were a real 
extension while it's actually handled by another module. Listing/saving 
keeps the alias name and options.

The "state" extension is the first example of such an alias.

diff --git a/extensions/libxt_conntrack.c b/extensions/libxt_conntrack.c
index c37f14d..92a42dc 100644
--- a/extensions/libxt_conntrack.c
+++ b/extensions/libxt_conntrack.c
@@ -794,6 +794,15 @@ conntrack_dump_ports(const char *prefix, const char *opt,
 		printf(" %s%s %u:%u", prefix, opt, port_low, port_high);
 }
 
+static const char *
+print_name_alias(const struct xt_entry_match *match)
+{
+	struct xt_conntrack_mtinfo1 *info = (void *)match->data;
+	
+	return info->match_flags & XT_CONNTRACK_STATE_ALIAS
+		? "state" : "conntrack";	
+}
+
 static void
 conntrack_dump(const struct xt_conntrack_mtinfo3 *info, const char *prefix,
                unsigned int family, bool numeric, bool v3)
@@ -801,7 +810,9 @@ conntrack_dump(const struct xt_conntrack_mtinfo3 *info, const char *prefix,
 	if (info->match_flags & XT_CONNTRACK_STATE) {
 		if (info->invert_flags & XT_CONNTRACK_STATE)
 			printf(" !");
-		printf(" %sctstate", prefix);
+		printf(" %s%s", prefix,
+		       info->match_flags & XT_CONNTRACK_STATE_ALIAS
+		       ? "state" : "ctstate");
 		print_state(info->state_mask);
 	}
 
@@ -1083,7 +1094,7 @@ static void state_ct1_parse(struct xt_option_call *cb)
 	struct xt_conntrack_mtinfo1 *sinfo = cb->data;
 
 	xtables_option_parse(cb);
-	sinfo->match_flags = XT_CONNTRACK_STATE;
+	sinfo->match_flags = XT_CONNTRACK_STATE | XT_CONNTRACK_STATE_ALIAS;
 	sinfo->state_mask = state_parse_states(cb->arg);
 	if (cb->invert)
 		sinfo->invert_flags |= XT_CONNTRACK_STATE;
@@ -1094,7 +1105,7 @@ static void state_ct23_parse(struct xt_option_call *cb)
 	struct xt_conntrack_mtinfo3 *sinfo = cb->data;
 
 	xtables_option_parse(cb);
-	sinfo->match_flags = XT_CONNTRACK_STATE;
+	sinfo->match_flags = XT_CONNTRACK_STATE | XT_CONNTRACK_STATE_ALIAS;
 	sinfo->state_mask = state_parse_states(cb->arg);
 	if (cb->invert)
 		sinfo->invert_flags |= XT_CONNTRACK_STATE;
@@ -1172,6 +1183,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.x6_fcheck     = conntrack_mt_check,
 		.print         = conntrack1_mt4_print,
 		.save          = conntrack1_mt4_save,
+		.alias	       = print_name_alias,
 		.x6_options    = conntrack2_mt_opts,
 	},
 	{
@@ -1186,6 +1198,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.x6_fcheck     = conntrack_mt_check,
 		.print         = conntrack1_mt6_print,
 		.save          = conntrack1_mt6_save,
+		.alias	       = print_name_alias,
 		.x6_options    = conntrack2_mt_opts,
 	},
 	{
@@ -1200,6 +1213,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.x6_fcheck     = conntrack_mt_check,
 		.print         = conntrack2_mt_print,
 		.save          = conntrack2_mt_save,
+		.alias	       = print_name_alias,
 		.x6_options    = conntrack2_mt_opts,
 	},
 	{
@@ -1214,6 +1228,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.x6_fcheck     = conntrack_mt_check,
 		.print         = conntrack2_mt6_print,
 		.save          = conntrack2_mt6_save,
+		.alias	       = print_name_alias,
 		.x6_options    = conntrack2_mt_opts,
 	},
 	{
@@ -1228,6 +1243,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.x6_fcheck     = conntrack_mt_check,
 		.print         = conntrack3_mt_print,
 		.save          = conntrack3_mt_save,
+		.alias	       = print_name_alias,
 		.x6_options    = conntrack3_mt_opts,
 	},
 	{
@@ -1242,6 +1258,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.x6_fcheck     = conntrack_mt_check,
 		.print         = conntrack3_mt6_print,
 		.save          = conntrack3_mt6_save,
+		.alias	       = print_name_alias,
 		.x6_options    = conntrack3_mt_opts,
 	},
 	{
@@ -1249,6 +1266,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.name          = "state",
 		.real_name     = "conntrack",
 		.revision      = 1,
+		.ext_flags     = XTABLES_EXT_ALIAS,
 		.version       = XTABLES_VERSION,
 		.size          = XT_ALIGN(sizeof(struct xt_conntrack_mtinfo1)),
 		.userspacesize = XT_ALIGN(sizeof(struct xt_conntrack_mtinfo1)),
@@ -1261,6 +1279,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.name          = "state",
 		.real_name     = "conntrack",
 		.revision      = 2,
+		.ext_flags     = XTABLES_EXT_ALIAS,
 		.version       = XTABLES_VERSION,
 		.size          = XT_ALIGN(sizeof(struct xt_conntrack_mtinfo2)),
 		.userspacesize = XT_ALIGN(sizeof(struct xt_conntrack_mtinfo2)),
@@ -1273,6 +1292,7 @@ static struct xtables_match conntrack_mt_reg[] = {
 		.name          = "state",
 		.real_name     = "conntrack",
 		.revision      = 3,
+		.ext_flags     = XTABLES_EXT_ALIAS,
 		.version       = XTABLES_VERSION,
 		.size          = XT_ALIGN(sizeof(struct xt_conntrack_mtinfo3)),
 		.userspacesize = XT_ALIGN(sizeof(struct xt_conntrack_mtinfo3)),
diff --git a/extensions/libxt_state.man b/extensions/libxt_state.man
index bd60468..ec096ca 100644
--- a/extensions/libxt_state.man
+++ b/extensions/libxt_state.man
@@ -1,4 +1,4 @@
-The "state" module is an obsolete version of "conntrack".
+The "state" extension is a subset of the "conntrack" module.
 "state" allows access to the connection tracking state for this packet.
 .TP
 [\fB!\fP] \fB\-\-state\fP \fIstate\fP
diff --git a/include/linux/netfilter/xt_conntrack.h b/include/linux/netfilter/xt_conntrack.h
index 74b904d..e971501 100644
--- a/include/linux/netfilter/xt_conntrack.h
+++ b/include/linux/netfilter/xt_conntrack.h
@@ -30,6 +30,7 @@ enum {
 	XT_CONNTRACK_REPLSRC_PORT = 1 << 10,
 	XT_CONNTRACK_REPLDST_PORT = 1 << 11,
 	XT_CONNTRACK_DIRECTION    = 1 << 12,
+	XT_CONNTRACK_STATE_ALIAS  = 1 << 13,
 };
 
 struct xt_conntrack_mtinfo1 {
diff --git a/include/xtables.h b/include/xtables.h
index 75de958..892c0b9 100644
--- a/include/xtables.h
+++ b/include/xtables.h
@@ -201,6 +201,10 @@ struct xtables_lmap {
 	struct xtables_lmap *next;
 };
 
+enum xtables_ext_flags {
+	XTABLES_EXT_ALIAS = 1 << 0,
+};
+
 /* Include file for additions: new matches and targets. */
 struct xtables_match
 {
@@ -217,6 +221,9 @@ struct xtables_match
 
 	/* Revision of match (0 by default). */
 	u_int8_t revision;
+	
+	/* Extension flags */
+	u_int8_t ext_flags;
 
 	u_int16_t family;
 
@@ -251,6 +258,9 @@ struct xtables_match
 	/* ip is struct ipt_ip * for example */
 	void (*save)(const void *ip, const struct xt_entry_match *match);
 
+	/* Print match name or alias*/
+	const char *(*alias)(const struct xt_entry_match *match);
+
 	/* Pointer to list of extra command-line options */
 	const struct option *extra_opts;
 
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 556647f..a5bcca3 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1000,7 +1000,8 @@ static int print_match_save(const struct xt_entry_match *e,
 		xtables_find_match(e->u.user.name, XTF_TRY_LOAD, NULL);
 
 	if (match) {
-		printf(" -m %s", e->u.user.name);
+		printf(" -m %s",
+		       match->alias ? match->alias(e) : e->u.user.name);
 
 		/* some matches don't provide a save function */
 		if (match->save)
@@ -1265,8 +1266,9 @@ static void command_match(struct iptables_command_state *cs)
 		strcpy(m->m->u.user.name, m->name);
 	} else {
 		strcpy(m->m->u.user.name, m->real_name);
-		fprintf(stderr, "WARNING: The %s match is obsolete. "
-		        "Use %s instead.\n", m->name, m->real_name);
+		if (!(m->ext_flags & XTABLES_EXT_ALIAS))
+			fprintf(stderr, "WARNING: The %s match is obsolete. "
+			        "Use %s instead.\n", m->name, m->real_name);
 	}
 	m->m->u.user.revision = m->revision;
 
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 00e3f01..6c53c54 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -991,7 +991,8 @@ static int print_match_save(const struct xt_entry_match *e,
 		xtables_find_match(e->u.user.name, XTF_TRY_LOAD, NULL);
 
 	if (match) {
-		printf(" -m %s", e->u.user.name);
+		printf(" -m %s",
+		       match->alias ? match->alias(e) : e->u.user.name);
 
 		/* some matches don't provide a save function */
 		if (match->save)
@@ -1259,8 +1260,9 @@ static void command_match(struct iptables_command_state *cs)
 		strcpy(m->m->u.user.name, m->name);
 	} else {
 		strcpy(m->m->u.user.name, m->real_name);
-		fprintf(stderr, "WARNING: The %s match is obsolete. "
-		        "Use %s instead.\n", m->name, m->real_name);
+		if (!(m->ext_flags & XTABLES_EXT_ALIAS))
+			fprintf(stderr, "WARNING: The %s match is obsolete. "
+			        "Use %s instead.\n", m->name, m->real_name);
 	}
 	m->m->u.user.revision = m->revision;
 
 
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]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Pablo Neira Ayuso]

Hi Jozsef,

On Tue, Jan 15, 2013 at 06:28:11PM +0100, Jozsef Kadlecsik wrote:
> On Tue, 15 Jan 2013, Jozsef Kadlecsik wrote:
> 
> > With passing one more internal flag, indicating that the "state" alias is 
> > used, the "conntrack" module can remain completely hidden and the user can 
> > list/save exactly the same command as the issued one.
> 
> Here follows the patch which introduces match module aliases (the same can 
> be done for targets as well). The alias is handled as it were a real 
> extension while it's actually handled by another module. Listing/saving 
> keeps the alias name and options.
> 
> The "state" extension is the first example of such an alias.

I like this symmetrical aliasing approach.

The aim is to get rid of redundant things in the kernel. And with
this, we can get that without disturbing users.

Would you do the same for the NOTRACK target?

Thanks.

Re: [PATCH]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Jozsef Kadlecsik]

On Fri, 18 Jan 2013, Pablo Neira Ayuso wrote:

> On Tue, Jan 15, 2013 at 06:28:11PM +0100, Jozsef Kadlecsik wrote:
> > On Tue, 15 Jan 2013, Jozsef Kadlecsik wrote:
> > 
> > > With passing one more internal flag, indicating that the "state" alias is 
> > > used, the "conntrack" module can remain completely hidden and the user can 
> > > list/save exactly the same command as the issued one.
> > 
> > Here follows the patch which introduces match module aliases (the same can 
> > be done for targets as well). The alias is handled as it were a real 
> > extension while it's actually handled by another module. Listing/saving 
> > keeps the alias name and options.
> > 
> > The "state" extension is the first example of such an alias.
> 
> I like this symmetrical aliasing approach.
> 
> The aim is to get rid of redundant things in the kernel. And with
> this, we can get that without disturbing users.
> 
> Would you do the same for the NOTRACK target?

Yes, but looking through the modules, I see a lot of inconsistencies: in 
listing match names capitalized, some match/target doesn't print its name, 
missing whitespaces, etc.

I'm going to fix those, but quite a lot of simplification could be 
achieved if the options at listing and saving could be generated by the 
same function in a match/target. Like instead of

	ttl match TTL == xxx

I propose to list the match as

	ttl eq xxx

(That is generate listings by striping off the dashes and the match/target 
name prefix from the options.)

The question is: do we have a "stable API" in listings, at this level?

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]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Jan Engelhardt]

On Tuesday 2013-01-22 22:47, Jozsef Kadlecsik wrote:
>
>I'm going to fix those, but quite a lot of simplification could be 
>achieved if the options at listing and saving could be generated by the 
>same function in a match/target. Like instead of
>
>	ttl match TTL == xxx
>
>I propose to list the match as
>
>	ttl eq xxx
>
>(That is generate listings by striping off the dashes and the match/target 
>name prefix from the options.)
>
>The question is: do we have a "stable API" in listings, at this level?

Only with -S/iptables-save, IMO.

Re: [PATCH]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Jozsef Kadlecsik]

On Tue, 22 Jan 2013, Jan Engelhardt wrote:

> On Tuesday 2013-01-22 22:47, Jozsef Kadlecsik wrote:
> >
> >I'm going to fix those, but quite a lot of simplification could be 
> >achieved if the options at listing and saving could be generated by the 
> >same function in a match/target. Like instead of
> >
> >	ttl match TTL == xxx
> >
> >I propose to list the match as
> >
> >	ttl eq xxx
> >
> >(That is generate listings by striping off the dashes and the match/target 
> >name prefix from the options.)
> >
> >The question is: do we have a "stable API" in listings, at this level?
> 
> Only with -S/iptables-save, IMO.

Yes, I believe so too. Still, better not break something important.

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]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Pablo Neira Ayuso]

Hi Jozsef,

On Tue, Jan 22, 2013 at 10:47:35PM +0100, Jozsef Kadlecsik wrote:
> On Fri, 18 Jan 2013, Pablo Neira Ayuso wrote:
> 
> > On Tue, Jan 15, 2013 at 06:28:11PM +0100, Jozsef Kadlecsik wrote:
> > > On Tue, 15 Jan 2013, Jozsef Kadlecsik wrote:
> > > 
> > > > With passing one more internal flag, indicating that the "state" alias is 
> > > > used, the "conntrack" module can remain completely hidden and the user can 
> > > > list/save exactly the same command as the issued one.
> > > 
> > > Here follows the patch which introduces match module aliases (the same can 
> > > be done for targets as well). The alias is handled as it were a real 
> > > extension while it's actually handled by another module. Listing/saving 
> > > keeps the alias name and options.
> > > 
> > > The "state" extension is the first example of such an alias.
> > 
> > I like this symmetrical aliasing approach.
> > 
> > The aim is to get rid of redundant things in the kernel. And with
> > this, we can get that without disturbing users.
> > 
> > Would you do the same for the NOTRACK target?
> 
> Yes, but looking through the modules, I see a lot of inconsistencies: in 
> listing match names capitalized, some match/target doesn't print its name, 
> missing whitespaces, etc.
> 
> I'm going to fix those, but quite a lot of simplification could be 
> achieved if the options at listing and saving could be generated by the 
> same function in a match/target. Like instead of
> 
> 	ttl match TTL == xxx
> 
> I propose to list the match as
> 
> 	ttl eq xxx
> 
> (That is generate listings by striping off the dashes and the match/target 
> name prefix from the options.)
> 
> The question is: do we have a "stable API" in listings, at this level?

I remember that, while discussing nfacct, some people mentioned that
they were using scripts to parse iptables -L -n to digest counters.

I have also found some perl extension [1] that parses `iptables -L -n'
output.

[1] http://search.cpan.org/dist/IPTables-Parse/

Re: [PATCH]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Jozsef Kadlecsik]

On Wed, 23 Jan 2013, Pablo Neira Ayuso wrote:

> On Tue, Jan 22, 2013 at 10:47:35PM +0100, Jozsef Kadlecsik wrote:
> > On Fri, 18 Jan 2013, Pablo Neira Ayuso wrote:
> > 
> > > On Tue, Jan 15, 2013 at 06:28:11PM +0100, Jozsef Kadlecsik wrote:
> > > > On Tue, 15 Jan 2013, Jozsef Kadlecsik wrote:
> > > > 
> > > > > With passing one more internal flag, indicating that the "state" alias is 
> > > > > used, the "conntrack" module can remain completely hidden and the user can 
> > > > > list/save exactly the same command as the issued one.
> > > > 
> > > > Here follows the patch which introduces match module aliases (the same can 
> > > > be done for targets as well). The alias is handled as it were a real 
> > > > extension while it's actually handled by another module. Listing/saving 
> > > > keeps the alias name and options.
> > > > 
> > > > The "state" extension is the first example of such an alias.
> > > 
> > > I like this symmetrical aliasing approach.
> > > 
> > > The aim is to get rid of redundant things in the kernel. And with
> > > this, we can get that without disturbing users.
> > > 
> > > Would you do the same for the NOTRACK target?
> > 
> > Yes, but looking through the modules, I see a lot of inconsistencies: in 
> > listing match names capitalized, some match/target doesn't print its name, 
> > missing whitespaces, etc.
> > 
> > I'm going to fix those, but quite a lot of simplification could be 
> > achieved if the options at listing and saving could be generated by the 
> > same function in a match/target. Like instead of
> > 
> > 	ttl match TTL == xxx
> > 
> > I propose to list the match as
> > 
> > 	ttl eq xxx
> > 
> > (That is generate listings by striping off the dashes and the match/target 
> > name prefix from the options.)
> > 
> > The question is: do we have a "stable API" in listings, at this level?
> 
> I remember that, while discussing nfacct, some people mentioned that
> they were using scripts to parse iptables -L -n to digest counters.

The counters part is not affected.
 
> I have also found some perl extension [1] that parses `iptables -L -n'
> output.
> 
> [1] http://search.cpan.org/dist/IPTables-Parse/

Sigh. Why 'iptables -L -n' is used and not 'iptables-save -c'?

Looking at the perl module, the proposed changes would break the parsing 
of the options of the SNAT/DNAT targets. It'd be not hard to prepare the 
module to handle current/proposed formats.

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]: Keep the "state" match as alias [Re: state match is obsolete 1.4.17]

[Pablo Neira Ayuso]

On Wed, Jan 23, 2013 at 10:00:19AM +0100, Jozsef Kadlecsik wrote:
> On Wed, 23 Jan 2013, Pablo Neira Ayuso wrote:
> 
> > On Tue, Jan 22, 2013 at 10:47:35PM +0100, Jozsef Kadlecsik wrote:
> > > On Fri, 18 Jan 2013, Pablo Neira Ayuso wrote:
> > > 
> > > > On Tue, Jan 15, 2013 at 06:28:11PM +0100, Jozsef Kadlecsik wrote:
> > > > > On Tue, 15 Jan 2013, Jozsef Kadlecsik wrote:
> > > > > 
> > > > > > With passing one more internal flag, indicating that the "state" alias is 
> > > > > > used, the "conntrack" module can remain completely hidden and the user can 
> > > > > > list/save exactly the same command as the issued one.
> > > > > 
> > > > > Here follows the patch which introduces match module aliases (the same can 
> > > > > be done for targets as well). The alias is handled as it were a real 
> > > > > extension while it's actually handled by another module. Listing/saving 
> > > > > keeps the alias name and options.
> > > > > 
> > > > > The "state" extension is the first example of such an alias.
> > > > 
> > > > I like this symmetrical aliasing approach.
> > > > 
> > > > The aim is to get rid of redundant things in the kernel. And with
> > > > this, we can get that without disturbing users.
> > > > 
> > > > Would you do the same for the NOTRACK target?
> > > 
> > > Yes, but looking through the modules, I see a lot of inconsistencies: in 
> > > listing match names capitalized, some match/target doesn't print its name, 
> > > missing whitespaces, etc.
> > > 
> > > I'm going to fix those, but quite a lot of simplification could be 
> > > achieved if the options at listing and saving could be generated by the 
> > > same function in a match/target. Like instead of
> > > 
> > > 	ttl match TTL == xxx
> > > 
> > > I propose to list the match as
> > > 
> > > 	ttl eq xxx
> > > 
> > > (That is generate listings by striping off the dashes and the match/target 
> > > name prefix from the options.)
> > > 
> > > The question is: do we have a "stable API" in listings, at this level?
> > 
> > I remember that, while discussing nfacct, some people mentioned that
> > they were using scripts to parse iptables -L -n to digest counters.
> 
> The counters part is not affected.
>  
> > I have also found some perl extension [1] that parses `iptables -L -n'
> > output.
> > 
> > [1] http://search.cpan.org/dist/IPTables-Parse/
> 
> Sigh. Why 'iptables -L -n' is used and not 'iptables-save -c'?

Good question :-). I don't have an answer for that, probably the
author.

> Looking at the perl module, the proposed changes would break the parsing 
> of the options of the SNAT/DNAT targets. It'd be not hard to prepare the 
> module to handle current/proposed formats.

Yes. That will fix that module for latest iptables, but would still
break with old iptables versions unless you add some workaround to
that script.

Don't get me wrong, I don't like those inconsistencies either. We have
to decide if we care to likely break existing applications parsing
iptables -L -n.

Re: state match is obsolete 1.4.17

[Jan Engelhardt]

On Wednesday 2013-01-16 00:27, Nick Edwards wrote:
>> On Tuesday 2013-01-15 06:09, Nick Edwards wrote:
>>
>>>WARNING: The state match is obsolete. Use conntrack instead.
>>>Getting these errors since upgrading to 1.4.17
>>
>> It is a warning, not an error. (An error would not let use you
>> the command at all.)
>
>I can understand that if there is a lot of pre warning, as others have
>mentioned, most guides show the former..
>And can you confirm my change is the correct method to obtain the same
>net result please?

Yes.

>Maybe the warning could be changed to
>WARNING: The state match is deprecated and will eventually go away.
>Use conntrack instead.
>That wont panic people into thinking the rule is not working.

Cannot guarantee that it will make it in, but I support your
position with this patch.


git://git.inai.de/iptables obsol
>From 6ef6c8821c26d5a8738cf90e397972c97986ac98 Mon Sep 17 00:00:00 2001
From: Jan Engelhardt <jengelh@inai.de>
Date: Wed, 16 Jan 2013 01:03:40 +0100
Subject: [PATCH] iptables: reword warning on using an alias

This by suggestion of Nick Edward.

References: http://marc.info/?l=netfilter&m=135829245822520&w=2
Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 iptables/ip6tables.c |    9 +++++----
 iptables/iptables.c  |    9 +++++----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 556647f..ba71aca 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1229,8 +1229,8 @@ static void command_jump(struct iptables_command_state *cs)
 		strcpy(cs->target->t->u.user.name, cs->jumpto);
 	} else {
 		strcpy(cs->target->t->u.user.name, cs->target->real_name);
-		fprintf(stderr, "WARNING: The %s target is obsolete. "
-		        "Use %s instead.\n",
+		fprintf(stderr, "WARNING: The %s target is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
 		        cs->jumpto, cs->target->real_name);
 	}
 	cs->target->t->u.user.revision = cs->target->revision;
@@ -1265,8 +1265,9 @@ static void command_match(struct iptables_command_state *cs)
 		strcpy(m->m->u.user.name, m->name);
 	} else {
 		strcpy(m->m->u.user.name, m->real_name);
-		fprintf(stderr, "WARNING: The %s match is obsolete. "
-		        "Use %s instead.\n", m->name, m->real_name);
+		fprintf(stderr, "WARNING: The %s match is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
+		        m->name, m->real_name);
 	}
 	m->m->u.user.revision = m->revision;
 
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 00e3f01..f0dc14b 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1222,8 +1222,8 @@ static void command_jump(struct iptables_command_state *cs)
 	} else {
 		/* Alias support for userspace side */
 		strcpy(cs->target->t->u.user.name, cs->target->real_name);
-		fprintf(stderr, "WARNING: The %s target is obsolete. "
-		        "Use %s instead.\n",
+		fprintf(stderr, "WARNING: The %s target is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
 		        cs->jumpto, cs->target->real_name);
 	}
 	cs->target->t->u.user.revision = cs->target->revision;
@@ -1259,8 +1259,9 @@ static void command_match(struct iptables_command_state *cs)
 		strcpy(m->m->u.user.name, m->name);
 	} else {
 		strcpy(m->m->u.user.name, m->real_name);
-		fprintf(stderr, "WARNING: The %s match is obsolete. "
-		        "Use %s instead.\n", m->name, m->real_name);
+		fprintf(stderr, "WARNING: The %s match is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
+		        m->name, m->real_name);
 	}
 	m->m->u.user.revision = m->revision;
 
-- 
1.7.10.4