[PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Andy Whitcroft]

It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
share constants include the corresponding include/netfilter/xp_*.h files.
Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
the norm.

Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 include/linux/netfilter_ipv4/ipt_policy.h  |    2 ++
 include/linux/netfilter_ipv6/ip6t_policy.h |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/netfilter_ipv4/ipt_policy.h b/include/linux/netfilter_ipv4/ipt_policy.h
index b9478a2..1037fb2 100644
--- a/include/linux/netfilter_ipv4/ipt_policy.h
+++ b/include/linux/netfilter_ipv4/ipt_policy.h
@@ -1,6 +1,8 @@
 #ifndef _IPT_POLICY_H
 #define _IPT_POLICY_H
 
+#include <linux/netfilter/xt_policy.h>
+
 #define IPT_POLICY_MAX_ELEM		XT_POLICY_MAX_ELEM
 
 /* ipt_policy_flags */
diff --git a/include/linux/netfilter_ipv6/ip6t_policy.h b/include/linux/netfilter_ipv6/ip6t_policy.h
index 6bab316..b1c449d 100644
--- a/include/linux/netfilter_ipv6/ip6t_policy.h
+++ b/include/linux/netfilter_ipv6/ip6t_policy.h
@@ -1,6 +1,8 @@
 #ifndef _IP6T_POLICY_H
 #define _IP6T_POLICY_H
 
+#include <linux/netfilter/xt_policy.h>
+
 #define IP6T_POLICY_MAX_ELEM		XT_POLICY_MAX_ELEM
 
 /* ip6t_policy_flags */
-- 
1.6.0.4.911.gc990

Re: [PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Patrick McHardy]

Andy Whitcroft wrote:
> It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
> share constants include the corresponding include/netfilter/xp_*.h files.
> Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
> the norm.

Does this actually fix a bug, or is it just for added consistency?

Re: [PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Jan Engelhardt]

On Wednesday 2008-11-19 19:08, Patrick McHardy wrote:

> Andy Whitcroft wrote:
>> It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
>> share constants include the corresponding include/netfilter/xp_*.h files.
>> Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
>> the norm.
>
> Does this actually fix a bug, or is it just for added consistency?

This can only be consistency, because every compile error would
be easy to fix by inclusion of xt_policy.h in the corresponding
.c file.
Which reminds me, when can we get rid of the ipt_*h files?

Re: [PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2008-11-19 19:08, Patrick McHardy wrote:
> 
>> Andy Whitcroft wrote:
>>> It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
>>> share constants include the corresponding include/netfilter/xp_*.h files.
>>> Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
>>> the norm.
>> Does this actually fix a bug, or is it just for added consistency?
> 
> This can only be consistency, because every compile error would
> be easy to fix by inclusion of xt_policy.h in the corresponding
> .c file.

It would also be easily fixed by this patch. Lets see what Andy
has to say.

> Which reminds me, when can we get rid of the ipt_*h files?

There is external code using these headers, so not at this point.
We could schedule them for removal, but I don't see point, keeping
those headers don't cause any maintenance work.

Re: [PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Andy Whitcroft]

On Wed, Nov 19, 2008 at 07:08:17PM +0100, Patrick McHardy wrote:
> Andy Whitcroft wrote:
>> It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
>> share constants include the corresponding include/netfilter/xp_*.h files.
>> Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
>> the norm.
>
> Does this actually fix a bug, or is it just for added consistency?

It was reported by an Ubuntu user who was compiling against them.  From
my point of view it seemed clearly inconsistant and therefore most likely
wrong.  So it seemed reasonable to fix it and push it upstream, if
there was a reason I was sure you'd soon put me straight.

-apw

Re: [PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Patrick McHardy]

Andy Whitcroft wrote:
> On Wed, Nov 19, 2008 at 07:08:17PM +0100, Patrick McHardy wrote:
>> Andy Whitcroft wrote:
>>> It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
>>> share constants include the corresponding include/netfilter/xp_*.h files.
>>> Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
>>> the norm.
>> Does this actually fix a bug, or is it just for added consistency?
> 
> It was reported by an Ubuntu user who was compiling against them.  From
> my point of view it seemed clearly inconsistant and therefore most likely
> wrong.  So it seemed reasonable to fix it and push it upstream, if
> there was a reason I was sure you'd soon put me straight.

I'm mainly asking in order to decide whether to push it for
2.7.28 or 2.6.29. So did the user report a compilation error
or something like that?

Re: [PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Andy Whitcroft]

On Thu, Nov 20, 2008 at 10:47:05AM +0100, Patrick McHardy wrote:
> Andy Whitcroft wrote:
>> On Wed, Nov 19, 2008 at 07:08:17PM +0100, Patrick McHardy wrote:
>>> Andy Whitcroft wrote:
>>>> It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
>>>> share constants include the corresponding include/netfilter/xp_*.h files.
>>>> Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
>>>> the norm.
>>> Does this actually fix a bug, or is it just for added consistency?
>>
>> It was reported by an Ubuntu user who was compiling against them.  From
>> my point of view it seemed clearly inconsistant and therefore most likely
>> wrong.  So it seemed reasonable to fix it and push it upstream, if
>> there was a reason I was sure you'd soon put me straight.
>
> I'm mainly asking in order to decide whether to push it for
> 2.7.28 or 2.6.29. So did the user report a compilation error
> or something like that?

We do have a bug open from an Ubuntu user who seems have hit the issue,
but details are scant, they did not report specifics of their use case.
I don't see it being particularly urgent, the work around is pretty simple
as I see it.  So I don't think there is any need to jump hoops to get it
into .28, we are pretty late in the cycle on that one.  For me knowing
its going to be upstream in .29 or wherever allows me to report that back
to the user.  If they are really insistant we can always pull the change
into our kernel as we won't have to carry it forever, though that is not
likely to be necessary.

-apw

Re: [PATCH 1/1] netfilter: ip{,6}t_policy.h should include xp_policy.h

[Patrick McHardy]

Andy Whitcroft wrote:
> On Thu, Nov 20, 2008 at 10:47:05AM +0100, Patrick McHardy wrote:
>> Andy Whitcroft wrote:
>>> On Wed, Nov 19, 2008 at 07:08:17PM +0100, Patrick McHardy wrote:
>>>> Andy Whitcroft wrote:
>>>>> It seems that all of the include/netfilter_{ipv4,ipv6}/{ipt,ip6t}_*.h which
>>>>> share constants include the corresponding include/netfilter/xp_*.h files.
>>>>> Neither ipt_policy.h not ip6t_policy.h do.  Make these consistant with
>>>>> the norm.
>>>> Does this actually fix a bug, or is it just for added consistency?
>>> It was reported by an Ubuntu user who was compiling against them.  From
>>> my point of view it seemed clearly inconsistant and therefore most likely
>>> wrong.  So it seemed reasonable to fix it and push it upstream, if
>>> there was a reason I was sure you'd soon put me straight.
>> I'm mainly asking in order to decide whether to push it for
>> 2.7.28 or 2.6.29. So did the user report a compilation error
>> or something like that?
> 
> We do have a bug open from an Ubuntu user who seems have hit the issue,
> but details are scant, they did not report specifics of their use case.
> I don't see it being particularly urgent, the work around is pretty simple
> as I see it.  So I don't think there is any need to jump hoops to get it
> into .28, we are pretty late in the cycle on that one.  For me knowing
> its going to be upstream in .29 or wherever allows me to report that back
> to the user.  If they are really insistant we can always pull the change
> into our kernel as we won't have to carry it forever, though that is not
> likely to be necessary.

OK, thanks for the information. Queued for 2.6.29.