iptables version defines

iptables version defines

[Thomas Jarosch]

Hello netfilter coreteam,

I'm currently looking for a way to support ipt_ACCOUNT
both for iptables 1.4.0 and iptables 1.4.1. Some function
names changed (addr_to_dotted() -> ipaddr_to_numeric())
and it would be easy to #ifdef around them.

The linux kernel provides multiple defines for its version number,
while iptables only supports a char* version string.

Would it make sense to add something like the kernel version defines
so one could write code like this:

#if IPTABLES_VERSION_CODE < IPTABLES_VERSION(1,4,1)
	abc
#else
	xyz
#endif

?

Another solution would be to have an "iptables" and "iptables-1.4.1"
directory in the pom archive, though I don't know if that is supported.
(and would lead to code duplication.)

Thanks,
Thomas

Re: iptables version defines

[Krzysztof Oledzki]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 809 bytes --]



On Fri, 30 May 2008, Thomas Jarosch wrote:

> Hello netfilter coreteam,
>
> I'm currently looking for a way to support ipt_ACCOUNT
> both for iptables 1.4.0 and iptables 1.4.1. Some function
> names changed (addr_to_dotted() -> ipaddr_to_numeric())
> and it would be easy to #ifdef around them.
>
> The linux kernel provides multiple defines for its version number,
> while iptables only supports a char* version string.
>
> Would it make sense to add something like the kernel version defines
> so one could write code like this:
>
> #if IPTABLES_VERSION_CODE < IPTABLES_VERSION(1,4,1)
> 	abc
> #else
> 	xyz
> #endif

#ifdef _XTABLES_H
init(struct xt_entry_target *t)
#else
init(struct ipt_entry_target *t, unsigned int *nfcache)
#endif

Best regards,

 				Krzysztof Olędzki

Re: iptables version defines

[Krzysztof Oledzki]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 935 bytes --]



On Fri, 30 May 2008, Krzysztof Oledzki wrote:

>
>
> On Fri, 30 May 2008, Thomas Jarosch wrote:
>
>> Hello netfilter coreteam,
>> 
>> I'm currently looking for a way to support ipt_ACCOUNT
>> both for iptables 1.4.0 and iptables 1.4.1. Some function
>> names changed (addr_to_dotted() -> ipaddr_to_numeric())
>> and it would be easy to #ifdef around them.
>> 
>> The linux kernel provides multiple defines for its version number,
>> while iptables only supports a char* version string.
>> 
>> Would it make sense to add something like the kernel version defines
>> so one could write code like this:
>> 
>> #if IPTABLES_VERSION_CODE < IPTABLES_VERSION(1,4,1)
>> 	abc
>> #else
>> 	xyz
>> #endif
>
> #ifdef _XTABLES_H
> init(struct xt_entry_target *t)
> #else
> init(struct ipt_entry_target *t, unsigned int *nfcache)
> #endif

Plese check the attached patch.

Best regards,

 				Krzysztof Olędzki

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1775 bytes --]

--- libipt_ACCOUNT.c	2007-12-14 10:42:16.000000000 +0100
+++ libipt_ACCOUNT.c-new	2008-05-12 23:14:15.000000000 +0200
@@ -30,14 +30,20 @@
 
 /* Initialize the target. */
 static void
+#ifdef _XTABLES_H
+init(struct xt_entry_target *t)
+#else
 init(struct ipt_entry_target *t, unsigned int *nfcache)
+#endif
 {
     struct ipt_acc_info *accountinfo = (struct ipt_acc_info *)t->data;
 
     accountinfo->table_nr = -1;
 
+#ifndef _XTABLES_H
     /* Can't cache this */
     *nfcache |= NFC_UNKNOWN;
+#endif
 }
 
 #define IPT_ACCOUNT_OPT_ADDR 0x01
@@ -47,8 +53,11 @@
    ate an option */
 static int
 parse(int c, char **argv, int invert, unsigned int *flags,
-      const struct ipt_entry *entry,
-      struct ipt_entry_target **target)
+#ifdef _XTABLES_H
+      const void *entry, struct xt_entry_target **target)
+#else
+      const struct ipt_entry *entry, struct ipt_entry_target **target)
+#endif
 {
     struct ipt_acc_info *accountinfo = (struct ipt_acc_info *)(*target)->data;
     struct in_addr *addrs = NULL, mask;
@@ -137,8 +146,13 @@
 
 /* Prints out the targinfo. */
 static void
+#ifdef _XTABLES_H
+print(const void *ip,
+      const struct xt_entry_target *target,
+#else
 print(const struct ipt_ip *ip,
       const struct ipt_entry_target *target,
+#endif
       int numeric)
 {
     print_it (ip, target, 0);
@@ -146,7 +160,13 @@
 
 /* Saves the union ipt_targinfo in parsable form to stdout. */
 static void
-save(const struct ipt_ip *ip, const struct ipt_entry_target *target)
+#ifdef _XTABLES_H
+save(const void *ip,
+     const struct xt_entry_target *target)
+#else
+save(const struct ipt_ip *ip,
+     const struct ipt_entry_target *target)
+#endif
 {
     print_it(ip, target, 1);
 }

Re: iptables version defines

[Thomas Jarosch]

Hi Krzysztof,

On Friday, 30. May 2008 11:56:28 you wrote:
> > #ifdef _XTABLES_H
> > init(struct xt_entry_target *t)
> > #else
> > init(struct ipt_entry_target *t, unsigned int *nfcache)
> > #endif
>
> Plese check the attached patch.

Thanks for the patch! I've developed something similar yesterday, though not 
as elegant as your version. This will solve the issue for iptables < 1.4.0.

The differences between 1.4.0 and the upcoming 1.4.1 are still an issue as 
some function names changed. The version defines could be of help here.

Thomas

Re: iptables version defines

[Jan Engelhardt]

On Friday 2008-05-30 12:06, Thomas Jarosch wrote:
>Hi Krzysztof,
>
>On Friday, 30. May 2008 11:56:28 you wrote:
>> > #ifdef _XTABLES_H
>> > init(struct xt_entry_target *t)
>> > #else
>> > init(struct ipt_entry_target *t, unsigned int *nfcache)
>> > #endif

Woah this is ridiculously ugly. (Remember, such constructs were
just eliminated from the kernel in the past years.)

There is Xtables-addons which provides enough glue so that there is no reason
to play dirty preprocessor tricks like these. xt-a uses a technique where an
extra backwards-API layer is in place that translates the API (mostly
parameter shuffling, etc) in an IMHO perfect fashion.

That's for the kernel part; the same applies to the iptables glue --
of which there is not any yet, because 1.4.0.77 is the minimum required
version because of the newly exported xtables.h, and I had to start
*somewhere*. My suggestion that you follow up on it ;-)

Re: iptables version defines

[Patrick McHardy]

Jan Engelhardt wrote:
> On Friday 2008-05-30 12:06, Thomas Jarosch wrote:
>> Hi Krzysztof,
>>
>> On Friday, 30. May 2008 11:56:28 you wrote:
>>>> #ifdef _XTABLES_H
>>>> init(struct xt_entry_target *t)
>>>> #else
>>>> init(struct ipt_entry_target *t, unsigned int *nfcache)
>>>> #endif
> 
> Woah this is ridiculously ugly. (Remember, such constructs were
> just eliminated from the kernel in the past years.)

I don't care about uglyness as long as it stays in external
code. So if someone sends me a patch to add this version
define, I'll add it.

Re: [patch] iptables version defines

[Patrick McHardy]

Thomas Jarosch wrote:
> Hi Patrick,
> 
>>>>>> #ifdef _XTABLES_H
>>>>>> init(struct xt_entry_target *t)
>>>>>> #else
>>>>>> init(struct ipt_entry_target *t, unsigned int *nfcache)
>>>>>> #endif
>>> Woah this is ridiculously ugly. (Remember, such constructs were
>>> just eliminated from the kernel in the past years.)
>> I don't care about uglyness as long as it stays in external
>> code. So if someone sends me a patch to add this version
>> define, I'll add it.
> 
> External code has to be "ugly" if you want to keep the user experience high.
> I don't feel like breaking ipt_ACCOUNT for older iptables versions without
> any real gain, it should work out of the box with iptables 1.4.0 and 1.4.1.
> 
> Attached is a patch to add the new defines. The macro XTABLES_VERSION is already in use, so I named it XTABLES_VERSION_CHECK. I've also tested
> that an empty XTABLES_VERSION_EXTRA in configure.ac works.
> 
> Now we can write code like this:
> 
> #if XTABLES_VERSION_CODE < XTABLES_VERSION_CHECK(1,5,0)
>     #warning You are obselete and will be assimilated.
> #endif


Looks good to me - I'll let it sit on netfilter-devel until tonight
though since my auto* knowlegde is close to zero :)

Re: iptables version defines

[Krzysztof Oledzki]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 983 bytes --]



On Fri, 30 May 2008, Krzysztof Oledzki wrote:

>
>
> On Fri, 30 May 2008, Thomas Jarosch wrote:
>
>> Hello netfilter coreteam,
>> 
>> I'm currently looking for a way to support ipt_ACCOUNT
>> both for iptables 1.4.0 and iptables 1.4.1. Some function
>> names changed (addr_to_dotted() -> ipaddr_to_numeric())
>> and it would be easy to #ifdef around them.
>> 
>> The linux kernel provides multiple defines for its version number,
>> while iptables only supports a char* version string.
>> 
>> Would it make sense to add something like the kernel version defines
>> so one could write code like this:
>> 
>> #if IPTABLES_VERSION_CODE < IPTABLES_VERSION(1,4,1)
>> 	abc
>> #else
>> 	xyz
>> #endif
>
> #ifdef _XTABLES_H
> init(struct xt_entry_target *t)
> #else
> init(struct ipt_entry_target *t, unsigned int *nfcache)
> #endif

Bzzz, sorry. Just noticed that you had asked about 1.4.0->1.4.1, not 
1.3->1.4.

Best regards,

 			Krzysztof Olędzki