All of lore.kernel.org
 help / color / mirror / Atom feed
* [ANNOUNCE] ipset 6.6 released
@ 2011-05-24  8:48 Jozsef Kadlecsik
  2011-05-24 11:09 ` Mr Dash Four
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-05-24  8:48 UTC (permalink / raw)
  To: netfilter, netfilter-devel

Hi,

Here comes ipset 6.6 with a couple of bugfixes and improvements. The terse 
list of the changes are:

Kernel part changes:
 - Use unified from/to address masking and check the usage
 - ip_set_flush returned -EPROTO instead of -IPSET_ERR_PROTOCOL, fixed
 - Take into account cidr value for the from address when creating the set
 - Adding ranges to hash types with timeout could still fail, fixed
 - Removed old, not used hashing method ip_set_chash
 - Remove variable 'ret' in type_pf_tdel(), which is set but not used
 - Use proper timeout parameter to jiffies conversion

Userspace changes:
 - Restore with bitmap:port and list:set types did not work, fixed
 - Accept "\r\n" terminated COMMIT command in restore files
 - Fix the message sequence number book-keeping
 - Protocol-level debugging support added
 - hash:net stress test in range notation added
 - ipset_mnl_query: in debug mode print the errno returned by the cb
   function
 - Accept "\r\n" terminated lines in restore files
 - Remove outdated checking of IPv6 support from configure.ac

I thank to Mr Dash Four for his tireless testings and helpful reports.

The next ipset package release will come with code simplifications, which 
mean dropping some support:

- iptree and iptreemap type support will be removed, because the automatic
  conversion to hash:ip is not fully backward compatible. The hash:ip and 
  hash:net types can be used instead.
- kernel version support below 2.6.37 will be dropped, to make code
  maintenance easier and more straightforward

You can download the source code of ipset from:
        http://ipset.netfilter.org
        ftp://ftp.netfilter.org/pub/ipset/
        git://git.netfilter.org/ipset.git

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24  8:48 [ANNOUNCE] ipset 6.6 released Jozsef Kadlecsik
@ 2011-05-24 11:09 ` Mr Dash Four
  2011-05-24 11:44   ` Jozsef Kadlecsik
  2011-05-25  1:58 ` Mr Dash Four
  2011-06-08  1:17 ` ipset 6.6 bug: subnet (mis)matching Mr Dash Four
  2 siblings, 1 reply; 18+ messages in thread
From: Mr Dash Four @ 2011-05-24 11:09 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter, netfilter-devel


> I thank to Mr Dash Four for his tireless testings and helpful reports.
>   
I thank you for your continuing development of this great tool which, at 
least in my case, hugely improved the stability and performance, as well 
as decreased the maintenance needed on all my systems as a direct result 
of employing this great tool!

> The next ipset package release will come with code simplifications, which 
> mean dropping some support:
>
> - iptree and iptreemap type support will be removed, because the automatic
>   conversion to hash:ip is not fully backward compatible. The hash:ip and 
>   hash:net types can be used instead.
> - kernel version support below 2.6.37 will be dropped, to make code
>   maintenance easier and more straightforward
>   
What are your plans with regards to the last point? The reason I am 
asking is because I am still on .35 (the last "stable" kernel released 
by Fedora) and I might not be able to use new releases if you introduce 
this restriction.

Also, would you consider my suggestion to include the number of members 
registered in a set to be displayed in the set headers (something like 
"Members: 15156") - this would help with managing the number of members 
in that set, which would be particularly useful for sets with large 
number of members.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24 11:09 ` Mr Dash Four
@ 2011-05-24 11:44   ` Jozsef Kadlecsik
  2011-05-24 11:54     ` Mr Dash Four
  0 siblings, 1 reply; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-05-24 11:44 UTC (permalink / raw)
  To: Mr Dash Four; +Cc: netfilter, netfilter-devel

On Tue, 24 May 2011, Mr Dash Four wrote:

> > The next ipset package release will come with code simplifications, which
> > mean dropping some support:
> > 
> > - iptree and iptreemap type support will be removed, because the automatic
> >   conversion to hash:ip is not fully backward compatible. The hash:ip and
> > hash:net types can be used instead.
> > - kernel version support below 2.6.37 will be dropped, to make code
> >   maintenance easier and more straightforward
> >   
> What are your plans with regards to the last point? The reason I am asking is
> because I am still on .35 (the last "stable" kernel released by Fedora) and I
> might not be able to use new releases if you introduce this restriction.

Hm, 2.6.35 can lessen the maintenance burden compared to the currently 
minimal supported version 2.6.34, because the main trouble comes from the 
differences between .34 and .35. So I think I can remove the older kernel
supports gradually and keep supporting 2.6.35 for a while.
 
> Also, would you consider my suggestion to include the number of members
> registered in a set to be displayed in the set headers (something like
> "Members: 15156") - this would help with managing the number of members in
> that set, which would be particularly useful for sets with large number of
> members.

The problem with it that the reported number can be inaccurate, at least 
in two cases:

- Elements can time out, so even if whatever number reported, by the
  time it's displayed, the set can even be completely empty. In the case
  of a huge set, it can even occur that the number of elements reported
  does not match the actual number of elements listed.
- Sets can be updated by the SET target

The first one is the main reason I dropped reporting the number of 
elements (the initial design of the new ipset included it).

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24 11:44   ` Jozsef Kadlecsik
@ 2011-05-24 11:54     ` Mr Dash Four
  2011-05-24 12:19       ` Jozsef Kadlecsik
  0 siblings, 1 reply; 18+ messages in thread
From: Mr Dash Four @ 2011-05-24 11:54 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter-devel


> Hm, 2.6.35 can lessen the maintenance burden compared to the currently 
> minimal supported version 2.6.34, because the main trouble comes from the 
> differences between .34 and .35. So I think I can remove the older kernel
> supports gradually and keep supporting 2.6.35 for a while.
>   
That would be appreciated as I think .35 is a pretty stable kernel and 
will be in use for a while yet (though I have to admit, I have patched 
this kernel extensively).

> The problem with it that the reported number can be inaccurate, at least 
> in two cases:
>
> - Elements can time out, so even if whatever number reported, by the
>   time it's displayed, the set can even be completely empty. In the case
>   of a huge set, it can even occur that the number of elements reported
>   does not match the actual number of elements listed.
> - Sets can be updated by the SET target
>
> The first one is the main reason I dropped reporting the number of 
> elements (the initial design of the new ipset included it).
>   
I see, reasonable point.

OK, would it be possible then to get this figure when I use restore - I 
am asking for this because, as you already know, ip ranges may mean 
inserting more elements than the range itself and the indicator I used 
up until now to determine how many set members I would have (the number 
of lines in my restore file) is no longer a reliable indicator, so I 
would need to know how many members I have inserted as a result of 
restore in order to determine the maxelem value, adjust it and thus save 
system resources. Do you see my point?


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24 11:54     ` Mr Dash Four
@ 2011-05-24 12:19       ` Jozsef Kadlecsik
  2011-05-24 12:22         ` Mr Dash Four
  0 siblings, 1 reply; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-05-24 12:19 UTC (permalink / raw)
  To: Mr Dash Four; +Cc: netfilter-devel

On Tue, 24 May 2011, Mr Dash Four wrote:

> > The problem with it that the reported number can be inaccurate, at least in
> > two cases:
> > 
> > - Elements can time out, so even if whatever number reported, by the
> >   time it's displayed, the set can even be completely empty. In the case
> >   of a huge set, it can even occur that the number of elements reported
> >   does not match the actual number of elements listed.
> > - Sets can be updated by the SET target
> > 
> > The first one is the main reason I dropped reporting the number of elements
> > (the initial design of the new ipset included it).
> >   
> I see, reasonable point.
> 
> OK, would it be possible then to get this figure when I use restore - I am
> asking for this because, as you already know, ip ranges may mean inserting
> more elements than the range itself and the indicator I used up until now to
> determine how many set members I would have (the number of lines in my restore
> file) is no longer a reliable indicator, so I would need to know how many
> members I have inserted as a result of restore in order to determine the
> maxelem value, adjust it and thus save system resources. Do you see my point?

Currently I can suggest you that tune both the hashsize and maxelem 
parameters: the hashsize tells the system the initial hash size (and thus 
the used memory resource) while maxelem tells up to what number of 
elements can be added to the set. You are free to enter quite high maxelem 
values, it does not waste memory. As you add more and more elements, the 
hash size is increased from the one specified by 'hashsize'. But you (or 
the SET targets) can't add more elements than "maxelem".

When you save a set, the current hashsize is saved and not the one 
specified at set creation time.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24 12:19       ` Jozsef Kadlecsik
@ 2011-05-24 12:22         ` Mr Dash Four
  2011-05-24 12:31           ` Jozsef Kadlecsik
  0 siblings, 1 reply; 18+ messages in thread
From: Mr Dash Four @ 2011-05-24 12:22 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter-devel


> Currently I can suggest you that tune both the hashsize and maxelem 
> parameters: the hashsize tells the system the initial hash size (and thus 
> the used memory resource) while maxelem tells up to what number of 
> elements can be added to the set. You are free to enter quite high maxelem 
> values, it does not waste memory. As you add more and more elements, the 
> hash size is increased from the one specified by 'hashsize'. But you (or 
> the SET targets) can't add more elements than "maxelem".
>
> When you save a set, the current hashsize is saved and not the one 
> specified at set creation time.
>   
Ah, I see! So maxelem does not have an impact on system resources as 
hashsize has, is that right? If that is the case, then I may as well 
leave the default (64k) as I'd never exceed this value. Thanks for your 
input, as always!


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24 12:22         ` Mr Dash Four
@ 2011-05-24 12:31           ` Jozsef Kadlecsik
  2011-05-25  2:33             ` Mr Dash Four
  0 siblings, 1 reply; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-05-24 12:31 UTC (permalink / raw)
  To: Mr Dash Four; +Cc: netfilter-devel

On Tue, 24 May 2011, Mr Dash Four wrote:

> > Currently I can suggest you that tune both the hashsize and maxelem
> > parameters: the hashsize tells the system the initial hash size (and thus
> > the used memory resource) while maxelem tells up to what number of elements
> > can be added to the set. You are free to enter quite high maxelem values, it
> > does not waste memory. As you add more and more elements, the hash size is
> > increased from the one specified by 'hashsize'. But you (or the SET targets)
> > can't add more elements than "maxelem".
> > 
> > When you save a set, the current hashsize is saved and not the one 
> > specified at set creation time.
> >   
> Ah, I see! So maxelem does not have an impact on system resources as 
> hashsize has, is that right? If that is the case, then I may as well 
> leave the default (64k) as I'd never exceed this value. Thanks for your 
> input, as always!

Yes, exactly. With the hashsize parameter, you tell the system how much 
memory is allocated for the set when created. With maxelem it is 
instructed at what point to stop increasing the set and refuse adding more 
elements.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24  8:48 [ANNOUNCE] ipset 6.6 released Jozsef Kadlecsik
  2011-05-24 11:09 ` Mr Dash Four
@ 2011-05-25  1:58 ` Mr Dash Four
  2011-05-25  7:23   ` Jozsef Kadlecsik
  2011-06-08  1:17 ` ipset 6.6 bug: subnet (mis)matching Mr Dash Four
  2 siblings, 1 reply; 18+ messages in thread
From: Mr Dash Four @ 2011-05-25  1:58 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter-devel


> Userspace changes:
>  - Restore with bitmap:port and list:set types did not work, fixed
>   
Having had the chance to test this now, I can say that it works, and 
what's more - the loading performance is much, much better - all my sets 
now load in about 2-3 seconds, while with the 4.5 version it took in 
excess of 10 minutes, completely hogging my CPU in the process. I 
haven't had the chance yet to judge the matching performance, but this 
is what I will do in the coming days.

I have found a bug, however. :-\

When I have multiple sets of different type to restore, each restore 
file normally ends with "COMMIT" statement for ipset to commit the whole 
transaction, or so I thought. If there is a mistake (syntax or any 
other) somewhere in the restore file, which prevents the restore 
process, ipset already commits everything up to that point, which I 
think is wrong.

For example, if I have this:

n privileged-ports bitmap:port range 1-1023 timeout 0
a privileged-ports 1-1023
n test-ports bitmap:port range 12770-19999 timeout 0
a test-ports 20000-30000
a test-ports 19999
n test-port bitmap:port range 29950-29950 timeout 0
a test-port 29950
COMMIT

There is an obvious error in line 4 above ("a test-ports 20000-30000" - 
this is out of the defined range for this set) - ipset should have 
aborted the whole transaction and not committed anything, but in 
practice, privileged-ports set is already registered and its members are 
already added.

Apart from the obvious error of ipset committing before the actual 
"COMMIT" has taken place, this raises another issue when I actually try 
to reload this file - I will get an error straight away as 
privileged-ports is already registered and that shouldn't be the case. 
Thought to let you know.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-24 12:31           ` Jozsef Kadlecsik
@ 2011-05-25  2:33             ` Mr Dash Four
  0 siblings, 0 replies; 18+ messages in thread
From: Mr Dash Four @ 2011-05-25  2:33 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter-devel


> Yes, exactly. With the hashsize parameter, you tell the system how much 
> memory is allocated for the set when created. With maxelem it is 
> instructed at what point to stop increasing the set and refuse adding more 
> elements.
>   
It turns out that ipset is more efficient in that respect as well. Over 
15k members take about 0.6MB of RAM, which isn't that bad!

I have one suggestion though: it would be nice if you could implement a 
"check" option to accompany restore so that ipset doesn't actually do 
anything, but just checks the syntax of that file (a dummy run, if you 
like) and reports any errors or omissions (it could be more than one!). 
It would be nice to have that feature and help in avoiding errors (see 
also my other post with regards to that).

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-25  1:58 ` Mr Dash Four
@ 2011-05-25  7:23   ` Jozsef Kadlecsik
  2011-05-25  8:32     ` Mr Dash Four
  0 siblings, 1 reply; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-05-25  7:23 UTC (permalink / raw)
  To: Mr Dash Four; +Cc: netfilter-devel

On Wed, 25 May 2011, Mr Dash Four wrote:

> > Userspace changes:
> >  - Restore with bitmap:port and list:set types did not work, fixed
> >   
> Having had the chance to test this now, I can say that it works, and what's
> more - the loading performance is much, much better - all my sets now load in
> about 2-3 seconds, while with the 4.5 version it took in excess of 10 minutes,
> completely hogging my CPU in the process. I haven't had the chance yet to
> judge the matching performance, but this is what I will do in the coming days.
> 
> I have found a bug, however. :-\
> 
> When I have multiple sets of different type to restore, each restore file
> normally ends with "COMMIT" statement for ipset to commit the whole
> transaction, or so I thought. If there is a mistake (syntax or any other)
> somewhere in the restore file, which prevents the restore process, ipset
> already commits everything up to that point, which I think is wrong.
> 
> For example, if I have this:
> 
> n privileged-ports bitmap:port range 1-1023 timeout 0
> a privileged-ports 1-1023
> n test-ports bitmap:port range 12770-19999 timeout 0
> a test-ports 20000-30000
> a test-ports 19999
> n test-port bitmap:port range 29950-29950 timeout 0
> a test-port 29950
> COMMIT
> 
> There is an obvious error in line 4 above ("a test-ports 20000-30000" - this
> is out of the defined range for this set) - ipset should have aborted the
> whole transaction and not committed anything, but in practice,
> privileged-ports set is already registered and its members are already added.
> 
> Apart from the obvious error of ipset committing before the actual "COMMIT"
> has taken place, this raises another issue when I actually try to reload this
> file - I will get an error straight away as privileged-ports is already
> registered and that shouldn't be the case. Thought to let you know.

If you have got a restore file with an error in it, fix the file and 
restore it again but with the '-!' flag. That way all already defined 
sets, elements are ignored (technically: not raised the error due to the 
clashing set/elements):

# ipset restore < file
<syntax error, fix the file>
# ipset -! restore < file

The "COMMIT" string is not required at all in ipset 6.x. It's just kept 
for backward compatibility reasons.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ANNOUNCE] ipset 6.6 released
  2011-05-25  7:23   ` Jozsef Kadlecsik
@ 2011-05-25  8:32     ` Mr Dash Four
  0 siblings, 0 replies; 18+ messages in thread
From: Mr Dash Four @ 2011-05-25  8:32 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter-devel


> If you have got a restore file with an error in it, fix the file and 
> restore it again but with the '-!' flag. That way all already defined 
> sets, elements are ignored (technically: not raised the error due to the 
> clashing set/elements):
>
> # ipset restore < file
> <syntax error, fix the file>
> # ipset -! restore < file
>
> The "COMMIT" string is not required at all in ipset 6.x. It's just kept 
> for backward compatibility reasons.
>   
Didn't know that, thanks Jozsef - I'll remove all the COMMITs in the 
future as well.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* ipset 6.6 bug: subnet (mis)matching
  2011-05-24  8:48 [ANNOUNCE] ipset 6.6 released Jozsef Kadlecsik
  2011-05-24 11:09 ` Mr Dash Four
  2011-05-25  1:58 ` Mr Dash Four
@ 2011-06-08  1:17 ` Mr Dash Four
  2011-06-08  7:07   ` Jozsef Kadlecsik
  2 siblings, 1 reply; 18+ messages in thread
From: Mr Dash Four @ 2011-06-08  1:17 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter

[root@test1 ~]# ipset n test hash:net family inet timeout 0
[root@test1 ~]# ipset a test 10.16.0.0-10.16.255.255
[root@test1 ~]# ipset l test
Name: test
Type: hash:net
Header: family inet hashsize 1024 maxelem 65536 timeout 0 
Size in memory: 16832
References: 0
Members:
10.16.0.0/16 timeout 0
[root@test1 ~]# ipset t test 10.16.224.0/24
10.16.224.0/24 is NOT in set test.
[root@test1 ~]# ipset t test 10.16.224.1   
10.16.224.1 is in set test.

That is plainly wrong!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: ipset 6.6 bug: subnet (mis)matching
  2011-06-08  1:17 ` ipset 6.6 bug: subnet (mis)matching Mr Dash Four
@ 2011-06-08  7:07   ` Jozsef Kadlecsik
  2011-06-08 10:07     ` Mr Dash Four
  0 siblings, 1 reply; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-06-08  7:07 UTC (permalink / raw)
  To: Mr Dash Four; +Cc: netfilter

On Wed, 8 Jun 2011, Mr Dash Four wrote:

> [root@test1 ~]# ipset n test hash:net family inet timeout 0
> [root@test1 ~]# ipset a test 10.16.0.0-10.16.255.255
> [root@test1 ~]# ipset l test
> Name: test
> Type: hash:net
> Header: family inet hashsize 1024 maxelem 65536 timeout 0 
> Size in memory: 16832
> References: 0
> Members:
> 10.16.0.0/16 timeout 0
> [root@test1 ~]# ipset t test 10.16.224.0/24
> 10.16.224.0/24 is NOT in set test.
> [root@test1 ~]# ipset t test 10.16.224.1   
> 10.16.224.1 is in set test.
> 
> That is plainly wrong!

No, that's a feature: it makes possible to check from userspace how the 
kernel would match an IP address in the set. Maybe it's badly worded in 
the manpage: "When testing entries, if a host address is tested, then the 
kernel tries to match the host address in the networks added to the set 
and reports the result accordingly."

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: ipset 6.6 bug: subnet (mis)matching
  2011-06-08  7:07   ` Jozsef Kadlecsik
@ 2011-06-08 10:07     ` Mr Dash Four
  2011-06-08 10:46       ` Jozsef Kadlecsik
  0 siblings, 1 reply; 18+ messages in thread
From: Mr Dash Four @ 2011-06-08 10:07 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: netfilter


>> [root@test1 ~]# ipset n test hash:net family inet timeout 0
>> [root@test1 ~]# ipset a test 10.16.0.0-10.16.255.255
>> [root@test1 ~]# ipset l test
>> Name: test
>> Type: hash:net
>> Header: family inet hashsize 1024 maxelem 65536 timeout 0 
>> Size in memory: 16832
>> References: 0
>> Members:
>> 10.16.0.0/16 timeout 0
>> [root@test1 ~]# ipset t test 10.16.224.0/24
>> 10.16.224.0/24 is NOT in set test.
>> [root@test1 ~]# ipset t test 10.16.224.1   
>> 10.16.224.1 is in set test.
>>
>> That is plainly wrong!
>>     
>
> No, that's a feature: it makes possible to check from userspace how the 
> kernel would match an IP address in the set.
You can't be serious! How could this be a "feature"?! It is a bug, clearly!

The /24 subnet (10.16.224.0-10.16.224.255) clearly covers the single IP 
address (10.16.224.1) and the test should be positive in both cases, as 
the /16 member of the set (10.16.0.0-10.16.255.255) covers both the 
single IP address as well as the /24subnet range tested. Apparently, 
that seems not to be the case. How am I suppose to test subnet ranges then?

I presume if I have 10.16.224.0/24 as a member and do a test with 
10.16.0.0/16 that would also return a negative match, isn't that the case?

>  Maybe it's badly worded in 
> the manpage: "When testing entries, if a host address is tested, then the 
> kernel tries to match the host address in the networks added to the set 
> and reports the result accordingly."
>   
That's all well and good for hosts, but ip ranges should either be 
tested properly or testing of those should be disabled altogether 
(which, if introduced, would be a severe restriction for me - I am not 
going to run a couple of thousand "ipset t" tests just to see that all 
of the single IP addresses in the range I am interested in match - that 
simply isn't going to happen!).


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: ipset 6.6 bug: subnet (mis)matching
  2011-06-08 10:07     ` Mr Dash Four
@ 2011-06-08 10:46       ` Jozsef Kadlecsik
  2011-06-08 11:21         ` Mr Dash Four
  0 siblings, 1 reply; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-06-08 10:46 UTC (permalink / raw)
  To: Mr Dash Four; +Cc: netfilter

On Wed, 8 Jun 2011, Mr Dash Four wrote:
 
> > > [root@test1 ~]# ipset n test hash:net family inet timeout 0
> > > [root@test1 ~]# ipset a test 10.16.0.0-10.16.255.255
> > > [root@test1 ~]# ipset l test
> > > Name: test
> > > Type: hash:net
> > > Header: family inet hashsize 1024 maxelem 65536 timeout 0 Size in memory:
> > > 16832
> > > References: 0
> > > Members:
> > > 10.16.0.0/16 timeout 0
> > > [root@test1 ~]# ipset t test 10.16.224.0/24
> > > 10.16.224.0/24 is NOT in set test.
> > > [root@test1 ~]# ipset t test 10.16.224.1   10.16.224.1 is in set test.
> > > 
> > > That is plainly wrong!
> > 
> > No, that's a feature: it makes possible to check from userspace how the
> > kernel would match an IP address in the set.
> You can't be serious! How could this be a "feature"?! It is a bug, clearly!
> 
> The /24 subnet (10.16.224.0-10.16.224.255) clearly covers the single IP
> address (10.16.224.1) and the test should be positive in both cases, as the
> /16 member of the set (10.16.0.0-10.16.255.255) covers both the single IP
> address as well as the /24subnet range tested. Apparently, that seems not to
> be the case. How am I suppose to test subnet ranges then?

I can only repeat myself: for the hash:*net* types the testing with a host 
address is special and handled as the kernel would test an IP address.
However network addresses are handled as a whole and overlapping is *not* 
checked. It's documented in the manpage.

Maybe an example helps: when you add 10.6.0.0/16, you add an "apple". You 
can also add 10.6.0.0/24, a "pear". Nothing prevents you, both can be the 
member of the set, because overlapping is not handled. At deleting, 
testing they are handled as "apple" and "pear", too. If "pear" is not 
added to the set, it's not "in" the set as a member - even if we know, 
that the elements of "apple" fully cover the elements of "pear". However 
at testing 10.16.224.1[/32] you test a "seed", which is checked inside 
"apple".

Merging with already existing elements (or breaking up if a network from a 
larger network were deleted) would be very expensive. I know, you compare 
the types to iptreemap, but iptreemap was a very special case where 
merging/breaking up was a natural process and costed almost nothing.

> I presume if I have 10.16.224.0/24 as a member and do a test with 10.16.0.0/16
> that would also return a negative match, isn't that the case?

Yes, of course.
 
> >  Maybe it's badly worded in the manpage: "When testing entries, if a host
> > address is tested, then the kernel tries to match the host address in the
> > networks added to the set and reports the result accordingly."
> >   
> That's all well and good for hosts, but ip ranges should either be tested
> properly or testing of those should be disabled altogether (which, if
> introduced, would be a severe restriction for me - I am not going to run a
> couple of thousand "ipset t" tests just to see that all of the single IP
> addresses in the range I am interested in match - that simply isn't going to
> happen!).

I don't quite follow you here: you can add, delete and test the elements 
by ipset. As a special feature, you can also test the members of elements, 
too by ipset.

Probably what you expect is the following:

    add 192.168.0.0/24
    add 192.168.1.0/24
		magically aggregate into 192.168.0.0/23
    add 192.168.2.0/24
    add 192.168.3.0/24
		magically aggregate into 192.168.0.0/22
    del 192.168.1.0/24
		break up into the elements
			192.168.0.0/24
			192.168.2.0/23
		
There's no such magic there.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: ipset 6.6 bug: subnet (mis)matching
  2011-06-08 10:46       ` Jozsef Kadlecsik
@ 2011-06-08 11:21         ` Mr Dash Four
  2011-06-08 11:43           ` Jozsef Kadlecsik
  0 siblings, 1 reply; 18+ messages in thread
From: Mr Dash Four @ 2011-06-08 11:21 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: 'netfilter@vger.kernel.org'


> I can only repeat myself: for the hash:*net* types the testing with a host 
> address is special and handled as the kernel would test an IP address.
> However network addresses are handled as a whole and overlapping is *not* 
> checked. It's documented in the manpage.
>   
I can understand that to a point as far as adding/deleting 
elements/subranges goes - it makes sense. Testing, however, is a 
different story.

> Maybe an example helps: when you add 10.6.0.0/16, you add an "apple". You 
> can also add 10.6.0.0/24, a "pear". Nothing prevents you, both can be the 
> member of the set, because overlapping is not handled. At deleting, 
> testing they are handled as "apple" and "pear", too. If "pear" is not 
> added to the set, it's not "in" the set as a member - even if we know, 
> that the elements of "apple" fully cover the elements of "pear". However 
> at testing 10.16.224.1[/32] you test a "seed", which is checked inside 
> "apple".
>   
Your basic presumption above is wrong - 10.16.224.1 *is* a member - i.e. 
it belongs, in network terms at least - to both 10.16.0.0/16 and 
10.16.224.0/24 ranges. Similarly, 10.16.224.0/24 also belongs to 
10.16.0.0/16 in the same way that 10.16.224.1 does to 10.16.224.0/24 and 
10.16.0.0/16.

10.16.224.1 is a member of the 10.16.224.0/24 "set", which, in turn, is 
also a member of 10.16.0.0/16 "set" (in mathematical as well as in 
networking terms). They are both in a way "subsets". Any tests for 
matching a subset belonging to a bigger set should return positive 
results. In the case of ipset they don't and that is my point.

There are no "apples" and "oranges" compared here as both ranges belong 
to each other, so they are not different from each other, apart from the 
fact that one set is bigger than the other. Basic grasp of maths would 
tell you all that.

> Merging with already existing elements (or breaking up if a network from a 
> larger network were deleted) would be very expensive. I know, you compare 
> the types to iptreemap, but iptreemap was a very special case where 
> merging/breaking up was a natural process and costed almost nothing.
>   
No, I don't make comparisons with iptreemap at all - there is no 
iptreemap set type involvement here, so there is no need for me to 
mention that type of set.

My basic (and only!) issue is that subnet ranges testing should either 
be implemented properly or not tested at all. The way I see it, that is 
not the case and I pointed out above why I think that is.

>> That's all well and good for hosts, but ip ranges should either be tested
>> properly or testing of those should be disabled altogether (which, if
>> introduced, would be a severe restriction for me - I am not going to run a
>> couple of thousand "ipset t" tests just to see that all of the single IP
>> addresses in the range I am interested in match - that simply isn't going to
>> happen!).
>>     
>
> I don't quite follow you here:
OK, if I have 10.16.0.0/16 as a member and then test a subnet range 
which is covered by the /16 address - again, lets say 10.16.1.0/24 - I 
expect to get a match as clearly, from both mathematical as well as 
networking terms, all "members" of 10.16.1.0/24 are also "members" of 
10.16.0.0/16, so the test result should return a positive. It doesn't!

I can understand a negative result if I test the other way around - not 
all "members" of 10.16.0.0/16 would belong to 10.16.1.0/24 and returning 
a negative test result in this instance is OK.

It doesn't work like that in ipset though - ipset seems to be operating 
either on "exact match" (say if I have 10.16.0.0/16 as a member and test 
with 10.16.0.0/16) or when a single ip address is tested, which I don't 
think is right and that was my point all along.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: ipset 6.6 bug: subnet (mis)matching
  2011-06-08 11:21         ` Mr Dash Four
@ 2011-06-08 11:43           ` Jozsef Kadlecsik
  2011-06-08 12:12             ` Mr Dash Four
  0 siblings, 1 reply; 18+ messages in thread
From: Jozsef Kadlecsik @ 2011-06-08 11:43 UTC (permalink / raw)
  To: Mr Dash Four; +Cc: 'netfilter@vger.kernel.org'

On Wed, 8 Jun 2011, Mr Dash Four wrote:

> OK, if I have 10.16.0.0/16 as a member and then test a subnet range which is
> covered by the /16 address - again, lets say 10.16.1.0/24 - I expect to get a
> match as clearly, from both mathematical as well as networking terms, all
> "members" of 10.16.1.0/24 are also "members" of 10.16.0.0/16, so the test
> result should return a positive. It doesn't!
> 
> I can understand a negative result if I test the other way around - not all
> "members" of 10.16.0.0/16 would belong to 10.16.1.0/24 and returning a
> negative test result in this instance is OK.
> 
> It doesn't work like that in ipset though - ipset seems to be operating either
> on "exact match" (say if I have 10.16.0.0/16 as a member and test with
> 10.16.0.0/16) or when a single ip address is tested, which I don't think is
> right and that was my point all along.

Current behaviour cannot be modified easily. There's no overlap checking 
(and aggregation/breaking up) in ipset, so it can't be prevented that 
someone adds overlapping elements to a set:

add 10.16.0.0/16
add 10.16.1.0/24

Now if 10.16.1.0/24 is tested as you suggest, one cannot tell whether the 
explicit network 10.16.1.0/24 is added to the set or the tested elements 
are just the members of a larger network.

Currently:
    test 10.16.1.1	=> test explicit and implicit membership
    test 10.16.1.0/24	=> test explicit membership only

What you propose is to test explicit and implicit membership in all cases.

My top priority now is to get the kernel in sync with ipset 6.7. It's just 
a much higher problem, because a lot of features and fixes are missing, 
even from the newest kernel version.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: ipset 6.6 bug: subnet (mis)matching
  2011-06-08 11:43           ` Jozsef Kadlecsik
@ 2011-06-08 12:12             ` Mr Dash Four
  0 siblings, 0 replies; 18+ messages in thread
From: Mr Dash Four @ 2011-06-08 12:12 UTC (permalink / raw)
  To: Jozsef Kadlecsik; +Cc: 'netfilter@vger.kernel.org'


> Current behaviour cannot be modified easily. There's no overlap checking 
> (and aggregation/breaking up) in ipset, so it can't be prevented that 
> someone adds overlapping elements to a set:
>
> add 10.16.0.0/16
> add 10.16.1.0/24
>
> Now if 10.16.1.0/24 is tested as you suggest, one cannot tell whether the 
> explicit network 10.16.1.0/24 is added to the set or the tested elements 
> are just the members of a larger network.
>
> Currently:
>     test 10.16.1.1	=> test explicit and implicit membership
>     test 10.16.1.0/24	=> test explicit membership only
>
> What you propose is to test explicit and implicit membership in all cases.
>   
The way I see it, you can take the base of a set member (10.16.0.0 in my 
case), build up a mask with the range (16), do the same with the range 
being tested and then match them with logical "and". If you get the same 
result as the set member - you have a match, if you don't there isn't a 
(complete) match.

Alternatively, you could loop through the members of the specified range 
to be tested and compare against the set members, but that would be a 
bit slower, though since this is done in userspace (i.e. interactively) 
speed wouldn't be that important compared to the kernel matching where 
good performance is needed.

> My top priority now is to get the kernel in sync with ipset 6.7. It's just 
> a much higher problem, because a lot of features and fixes are missing, 
> even from the newest kernel version.
>   
I am not asking or demanding anything to be implemented/corrected 
immediately - I am simply making you aware of a bug in ipset I found 
last night. If/When you are going to fix it it is for you to decide - 
you are the author of ipset, not me - I am just a user.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2011-06-08 12:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-24  8:48 [ANNOUNCE] ipset 6.6 released Jozsef Kadlecsik
2011-05-24 11:09 ` Mr Dash Four
2011-05-24 11:44   ` Jozsef Kadlecsik
2011-05-24 11:54     ` Mr Dash Four
2011-05-24 12:19       ` Jozsef Kadlecsik
2011-05-24 12:22         ` Mr Dash Four
2011-05-24 12:31           ` Jozsef Kadlecsik
2011-05-25  2:33             ` Mr Dash Four
2011-05-25  1:58 ` Mr Dash Four
2011-05-25  7:23   ` Jozsef Kadlecsik
2011-05-25  8:32     ` Mr Dash Four
2011-06-08  1:17 ` ipset 6.6 bug: subnet (mis)matching Mr Dash Four
2011-06-08  7:07   ` Jozsef Kadlecsik
2011-06-08 10:07     ` Mr Dash Four
2011-06-08 10:46       ` Jozsef Kadlecsik
2011-06-08 11:21         ` Mr Dash Four
2011-06-08 11:43           ` Jozsef Kadlecsik
2011-06-08 12:12             ` Mr Dash Four

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.