netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* NAT regression in next tree
       [not found] ` <201002171526.02493.arnd@arndb.de>
@ 2010-02-19  1:36   ` Stephen Hemminger
  2010-02-19  5:45     ` Patrick McHardy
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2010-02-19  1:36 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, netfilter-devel

Something in net-next tree broke bridging of virtual nets.
My local VM's can no longer access external networks.

It is a NAT problem. One of the recent netfilter changes is causing
the packets to not have there source address rewritten.

I see:
    VM1  -- 192.168.100.0/24 -- HOST -- 192.168.1.0/24 -- ROUTER
                           virbr0    eth0

Even a simple ping from VM1 doesn't get responded to because
the 192.168.100.X source address is not getting rewritten.

iptables -n -v -v -L (on host) after several attempted packets
-----------------
Chain INPUT (policy ACCEPT 2767 packets, 1347K bytes)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 ACCEPT     udp  --  virbr1 *       0.0.0.0/0            0.0.0.0/0           udp dpt:53 
    0     0 ACCEPT     tcp  --  virbr1 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:53 
    0     0 ACCEPT     udp  --  virbr1 *       0.0.0.0/0            0.0.0.0/0           udp dpt:67 
    0     0 ACCEPT     tcp  --  virbr1 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:67 
    0     0 ACCEPT     udp  --  virbr2 *       0.0.0.0/0            0.0.0.0/0           udp dpt:53 
    0     0 ACCEPT     tcp  --  virbr2 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:53 
    0     0 ACCEPT     udp  --  virbr2 *       0.0.0.0/0            0.0.0.0/0           udp dpt:67 
    0     0 ACCEPT     tcp  --  virbr2 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:67 
    5   295 ACCEPT     udp  --  virbr0 *       0.0.0.0/0            0.0.0.0/0           udp dpt:53 
    0     0 ACCEPT     tcp  --  virbr0 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:53 
    2   656 ACCEPT     udp  --  virbr0 *       0.0.0.0/0            0.0.0.0/0           udp dpt:67 
    0     0 ACCEPT     tcp  --  virbr0 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:67 
    0     0 ACCEPT     udp  --  virbr4 *       0.0.0.0/0            0.0.0.0/0           udp dpt:53 
    0     0 ACCEPT     tcp  --  virbr4 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:53 
    0     0 ACCEPT     udp  --  virbr4 *       0.0.0.0/0            0.0.0.0/0           udp dpt:67 
    0     0 ACCEPT     tcp  --  virbr4 *       0.0.0.0/0            0.0.0.0/0           tcp dpt:67 

Chain FORWARD (policy ACCEPT 0 packets, 0 bytes)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 ACCEPT     all  --  virbr1 virbr1  0.0.0.0/0            0.0.0.0/0           
    0     0 REJECT     all  --  *      virbr1  0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 
    0     0 REJECT     all  --  virbr1 *       0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 
    0     0 ACCEPT     all  --  virbr2 virbr2  0.0.0.0/0            0.0.0.0/0           
    0     0 REJECT     all  --  *      virbr2  0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 
    0     0 REJECT     all  --  virbr2 *       0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 
    0     0 ACCEPT     all  --  *      virbr0  0.0.0.0/0            192.168.100.0/24    state RELATED,ESTABLISHED 
   15  1188 ACCEPT     all  --  virbr0 *       192.168.100.0/24     0.0.0.0/0           
    0     0 ACCEPT     all  --  virbr0 virbr0  0.0.0.0/0            0.0.0.0/0           
    0     0 REJECT     all  --  *      virbr0  0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 
    0     0 REJECT     all  --  virbr0 *       0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 
    0     0 ACCEPT     all  --  virbr4 virbr4  0.0.0.0/0            0.0.0.0/0           
    0     0 REJECT     all  --  *      virbr4  0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 
    0     0 REJECT     all  --  virbr4 *       0.0.0.0/0            0.0.0.0/0           reject-with icmp-port-unreachable 

Chain OUTPUT (policy ACCEPT 2720 packets, 1311K bytes)
 pkts bytes target     prot opt in     out     source               destination         
libiptc vlibxtables.so.2. 6000 bytes.
Table `filter'
Hooks: pre/in/fwd/out/post = ffffffff/0/d18/1628/ffffffff
Underflows: pre/in/fwd/out/post = ffffffff/c80/1590/1628/ffffffff
Entry 0 (0):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr1'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 1 (200):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr1'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 2 (400):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr1'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 3 (600):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr1'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 4 (800):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr2'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 5 (1000):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr2'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 6 (1200):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr2'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 7 (1400):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr2'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 8 (1600):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr0'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 5 packets, 295 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 9 (1800):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr0'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 10 (2000):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr0'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 2 packets, 656 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 11 (2200):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr0'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 12 (2400):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr4'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 13 (2600):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr4'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 14 (2800):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr4'/XXXXXXX.........to `'/................
Protocol: 17
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `udp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 15 (3000):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr4'/XXXXXXX.........to `'/................
Protocol: 6
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `tcp'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 16 (3200):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 2767 packets, 1347401 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 17 (3352):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr1'/XXXXXXX.........to `virbr1'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 18 (3504):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `virbr1'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 19 (3656):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr1'/XXXXXXX.........to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 20 (3808):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr2'/XXXXXXX.........to `virbr2'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 21 (3960):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `virbr2'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 22 (4112):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr2'/XXXXXXX.........to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 23 (4264):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 192.168.100.0/255.255.255.0
Interface: `'/................to `virbr0'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Match name: `state'
Target name: `' [40]
verdict=NF_ACCEPT

Entry 24 (4456):
SRC IP: 192.168.100.0/255.255.255.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr0'/XXXXXXX.........to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 15 packets, 1188 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 25 (4608):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr0'/XXXXXXX.........to `virbr0'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 26 (4760):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `virbr0'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 27 (4912):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr0'/XXXXXXX.........to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 28 (5064):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr4'/XXXXXXX.........to `virbr4'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 29 (5216):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `virbr4'/XXXXXXX.........
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 30 (5368):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `virbr4'/XXXXXXX.........to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `REJECT' [40]

Entry 31 (5520):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 32 (5672):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 2720 packets, 1310915 bytes
Cache: 00000000
Target name: `' [40]
verdict=NF_ACCEPT

Entry 33 (5824):
SRC IP: 0.0.0.0/0.0.0.0
DST IP: 0.0.0.0/0.0.0.0
Interface: `'/................to `'/................
Protocol: 0
Flags: 00
Invflags: 00
Counters: 0 packets, 0 bytes
Cache: 00000000
Target name: `ERROR' [64]
error=`ERROR'


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

* Re: NAT regression in next tree
  2010-02-19  1:36   ` NAT regression in next tree Stephen Hemminger
@ 2010-02-19  5:45     ` Patrick McHardy
  2010-02-19  5:51       ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Patrick McHardy @ 2010-02-19  5:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev, netfilter-devel

Stephen Hemminger wrote:
> Something in net-next tree broke bridging of virtual nets.
> My local VM's can no longer access external networks.
> 
> It is a NAT problem. One of the recent netfilter changes is causing
> the packets to not have there source address rewritten.
> 
> I see:
>     VM1  -- 192.168.100.0/24 -- HOST -- 192.168.1.0/24 -- ROUTER
>                            virbr0    eth0
> 
> Even a simple ping from VM1 doesn't get responded to because
> the 192.168.100.X source address is not getting rewritten.

I'll try to reproduce it locally. What is the HEAD of the broken
tree you're running?

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

* Re: NAT regression in next tree
  2010-02-19  5:45     ` Patrick McHardy
@ 2010-02-19  5:51       ` Stephen Hemminger
  2010-02-19  7:06         ` Patrick McHardy
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2010-02-19  5:51 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, netfilter-devel

On Fri, 19 Feb 2010 06:45:43 +0100
Patrick McHardy <kaber@trash.net> wrote:

> Stephen Hemminger wrote:
> > Something in net-next tree broke bridging of virtual nets.
> > My local VM's can no longer access external networks.
> > 
> > It is a NAT problem. One of the recent netfilter changes is causing
> > the packets to not have there source address rewritten.
> > 
> > I see:
> >     VM1  -- 192.168.100.0/24 -- HOST -- 192.168.1.0/24 -- ROUTER
> >                            virbr0    eth0
> > 
> > Even a simple ping from VM1 doesn't get responded to because
> > the 192.168.100.X source address is not getting rewritten.
> 
> I'll try to reproduce it locally. What is the HEAD of the broken
> tree you're running?

commit 37ee3d5b3e979a168536e7e2f15bd1e769cb4122
Author: Patrick McHardy <kaber@trash.net>
Date:   Thu Feb 18 19:04:44 2010 +0100

    netfilter: nf_defrag_ipv4: fix compilation error with NF_CONNTRACK=n



-- 

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

* Re: NAT regression in next tree
  2010-02-19  5:51       ` Stephen Hemminger
@ 2010-02-19  7:06         ` Patrick McHardy
  2010-02-19  7:20           ` Eric Dumazet
  0 siblings, 1 reply; 7+ messages in thread
From: Patrick McHardy @ 2010-02-19  7:06 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev, netfilter-devel

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

Stephen Hemminger wrote:
> On Fri, 19 Feb 2010 06:45:43 +0100
> Patrick McHardy <kaber@trash.net> wrote:
> 
>> Stephen Hemminger wrote:
>>> Something in net-next tree broke bridging of virtual nets.
>>> My local VM's can no longer access external networks.
>>>
>>> It is a NAT problem. One of the recent netfilter changes is causing
>>> the packets to not have there source address rewritten.
>>>
>>> I see:
>>>     VM1  -- 192.168.100.0/24 -- HOST -- 192.168.1.0/24 -- ROUTER
>>>                            virbr0    eth0
>>>
>>> Even a simple ping from VM1 doesn't get responded to because
>>> the 192.168.100.X source address is not getting rewritten.
>> I'll try to reproduce it locally. What is the HEAD of the broken
>> tree you're running?
> 
> commit 37ee3d5b3e979a168536e7e2f15bd1e769cb4122
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Thu Feb 18 19:04:44 2010 +0100
> 
>     netfilter: nf_defrag_ipv4: fix compilation error with NF_CONNTRACK=n

This patch should fix it.


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

commit 4bac6b180771f7ef5275b1a6d88e630ca3a3d6f0
Author: Patrick McHardy <kaber@trash.net>
Date:   Fri Feb 19 08:03:28 2010 +0100

    netfilter: restore POST_ROUTING hook in NF_HOOK_COND
    
    Commit 2249065 ("netfilter: get rid of the grossness in netfilter.h")
    inverted the logic for conditional hook invocation, breaking the
    POST_ROUTING hook invoked by ip_output().
    
    Correct the logic and remove an unnecessary initialization.
    
    Reported-by: Stephen Hemminger <shemminger@vyatta.com>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 7007945..89341c3 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -212,8 +212,9 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
 	     struct net_device *in, struct net_device *out,
 	     int (*okfn)(struct sk_buff *), bool cond)
 {
-	int ret = 1;
-	if (cond ||
+	int ret;
+
+	if (!cond ||
 	    (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
 		ret = okfn(skb);
 	return ret;

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

* Re: NAT regression in next tree
  2010-02-19  7:06         ` Patrick McHardy
@ 2010-02-19  7:20           ` Eric Dumazet
  2010-02-19  7:27             ` Patrick McHardy
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2010-02-19  7:20 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Stephen Hemminger, David Miller, netdev, netfilter-devel

Le vendredi 19 février 2010 à 08:06 +0100, Patrick McHardy a écrit :
> Stephen Hemminger wrote:
> > On Fri, 19 Feb 2010 06:45:43 +0100
> > Patrick McHardy <kaber@trash.net> wrote:
> > 
> >> Stephen Hemminger wrote:
> >>> Something in net-next tree broke bridging of virtual nets.
> >>> My local VM's can no longer access external networks.
> >>>
> >>> It is a NAT problem. One of the recent netfilter changes is causing
> >>> the packets to not have there source address rewritten.
> >>>
> >>> I see:
> >>>     VM1  -- 192.168.100.0/24 -- HOST -- 192.168.1.0/24 -- ROUTER
> >>>                            virbr0    eth0
> >>>
> >>> Even a simple ping from VM1 doesn't get responded to because
> >>> the 192.168.100.X source address is not getting rewritten.
> >> I'll try to reproduce it locally. What is the HEAD of the broken
> >> tree you're running?
> > 
> > commit 37ee3d5b3e979a168536e7e2f15bd1e769cb4122
> > Author: Patrick McHardy <kaber@trash.net>
> > Date:   Thu Feb 18 19:04:44 2010 +0100
> > 
> >     netfilter: nf_defrag_ipv4: fix compilation error with NF_CONNTRACK=n
> 
> This patch should fix it.
> 
> pièce jointe document texte brut (x)
> commit 4bac6b180771f7ef5275b1a6d88e630ca3a3d6f0
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Fri Feb 19 08:03:28 2010 +0100
> 
>     netfilter: restore POST_ROUTING hook in NF_HOOK_COND
>     
>     Commit 2249065 ("netfilter: get rid of the grossness in netfilter.h")
>     inverted the logic for conditional hook invocation, breaking the
>     POST_ROUTING hook invoked by ip_output().
>     
>     Correct the logic and remove an unnecessary initialization.
>     
>     Reported-by: Stephen Hemminger <shemminger@vyatta.com>
>     Signed-off-by: Patrick McHardy <kaber@trash.net>
> 
> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> index 7007945..89341c3 100644
> --- a/include/linux/netfilter.h
> +++ b/include/linux/netfilter.h
> @@ -212,8 +212,9 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
>  	     struct net_device *in, struct net_device *out,
>  	     int (*okfn)(struct sk_buff *), bool cond)
>  {
> -	int ret = 1;
> -	if (cond ||
> +	int ret;
> +
> +	if (!cond ||
>  	    (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
>  		ret = okfn(skb);
>  	return ret;

I dont quite get it

Original code was :


#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)                \
({int __ret;                                                                  \
if ((cond) || (__ret = nf_hook_thresh(pf, hook, (skb), indev, outdev, okfn, INT_MIN)) == 1)\
       __ret = (okfn)(skb);                                                   \
__ret;})


There was no condition inversion.



--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: NAT regression in next tree
  2010-02-19  7:20           ` Eric Dumazet
@ 2010-02-19  7:27             ` Patrick McHardy
  2010-02-19 18:11               ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Patrick McHardy @ 2010-02-19  7:27 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, David Miller, netdev, netfilter-devel

Eric Dumazet wrote:
> Le vendredi 19 février 2010 à 08:06 +0100, Patrick McHardy a écrit :
>>     netfilter: restore POST_ROUTING hook in NF_HOOK_COND
>>     
>>     Commit 2249065 ("netfilter: get rid of the grossness in netfilter.h")
>>     inverted the logic for conditional hook invocation, breaking the
>>     POST_ROUTING hook invoked by ip_output().
>>     
>>     Correct the logic and remove an unnecessary initialization.
>>     
>>     Reported-by: Stephen Hemminger <shemminger@vyatta.com>
>>     Signed-off-by: Patrick McHardy <kaber@trash.net>
>>
>> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
>> index 7007945..89341c3 100644
>> --- a/include/linux/netfilter.h
>> +++ b/include/linux/netfilter.h
>> @@ -212,8 +212,9 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
>>  	     struct net_device *in, struct net_device *out,
>>  	     int (*okfn)(struct sk_buff *), bool cond)
>>  {
>> -	int ret = 1;
>> -	if (cond ||
>> +	int ret;
>> +
>> +	if (!cond ||
>>  	    (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
>>  		ret = okfn(skb);
>>  	return ret;
> 
> I dont quite get it
> 
> Original code was :
> 
> 
> #define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)                \
> ({int __ret;                                                                  \
> if ((cond) || (__ret = nf_hook_thresh(pf, hook, (skb), indev, outdev, okfn, INT_MIN)) == 1)\
>        __ret = (okfn)(skb);                                                   \
> __ret;})
> 
> 
> There was no condition inversion.

Right, I quoted the wrong patch, it was actually broken in
23f3733 ("netfilter: reduce NF_HOOK by one argument"), which
moved the cond check from nf_hook_thresh() to NF_HOOK_COND().
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: NAT regression in next tree
  2010-02-19  7:27             ` Patrick McHardy
@ 2010-02-19 18:11               ` Stephen Hemminger
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2010-02-19 18:11 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Eric Dumazet, David Miller, netdev, netfilter-devel

On Fri, 19 Feb 2010 08:27:33 +0100
Patrick McHardy <kaber@trash.net> wrote:

> Eric Dumazet wrote:
> > Le vendredi 19 février 2010 à 08:06 +0100, Patrick McHardy a écrit :
> >>     netfilter: restore POST_ROUTING hook in NF_HOOK_COND
> >>     
> >>     Commit 2249065 ("netfilter: get rid of the grossness in netfilter.h")
> >>     inverted the logic for conditional hook invocation, breaking the
> >>     POST_ROUTING hook invoked by ip_output().
> >>     
> >>     Correct the logic and remove an unnecessary initialization.
> >>     
> >>     Reported-by: Stephen Hemminger <shemminger@vyatta.com>
> >>     Signed-off-by: Patrick McHardy <kaber@trash.net>
> >>
> >> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> >> index 7007945..89341c3 100644
> >> --- a/include/linux/netfilter.h
> >> +++ b/include/linux/netfilter.h
> >> @@ -212,8 +212,9 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
> >>  	     struct net_device *in, struct net_device *out,
> >>  	     int (*okfn)(struct sk_buff *), bool cond)
> >>  {
> >> -	int ret = 1;
> >> -	if (cond ||
> >> +	int ret;
> >> +
> >> +	if (!cond ||
> >>  	    (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
> >>  		ret = okfn(skb);
> >>  	return ret;
> > 
> > I dont quite get it
> > 
> > Original code was :
> > 
> > 
> > #define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)                \
> > ({int __ret;                                                                  \
> > if ((cond) || (__ret = nf_hook_thresh(pf, hook, (skb), indev, outdev, okfn, INT_MIN)) == 1)\
> >        __ret = (okfn)(skb);                                                   \
> > __ret;})
> > 
> > 
> > There was no condition inversion.
> 
> Right, I quoted the wrong patch, it was actually broken in
> 23f3733 ("netfilter: reduce NF_HOOK by one argument"), which
> moved the cond check from nf_hook_thresh() to NF_HOOK_COND().

Yes, this fixes the problem I was seeing.

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

-- 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2010-02-19 18:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20100216173658.519b6245@nehalam>
     [not found] ` <201002171526.02493.arnd@arndb.de>
2010-02-19  1:36   ` NAT regression in next tree Stephen Hemminger
2010-02-19  5:45     ` Patrick McHardy
2010-02-19  5:51       ` Stephen Hemminger
2010-02-19  7:06         ` Patrick McHardy
2010-02-19  7:20           ` Eric Dumazet
2010-02-19  7:27             ` Patrick McHardy
2010-02-19 18:11               ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).