[PATCH] Reserve nfnetlink subsys ids.

[PATCH] Reserve nfnetlink subsys ids.

[Jozsef Kadlecsik]

Hi Patrick,

A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 include/linux/netfilter/nfnetlink.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index c600083..e0f2ab0 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -46,7 +46,9 @@ struct nfgenmsg {
 #define NFNL_SUBSYS_CTNETLINK_EXP	2
 #define NFNL_SUBSYS_QUEUE		3
 #define NFNL_SUBSYS_ULOG		4
-#define NFNL_SUBSYS_COUNT		5
+#define NFNL_SUBSYS_NFTABLES		5
+#define NFNL_SUBSYS_IPSET		6
+#define NFNL_SUBSYS_COUNT		7
 
 #ifdef __KERNEL__
 
-- 
1.5.4.3

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

Re: [PATCH] Reserve nfnetlink subsys ids.

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> Hi Patrick,
> 
> A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)

Its usually frowned upon to reserve IDs that are not used yet.
We can certainly coordinate so nftables and ipset won't clash
unless we add a different subsystem (we actually did, OSF is
new in nf-next-2.6.git). But is there a reason for adding the
IPSET id at this time instead of waiting until we merge it?

Re: [PATCH] Reserve nfnetlink subsys ids.

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Jozsef Kadlecsik wrote:
>> Hi Patrick,
>>
>> A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)
> 
> Its usually frowned upon to reserve IDs that are not used yet.
> We can certainly coordinate so nftables and ipset won't clash
> unless we add a different subsystem (we actually did, OSF is
> new in nf-next-2.6.git). But is there a reason for adding the
> IPSET id at this time instead of waiting until we merge it?

I think that the problem is that Jozsef then would need to distribute a
patch for nfnetlink to add the new ipset subsystem. This would make
easier for him I think. Moreover, there's a compromise to merge it so,
why not include it?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [PATCH] Reserve nfnetlink subsys ids.

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Jozsef Kadlecsik wrote:
>>> Hi Patrick,
>>>
>>> A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)
>> Its usually frowned upon to reserve IDs that are not used yet.
>> We can certainly coordinate so nftables and ipset won't clash
>> unless we add a different subsystem (we actually did, OSF is
>> new in nf-next-2.6.git). But is there a reason for adding the
>> IPSET id at this time instead of waiting until we merge it?
> 
> I think that the problem is that Jozsef then would need to distribute a
> patch for nfnetlink to add the new ipset subsystem. This would make
> easier for him I think. Moreover, there's a compromise to merge it so,
> why not include it?

Oh, I didn't reply your question. I think that he wants some feedback
before merging it. I should not talk on behalf of others :-), Jozsef?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [PATCH] Reserve nfnetlink subsys ids.

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Jozsef Kadlecsik wrote:
>>> Hi Patrick,
>>>
>>> A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)
>> Its usually frowned upon to reserve IDs that are not used yet.
>> We can certainly coordinate so nftables and ipset won't clash
>> unless we add a different subsystem (we actually did, OSF is
>> new in nf-next-2.6.git). But is there a reason for adding the
>> IPSET id at this time instead of waiting until we merge it?
> 
> I think that the problem is that Jozsef then would need to distribute a
> patch for nfnetlink to add the new ipset subsystem. This would make
> easier for him I think.

But there's a patch anyways until its merged, no? I mean, it doesn't
add any code, so it isn't even necessary to rebuild the kernel.

> Moreover, there's a compromise to merge it so, why not include it?

As I said, IDs are usually not reserved until something is merged,
even if there is agreement to merge it. I guess there has been too
much bad experience with reserved IDs and code that never got merged.
I have to justify the patches I merge myself and so far I can't.

Re: [PATCH] Reserve nfnetlink subsys ids.

[Jozsef Kadlecsik]

On Thu, 11 Jun 2009, Patrick McHardy wrote:

> Pablo Neira Ayuso wrote:
> > Patrick McHardy wrote:
> > > Jozsef Kadlecsik wrote:
> > > > Hi Patrick,
> > > > 
> > > > A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)
> As I said, IDs are usually not reserved until something is merged,
> even if there is agreement to merge it. I guess there has been too
> much bad experience with reserved IDs and code that never got merged.
> I have to justify the patches I merge myself and so far I can't.

You're right, of course. I'm going to find another way for a smooth 
protocol migration.

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

Re: [PATCH] Reserve nfnetlink subsys ids.

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> On Thu, 11 Jun 2009, Patrick McHardy wrote:
> 
>> Pablo Neira Ayuso wrote:
>>> Patrick McHardy wrote:
>>>> Jozsef Kadlecsik wrote:
>>>>> Hi Patrick,
>>>>>
>>>>> A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)
>> As I said, IDs are usually not reserved until something is merged,
>> even if there is agreement to merge it. I guess there has been too
>> much bad experience with reserved IDs and code that never got merged.
>> I have to justify the patches I merge myself and so far I can't.
> 
> You're right, of course. I'm going to find another way for a smooth 
> protocol migration.

I didn't mean to finally say no, just wondering if there are special
circumstances which would justify an exception.

Re: [PATCH] Reserve nfnetlink subsys ids.

[Jozsef Kadlecsik]

On Fri, 12 Jun 2009, Patrick McHardy wrote:

> Jozsef Kadlecsik wrote:
> > On Thu, 11 Jun 2009, Patrick McHardy wrote:
> > 
> > > Pablo Neira Ayuso wrote:
> > > > Patrick McHardy wrote:
> > > > > Jozsef Kadlecsik wrote:
> > > > > > Hi Patrick,
> > > > > > 
> > > > > > A tiny patch to add nfnetlink subsys ids for nftables and ipset. :-)
> > > As I said, IDs are usually not reserved until something is merged,
> > > even if there is agreement to merge it. I guess there has been too
> > > much bad experience with reserved IDs and code that never got merged.
> > > I have to justify the patches I merge myself and so far I can't.
> > 
> > You're right, of course. I'm going to find another way for a smooth protocol
> > migration.
> 
> I didn't mean to finally say no, just wondering if there are special
> circumstances which would justify an exception.

No need for an exception, there's a much nicer solution. 

What is my main problem actually? That currently there is no way for an 
extension-specific error reporting from the kernel to iptables when 
there's some problem with a rule. Therefore in order to catch the typical 
mistakes, I have to check the existence of the set specified on the 
iptables command line before the rules are sent to the kernel. It is done 
by querying the kernel about the set, currently via *sockopt calls. But 
when ipset is migrated to nfnetlink, it'd mean libnfnetlink dependecy, 
just for the sake of the set match/target in iptables. That's a too high 
price and I'm not willing to pay it.

So what I'm working on it is a protocol change in iptables itself (:-), 
which is fully backward compatible.

- add a new sockopt option, IPT_SO_GET_REPLACE, which is used instead of
  IPT_SO_SET_REPLACE (if supported by the kernel.  New iptables will 
  use IPT_SO_SET_REPLACE with old kernels.)
- new checkentry functions, which return the extension-specific error 
  codes instead of a simple boolean value
- if any error is detected by the checkentry funtions, IPT_SO_GET_REPLACE 
  returns the corresponding full ipt_entry, with the offset stored in 
  comefrom to the match/target which produced the error
- the userspace match/targets, with their new error function, 
  can translate the received error code to the appropriate error message 
  and can insert any specific data into the text using the offset into the 
  ipt_entry.

So we'll be able to report back exactly what is wrong with the given rule.
No need anymore to print 'Run `dmesg' for more information.' :-).

And from the set match/target point of view, I won't have to query the 
kernel at all :-)).

So far, working on the kernel part, surprisingly small modifications are 
required.

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

Re: [PATCH] Reserve nfnetlink subsys ids.

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> On Fri, 12 Jun 2009, Patrick McHardy wrote:
> 
>> I didn't mean to finally say no, just wondering if there are special
>> circumstances which would justify an exception.
> 
> No need for an exception, there's a much nicer solution. 
> 
> What is my main problem actually? That currently there is no way for an 
> extension-specific error reporting from the kernel to iptables when 
> there's some problem with a rule. Therefore in order to catch the typical 
> mistakes, I have to check the existence of the set specified on the 
> iptables command line before the rules are sent to the kernel. It is done 
> by querying the kernel about the set, currently via *sockopt calls. But 
> when ipset is migrated to nfnetlink, it'd mean libnfnetlink dependecy, 
> just for the sake of the set match/target in iptables. That's a too high 
> price and I'm not willing to pay it.
> 
> So what I'm working on it is a protocol change in iptables itself (:-), 
> which is fully backward compatible.
> 
> - add a new sockopt option, IPT_SO_GET_REPLACE, which is used instead of
>   IPT_SO_SET_REPLACE (if supported by the kernel.  New iptables will 
>   use IPT_SO_SET_REPLACE with old kernels.)
> - new checkentry functions, which return the extension-specific error 
>   codes instead of a simple boolean value
> - if any error is detected by the checkentry funtions, IPT_SO_GET_REPLACE 
>   returns the corresponding full ipt_entry, with the offset stored in 
>   comefrom to the match/target which produced the error
> - the userspace match/targets, with their new error function, 
>   can translate the received error code to the appropriate error message 
>   and can insert any specific data into the text using the offset into the 
>   ipt_entry.
> 
> So we'll be able to report back exactly what is wrong with the given rule.
> No need anymore to print 'Run `dmesg' for more information.' :-).

That sounds pretty cool.

> And from the set match/target point of view, I won't have to query the 
> kernel at all :-)).
> 
> So far, working on the kernel part, surprisingly small modifications are 
> required.

I'm looking forward to finally have reasonable error reporting :)

Re: [PATCH] Reserve nfnetlink subsys ids.

[Jan Engelhardt]

On Friday 2009-06-12 15:07, Patrick McHardy wrote:
>
> I'm looking forward to finally have reasonable error reporting :)

Don't worry. Even with the "See dmesg" indirection, our error
reporting is much superior than iproute2's.