2.6.12: connection tracking broken?

2.6.12: connection tracking broken?

[Chris Rankin]

Hi,

I have just tried upgrading my firewall to 2.6.12, but neither of the following rules in my
FORWARD table was allowing return traffic:

 1109  814K ACCEPT     all  --  ppp0   br0     anywhere             anywhere         ctstate
RELATED,ESTABLISHED
  11M   13G ACCEPT     all  --  ppp0   br0     anywhere             anywhere         state
RELATED,ESTABLISHED

I have currently returned to using 2.6.11.11, where the identical configuration works fine. br0 is
a bridge device containing two e100 devices, and ppp0 is my PPPoE DSL link. I am using iptables
1.3.1.

Cheers,
Chris



	
	
		
___________________________________________________________ 
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com

Re: 2.6.12: connection tracking broken?

[Jan Engelhardt]

>Hi,
>
>I have just tried upgrading my firewall to 2.6.12, but neither of the following rules in my
>FORWARD table was allowing return traffic:

You forget about INPUT and OUTPUT. If you drop everything in INPUT, there's 
nothing to FORWARD.

> 1109  814K ACCEPT     all  --  ppp0   br0     anywhere             anywhere         ctstate
>RELATED,ESTABLISHED
>  11M   13G ACCEPT     all  --  ppp0   br0     anywhere             anywhere         state
>RELATED,ESTABLISHED
>
>I have currently returned to using 2.6.11.11, where the identical configuration works fine. br0 is
>a bridge device containing two e100 devices, and ppp0 is my PPPoE DSL link. I am using iptables
>1.3.1.


Jan Engelhardt                                                               
--                                                                            
| Gesellschaft fuer Wissenschaftliche Datenverarbeitung Goettingen,
| Am Fassberg, 37077 Goettingen, www.gwdg.de

Re: 2.6.12: connection tracking broken?

[Tobias DiPasquale]

On 6/18/05, Jan Engelhardt <jengelh@linux01.gwdg.de> wrote:
> >I have just tried upgrading my firewall to 2.6.12, but neither of the following rules in my
> >FORWARD table was allowing return traffic:
> 
> You forget about INPUT and OUTPUT. If you drop everything in INPUT, there's
> nothing to FORWARD.

No. INPUT/OUTPUT rules have nothing to do with FORWARDed traffic,
since a packet is either locally destined (INPUT), locally originated
(OUTPUT) or being forwarded (FORWARD).

> > 1109  814K ACCEPT     all  --  ppp0   br0     anywhere             anywhere         ctstate
> >RELATED,ESTABLISHED
> >  11M   13G ACCEPT     all  --  ppp0   br0     anywhere             anywhere         state
> >RELATED,ESTABLISHED
> >
> >I have currently returned to using 2.6.11.11, where the identical configuration works fine. br0 is
> >a bridge device containing two e100 devices, and ppp0 is my PPPoE DSL link. I am using iptables
> >1.3.1.

Did you have /proc/sys/net/ipv4/ip_forward turned on?

-- 
[ Tobias DiPasquale ]
0x636f6465736c696e67657240676d61696c2e636f6d

Re: 2.6.12: connection tracking broken?

[Chris Rankin]

--- Tobias DiPasquale <codeslinger@gmail.com> wrote:
> Did you have /proc/sys/net/ipv4/ip_forward turned on?

Yes, because otherwise it wouldn't work in 2.6.11.11 either. My userspace environment is identical
between 2.6.12 an 2.6.11.11, but the RELATED and ESTABLISHED packets aren't forwarded across the
bridge in 2.6.12.

And no, I haven't tried recompiling my userspace iptables tools. Why would this make any
difference? "iptables -L -v" reports exactly what I expect to see.

Cheers,
Chris



		
___________________________________________________________ 
How much free photo storage do you get? Store your holiday 
snaps for FREE with Yahoo! Photos http://uk.photos.yahoo.com

Re: 2.6.12: connection tracking broken?

[Santiago Garcia Mantinan]

> I have just tried upgrading my firewall to 2.6.12, but neither of the
> following rules in my FORWARD table was allowing return traffic:

This seems to happen only if you use bridge interfaces, as you said it is
something related to connection tracking otherwise netfilter seems to work
ok.

I have sent this right now to the bridge list, I'm copying it here so that
more info is available about this bug.


---------------------------------------------------------------------------
Hi!

As noted by Chris Rankin on a mail to netfilter-devel and to the
linux-kernel mailing list (subject: 2.6.12: connection tracking broken?),
there is a problem with the connection tracking of iptables when one of the
interfaces is a bridge.

On my tests here I have setup a connection between two machines using a real
interface (eth0) and then the same setup using a bridge interface (br0) to
which that interface had been enslaved:

The setup had modules ipt_LOG, ipt_state, ip_conntrack, iptable_filter and
ip_tables loaded, as well as bridge and I loaded a simple set of rules,
exactly the same set each time but changing the interface name, I'll just
write the setup for br0, but the setup was the same one for eth0 with that
little change:

iptables -A INPUT -i br0 -m state --state RELATED,ESTABLISHED -j ACCEPT
iptables -A INPUT -i br0 -j LOG --log-level 7 --log-prefix "NOTESTABLISHED "
iptables -A INPUT -i br0 -j DROP

this set of rules with eth0 on them worked ok when I tried to telnet a port
on a remote machine (192.168.0.1) from the local machine (192.168.0.2),
concretelly the test was a telnet to port 22 where the ssh daemon was
listening. However, when I did the same test using the br0 interface, I got
this logged:

NOTESTABLISHED IN=br0 OUT= PHYSIN=eth0
MAC=00:50:ba:54:39:8c:00:48:54:6a:58:90:08:00 SRC=192.168.0.1
DST=192.168.0.2 LEN=60 TOS=0x00 PREC=0x00 TTL=64 ID=0 DF PROTO=TCP SPT=22
DPT=48448 WINDOW=5792 RES=0x00 ACK SYN URGP=0 

doing a grep for 192.168.0.1 on /proc/net/ip_conntrack* returned nothing,
however netstat showed the connection:
tcp        0      1 192.168.0.2:48448       192.168.0.1:22          SYN_SENT

I believe that iptables works ok execept for the connection tracking, but I
have not tested this fully.

Machines were I tried this were a Pentium III and an AMD K6II both with
kernel 2.6.12, I know this is happening at least from RC5, but at that time
I didn't have the time to check and I thought it was due to the kernel
bridge firewall being loaded, the tests I did today with 2.6.12 final didn't
have the kernel bridge firewall, just normal bridge and normal iptables.

If you need any other info to check this just ask for it.

Regards...
-- 
Manty/BestiaTester -> http://manty.net

Re: 2.6.12: connection tracking broken?

[Santiago Garcia Mantinan]

> I have sent this right now to the bridge list, I'm copying it here so that
> more info is available about this bug.

I have selected patches from 2.6.12 that I thought could be related to this
issue, and I have finaly identified this patch...

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b31e5b1bb53b99dfd5e890aa07e943aff114ae1c

as the patch causing the problem, I have reversed it on my kernel tree and
now the firewall is working again.

I have not really looked at what the patch does and how it does that, I have
just identified it as the one causing the break of this connection tracking
relating to the bridges.

It seems this is affecting more stuff than the connection tracking on
bridges, I have a friend also suffering problems relating to the firewall in
2.6.12 and nothing to do with the bridge, but he is not around now so I
cannot confirm it is due to this patch also. His problem may be something
related to loopback. I'll try to contact him tomorrow and tell him to test
with this patch reversed.

Regards...
-- 
Manty/BestiaTester -> http://manty.net

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

Santiago Garcia Mantinan wrote:
>>I have sent this right now to the bridge list, I'm copying it here so that
>>more info is available about this bug.
> 
> 
> I have selected patches from 2.6.12 that I thought could be related to this
> issue, and I have finaly identified this patch...
> 
> http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b31e5b1bb53b99dfd5e890aa07e943aff114ae1c
> 
> as the patch causing the problem, I have reversed it on my kernel tree and
> now the firewall is working again.
> 
> I have not really looked at what the patch does and how it does that, I have
> just identified it as the one causing the break of this connection tracking
> relating to the bridges.

The patch drops the conntrack reference when a packet leaves IP to avoid
problems with module unload because of indefinitely queued packets.
The bridge-netfilter code defers calling of some NF_IP_* hooks to the
bridge layer, when the conntrack reference is already gone, so the entry
is neither confirmed (enters the hashtable) nor available for use by
matches or targets. Reverting the patch is not a good option, I'll look
into other possiblities.

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Herbert Xu]

Patrick McHardy <kaber@trash.net> wrote:
>
> The bridge-netfilter code defers calling of some NF_IP_* hooks to the
> bridge layer, when the conntrack reference is already gone, so the entry

Why does it defer them at all? Shouldn't the fact that the device is
bridged be transparent to the IP layer?

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: 2.6.12: connection tracking broken?

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 20 Jun 2005 10:05:42 +1000

> Patrick McHardy <kaber@trash.net> wrote:
> >
> > The bridge-netfilter code defers calling of some NF_IP_* hooks to the
> > bridge layer, when the conntrack reference is already gone, so the entry
> 
> Why does it defer them at all? Shouldn't the fact that the device is
> bridged be transparent to the IP layer?

The bridge netfilter layer uses netif_rx(skb) at the deepest level in
order to avoid too deep stack usage.

This is also why the NF_HOOK*() macros were semantically changed
a little bit several months ago.

Re: 2.6.12: connection tracking broken?

[Herbert Xu]

On Sun, Jun 19, 2005 at 05:18:13PM -0700, David S. Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Mon, 20 Jun 2005 10:05:42 +1000
>
> > Why does it defer them at all? Shouldn't the fact that the device is
> > bridged be transparent to the IP layer?
> 
> The bridge netfilter layer uses netif_rx(skb) at the deepest level in
> order to avoid too deep stack usage.

Sorry, but I don't see the connection between this and deferring
NF_IP_* hooks on the transmit path.

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: 2.6.12: connection tracking broken?

[Patrick McHardy]

On Mon, 20 Jun 2005, Herbert Xu wrote:
> Patrick McHardy <kaber@trash.net> wrote:
>>
>> The bridge-netfilter code defers calling of some NF_IP_* hooks to the
>> bridge layer, when the conntrack reference is already gone, so the entry
>
> Why does it defer them at all? Shouldn't the fact that the device is
> bridged be transparent to the IP layer?

I couldn't figure out the reason, it seems to have something to do
with setting up device pointers for iptables and ebtables. It looks
like the only way to fix this problem without keeping the conntrack
reference while packets are queued at the device is to avoid defering
the NF_IP_* hooks. Bart, can you explain why the hooks are defered
please?

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Bart De Schuymer]

Op ma, 20-06-2005 te 04:45 +0200, schreef Patrick McHardy:
> On Mon, 20 Jun 2005, Herbert Xu wrote:
> > Patrick McHardy <kaber@trash.net> wrote:
> >>
> >> The bridge-netfilter code defers calling of some NF_IP_* hooks to the
> >> bridge layer, when the conntrack reference is already gone, so the entry
> >
> > Why does it defer them at all? Shouldn't the fact that the device is
> > bridged be transparent to the IP layer?
> 
> I couldn't figure out the reason, it seems to have something to do
> with setting up device pointers for iptables and ebtables. It looks
> like the only way to fix this problem without keeping the conntrack
> reference while packets are queued at the device is to avoid defering
> the NF_IP_* hooks. Bart, can you explain why the hooks are defered
> please?

This is done so that iptables knows which bridge port the output device
is, using the iptables physdev match.

Can't you release the conntrack reference with a function registered on
the POSTROUTING hook with a prio higher than nat POSTROUTING (or
something like that)?

cheers,
Bart

Re: 2.6.12: connection tracking broken?

[Harald Welte]

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

On Sat, Jun 18, 2005 at 04:57:49PM +0200, Jan Engelhardt wrote:
> You forget about INPUT and OUTPUT. If you drop everything in INPUT, there's 
> nothing to FORWARD.

he was talking about iptables, not ipchains.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

Bart De Schuymer wrote:
> Op ma, 20-06-2005 te 04:45 +0200, schreef Patrick McHardy:
> 
>> Bart, can you explain why the hooks are defered please?
> 
> This is done so that iptables knows which bridge port the output device
> is, using the iptables physdev match.

In which cases is this necessary? AFAICT the output device is determined
in br_handle_frame_finish() for a normally bridged packet.

> Can't you release the conntrack reference with a function registered on
> the POSTROUTING hook with a prio higher than nat POSTROUTING (or
> something like that)?

We would have to hold the reference while the packet is queued at the
device for the bridge case, which we want to avoid.

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Bart De Schuymer]

Op ma, 20-06-2005 te 14:15 +0200, schreef Patrick McHardy:
> Bart De Schuymer wrote:
> > Op ma, 20-06-2005 te 04:45 +0200, schreef Patrick McHardy:
> > 
> >> Bart, can you explain why the hooks are defered please?
> > 
> > This is done so that iptables knows which bridge port the output device
> > is, using the iptables physdev match.
> 
> In which cases is this necessary? AFAICT the output device is determined
> in br_handle_frame_finish() for a normally bridged packet.

When the _routing_ decision sends the packet to br0 (a bridge device),
it is unknown which bridge port(s) the packet will be sent out. This is
only known after the packet enters the bridge code. Therefore, for
iptables to know the bridge port out device, the hooks must be postponed
until in the bridge code.

> > Can't you release the conntrack reference with a function registered on
> > the POSTROUTING hook with a prio higher than nat POSTROUTING (or
> > something like that)?
> 
> We would have to hold the reference while the packet is queued at the
> device for the bridge case, which we want to avoid.

Trust me, people will complain if they can no longer use the physdev
match for routed packets.
People using a bridging firewall will just have to live with the fact
that the reference is held until in the bridge code.

cheers,
Bart

Re: 2.6.12: connection tracking broken?

[Phil Oester]

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

The changes were originally made to fix conntrack unload problems with
raw sockets.  My original patch performed the nf_reset in the socket
code, but Patrick suggested moving it to ip_output.  The below patch
reverts the ip_output changes, and implements the original suggested
changes to raw socket handling.  

While this is unlikely to be the permanent solution, it will fix the
current bridging problems while retaining the raw socket fixes.  I'd
suggest that this could be included in -stable while researching
other solutions.

Phil

Signed-off-by: Phil Oester <kernel@linuxace.com>



[-- Attachment #2: patch-fixbridge --]
[-- Type: text/plain, Size: 1938 bytes --]

diff -purN linux-orig/net/ipv4/ip_output.c linux-new/net/ipv4/ip_output.c
--- linux-orig/net/ipv4/ip_output.c	2005-06-17 15:48:29.000000000 -0400
+++ linux-new/net/ipv4/ip_output.c	2005-06-20 14:47:58.000000000 -0400
@@ -196,8 +196,6 @@ static inline int ip_finish_output2(stru
 	nf_debug_ip_finish_output2(skb);
 #endif /*CONFIG_NETFILTER_DEBUG*/
 
-	nf_reset(skb);
-
 	if (hh) {
 		int hh_alen;
 
diff -purN linux-orig/net/ipv4/netfilter/ip_conntrack_standalone.c linux-new/net/ipv4/netfilter/ip_conntrack_standalone.c
--- linux-orig/net/ipv4/netfilter/ip_conntrack_standalone.c	2005-06-17 15:48:29.000000000 -0400
+++ linux-new/net/ipv4/netfilter/ip_conntrack_standalone.c	2005-06-20 14:47:58.000000000 -0400
@@ -432,6 +432,13 @@ static unsigned int ip_conntrack_defrag(
 				        const struct net_device *out,
 				        int (*okfn)(struct sk_buff *))
 {
+#if !defined(CONFIG_IP_NF_NAT) && !defined(CONFIG_IP_NF_NAT_MODULE)
+	/* Previously seen (loopback)?  Ignore.  Do this before
+           fragment check. */
+	if ((*pskb)->nfct)
+		return NF_ACCEPT;
+#endif
+
 	/* Gather fragments. */
 	if ((*pskb)->nh.iph->frag_off & htons(IP_MF|IP_OFFSET)) {
 		*pskb = ip_ct_gather_frags(*pskb,
diff -purN linux-orig/net/packet/af_packet.c linux-new/net/packet/af_packet.c
--- linux-orig/net/packet/af_packet.c	2005-06-17 15:48:29.000000000 -0400
+++ linux-new/net/packet/af_packet.c	2005-06-20 14:48:38.000000000 -0400
@@ -274,6 +274,9 @@ static int packet_rcv_spkt(struct sk_buf
 	dst_release(skb->dst);
 	skb->dst = NULL;
 
+	/* drop conntrack reference */
+	nf_reset(skb);
+
 	spkt = (struct sockaddr_pkt*)skb->cb;
 
 	skb_push(skb, skb->data-skb->mac.raw);
@@ -517,6 +520,9 @@ static int packet_rcv(struct sk_buff *sk
 	dst_release(skb->dst);
 	skb->dst = NULL;
 
+	/* drop conntrack reference */
+	nf_reset(skb);
+
 	spin_lock(&sk->sk_receive_queue.lock);
 	po->stats.tp_packets++;
 	__skb_queue_tail(&sk->sk_receive_queue, skb);

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

Bart De Schuymer wrote:
> When the _routing_ decision sends the packet to br0 (a bridge device),
> it is unknown which bridge port(s) the packet will be sent out. This is
> only known after the packet enters the bridge code. Therefore, for
> iptables to know the bridge port out device, the hooks must be postponed
> until in the bridge code.

This seems to be a really bad idea, for a single match that violates
layering we need to deal with this inconsistency. It's not just the
conntrack reference, with IPsec the packet passed to the defered hooks
is totally different from when it was still inside IP, which for example
breaks the policy match.

> Trust me, people will complain if they can no longer use the physdev
> match for routed packets.

I'm sure they will, now that they got used to it. Why can't people
match on the bridge port inside ebtables and only match on the device
within iptables? Is there a case that can't be handled this way?

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

Phil Oester wrote:
> The changes were originally made to fix conntrack unload problems with
> raw sockets.  My original patch performed the nf_reset in the socket
> code, but Patrick suggested moving it to ip_output.  The below patch
> reverts the ip_output changes, and implements the original suggested
> changes to raw socket handling.  
> 
> While this is unlikely to be the permanent solution, it will fix the
> current bridging problems while retaining the raw socket fixes.  I'd
> suggest that this could be included in -stable while researching
> other solutions.

I disagree. We will probably have a final solution within a few days,
so I don't think we need to submit "temporary" fixes yet. Your patch
actually causes a regression for people not using bringe-netfilter,
unplugging a network cable can cause the conntrack module to become
unloadable again.

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Bart De Schuymer]

Op di, 21-06-2005 te 01:22 +0200, schreef Patrick McHardy:
> This seems to be a really bad idea, for a single match that violates
> layering we need to deal with this inconsistency. It's not just the
> conntrack reference, with IPsec the packet passed to the defered hooks
> is totally different from when it was still inside IP, which for example
> breaks the policy match.

I agree it is annoying.

> > Trust me, people will complain if they can no longer use the physdev
> > match for routed packets.
> 
> I'm sure they will, now that they got used to it. Why can't people
> match on the bridge port inside ebtables and only match on the device
> within iptables? Is there a case that can't be handled this way?

ebtables doesn't have all the targets/matches that iptables has.
Perhaps a rule structure using marking can always be used so that the
ACCEPT/DROP can be deferred until inside ebtables, I don't know if that
will always be possible though.

Deferring the hooks makes the bridge-nf code alot more complicated, so I
would be glad to get rid of it if it is the right thing to do. But
backwards compatibility can't be maintained and I'd be surprised if
every ruleset that now works will still be possible using an
iptables/ebtables scheme.
It has worked fine in the past and I see no reason why to stop making it
work now because of some recently found and unrelated referencing
problem.
Of course, if the netfilter people decide it should be removed, then so
be it.

cheers,
Bart

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

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

Bart De Schuymer wrote:
> Deferring the hooks makes the bridge-nf code alot more complicated, so I
> would be glad to get rid of it if it is the right thing to do. But
> backwards compatibility can't be maintained and I'd be surprised if
> every ruleset that now works will still be possible using an
> iptables/ebtables scheme.

I unfortunately don't see a way to remove it, but we should keep
thinking about it. Can you please check if the attached patch is
correct? It should exclude all packets handled by bridge-netfilter
from having their conntrack reference dropped. I didn't add nf_reset()'s
to the bridging code because with tc actions the packets can end up
anywhere else anyway, and this will hopefully get fixed right sometime.

BTW. this line from ip_sabotage_out() looks wrong, it will clear all
flags instead of setting the BRNF_DONT_TAKE_PARENT flag (second
patch):

                        nf_bridge->mask &= BRNF_DONT_TAKE_PARENT;

Signed-off-by: Patrick McHardy <kaber@trash.net>

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 448 bytes --]

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -188,7 +188,12 @@ static inline int ip_finish_output2(stru
 		skb = skb2;
 	}
 
-	nf_reset(skb);
+#ifdef CONFIG_BRIDGE_NETFILTER
+	/* bridge-netfilter defers calling some IP hooks to the bridge layer and
+	 * still needs the conntrack reference */
+	if (skb->nf_bridge == NULL)
+#endif
+		nf_reset(skb);
 
 	if (hh) {
 		int hh_alen;

[-- Attachment #3: 10.diff --]
[-- Type: text/x-patch, Size: 573 bytes --]

diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -882,7 +882,7 @@ static unsigned int ip_sabotage_out(unsi
 		 * doesn't use the bridge parent of the indev by using
 		 * the BRNF_DONT_TAKE_PARENT mask. */
 		if (hook == NF_IP_FORWARD && nf_bridge->physindev == NULL) {
-			nf_bridge->mask &= BRNF_DONT_TAKE_PARENT;
+			nf_bridge->mask |= BRNF_DONT_TAKE_PARENT;
 			nf_bridge->physindev = (struct net_device *)in;
 		}
 #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)

Re: 2.6.12: connection tracking broken?

[Bart De Schuymer]

Op di, 21-06-2005 te 17:16 +0200, schreef Patrick McHardy:
> I unfortunately don't see a way to remove it, but we should keep
> thinking about it. Can you please check if the attached patch is
> correct? It should exclude all packets handled by bridge-netfilter
> from having their conntrack reference dropped. I didn't add nf_reset()'s
> to the bridging code because with tc actions the packets can end up
> anywhere else anyway, and this will hopefully get fixed right sometime.

Looks good.
Perhaps a compile time option to disable postponing the hooks would be
nice...

> BTW. this line from ip_sabotage_out() looks wrong, it will clear all
> flags instead of setting the BRNF_DONT_TAKE_PARENT flag (second
> patch):
> 
>                         nf_bridge->mask &= BRNF_DONT_TAKE_PARENT;

Thanks,
Bart

Re: 2.6.12: connection tracking broken?

[Chris Wright]

* Bart De Schuymer (bdschuym@pandora.be) wrote:
> Op di, 21-06-2005 te 17:16 +0200, schreef Patrick McHardy:
> > I unfortunately don't see a way to remove it, but we should keep
> > thinking about it. Can you please check if the attached patch is
> > correct? It should exclude all packets handled by bridge-netfilter
> > from having their conntrack reference dropped. I didn't add nf_reset()'s
> > to the bridging code because with tc actions the packets can end up
> > anywhere else anyway, and this will hopefully get fixed right sometime.
> 
> Looks good.

Is this one good for -stable?

thanks,
-chris

Re: 2.6.12: connection tracking broken?

[David S. Miller]

From: Chris Wright <chrisw@osdl.org>
Subject: Re: 2.6.12: connection tracking broken?
Date: Tue, 21 Jun 2005 14:23:17 -0700

> * Bart De Schuymer (bdschuym@pandora.be) wrote:
> > Op di, 21-06-2005 te 17:16 +0200, schreef Patrick McHardy:
> > > I unfortunately don't see a way to remove it, but we should keep
> > > thinking about it. Can you please check if the attached patch is
> > > correct? It should exclude all packets handled by bridge-netfilter
> > > from having their conntrack reference dropped. I didn't add nf_reset()'s
> > > to the bridging code because with tc actions the packets can end up
> > > anywhere else anyway, and this will hopefully get fixed right sometime.
> > 
> > Looks good.
> 
> Is this one good for -stable?

We will push it to stable@kernel.org if we deem it should.

I do review the stream going into 2.6.x, and decide if it
should be bound for -stable as well, so you never need to
inquire about this specifically.

Re: 2.6.12: connection tracking broken?

[Chris Wright]

* David S. Miller (davem@davemloft.net) wrote:
> We will push it to stable@kernel.org if we deem it should.
> 
> I do review the stream going into 2.6.x, and decide if it
> should be bound for -stable as well, so you never need to
> inquire about this specifically.

Great, much appreciated.

thanks,
-chris

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

David S. Miller wrote:
>>Is this one good for -stable?
> 
> We will push it to stable@kernel.org if we deem it should.

I would like to get confirmation from someone affected by this
bug, after that I think it should go in -stable. Chris, could
you give it a try?

Thanks
Patrick

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

Bart De Schuymer wrote:
> Op di, 21-06-2005 te 17:16 +0200, schreef Patrick McHardy:
> 
>>I unfortunately don't see a way to remove it, but we should keep
>>thinking about it. Can you please check if the attached patch is
>>correct? It should exclude all packets handled by bridge-netfilter
>>from having their conntrack reference dropped. I didn't add nf_reset()'s
>>to the bridging code because with tc actions the packets can end up
>>anywhere else anyway, and this will hopefully get fixed right sometime.
> 
> Looks good.

Thanks.

> Perhaps a compile time option to disable postponing the hooks would be
> nice...

I think we want to reduce the number of possible paths to ideally one,
not make them a compile-time option.

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Herbert Xu]

On Tue, Jun 21, 2005 at 05:16:05PM +0200, Patrick McHardy wrote:
> Bart De Schuymer wrote:
> > Deferring the hooks makes the bridge-nf code alot more complicated, so I
> > would be glad to get rid of it if it is the right thing to do. But
> > backwards compatibility can't be maintained and I'd be surprised if
> > every ruleset that now works will still be possible using an
> > iptables/ebtables scheme.
> 
> I unfortunately don't see a way to remove it, but we should keep
> thinking about it. Can you please check if the attached patch is
> correct? It should exclude all packets handled by bridge-netfilter
> from having their conntrack reference dropped. I didn't add nf_reset()'s
> to the bridging code because with tc actions the packets can end up
> anywhere else anyway, and this will hopefully get fixed right sometime.

I think this patch will be fine for 2.6.12.*.

Longer term though we should obsolete the ipt_physdev module.  The
rationale there is that this creates a precedence that we can't
possibly maintain in a consistent way.  For example, we don't have
a target that matches by hardware MAC address.  If you wanted to
do that, you'd hook into the arptables interface rather than deferring
iptables after the creation of the hardware header.

This can be done in two stages to minimise pain for people already
using it:

1) We rewrite ipt_physdev to do the lookups necessary to get the output
physical devices through the bridge layer.  Of course this may not be
the real output device due to changes in the environment.  So this should
be accompanied with a warning that users should switch to ebt.

2) We remove the iptables deferring since ipt_physdev will no longer need
it.

3) After a set period (say a year or so) we remove ipt_physdev altogether.

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: 2.6.12: connection tracking broken?

[Chris Rankin]

--- Patrick McHardy <kaber@trash.net> wrote:
> I would like to get confirmation from someone affected by this
> bug, after that I think it should go in -stable. Chris, could
> you give it a try?

I trust you're talking about the following patches?

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -188,7 +188,12 @@ static inline int ip_finish_output2(stru
                skb = skb2;
        }

-       nf_reset(skb);
+#ifdef CONFIG_BRIDGE_NETFILTER
+       /* bridge-netfilter defers calling some IP hooks to the bridge layer and
+        * still needs the conntrack reference */
+       if (skb->nf_bridge == NULL)
+#endif
+               nf_reset(skb);

        if (hh) {
                int hh_alen;

diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -882,7 +882,7 @@ static unsigned int ip_sabotage_out(unsi
                 * doesn't use the bridge parent of the indev by using
                 * the BRNF_DONT_TAKE_PARENT mask. */
                if (hook == NF_IP_FORWARD && nf_bridge->physindev == NULL) {
-                       nf_bridge->mask &= BRNF_DONT_TAKE_PARENT;
+                       nf_bridge->mask |= BRNF_DONT_TAKE_PARENT;
                        nf_bridge->physindev = (struct net_device *)in;
                }
 #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)

I have just installed them, and my bridging firewall is working again with 2.6.12.

Thanks,
Chris



		
___________________________________________________________ 
How much free photo storage do you get? Store your holiday 
snaps for FREE with Yahoo! Photos http://uk.photos.yahoo.com

Re: 2.6.12: connection tracking broken?

[Carl-Daniel Hailfinger]

Herbert Xu schrieb:
> 
> Longer term though we should obsolete the ipt_physdev module.  The
> rationale there is that this creates a precedence that we can't
> possibly maintain in a consistent way.  For example, we don't have
> a target that matches by hardware MAC address.  If you wanted to
> do that, you'd hook into the arptables interface rather than deferring
> iptables after the creation of the hardware header.
> 
> This can be done in two stages to minimise pain for people already
> using it:
> 
> 1) We rewrite ipt_physdev to do the lookups necessary to get the output
> physical devices through the bridge layer.  Of course this may not be
> the real output device due to changes in the environment.  So this should
> be accompanied with a warning that users should switch to ebt.
> 
> 2) We remove the iptables deferring since ipt_physdev will no longer need
> it.
> 
> 3) After a set period (say a year or so) we remove ipt_physdev altogether.

For my local setup it is already a minor PITA that there is no tool
combining the functionality of arptables, ebtables and iptables, but
I can cope with the help of marking and ipt_physdev. If that doesn't
work reliably anymore, I'll be stuck.

Wasn't someone working on a unified framework for *tables? IIRC that
would have been pkttables, but Harald(?) said there was not much
code there yet.

Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

On Thu, 23 Jun 2005, Herbert Xu wrote:
> Longer term though we should obsolete the ipt_physdev module.  The
> rationale there is that this creates a precedence that we can't
> possibly maintain in a consistent way.  For example, we don't have
> a target that matches by hardware MAC address.  If you wanted to
> do that, you'd hook into the arptables interface rather than deferring
> iptables after the creation of the hardware header.

Agreed.

> This can be done in two stages to minimise pain for people already
> using it:
>
> 1) We rewrite ipt_physdev to do the lookups necessary to get the output
> physical devices through the bridge layer.  Of course this may not be
> the real output device due to changes in the environment.  So this should
> be accompanied with a warning that users should switch to ebt.

IMO without defering the hooks the physdev match becomes pretty useless
for locally generated packets. The bridge layer clones packets that are
delivered to multiple ports and calls the NF_IP_* hooks for each clone, so
each clone can be treated seperately. In the IP layer there is only a
single packet, so clones for different ports couldn't be treated
seperately anymore and the semantic of the physdev match would need to be
changed to match on any bridge port in this case, which would probably
break existing rulesets.

> 2) We remove the iptables deferring since ipt_physdev will no longer need
> it.
>
> 3) After a set period (say a year or so) we remove ipt_physdev altogether.

How about we schedule it for removal in a year, keep the workaround
until then and then remove the hook defering?

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

On Thu, 23 Jun 2005, Carl-Daniel Hailfinger wrote:

> Herbert Xu schrieb:
>>
>> 3) After a set period (say a year or so) we remove ipt_physdev altogether.
>
> For my local setup it is already a minor PITA that there is no tool
> combining the functionality of arptables, ebtables and iptables, but
> I can cope with the help of marking and ipt_physdev. If that doesn't
> work reliably anymore, I'll be stuck.

You would still be able to mark packets in iptables and match on that
mark in ebtables, where filtering on the bridge port can be performed.

> Wasn't someone working on a unified framework for *tables? IIRC that
> would have been pkttables, but Harald(?) said there was not much
> code there yet.

Not much has changed AFAIK, but pkttables wouldn't change the fact
that the bridge port isn't available at the IP layer.

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Herbert Xu]

On Thu, Jun 23, 2005 at 05:26:30AM +0200, Patrick McHardy wrote:
>
> >3) After a set period (say a year or so) we remove ipt_physdev altogether.
> 
> How about we schedule it for removal in a year, keep the workaround
> until then and then remove the hook defering?

That's fine by me.

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: 2.6.12: connection tracking broken?

[Bart De Schuymer]

Op do, 23-06-2005 te 07:49 +1000, schreef Herbert Xu:
> Longer term though we should obsolete the ipt_physdev module.  The
> rationale there is that this creates a precedence that we can't
> possibly maintain in a consistent way.  For example, we don't have
> a target that matches by hardware MAC address.  If you wanted to
> do that, you'd hook into the arptables interface rather than deferring
> iptables after the creation of the hardware header.

Iptables also sees purely bridged packets and at least for these packets
the physdev module is useful and harmless. I think removing physdev
alltogether is a bit drastic.

I wonder what flood of messages from angry users the removal of the
physdev functionality for routed packets will stirr.

cheers,
Bart

Re: [Ebtables-devel] Re: 2.6.12: connection tracking broken?

[Bart De Schuymer]

Op do, 23-06-2005 te 02:02 +0200, schreef Carl-Daniel Hailfinger:
> For my local setup it is already a minor PITA that there is no tool
> combining the functionality of arptables, ebtables and iptables, but
> I can cope with the help of marking and ipt_physdev. If that doesn't
> work reliably anymore, I'll be stuck.
> 
> Wasn't someone working on a unified framework for *tables? IIRC that
> would have been pkttables, but Harald(?) said there was not much
> code there yet.

On my todo list is making the arptables matches usable in ebtables
rules. I think it would be very nice if {eb,ip,ip6,arp}tables modules
could be used together.

cheers,
Bart

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

Chris Rankin wrote:
> I have just installed them, and my bridging firewall is working again with 2.6.12.

Thanks Chris. Dave, did you get the patches or do you want me to send
them again?

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[David S. Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 23 Jun 2005 19:42:38 +0200

> Thanks Chris. Dave, did you get the patches or do you want me to send
> them again?

I have the patch, can you give me a nice changelog entry
for it?

Thanks.

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

David S. Miller wrote:
> I have the patch, can you give me a nice changelog entry
> for it?

Here you go:

In 2.6.12 we started dropping the conntrack reference when a packet
leaves the IP layer. This broke connection tracking on a bridge,
because bridge-netfilter defers calling some NF_IP_* hooks to the bridge
layer for locally generated packets going out a bridge, where the
conntrack reference is no longer available. This patch keeps the
reference in this case as a temporary solution, long term we will
remove the defered hook calling. No attempt is made to drop the
reference in the bridge-code when it is no longer needed, tc actions
could already have sent the packet anywhere.

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[Harald Welte]

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

On Thu, Jun 23, 2005 at 06:23:20AM +0000, Bart De Schuymer wrote:
> Op do, 23-06-2005 te 07:49 +1000, schreef Herbert Xu:
> > Longer term though we should obsolete the ipt_physdev module.  The
> > rationale there is that this creates a precedence that we can't
> > possibly maintain in a consistent way.  For example, we don't have
> > a target that matches by hardware MAC address.  If you wanted to
> > do that, you'd hook into the arptables interface rather than deferring
> > iptables after the creation of the hardware header.
> 
> Iptables also sees purely bridged packets and at least for these packets
> the physdev module is useful and harmless. I think removing physdev
> alltogether is a bit drastic.
> 
> I wonder what flood of messages from angry users the removal of the
> physdev functionality for routed packets will stirr.

I have to agree with Bart.  I don't know any bridging-packetfilter setup
that doesn't use ipt_physdev in FORWARD :(

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: 2.6.12: connection tracking broken?

[Patrick McHardy]

Harald Welte wrote:
> I have to agree with Bart.  I don't know any bridging-packetfilter setup
> that doesn't use ipt_physdev in FORWARD :(

It shouldn't be a problem to MARK in ebtables and use the marks instead.
So far I think we can only remove the support for locally generated
packets, but the way bridge-netfilter and ip-netfilter interact causes
some more problems and inconsistencies which I would like to look into
first. One thing I've noticed is that NF_IP_LOCAL_OUT, NF_IP_FORWARD and
NF_IP_POST_ROUTING get called for every clone when packets are delivered
to multiple ports, which causes unexpected results with REJECT (many
packets sent in response to a single one) and probably others. This
could be avoided by only passing packets to the NF_IP_* hooks once, but
that would make the physdev match useless.

Another problem is defragmentation, we've added the user argument to
ip_defrag to avoid packets from jumping through the stack between
different callers. With bridge-netfilter defragmentation in
NF_IP_PRE_ROUTING can be reached from both the IP layer and the bridge
layer, so packets can still jump (see netfilter bugzilla #339).

I expect there are more problems, I hope I can find some time to look
into it this week.

Regards
Patrick

Re: 2.6.12: connection tracking broken?

[David S. Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Fri, 24 Jun 2005 10:39:08 +0200

> In 2.6.12 we started dropping the conntrack reference when a packet
> leaves the IP layer. This broke connection tracking on a bridge,
> because bridge-netfilter defers calling some NF_IP_* hooks to the bridge
> layer for locally generated packets going out a bridge, where the
> conntrack reference is no longer available. This patch keeps the
> reference in this case as a temporary solution, long term we will
> remove the defered hook calling. No attempt is made to drop the
> reference in the bridge-code when it is no longer needed, tc actions
> could already have sent the packet anywhere.

Patch applied and pushed to stable@kernel.org

Thanks a lot.