patch: policy update by id

patch: policy update by id

[Jamal Hadi Salim]

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


Herbert,

I think this is a reasonable thing to have. I should be able to update
(just like delete or get) a policy by id. And that it trumps a selector
when both are specified..
I am assuming ids are unique - otherwise we have a serious problem.
If this patch looks good, please add it to your patch collection that
you are queueing for Dave; btw - is that stuff supposed to show up
around .13?

cheers,
jamal

[-- Attachment #2: polid_p --]
[-- Type: text/plain, Size: 539 bytes --]

--- a/net/xfrm/xfrm_policy.c	2005/04/27 11:32:13	1.1
+++ b/net/xfrm/xfrm_policy.c	2005/04/27 11:48:42
@@ -345,7 +345,9 @@
 
 	write_lock_bh(&xfrm_policy_lock);
 	for (p = &xfrm_policy_list[dir]; (pol=*p)!=NULL;) {
-		if (!delpol && memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0) {
+		if (!delpol && 
+		((!excl && policy->id && (policy->id == pol->id)) || 
+		 (memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0))) {
 			if (excl) {
 				write_unlock_bh(&xfrm_policy_lock);
 				return -EEXIST;

Re: patch: policy update by id

[Patrick McHardy]

Jamal Hadi Salim wrote:
> I think this is a reasonable thing to have. I should be able to update
> (just like delete or get) a policy by id. And that it trumps a selector
> when both are specified..

struct xfrm_policy doesn't have an id member in my tree. Did I miss
something or do you mean to use index?

Regards
Patrick

Re: patch: policy update by id

[jamal]

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

On Wed, 2005-27-04 at 07:54 -0400, Jamal Hadi Salim wrote:
> Herbert,
> 
> I think this is a reasonable thing to have. I should be able to update
> (just like delete or get) a policy by id. And that it trumps a selector
> when both are specified..
> I am assuming ids are unique - otherwise we have a serious problem.
> If this patch looks good, please add it to your patch collection that
> you are queueing for Dave; btw - is that stuff supposed to show up
> around .13?
> 

Sorry, should have tried to compile first ;->

here it is. I will test in 5 minutes or so.

cheers,
jamal

[-- Attachment #2: polid_p --]
[-- Type: text/plain, Size: 579 bytes --]

--- /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 11:32:13	1.1
+++ /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 12:23:03
@@ -345,7 +345,9 @@
 
 	write_lock_bh(&xfrm_policy_lock);
 	for (p = &xfrm_policy_list[dir]; (pol=*p)!=NULL;) {
-		if (!delpol && memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0) {
+		if (!delpol && 
+		((!excl && policy->id && (policy->index == pol->index)) || 
+		 (memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0))) {
 			if (excl) {
 				write_unlock_bh(&xfrm_policy_lock);
 				return -EEXIST;

Re: patch: policy update by id

[Jamal Hadi Salim]

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


dammit. here it is now compiling ;->

cheers,
jamal

[-- Attachment #2: polid_p --]
[-- Type: text/plain, Size: 582 bytes --]

--- /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 11:32:13	1.1
+++ /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 12:25:24
@@ -345,7 +345,9 @@
 
 	write_lock_bh(&xfrm_policy_lock);
 	for (p = &xfrm_policy_list[dir]; (pol=*p)!=NULL;) {
-		if (!delpol && memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0) {
+		if (!delpol && 
+		((!excl && policy->index && (policy->index == pol->index)) || 
+		 (memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0))) {
 			if (excl) {
 				write_unlock_bh(&xfrm_policy_lock);
 				return -EEXIST;

Re: patch: policy update by id

[Jamal Hadi Salim]

On Wed, 2005-27-04 at 14:18 +0200, Patrick McHardy wrote:

> struct xfrm_policy doesn't have an id member in my tree. Did I miss
> something or do you mean to use index?
> 

I meant index, sorry. I am going to give it a quick test in about 3
minutes.

cheers,
jamal

Re: patch: policy update by id

[jamal]

On Wed, 2005-27-04 at 08:28 -0400, Jamal Hadi Salim wrote:
> On Wed, 2005-27-04 at 14:18 +0200, Patrick McHardy wrote:
> 
> > struct xfrm_policy doesn't have an id member in my tree. Did I miss
> > something or do you mean to use index?
> > 
> 
> I meant index, sorry. I am going to give it a quick test in about 3
> minutes.
> 

Sorry, cant test, have to rush to work - and it looks like the
priorities may interfere. I will test when i get back home. It also
appears there may be a bug in ip x.


cheers,
jamal

Re: patch: policy update by id

[Herbert Xu]

On Wed, Apr 27, 2005 at 08:27:06AM -0400, Jamal Hadi Salim wrote:
>
> --- /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 11:32:13	1.1
> +++ /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 12:25:24
> @@ -345,7 +345,9 @@
>  
>  	write_lock_bh(&xfrm_policy_lock);
>  	for (p = &xfrm_policy_list[dir]; (pol=*p)!=NULL;) {
> -		if (!delpol && memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0) {
> +		if (!delpol && 
> +		((!excl && policy->index && (policy->index == pol->index)) || 
> +		 (memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0))) {

I have no problems with the idea itself.  However, I have a couple of
minor issues with this patch :)

First of all please align the continued lines to the if expression, e.g.,
		if (!delpol && 
		    (policy->index && (policy->index == pol->index)) || 

Also the excl check doesn't make sense.  You should let the
following excl check take place after you've found out that
the indices is identical.

>  			if (excl) {
>  				write_unlock_bh(&xfrm_policy_lock);
>  				return -EEXIST;

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[jamal]

On Thu, 2005-28-04 at 09:39 +1000, Herbert Xu wrote:
> On Wed, Apr 27, 2005 at 08:27:06AM -0400, Jamal Hadi Salim wrote:
> >
> > --- /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 11:32:13	1.1
> > +++ /usr/src/26117-mod/net/xfrm/xfrm_policy.c	2005/04/27 12:25:24
> > @@ -345,7 +345,9 @@
> >  
> >  	write_lock_bh(&xfrm_policy_lock);
> >  	for (p = &xfrm_policy_list[dir]; (pol=*p)!=NULL;) {
> > -		if (!delpol && memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0) {
> > +		if (!delpol && 
> > +		((!excl && policy->index && (policy->index == pol->index)) || 
> > +		 (memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0))) {
> 
> I have no problems with the idea itself.  However, I have a couple of
> minor issues with this patch :)
> 
> First of all please align the continued lines to the if expression, e.g.,
> 		if (!delpol && 
> 		    (policy->index && (policy->index == pol->index)) || 
> 

np. sorry, i was rushing out - testing as we speak.

> Also the excl check doesn't make sense.  You should let the
> following excl check take place after you've found out that
> the indices is identical.
> 

the policy->index is only relevant for the update not the add;
the update could also be done by selector. So i didnt follow.

cheers,
jamal 

Re: patch: policy update by id

[Herbert Xu]

On Wed, Apr 27, 2005 at 09:13:36PM -0400, jamal wrote:
>
> > > +		if (!delpol && 
> > > +		((!excl && policy->index && (policy->index == pol->index)) || 
> > > +		 (memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0))) {
>
> > Also the excl check doesn't make sense.  You should let the
> > following excl check take place after you've found out that
> > the indices is identical.
> 
> the policy->index is only relevant for the update not the add;
> the update could also be done by selector. So i didnt follow.

I see.  In that case you want to change your expression above
so that the memcmp is never done if excl is off and the index
is non-zero.  Otherwise this will result in non-deterministic
behaviour as the result will change depending on whether the
first hit is an index match or a selector match.

Actually, would it be so bad to check the policy->index for the
add case? It does have a well-defined meaning there.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[Herbert Xu]

On Thu, Apr 28, 2005 at 11:21:35AM +1000, herbert wrote:
> 
> I see.  In that case you want to change your expression above
> so that the memcmp is never done if excl is off and the index
> is non-zero.  Otherwise this will result in non-deterministic
> behaviour as the result will change depending on whether the
> first hit is an index match or a selector match.

Sorry, the index match needs more work.  We need to maintain
these invariants:

1) There is only one policy with a given selector.
2) There is only one policy with a given index.

So to allow matching by index when updating, we need to deal
with the possibility of having to delete two existing policies.
The current code simply can't deal with that.

So if we're going to do this we'll need a bigger patch :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[jamal]

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


I found a bug in the kernel that i initially thought was in "ip x p". 
If you specify an index when creating a new rule,  the kernel overrides
it regardless.
So i can now update by index with attached patch.

On Thu, 2005-28-04 at 11:21 +1000, Herbert Xu wrote:

> I see.  In that case you want to change your expression above
> so that the memcmp is never done if excl is off and the index
> is non-zero.

Hrm. Thinking... So you want to exclude the selector check if someone
updating ever specified the index? That may change things a little, no?
Give me a clever expression.

>   Otherwise this will result in non-deterministic
> behaviour as the result will change depending on whether the
> first hit is an index match or a selector match.
> 

I was trying to emulate the get/del. There if p->index is specified
it trumps the selector as a search key. 
 
> Actually, would it be so bad to check the policy->index for the
> add case? It does have a well-defined meaning there.

That may not be totally unreasonable depending on what you mean by 
"well defined meaning" ;->
If we want to ensure that theres a uniqueness of indices, then it makes
sense. i.e  noone should be able to add either a selector or index which
match what already is in the SPD (per direction and probably ifindex).
Is that what you mean?

cheers,
jamal

[-- Attachment #2: polid_p2 --]
[-- Type: text/plain, Size: 944 bytes --]

--- a/net/xfrm/xfrm_policy.c	2005/04/27 11:32:13	1.1
+++ b/net/xfrm/xfrm_policy.c	2005/04/28 01:22:42
@@ -345,7 +345,10 @@
 
 	write_lock_bh(&xfrm_policy_lock);
 	for (p = &xfrm_policy_list[dir]; (pol=*p)!=NULL;) {
-		if (!delpol && memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0) {
+		if (!delpol && 
+		    ((!excl && policy->index && 
+		      (policy->index == pol->index)) || 
+		    (memcmp(&policy->selector, &pol->selector, sizeof(pol->selector)) == 0))) {
 			if (excl) {
 				write_unlock_bh(&xfrm_policy_lock);
 				return -EEXIST;
@@ -370,7 +373,9 @@
 	policy->next = *p;
 	*p = policy;
 	atomic_inc(&flow_cache_genid);
-	policy->index = delpol ? delpol->index : xfrm_gen_index(dir);
+	if (!policy->index)
+		policy->index = delpol ? delpol->index : xfrm_gen_index(dir);
+
 	policy->curlft.add_time = (unsigned long)xtime.tv_sec;
 	policy->curlft.use_time = 0;
 	if (!mod_timer(&policy->timer, jiffies + HZ))

Re: patch: policy update by id

[Herbert Xu]

On Wed, Apr 27, 2005 at 09:44:40PM -0400, jamal wrote:
> 
> > I see.  In that case you want to change your expression above
> > so that the memcmp is never done if excl is off and the index
> > is non-zero.
> 
> Hrm. Thinking... So you want to exclude the selector check if someone
> updating ever specified the index? That may change things a little, no?
> Give me a clever expression.

Please see my last email.  To do index updates correctly we'll need
to modify the current code so that it is able to delete two existing
policies.  We may have two existing policies if one matches the index
while the other matches the selector.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[jamal]

On Thu, 2005-28-04 at 11:30 +1000, Herbert Xu wrote:
> On Thu, Apr 28, 2005 at 11:21:35AM +1000, herbert wrote:
> > 
> > I see.  In that case you want to change your expression above
> > so that the memcmp is never done if excl is off and the index
> > is non-zero.  Otherwise this will result in non-deterministic
> > behaviour as the result will change depending on whether the
> > first hit is an index match or a selector match.
> 
> Sorry, the index match needs more work.  We need to maintain
> these invariants:
> 
> 1) There is only one policy with a given selector.
> 2) There is only one policy with a given index.
> 
> So to allow matching by index when updating, we need to deal
> with the possibility of having to delete two existing policies.
> The current code simply can't deal with that.
> 

Well, while snooping i was bothered as well. I am not sure i agree with
your #1 above ;->

1) It would seem to me that the priority field is to be used 
as a ambiguity resolver (thats what a gazillion other classification
schemes do). 

Lets take an example of an add:
If i specify a priority and a selector matches when doing an add, then
the priority being different should allow me to add the rule even if the
selectors match. 
Current behavior: We dont allow entering multiple selectors with the
same value even if i specify a different prio.

2) index really oughta be unique across the SPD.  
Current behavior: I can add several new rules with the same index.

I realize what i am asking in #2 is the opposite of what i ask for in 
#1 - the big unresolved question is: if both selector and index are
going to be keys to manipulating the SPD, then their behavior needs to
be consistent with each other. I really like to see the index being
unique, but the selector being priority disambiguated.


> So if we're going to do this we'll need a bigger patch :)

Lets agree on the principles first ;-> The patch i sent maintains the
status quo.

cheers,
jamal

Re: patch: policy update by id

[jamal]

On Thu, 2005-28-04 at 11:48 +1000, Herbert Xu wrote:
> On Wed, Apr 27, 2005 at 09:44:40PM -0400, jamal wrote:
> > 
> > > I see.  In that case you want to change your expression above
> > > so that the memcmp is never done if excl is off and the index
> > > is non-zero.
> > 
> > Hrm. Thinking... So you want to exclude the selector check if someone
> > updating ever specified the index? That may change things a little, no?
> > Give me a clever expression.
> 
> Please see my last email.

And you my other email ;->

> To do index updates correctly we'll need
> to modify the current code so that it is able to delete two existing
> policies.  We may have two existing policies if one matches the index
> while the other matches the selector.
> 

i think if we say index is unique per direction then lets settle in
rejecting adds which try to reinsert the same index. So theres no
ambiguity in deleting by index.
Not specifying the index means one which is unique is generated by the
kernel.
Specifying x rules with exact same selector in absence of a index i
believe should be allowed. There the disambiguation in add is via the
priority. Deleting of such entries should be per first seen i.e highest
priority.  
Thoughts?

cheers,
jamal

Re: patch: policy update by id

[Herbert Xu]

On Wed, Apr 27, 2005 at 09:52:20PM -0400, jamal wrote:
>
> > 1) There is only one policy with a given selector.
> > 2) There is only one policy with a given index.
> 
> Well, while snooping i was bothered as well. I am not sure i agree with
> your #1 above ;->
> 
> 1) It would seem to me that the priority field is to be used 
> as a ambiguity resolver (thats what a gazillion other classification
> schemes do). 

You know what, I actually agree with you :) But you'll need to convince
Dave:

http://www.uwsg.iu.edu/hypermail/linux/net/0305.3/0018.html

However, this doesn't change the fact that you may need to delete
two policies.

> 2) index really oughta be unique across the SPD.  
> Current behavior: I can add several new rules with the same index.

Not really.  The kernel ignores the index supplied when you're
adding them.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[jamal]

On Thu, 2005-28-04 at 12:07 +1000, Herbert Xu wrote:

> You know what, I actually agree with you :) But you'll need to convince
> Dave:
> 
> http://www.uwsg.iu.edu/hypermail/linux/net/0305.3/0018.html
> 
> However, this doesn't change the fact that you may need to delete
> two policies.
> 

It certainly may be simpler to just allow no more than selector.
It reduces the value of priorities to be resolving ambiguities between
matches perhaps with overlapping areas by prefix lengths.

> > 2) index really oughta be unique across the SPD.  
> > Current behavior: I can add several new rules with the same index.
> 
> Not really.  The kernel ignores the index supplied when you're
> adding them.
> 

Whats the point of index then?

cheers,
jamal

Re: patch: policy update by id

[Herbert Xu]

On Wed, Apr 27, 2005 at 10:20:32PM -0400, jamal wrote:
>
> > > 2) index really oughta be unique across the SPD.  
> > > Current behavior: I can add several new rules with the same index.
> > 
> > Not really.  The kernel ignores the index supplied when you're
> > adding them.
> 
> Whats the point of index then?

So that you can delete policies without specifying the whole selector.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[jamal]

On Thu, 2005-28-04 at 12:22 +1000, Herbert Xu wrote:
> On Wed, Apr 27, 2005 at 10:20:32PM -0400, jamal wrote:

> > Whats the point of index then?
> 
> So that you can delete policies without specifying the whole selector.
> 

Thats fine - same with get by index.
But if i am managing the policies I should be able to specify the
indices of choice. The kernel should assign me one when i dont define an
index. It should also reject what i pass if it the index is already in
use.  This is a very standard scheme for managing tables.

cheers,
jamal

Re: patch: policy update by id

[David S. Miller]

On Thu, 28 Apr 2005 12:07:54 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> You know what, I actually agree with you :) But you'll need to convince
> Dave:
> 
> http://www.uwsg.iu.edu/hypermail/linux/net/0305.3/0018.html

I'm willing to reneg on that position if you can convince me
that security minded folks won't be surprised by this pseudo-
aliasing.  For example, do firewall systems tend to support
such priority schemes?  If so, I guess we can do it.

Re: patch: policy update by id

[Herbert Xu]

On Wed, Apr 27, 2005 at 07:43:56PM -0700, David S. Miller wrote:
> 
> I'm willing to reneg on that position if you can convince me
> that security minded folks won't be surprised by this pseudo-
> aliasing.  For example, do firewall systems tend to support
> such priority schemes?  If so, I guess we can do it.

Well netfilter certainly follows this scheme:

$ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
$ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
$ iptables -v -L INPUT -n
Chain INPUT (policy ACCEPT 0 packets, 0 bytes)
 pkts bytes target     prot opt in     out     source               destination 
    0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     
    0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[jamal]

On Wed, 2005-27-04 at 19:43 -0700, David S. Miller wrote:
> On Thu, 28 Apr 2005 12:07:54 +1000
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > You know what, I actually agree with you :) But you'll need to convince
> > Dave:
> > 
> > http://www.uwsg.iu.edu/hypermail/linux/net/0305.3/0018.html
> 
> I'm willing to reneg on that position if you can convince me
> that security minded folks won't be surprised by this pseudo-
> aliasing.  For example, do firewall systems tend to support
> such priority schemes?  If so, I guess we can do it.

Well, the tc classifiers are a good example. Priorities are used 
as ambiguity resolvers. 

After reading that URL though i think either way would be fine ..

rule1:
reject ipsrc A/32 ipdst B/32 with different priorities if entered more
than once; 
** but we allow the second rule ipsrc A/24 ipdst B/24 - only thing would
probably be useful to add is ensure a different priority is used. This
may be a little involved.

BTW, a weird ambiguity resolver is iptables - it just prepends rules.

cheers,
jamal

Re: patch: policy update by id

[jamal]

On Thu, 2005-28-04 at 12:56 +1000, Herbert Xu wrote:

> Well netfilter certainly follows this scheme:
> 
> $ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
> $ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
> $ iptables -v -L INPUT -n
> Chain INPUT (policy ACCEPT 0 packets, 0 bytes)
>  pkts bytes target     prot opt in     out     source               destination 
>     0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     
>     0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     
> 

Which is bizare to say the least. If you delete, only the first one gets
deleted.

cheers,
jamal

Re: patch: policy update by id

[Herbert Xu]

On Wed, Apr 27, 2005 at 11:16:00PM -0400, jamal wrote:
> On Thu, 2005-28-04 at 12:56 +1000, Herbert Xu wrote:
> 
> > Well netfilter certainly follows this scheme:
> > 
> > $ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
> > $ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
> > $ iptables -v -L INPUT -n
> > Chain INPUT (policy ACCEPT 0 packets, 0 bytes)
> >  pkts bytes target     prot opt in     out     source               destination 
> >     0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     
> >     0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     
> 
> Which is bizare to say the least. If you delete, only the first one gets
> deleted.

It isn't that strange.  It's also done using indices except that the
indices aren't fixed.  Do delete the second rule you would say

iptables -D INPUT 2
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: patch: policy update by id

[Thomas Graf]

* Herbert Xu <20050428032045.GA24041@gondor.apana.org.au> 2005-04-28 13:20
> On Wed, Apr 27, 2005 at 11:16:00PM -0400, jamal wrote:
> > On Thu, 2005-28-04 at 12:56 +1000, Herbert Xu wrote:
> > 
> > > Well netfilter certainly follows this scheme:
> > > 
> > > $ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
> > > $ iptables -I INPUT -s 3.3.3.3 -d 4.4.4.4 -j ACCEPT
> > > $ iptables -v -L INPUT -n
> > > Chain INPUT (policy ACCEPT 0 packets, 0 bytes)
> > >  pkts bytes target     prot opt in     out     source               destination 
> > >     0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     
> > >     0     0 ACCEPT     all  --  *      *       3.3.3.3              4.4.4.4     
> > 
> > Which is bizare to say the least. If you delete, only the first one gets
> > deleted.
> 
> It isn't that strange.  It's also done using indices except that the
> indices aren't fixed.  Do delete the second rule you would say
> 
> iptables -D INPUT 2

Except for when another iptables instance has modified the ordering of
the rules by inserting or deleting a rule in the meantime. Please do
not adopt this scheme, it's completely unreliable.

Re: patch: policy update by id

[Patrick McHardy]

Thomas Graf wrote:
> * Herbert Xu <20050428032045.GA24041@gondor.apana.org.au> 2005-04-28 13:20
> 
>>iptables -D INPUT 2
> 
> Except for when another iptables instance has modified the ordering of
> the rules by inserting or deleting a rule in the meantime. Please do
> not adopt this scheme, it's completely unreliable.

Yes, if you don't know the ordering of your ruleset it is unreliable :)

Regards
Patrick

Re: patch: policy update by id

[Thomas Graf]

* Patrick McHardy <4270D286.7060301@trash.net> 2005-04-28 14:09
> Thomas Graf wrote:
> >* Herbert Xu <20050428032045.GA24041@gondor.apana.org.au> 2005-04-28 13:20
> >
> >>iptables -D INPUT 2
> >
> >Except for when another iptables instance has modified the ordering of
> >the rules by inserting or deleting a rule in the meantime. Please do
> >not adopt this scheme, it's completely unreliable.
> 
> Yes, if you don't know the ordering of your ruleset it is unreliable :)

Even if you know the ordering, the knowledge may expire very fast. It's
getting more and more common to automate the insertion and deletion of
rules via scripts triggered by events. I'm more or less fine with this
choice but it's nowhere near an index but rather just a line based
offset. I've seen user interfaces relying on -D, guess what happens if
more than one person uses the interface. It wouldn't cost much to
introduce a 64bit generated identifier and return that it userspace
but it would give higher level applications a chance to reidentify
rules, safely delete a certain rule, and would probably speed up the
deletion of a single rule out of a rather big ruleset considerably.