[PATCH}: Make MARK target terminate (resend)

[PATCH}: Make MARK target terminate (resend)

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

Hi.

After not receiving a response for two weeks second try:

The attached patch adds a new option --terminate to the MARK target 
which lets the user choose
if MARK should return IPT_CONTINUE (normal behaviour) or NF_ACCEPT (to 
terminate further rule processing).
This can be useful if you have complex marking rules, f.e. for QoS.
The current patch is against a clean MARK target, if anyone wishes i can 
also make a patch which will apply after MARK-operations.
A CONNMARK patch will follow but currently CONNMARK doesn't apply clean 
against 2.4.18/2.4.19-pre10 ..

Bye,
Patrick


[-- Attachment #2: iptables-terminate.diff --]
[-- Type: text/plain, Size: 3166 bytes --]

diff -urN userspace-clean/patch-o-matic/extra/MARK-terminate.patch userspace/patch-o-matic/extra/MARK-terminate.patch
--- userspace-clean/patch-o-matic/extra/MARK-terminate.patch	Thu Jan  1 01:00:00 1970
+++ userspace/patch-o-matic/extra/MARK-terminate.patch	Mon Jun 17 05:36:21 2002
@@ -0,0 +1,23 @@
+diff -urN linux/include/linux/netfilter_ipv4/ipt_MARK.h linux-2.4.18-terminate/include/linux/netfilter_ipv4/ipt_MARK.h
+--- linux/include/linux/netfilter_ipv4/ipt_MARK.h	Fri Mar 17 19:56:20 2000
++++ linux-2.4.18-terminate/include/linux/netfilter_ipv4/ipt_MARK.h	Mon Jun 17 05:07:29 2002
+@@ -3,6 +3,7 @@
+ 
+ struct ipt_mark_target_info {
+ 	unsigned long mark;
++	u_int8_t terminate;
+ };
+ 
+ #endif /*_IPT_MARK_H_target*/
+diff -urN linux/net/ipv4/netfilter/ipt_MARK.c linux-2.4.18-terminate/net/ipv4/netfilter/ipt_MARK.c
+--- linux/net/ipv4/netfilter/ipt_MARK.c	Sun Sep 30 21:26:08 2001
++++ linux-2.4.18-terminate/net/ipv4/netfilter/ipt_MARK.c	Mon Jun 17 05:07:58 2002
+@@ -21,7 +21,7 @@
+ 		(*pskb)->nfmark = markinfo->mark;
+ 		(*pskb)->nfcache |= NFC_ALTERED;
+ 	}
+-	return IPT_CONTINUE;
++	return markinfo->terminate ? NF_ACCEPT : IPT_CONTINUE;
+ }
+ 
+ static int
diff -urN userspace-clean/patch-o-matic/extra/MARK-terminate.patch.help userspace/patch-o-matic/extra/MARK-terminate.patch.help
--- userspace-clean/patch-o-matic/extra/MARK-terminate.patch.help	Thu Jan  1 01:00:00 1970
+++ userspace/patch-o-matic/extra/MARK-terminate.patch.help	Mon Jun 17 05:36:21 2002
@@ -0,0 +1,9 @@
+Author: Patrick McHardy <kaber@trash.net>
+Status: Working
+
+This patch adds a new option --terminate to MARK to terminate chain traversal.
+
+# iptables -t mangle -A PREROUTING -m tos --tos 0x08 -j MARK --set-mark 0x15 --terminate
+
+***** WARNING ***** This patch also patch the userspace directory which means that you
+                    you have to recompile and reinstall the iptables package after that.
diff -urN userspace-clean/patch-o-matic/extra/MARK-terminate.patch.userspace userspace/patch-o-matic/extra/MARK-terminate.patch.userspace
--- userspace-clean/patch-o-matic/extra/MARK-terminate.patch.userspace	Thu Jan  1 01:00:00 1970
+++ userspace/patch-o-matic/extra/MARK-terminate.patch.userspace	Mon Jun 17 05:36:21 2002
@@ -0,0 +1,42 @@
+diff -urN extensions/libipt_MARK.c extensions/libipt_MARK.c
+--- extensions/libipt_MARK.c	Wed May 29 15:08:16 2002
++++ extensions/libipt_MARK.c	Mon Jun 17 05:21:16 2002
+@@ -26,6 +26,7 @@
+ 
+ static struct option opts[] = {
+ 	{ "set-mark", 1, 0, '1' },
++	{ "terminate", 0, 0, '2' },
+ 	{ 0 }
+ };
+ 
+@@ -57,6 +58,10 @@
+ 		*flags = 1;
+ 		break;
+ 
++	case '2':
++		markinfo->terminate = 1;
++		break;
++		
+ 	default:
+ 		return 0;
+ 	}
+@@ -88,6 +93,9 @@
+ 		(const struct ipt_mark_target_info *)target->data;
+ 	printf("MARK set ");
+ 	print_mark(markinfo->mark, numeric);
++	
++	if (markinfo->terminate)
++		printf("terminate ");
+ }
+ 
+ /* Saves the union ipt_targinfo in parsable form to stdout. */
+@@ -98,6 +106,9 @@
+ 		(const struct ipt_mark_target_info *)target->data;
+ 
+ 	printf("--set-mark 0x%lx ", markinfo->mark);
++
++	if (markinfo->terminate)
++		printf("--terminate ");
+ }
+ 
+ static

Re: [PATCH}: Make MARK target terminate (resend)

[Patrick Schaaf]

> After not receiving a response for two weeks second try:

Sorry. Here we go:

> The attached patch adds a new option --terminate to the MARK target 
> which lets the user choose if MARK should return IPT_CONTINUE
> (normal behaviour) or NF_ACCEPT (to terminate further rule processing).
[...]
> A CONNMARK patch will follow

Will you also add this to LOG, ULOG, and any other IPT_CONTINUE target
that may come up in the future? In my opinion, this is misguided, because
it leads to much code duplication in target modules.

There is already a flexible, but somewhat ugly, way to do what you want:
create a user defined chain that first MARKs then ACCEPTs (or does whatever
else one may want to be done after MARK). Obviously, this is a bit ugly
because you need one such chain per MARK value.

The good way to do it, in my opinion, would be to permit more than one
target per iptables rule. You could then write

	iptables -A somewhere -m something -j MARK --mark 1 -j ACCEPT

Note that I do _not_ oppose adding your --terminate option to MARK, as a
stopgap measure. If that is helpful now, it should be done, IMHO.

However, I'd like to hear people's opinions on the multitarget approach.

best regards
  Patrick

Re: [PATCH}: Make MARK target terminate (resend)

[Patrick McHardy]

Hi.

Patrick Schaaf wrote:

>>After not receiving a response for two weeks second try:
>>
>
>Sorry. Here we go:
>
>>The attached patch adds a new option --terminate to the MARK target 
>>which lets the user choose if MARK should return IPT_CONTINUE
>>(normal behaviour) or NF_ACCEPT (to terminate further rule processing).
>>
>[...]
>
>>A CONNMARK patch will follow
>>
>
>Will you also add this to LOG, ULOG, and any other IPT_CONTINUE target
>that may come up in the future? In my opinion, this is misguided, because
>it leads to much code duplication in target modules.
>
Hmm probably not :) Although i can't see why someone would like (U)LOG 
to return NF_ACCEPT
you're point is clear ..

>
>
>There is already a flexible, but somewhat ugly, way to do what you want:
>create a user defined chain that first MARKs then ACCEPTs (or does whatever
>else one may want to be done after MARK). Obviously, this is a bit ugly
>because you need one such chain per MARK value.
>
Thats not really what i want to do .. especially since we need lots of 
marking rules, basically everything
allowed needs to be marked with different values.

>
>The good way to do it, in my opinion, would be to permit more than one
>target per iptables rule. You could then write
>
>	iptables -A somewhere -m something -j MARK --mark 1 -j ACCEPT
>
>Note that I do _not_ oppose adding your --terminate option to MARK, as a
>stopgap measure. If that is helpful now, it should be done, IMHO.
>
>However, I'd like to hear people's opinions on the multitarget approach.
>
I like that idea very much, although (like Henrik already suggested) i 
favour the idea of something new
between targets and matches for IPT_CONTINUE targets ..

Bye,
Patrick

Re: [PATCH}: Make MARK target terminate (resend)

[Henrik Nordstrom]

On Saturday 29 June 2002 11.46, Patrick McHardy wrote:

> A CONNMARK patch will follow but currently CONNMARK doesn't apply
> clean against 2.4.18/2.4.19-pre10 ..

Note: There is two versions of the CONNMARK patch. The one in extra 
applies if you are using the new_nat patch, the one on old_nat if 
not.

Your last posting did stir up some discussion on how to deal with 
this. Adding a "terminate" option to each and every of these 
psuedo-targets is clearly not the way to go, and only cover a very 
small subset of what is needed.

I proposed adding a new class of iptables things between matches and 
targets, being neither a match for filtering or a target that 
determines the ultimate fate of the packet. The names proposed for 
these in the discussion was modifiers or actions.

The implementation of these can be done without needing to change the 
kernel iptables API by simply piggying back on the match list in the 
table structure. The modifiers/actions need to register themselves as 
a match, and for compability with old rulesets and/or userspace tools 
as a target as well..

The userspace tools need to have a new option for calling a 
modifier/action. These should clearly be separated from matches.

So the question to the Netfilter core team is if it would be OK to add 
a new option and "module class" to the userspace tools, and have the 
existing IPT_CONTINUE targets dual-register as both a target and a 
match. I can try to whip something together if this is seen as 
something acceptable. Should be fully backwards/forward compatible 
with existing rulesets with only a minimal amount of code 
duplication. The only compability issue is that if you make use the 
new feature then you cannot go back to a older userspace or kernel..

Regards
Henrik

Re: [PATCH}: Make MARK target terminate (resend)

[Brad Chapman]

Mr. Nordstrom,

--- Henrik Nordstrom <hno@marasystems.com> wrote:
> On Saturday 29 June 2002 11.46, Patrick McHardy wrote:
> 
> > A CONNMARK patch will follow but currently CONNMARK doesn't apply
> > clean against 2.4.18/2.4.19-pre10 ..
> 
> Note: There is two versions of the CONNMARK patch. The one in extra 
> applies if you are using the new_nat patch, the one on old_nat if 
> not.
> 
> Your last posting did stir up some discussion on how to deal with 
> this. Adding a "terminate" option to each and every of these 
> psuedo-targets is clearly not the way to go, and only cover a very 
> small subset of what is needed.
> 
> I proposed adding a new class of iptables things between matches and 
> targets, being neither a match for filtering or a target that 
> determines the ultimate fate of the packet. The names proposed for 
> these in the discussion was modifiers or actions.

        I think modifier is a better term. "Action" sounds more like "target"
and may confuse new users of iptables.

> 
> The implementation of these can be done without needing to change the 
> kernel iptables API by simply piggying back on the match list in the 
> table structure. The modifiers/actions need to register themselves as 
> a match, and for compability with old rulesets and/or userspace tools 
> as a target as well..
> 
> The userspace tools need to have a new option for calling a 
> modifier/action. These should clearly be separated from matches.

        Has the option letter been decided on yet?

> 
> So the question to the Netfilter core team is if it would be OK to add 
> a new option and "module class" to the userspace tools, and have the 
> existing IPT_CONTINUE targets dual-register as both a target and a 
> match. I can try to whip something together if this is seen as 
> something acceptable. Should be fully backwards/forward compatible 
> with existing rulesets with only a minimal amount of code 
> duplication. The only compability issue is that if you make use the 
> new feature then you cannot go back to a older userspace or kernel.

        I would like to see a feature like this because it makes the iptables 
syntax cleaner and easier to understand. The only difficulty I see is backward
compatibility with older kernels and older iptables installations. You mentioned
using a piggyback mechanism for the ipt_match/ipt_target structs to prevent
API changes. Will a new API eventually emerge (ipt_modifier)? And if a new API
does emerge, what kernel version would it be aimed for (2.4.xx? 2.5.xx?)

        Also, BTW: will this facility appear in iptables2 as well?

> 
> Regards
> Henrik
>

Brad 


=====
Brad Chapman

Permanent e-mail: kakadu_croc@yahoo.com

__________________________________________________
Do You Yahoo!?
Yahoo! - Official partner of 2002 FIFA World Cup
http://fifaworldcup.yahoo.com

Re: [PATCH}: Make MARK target terminate (resend)

[Balazs Scheidler]

On Sat, Jun 29, 2002 at 12:36:36PM +0200, Henrik Nordstrom wrote:
> On Saturday 29 June 2002 11.46, Patrick McHardy wrote:
> So the question to the Netfilter core team is if it would be OK to add 
> a new option and "module class" to the userspace tools, and have the 
> existing IPT_CONTINUE targets dual-register as both a target and a 
> match. I can try to whip something together if this is seen as 
> something acceptable. Should be fully backwards/forward compatible 
> with existing rulesets with only a minimal amount of code 
> duplication. The only compability issue is that if you make use the 
> new feature then you cannot go back to a older userspace or kernel..

I for one would second a feature like this. I see a good number of places
where it could be used (the long standing missing -l option is one example)

-- 
Bazsi
PGP info: KeyID 9AF8D0A9 Fingerprint CD27 CFB0 802C 0944 9CFD 804E C82C 8EB1

Re: [PATCH}: Make MARK target terminate (resend)

[Harald Welte]

On Mon, Jul 01, 2002 at 09:50:18AM +0200, Balazs Scheidler wrote:
> On Sat, Jun 29, 2002 at 12:36:36PM +0200, Henrik Nordstrom wrote:
> > On Saturday 29 June 2002 11.46, Patrick McHardy wrote:
> > So the question to the Netfilter core team is if it would be OK to add 
> > a new option and "module class" to the userspace tools, and have the 
> > existing IPT_CONTINUE targets dual-register as both a target and a 
> > match. I can try to whip something together if this is seen as 
> > something acceptable. Should be fully backwards/forward compatible 
> > with existing rulesets with only a minimal amount of code 
> > duplication. The only compability issue is that if you make use the 
> > new feature then you cannot go back to a older userspace or kernel..
> 
> I for one would second a feature like this. I see a good number of places
> where it could be used (the long standing missing -l option is one example)

It is definitely useful.  I'd love to have this additional flexibility.
A couple of issues, though:

1) definitely 2.5.x thing - thus it should wait until pkt_tables becomes
   more apparent
2) I think this heavily confuses users, since a target can now be used
   as a 'match'.  The current concept of netfilter/iptables is very
   clear and well-defined.  Adding kludges like this sort-of breaks this
   concept...  

> Bazsi

-- 
Live long and prosper
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
============================================================================
GCS/E/IT d- s-: a-- C+++ UL++++$ P+++ L++++$ E--- W- N++ o? K- w--- O- M- 
V-- PS+ PE-- Y+ PGP++ t++ 5-- !X !R tv-- b+++ DI? !D G+ e* h+ r% y+(*)

Re: [PATCH}: Make MARK target terminate (resend)

[Jozsef Kadlecsik]

On Tue, 2 Jul 2002, Harald Welte wrote:

> On Mon, Jul 01, 2002 at 09:50:18AM +0200, Balazs Scheidler wrote:
> > On Sat, Jun 29, 2002 at 12:36:36PM +0200, Henrik Nordstrom wrote:
> > > On Saturday 29 June 2002 11.46, Patrick McHardy wrote:
> > > So the question to the Netfilter core team is if it would be OK to add
> > > a new option and "module class" to the userspace tools, and have the

> It is definitely useful.  I'd love to have this additional flexibility.
> A couple of issues, though:
>
> 1) definitely 2.5.x thing - thus it should wait until pkt_tables becomes
>    more apparent
> 2) I think this heavily confuses users, since a target can now be used
>    as a 'match'.  The current concept of netfilter/iptables is very
>    clear and well-defined.  Adding kludges like this sort-of breaks this
>    concept...

Also, no one answered the question yet on how to name an action module
without breaking the current clear distiction in name between match and
target modules.

If the pseudo targets were converted to match modules, most of them
could be unified with an already existing match module, which would
result a more compact codebase (tcpmss/TCPMSS, ttl/TTL, tos/TOS, etc,
etc.). I just repeat myself: the recent match is a nice example for
this approach.

> > - multiple targets
> >
> >   It has been rejected several times with good reasons: too error-prone
> >   for the users and it would require heavy modifications both in the
> >   kernel and the userspace.
>
> the 'too heavy modification' issue is not really a problem anymore,
> since the undergoing 'pkt_tables' rewrite [shared infrastructure for
> iptables, ip6tables, arptables] and the linked-list rewrite.

Then one should consider this approach as well. If we had the possibility
to specify multiple targets then the current clean concepts could be kept
as well.

new action class:
	pro: no ordering problem
	pro: clean separation of functionalities
	con: naming issue

match module:
	pro: no naming issue, current well known concepts can be kept
	pro: couple of modules can be unified
	con: ordering issue

multiple targets:
	pro: no naming issue, current well known concepts can be kept
	pro: no change required in the existing modules
	con: ordering issue (-j ACCEPT -j TTL)

Regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
WWW-Home: http://www.kfki.hu/~kadlec
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH}: Make MARK target terminate (resend)

[Patrick Schaaf]

For the record:

> match module:
> 	pro: no naming issue, current well known concepts can be kept
> 	pro: couple of modules can be unified
> 	con: ordering issue

I strongly prefer this solution, with the added requirement that order
issues should be defined clearly, and have a clear relationship to the
order given on the command line. If this is not the case now, this is
the time to fix it, IMHO.

best regards
  Patrick

Re: [PATCH}: Make MARK target terminate (resend)

[Fabrice MARIE]

Hello,

On Wednesday 03 July 2002 19:24, Jozsef Kadlecsik wrote:
>[...]
> new action class:
> 	pro: no ordering problem
> 	pro: clean separation of functionalities
> 	con: naming issue
> match module:
> 	pro: no naming issue, current well known concepts can be kept
> 	pro: couple of modules can be unified
> 	con: ordering issue
> multiple targets:
> 	pro: no naming issue, current well known concepts can be kept
> 	pro: no change required in the existing modules
> 	con: ordering issue (-j ACCEPT -j TTL)

I proposed the last one some time ago. A solution to the ordering
issue is to have two kind of targets:
1- terminal target (ie ACCEPT, DROP, REJECT, jump to chain, etc...)
2- non terminal target (ie TTL, MARK, IPV4OPTSSTRIP, etc...)
The userland would basically use the order the admin provide,
and would reply with an error if a non-terminal target is used
after a terminal target.

But I was answered that it was impossible/difficult with the current
framework (which I agree).

I was also answered that it would break what people are used to,
(having only one target) which I don't really agree with.
I believe it's very natural to want to perform several things if a condition
match, just like you would do in C a
if (condition1) {
   action1();
   action2();
   action3();
}
instead of a
if (condition1)
   action1();
if (condition1)
   action2();
if (condition1)
   action3();

I believe it's important to be able to have several targets,
or whatever you call them, since you will avoid testing the condition
several times (which you cannot do with some matches without changing
their returned value : nth, random, recent, etc...etc...).

This might be something we can think about when designing the
framework for 2.5, since it's got to be rewritten to support
netlink.

Have a nice day,

Fabrice.
--
Fabrice MARIE
Senior R&D Engineer
Celestix Networks
http://www.celestix.com/

"Silly hacker, root is for administrators"
       -Unknown

Re: [PATCH}: Make MARK target terminate (resend)

[Hervé Eychenne]

On Wed, Jul 03, 2002 at 08:41:48PM +0800, Fabrice MARIE wrote:

> On Wednesday 03 July 2002 19:24, Jozsef Kadlecsik wrote:

 Hi,

> I believe it's very natural to want to perform several things if a condition
> match, just like you would do in C a
> if (condition1) {
>    action1();
>    action2();
>    action3();
> }
> instead of a
> if (condition1)
>    action1();
> if (condition1)
>    action2();
> if (condition1)
>    action3();

That's what user-defined chains are for.  I agree it is not very
elegant to create a chain for every action group, but there are
generally no plenty of these.

 RV

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

Re: [PATCH}: Make MARK target terminate (resend)

[Henrik Nordstrom]

On Wednesday 03 July 2002 14.41, Fabrice MARIE wrote:

> I proposed the last one some time ago. A solution to the ordering
> issue is to have two kind of targets:
> 1- terminal target (ie ACCEPT, DROP, REJECT, jump to chain, etc...)
> 2- non terminal target (ie TTL, MARK, IPV4OPTSSTRIP, etc...)
> The userland would basically use the order the admin provide,
> and would reply with an error if a non-terminal target is used
> after a terminal target.
>
> But I was answered that it was impossible/difficult with the
> current framework (which I agree).

Not really. To address this in the current framework one only needs to 
piggyback on the match list.

The kernel components needs to be modified to register themselves as 
matches as well as targets.

The userspace components needs to be extended with a additional flag 
telling the userspace tool that the "target" is a non-terminal target 
and can be installed as a match in the kernel space table if multiple 
targets are used.

The same datatypes should be possible to use for both the match info 
and the target info.

> I was also answered that it would break what people are used to,
> (having only one target) which I don't really agree with.

Neither do I.

Regards
Henrik

Re: [PATCH}: Make MARK target terminate (resend)

[Harald Welte]

On Wed, Jul 03, 2002 at 01:24:46PM +0200, Jozsef Kadlecsik wrote:
> Also, no one answered the question yet on how to name an action module
> without breaking the current clear distiction in name between match and
> target modules.

i would just make them use uppercase, like targets.

> If the pseudo targets were converted to match modules, most of them
> could be unified with an already existing match module, which would
> result a more compact codebase (tcpmss/TCPMSS, ttl/TTL, tos/TOS, etc,
> etc.). I just repeat myself: the recent match is a nice example for
> this approach.

no, i really don't like this approach. unifying the codebase can be done
differently [like implementing match+target in one .c file]

> > > - multiple targets
> > >
> > >   It has been rejected several times with good reasons: too error-prone
> > >   for the users and it would require heavy modifications both in the
> > >   kernel and the userspace.
> >
> > the 'too heavy modification' issue is not really a problem anymore,
> > since the undergoing 'pkt_tables' rewrite [shared infrastructure for
> > iptables, ip6tables, arptables] and the linked-list rewrite.
> 
> Then one should consider this approach as well. If we had the possibility
> to specify multiple targets then the current clean concepts could be kept
> as well.

yes. But then, how do we distinguish between terminating targets [where
we can have only one per rule] and non-terminating targets AKA actions,
where we can have multiple.

> new action class:
> 	pro: no ordering problem
> 	pro: clean separation of functionalities
> 	con: naming issue
> 
> match module:
> 	pro: no naming issue, current well known concepts can be kept
> 	pro: couple of modules can be unified
> 	con: ordering issue

it just breaks the concept.  a match is by definition something which
matches against a certain criteria. it is not supposed to do anything.

> Jozsef

-- 
Live long and prosper
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
============================================================================
GCS/E/IT d- s-: a-- C+++ UL++++$ P+++ L++++$ E--- W- N++ o? K- w--- O- M- 
V-- PS+ PE-- Y+ PGP++ t++ 5-- !X !R tv-- b+++ DI? !D G+ e* h+ r% y+(*)

Re: [PATCH}: Make MARK target terminate (resend)

[Fabrice MARIE]

Hello Harald,

On Friday 05 July 2002 07:58, Harald Welte wrote:
> [...]
> yes. But then, how do we distinguish between terminating targets [where
> we can have only one per rule] and non-terminating targets AKA actions,
> where we can have multiple.

You could just add a boolean field 'terminating' to the iptables_target.
Then, make sure iptables abort and complains if it sees more than one terminating
target being requested in a single rule.

Iptables could even give a warning (and reorder on the fly if necessary) if a
terminating target is found before a non-terminating target.

For example, the iptables_target struct of the TTL target would become :

static
struct iptables_target TTL = { NULL,
        "TTL",
        IPTABLES_VERSION,
        IPT_ALIGN(sizeof(struct ipt_TTL_info)),
        IPT_ALIGN(sizeof(struct ipt_TTL_info)),
        &help,
        &init,
        &parse,
        &final_check,
        &print,
        &save,
-       opts
+       opts,
+       0 /* target is NOT terminating*/
};

and the iptables_target struct of the REJECT target would become :

static
struct iptables_target reject
= { NULL,
    "REJECT",
    IPTABLES_VERSION,
    IPT_ALIGN(sizeof(struct ipt_reject_info)),
    IPT_ALIGN(sizeof(struct ipt_reject_info)),
    &help,
    &init,
    &parse,
    &final_check,
    &print,
    &save,
-   opts
+   opts,
+   1 /* target is terminating */
};

But now, if you don't want to use the match piggybacking trick that Jozsef & Henrik
mentionned, then we can't implement that right now.
Do you think multiple targets is worth including in the design of the next
netfilter framework ?

I bielieve we should do that, multiple actions for one condition is very natural,
and I believe the usage of a custom chain for each of theses rules is a bit overkill..

Any thoughts ?

Fabrice.
--
Fabrice MARIE
Senior R&D Engineer
Celestix Networks
http://www.celestix.com/

"Silly hacker, root is for administrators"
       -Unknown

Re: [PATCH}: Make MARK target terminate (resend)

[Harald Welte]

On Fri, Jul 05, 2002 at 12:01:21PM +0800, Fabrice MARIE wrote:
> 
> Hello Harald,
> 
> On Friday 05 July 2002 07:58, Harald Welte wrote:
> > [...]
> > yes. But then, how do we distinguish between terminating targets [where
> > we can have only one per rule] and non-terminating targets AKA actions,
> > where we can have multiple.
> 
> You could just add a boolean field 'terminating' to the iptables_target.
> Then, make sure iptables abort and complains if it sees more than one
> terminating target being requested in a single rule.

no, it's not about how to distinguish it internally.  It was more like:
How does the user know which targets terminate and which don't [just by
looking at the name or it's usage]

> But now, if you don't want to use the match piggybacking trick that
> Jozsef & Henrik mentionned, then we can't implement that right now.

no.  There is no reason in implementing it right now anyway.  A change
like this would not appeear in 2.4.x anyway...

> Do you think multiple targets is worth including in the design of the next
> netfilter framework ?

it's not a big issue anyway. Instead of a fixed single target entry,
there is a linked list.  I'm already working on that code..

> I bielieve we should do that, multiple actions for one condition is
> very natural, and I believe the usage of a custom chain for each of
> theses rules is a bit overkill..

yes, it helps in some cases, but not in all.

> Any thoughts ?
> Fabrice.

-- 
Live long and prosper
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
============================================================================
GCS/E/IT d- s-: a-- C+++ UL++++$ P+++ L++++$ E--- W- N++ o? K- w--- O- M- 
V-- PS+ PE-- Y+ PGP++ t++ 5-- !X !R tv-- b+++ DI? !D G+ e* h+ r% y+(*)

Re: [PATCH}: Make MARK target terminate (resend)

[Patrick Schaaf]

Hi Harald,

On Fri, Jul 05, 2002 at 04:21:27PM +0200, Harald Welte wrote:
> 
> > You could just add a boolean field 'terminating' to the iptables_target.
> > Then, make sure iptables abort and complains if it sees more than one
> > terminating target being requested in a single rule.
> 
> no, it's not about how to distinguish it internally.  It was more like:
> How does the user know which targets terminate and which don't [just by
> looking at the name or it's usage]

Random notice: the same question waits for the user who wants to understand
the action of some "previous user defined chain" he just sees. Does that
user defined chain terminate in any case?

My point? iptables rulesets tend to become sufficiently complex in a
short time so that "vague inspection" won't make a given ruleset
easily understandable. IMHO that's a tribute to the flexibility
we have with iptables. Engage brain before making modifications.

have a nice weekend
  Patrick

Re: [PATCH}: Make MARK target terminate (resend)

[Joakim Axelsson]

2002-07-03 13:24:46+0200, Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> ->
>
> Also, no one answered the question yet on how to name an action module
> without breaking the current clear distiction in name between match and
> target modules.
> 

Well I did when I suggested the name/class "action". An action is named with
a capital first and the rest lowercase.

Example.

-m matchexample -a Actionexample -j TARGETEXAMPLE



-- 
/Joakim Axelsson A.K.A Gozem@EFnet & OPN

Re: [PATCH}: Make MARK target terminate (resend)

[Jozsef Kadlecsik]

On Sat, 29 Jun 2002, Henrik Nordstrom wrote:

[...]
> I proposed adding a new class of iptables things between matches and
> targets, being neither a match for filtering or a target that
> determines the ultimate fate of the packet. The names proposed for
> these in the discussion was modifiers or actions.

I believe we have four possibilities

- multiple targets

  It has been rejected several times with good reasons: too error-prone
  for the users and it would require heavy modifications both in the
  kernel and the userspace.

- a new class: actions

  With the new class, we would have to following skeleton of a rule:

	IP match data
	list of matches
	list of actions
	single target

  Using any action would make sense only when the target is ACCEPT and
  alike, so the actions function as 'always true' matches.

  One also has to note, that we have a nice, visible separation of matches
  and targets by name: matches are lowercased, while targets are
  uppercased. How could actions be fit into this scheme? How could one
  decide by glimpse that we are speaking about a match, action or
  target?

  [I'd name the new class as 'action' instead of 'modifier', because '-m'
  is reserverd but '-a' is not.]

- rewrite the IPT_CONTINUE targets as matches

  What would be solved by actions so nicely is biting here:
  order-dependecy. Users should keep in mind, that the rule of
  thumb is that 'always true' matches must come at the end of the rules.
  But one has to note that the 'match' instead of 'action' approach would
  lead a little bit more flexibility as well: actions have only effect
  when all matches are true in the rule. (What is drawback from one point
  of view, advantage from another one.)

  The cases, when we have match/target pairs (like 'mark/MARK, ecn/ECN,
  dscp/DSCP, etc) could be unified into single matches. (The 'recent'
  match is a really nice example for such a solution. It could have been
  introduced as a separated 'recent' match and a 'RECENT' target as well.)

- do nothing: the problem can always be solved by introducing custom
  chains :-)

> So the question to the Netfilter core team is if it would be OK to add
> a new option and "module class" to the userspace tools, and have the
> existing IPT_CONTINUE targets dual-register as both a target and a
> match. I can try to whip something together if this is seen as

In my opinion the match solution would be better, cleaner.

Regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
WWW-Home: http://www.kfki.hu/~kadlec
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH}: Make MARK target terminate (resend)

[Henrik Nordstrom]

Jozsef Kadlecsik wrote:

> - rewrite the IPT_CONTINUE targets as matches

I am not very fond of this.. besides the order dependency it also has the 
question on how to easily determine what will happen with the packet.. No 
obvious distinction between something that matches packets and something that 
modifies packets or internal system state (conntrack, nfmark, ippools etc..).

> - do nothing: the problem can always be solved by introducing custom
>   chains :-)

Well.. Not really. Consider for example the LOG "target" where one wants to 
use the --log-prefix option to log different cases. Would require a custom 
chain per case which is quite cumbersome. But sure, it is in theory doable 
just as having all rules duplicated in a single chain is.

> > So the question to the Netfilter core team is if it would be OK to add
> > a new option and "module class" to the userspace tools, and have the
> > existing IPT_CONTINUE targets dual-register as both a target and a
> > match. I can try to whip something together if this is seen as
>
> In my opinion the match solution would be better, cleaner.

So your current opinion is that the IPT_CONTINUE targets should be rewritten 
as matches?

Regards
Henrik

Re: [PATCH}: Make MARK target terminate (resend)

[Jozsef Kadlecsik]

On Mon, 1 Jul 2002, Henrik Nordstrom wrote:

> > - rewrite the IPT_CONTINUE targets as matches
>
> I am not very fond of this.. besides the order dependency it also has the
> question on how to easily determine what will happen with the packet.. No
> obvious distinction between something that matches packets and something that
> modifies packets or internal system state (conntrack, nfmark, ippools etc..).

Nothing much can be do about order dependecy except the clear
documentation of the feature. When the prestate/raw table will be ready
(oh well, time...) then it'll be a good aid in spotting misconfigured
rules.

I see that the apparent distinction between true matches and matches with
side effect would be lost compared to the case of separated matches,
actions. Hoewer, would it help to the end user if there were a separated
interface to the matches with side-effect (actions)?

What would be the rule for an action name? I fear it would create a
confusion about which module is a match, which one is an action if an
action name would be lowercased.

Something new must be introduced. The question is which one more coherent
*and* more user-friendly.

> > In my opinion the match solution would be better, cleaner.
>
> So your current opinion is that the IPT_CONTINUE targets should be rewritten
> as matches?

This is my current personal opinion. The naming issue of actions really
disturbs me.

Regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
WWW-Home: http://www.kfki.hu/~kadlec
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH}: Make MARK target terminate (resend)

[Harald Welte]

On Mon, Jul 01, 2002 at 11:47:09AM +0200, Jozsef Kadlecsik wrote:
> On Sat, 29 Jun 2002, Henrik Nordstrom wrote:
> 
> [...]
> > I proposed adding a new class of iptables things between matches and
> > targets, being neither a match for filtering or a target that
> > determines the ultimate fate of the packet. The names proposed for
> > these in the discussion was modifiers or actions.
> 
> I believe we have four possibilities
> 
> - multiple targets
> 
>   It has been rejected several times with good reasons: too error-prone
>   for the users and it would require heavy modifications both in the
>   kernel and the userspace.

the 'too heavy modification' issue is not really a problem anymore,
since the undergoing 'pkt_tables' rewrite [shared infrastructure for
iptables, ip6tables, arptables] and the linked-list rewrite.

> - a new class: actions
> 
>   With the new class, we would have to following skeleton of a rule:
> 
> 	IP match data
> 	list of matches
> 	list of actions
> 	single target
> 
>   Using any action would make sense only when the target is ACCEPT and
>   alike, so the actions function as 'always true' matches.
> 
>   One also has to note, that we have a nice, visible separation of matches
>   and targets by name: matches are lowercased, while targets are
>   uppercased. How could actions be fit into this scheme? How could one
>   decide by glimpse that we are speaking about a match, action or
>   target?
> 
>   [I'd name the new class as 'action' instead of 'modifier', because '-m'
>   is reserverd but '-a' is not.]

this sounds the most reasonable idea to me. 

> In my opinion the match solution would be better, cleaner.

I think introducing actions would be the way to go. but I'm not really
convinced of any of the 'solutions'.

> Regards,
> Jozsef

-- 
Live long and prosper
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
============================================================================
GCS/E/IT d- s-: a-- C+++ UL++++$ P+++ L++++$ E--- W- N++ o? K- w--- O- M- 
V-- PS+ PE-- Y+ PGP++ t++ 5-- !X !R tv-- b+++ DI? !D G+ e* h+ r% y+(*)

Re: [PATCH}: Make MARK target terminate (resend)

[Patrick McHardy]

Henrik Nordstrom wrote:

>On Saturday 29 June 2002 11.46, Patrick McHardy wrote:
>
>>A CONNMARK patch will follow but currently CONNMARK doesn't apply
>>clean against 2.4.18/2.4.19-pre10 ..
>>
>
>Note: There is two versions of the CONNMARK patch. The one in extra 
>applies if you are using the new_nat patch, the one on old_nat if 
>not.
>
thanks, so it was my mistake :)
i also need a terminating CONNMARK, but i guess i'm going to wait with
it until it's clear where this is going ..

>
>Your last posting did stir up some discussion on how to deal with 
>this. Adding a "terminate" option to each and every of these 
>psuedo-targets is clearly not the way to go, and only cover a very 
>small subset of what is needed.
>

also at least i do not plan do something like that :) you're right of
course, the terminate option to MARK was just something i needed now ..

>
>
>I proposed adding a new class of iptables things between matches and 
>targets, being neither a match for filtering or a target that 
>determines the ultimate fate of the packet. The names proposed for 
>these in the discussion was modifiers or actions.
>

I like the idea very much. The name 'actions' would be better in my
opinion, LOG f.e. doesn't sound like a modifier.

>
>
>The implementation of these can be done without needing to change the 
>kernel iptables API by simply piggying back on the match list in the 
>table structure. The modifiers/actions need to register themselves as 
>a match, and for compability with old rulesets and/or userspace tools 
>as a target as well.
>
>The userspace tools need to have a new option for calling a 
>modifier/action. These should clearly be separated from matches.
>
>So the question to the Netfilter core team is if it would be OK to add 
>a new option and "module class" to the userspace tools, and have the 
>existing IPT_CONTINUE targets dual-register as both a target and a 
>match. I can try to whip something together if this is seen as 
>something acceptable. Should be fully backwards/forward compatible 
>with existing rulesets with only a minimal amount of code 
>duplication. The only compability issue is that if you make use the 
>new feature then you cannot go back to a older userspace or kernel..
>
>
So all IPT_CONTINUE targets would need to be changed to register as a
module too .. if you need some help doing the silly-work just drop me
a mail ..

Bye
Patrick