[iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[Olaf Rempel]

hi list

>From 1.3.0 all chain names were alphabetically sorted when listing tables.
A user-defined chain 'AAA' is listed before the buildin chain "INPUT", and 
"FORWARD" is listed before "INPUT".

I've created two patches to revert it to the "old" behavior:
first buildin chains, sorted by hooknum, than userdefined chains, sorted by name.

OK, why two patches? 
The first one need some preconditions: 
*all* buildin chains need to be "in order" and in front of *any* user chain 
when parsing the kernel-list.

I was not sure about this, so I made a second patch, that does not need these
preconditions. 

Olaf


diff -uNr iptables-1.3.0.org/libiptc/libiptc.c iptables-1.3.0/libiptc/libiptc.c
--- iptables-1.3.0.org/libiptc/libiptc.c	2005-02-12 21:05:32.000000000 +0100
+++ iptables-1.3.0/libiptc/libiptc.c	2005-03-04 17:12:17.909906736 +0100
@@ -396,10 +396,13 @@
 {
 	struct chain_head *tmp;
 
-	list_for_each_entry(tmp, &h->chains, list) {
-		if (strcmp(c->name, tmp->name) <= 0) {
-			list_add(&c->list, tmp->list.prev);
-			return;
+	/* sort only user defined chains */
+	if (!c->hooknum) {
+		list_for_each_entry(tmp, &h->chains, list) {
+			if (strcmp(c->name, tmp->name) <= 0) {
+				list_add(&c->list, tmp->list.prev);
+				return;
+			}
 		}
 	}


diff -uNr iptables-1.3.0.org/libiptc/libiptc.c iptables-1.3.0/libiptc/libiptc.c
--- iptables-1.3.0.org/libiptc/libiptc.c	2005-02-12 21:05:32.000000000 +0100
+++ iptables-1.3.0/libiptc/libiptc.c	2005-03-04 17:20:12.720724520 +0100
@@ -396,10 +396,23 @@
 {
 	struct chain_head *tmp;
 
-	list_for_each_entry(tmp, &h->chains, list) {
-		if (strcmp(c->name, tmp->name) <= 0) {
-			list_add(&c->list, tmp->list.prev);
-			return;
+	/* sort only user defined chains */
+	if (!c->hooknum) {
+		/* sort user-defined by name and after builtins */
+		list_for_each_entry(tmp, &h->chains, list) {
+			if (!tmp->hooknum && strcmp(c->name, tmp->name) <= 0) {
+				list_add(&c->list, tmp->list.prev);
+				return;
+			}
+		}
+	
+	} else {
+		/* sort builtins by hooknum and before user-defined */
+		list_for_each_entry(tmp, &h->chains, list) {
+			if (!tmp->hooknum || tmp->hooknum > c->hooknum) {
+				list_add(&c->list, tmp->list.prev);
+				return;
+			}
 		}
 	}
 

Re: [iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[Phil Oester]

On Fri, Mar 04, 2005 at 09:21:14PM +0100, Olaf Rempel wrote:
> hi list
> 
> >From 1.3.0 all chain names were alphabetically sorted when listing tables.
> A user-defined chain 'AAA' is listed before the buildin chain "INPUT", and 
> "FORWARD" is listed before "INPUT".
> 
> I've created two patches to revert it to the "old" behavior:
> first buildin chains, sorted by hooknum, than userdefined chains, sorted by name.

Why is the old behaviour better than the current?  Any scripts you have
should be smart enough to check for '^Chain ______' to determine chain.

Phil

Re: [iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[Patrick McHardy]

Phil Oester wrote:
> On Fri, Mar 04, 2005 at 09:21:14PM +0100, Olaf Rempel wrote:
>>
>>I've created two patches to revert it to the "old" behavior:
>>first buildin chains, sorted by hooknum, than userdefined chains, sorted by name.
> 
> Why is the old behaviour better than the current?  Any scripts you have
> should be smart enough to check for '^Chain ______' to determine chain.

Unfortunately not all are, so best chance of no breakage is keeping the
old behaviour. I can't remember where, but someone already reported a
broken script because of this change.

Regards
Patrick

Re: [iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[Olaf Rempel]

On Fri, 4 Mar 2005 14:08:27 -0800
Phil Oester <kernel@linuxace.com> wrote:

> Why is the old behaviour better than the current?  Any scripts you have
> should be smart enough to check for '^Chain ______' to determine chain.

It's not about scripts. I just think it's easier to understand: 
Having the the builtins arranged like the packets traverse them, and 
being on top of the chains they "jump" into. 

I see no advantage of changing to the new behavior, only that it confuses
people that had worked for years with the old behavior.

And last but not least:
libiptc.c:565 	/* FIXME: sort chains */

I read this like: not implemented yet :)

Olaf

Re: [iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[Patrick McHardy]

Olaf Rempel wrote:
> OK, why two patches? 
> The first one need some preconditions: 
> *all* buildin chains need to be "in order" and in front of *any* user chain 
> when parsing the kernel-list.
> 
> I was not sure about this, so I made a second patch, that does not need these
> preconditions. 

I assume with "in order", you mean it relies on the current numbering of
the hooks:

/* After promisc drops, checksum checks. */
#define NF_IP_PRE_ROUTING       0
/* If the packet is destined for this box. */
#define NF_IP_LOCAL_IN          1
/* If the packet is destined for another interface. */
#define NF_IP_FORWARD           2
/* Packets coming from a local process. */
#define NF_IP_LOCAL_OUT         3
/* Packets about to hit the wire. */
#define NF_IP_POST_ROUTING      4

This seems fine to me. I'm going to apply your first patch, thanks.

Regards
Patrick

Re: [iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[Jonas Berlin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Patrick McHardy on 2005-03-04 23:04 UTC:

>> OK, why two patches? The first one need some preconditions: *all*
>> buildin chains need to be "in order" and in front of *any* user chain
>> when parsing the kernel-list.
> 
> I assume with "in order", you mean it relies on the current numbering of
> the hooks:
> 
> This seems fine to me. I'm going to apply your first patch, thanks.

What he meant by "in order" was that when getting a list of chains from
the kernel, they would be returned like this:

PREROUTING
INPUT
FORWARD
OUTPUT
POSTROUTING
USER_CHAIN_B
USER_CHAIN_A
USER_CHAIN_C
...

I.e. builtin chains first, and then user chains in random order.

This is not the case however (at least with my 2.6.11 kernel), they seem
to come in a vaguely alphabetic order, with builtin chains being among
the user chains like:

USER_CHAIN_A
PREROUTING
INPUT
USER_CHAIN_B
FORWARD
OUTPUT
POSTROUTING
USER_CHAIN_C
...

Thus, his second patch is necessary (at least for me) in order to get
them in the order

<builtin chains sorted logically>
<user chains sorted alphabetically>

I tested switching to the second patch and the results were Good. :)

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCZRrhxyF48ZTvn+4RAiNKAJ0U96HA8lxIkWUvn8+vE4zEY7jHgACeJjaD
94uZ+M79//yeo0xQi3397SA=
=GShI
-----END PGP SIGNATURE-----

Re: [iptables 1.3.0 / libiptc patch] sort chains by hooknum/names

[Herve Eychenne]

On Fri, Mar 04, 2005 at 09:21:14PM +0100, Olaf Rempel wrote:

> hi list

> >From 1.3.0 all chain names were alphabetically sorted when listing tables.
> A user-defined chain 'AAA' is listed before the buildin chain "INPUT", and 
> "FORWARD" is listed before "INPUT".

> I've created two patches to revert it to the "old" behavior:
> first buildin chains, sorted by hooknum, than userdefined chains, sorted by name.

Appart from the fact that it restores the old behavior (and avoids
breakage of some badly written scripts or apps), can you give a
single reason why the listing should be sorted?

Why not sort the output of a lot of commands (like lsof, netstat, ps,
etc.) then?  Hint: probably because there is no interest in wasting CPU
time for this.

 Herve

-- 
 _
(°=  Hervé Eychenne
//)
v_/_ WallFire project:  http://www.wallfire.org/