TEE broken in 3.6

TEE broken in 3.6

[Torsten Luettgert]

Hi,

today, I found that the TEE target doesn't work any longer in 3.6 and
following kernels. The problem is that it tries to ARP-lookup the
original destination address of the forwarded packet, not the address
of the gateway.

I git bisected and found that this commit broke it:

commit f8126f1d5136be1ca1a3536d43ad7a710b5620f8
Author: David S. Miller <davem@davemloft.net>
Date:   Fri Jul 13 05:03:45 2012 -0700

    ipv4: Adjust semantics of rt->rt_gateway.
    
    In order to allow prefixed routes, we have to adjust how rt_gateway
    is set and interpreted.
    
    The new interpretation is:
    
    1) rt_gateway == 0, destination is on-link, nexthop is iph->daddr
    
    2) rt_gateway != 0, destination requires a nexthop gateway
    
    Abstract the fetching of the proper nexthop value using a new
    inline helper, rt_nexthop(), as suggested by Joe Perches.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Tested-by: Vijay Subramanian <subramanian.vijay@gmail.com>

which seems plausible. I'm not versed enough in netfilter/routing code
to fix it myself. Could any of you wizards help?

Thanks,
Torsten

Re: TEE broken in 3.6

[Eric Dumazet]

On Tue, 2012-10-16 at 21:13 +0200, Torsten Luettgert wrote:
> Hi,
> 
> today, I found that the TEE target doesn't work any longer in 3.6 and
> following kernels. The problem is that it tries to ARP-lookup the
> original destination address of the forwarded packet, not the address
> of the gateway.
> 
> I git bisected and found that this commit broke it:
> 
> commit f8126f1d5136be1ca1a3536d43ad7a710b5620f8
> Author: David S. Miller <davem@davemloft.net>
> Date:   Fri Jul 13 05:03:45 2012 -0700
> 
>     ipv4: Adjust semantics of rt->rt_gateway.
>     
>     In order to allow prefixed routes, we have to adjust how rt_gateway
>     is set and interpreted.
>     
>     The new interpretation is:
>     
>     1) rt_gateway == 0, destination is on-link, nexthop is iph->daddr
>     
>     2) rt_gateway != 0, destination requires a nexthop gateway
>     
>     Abstract the fetching of the proper nexthop value using a new
>     inline helper, rt_nexthop(), as suggested by Joe Perches.
>     
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>     Tested-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> 
> which seems plausible. I'm not versed enough in netfilter/routing code
> to fix it myself. Could any of you wizards help?

Hmm...

Could you try latest tree, with the following patch ?

diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index ee2e5bc..bd93e51 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -70,6 +70,7 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info)
 	fl4.daddr = info->gw.ip;
 	fl4.flowi4_tos = RT_TOS(iph->tos);
 	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_flags = FLOWI_FLAG_KNOWN_NH;
 	rt = ip_route_output_key(net, &fl4);
 	if (IS_ERR(rt))
 		return false;

Re: TEE broken in 3.6

[Torsten Luettgert]

On Tue, 16 Oct 2012 22:00:18 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Could you try latest tree, with the following patch ?

Yeah, that works.

I'm still a bit sad, though, since I'll have to wait for 3.7 now (no
FLOWI_FLAG_KNOWN_NH in 3.6.*), or is there any solution for those "old"
kernels, too?

Regards,
Torsten

Re: TEE broken in 3.6

[Eric Dumazet]

On Wed, 2012-10-17 at 09:45 +0200, Torsten Luettgert wrote:
> On Tue, 16 Oct 2012 22:00:18 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> > Could you try latest tree, with the following patch ?
> 
> Yeah, that works.
> 
> I'm still a bit sad, though, since I'll have to wait for 3.7 now (no
> FLOWI_FLAG_KNOWN_NH in 3.6.*), or is there any solution for those "old"
> kernels, too?
> 
> Regards,
> Torsten

I believe its stable material, it will reach 3.6.3 or 3.6.4 dont worry.

Thanks

Re: TEE broken in 3.6

[Pablo Neira Ayuso]

On Wed, Oct 17, 2012 at 09:48:51AM +0200, Eric Dumazet wrote:
> On Wed, 2012-10-17 at 09:45 +0200, Torsten Luettgert wrote:
> > On Tue, 16 Oct 2012 22:00:18 +0200
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > 
> > > Could you try latest tree, with the following patch ?
> > 
> > Yeah, that works.
> > 
> > I'm still a bit sad, though, since I'll have to wait for 3.7 now (no
> > FLOWI_FLAG_KNOWN_NH in 3.6.*), or is there any solution for those "old"
> > kernels, too?
> > 
> > Regards,
> > Torsten
> 
> I believe its stable material, it will reach 3.6.3 or 3.6.4 dont worry.

Sure. I'll pass this to -stable once it hits Linus' tree. It may take
a while though.

Re: TEE broken in 3.6

[Pablo Neira Ayuso]

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

On Wed, Oct 17, 2012 at 09:48:51AM +0200, Eric Dumazet wrote:
> On Wed, 2012-10-17 at 09:45 +0200, Torsten Luettgert wrote:
> > On Tue, 16 Oct 2012 22:00:18 +0200
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > 
> > > Could you try latest tree, with the following patch ?
> > 
> > Yeah, that works.
> > 
> > I'm still a bit sad, though, since I'll have to wait for 3.7 now (no
> > FLOWI_FLAG_KNOWN_NH in 3.6.*), or is there any solution for those "old"
> > kernels, too?
> > 
> > Regards,
> > Torsten
> 
> I believe its stable material, it will reach 3.6.3 or 3.6.4 dont worry.

@Eric: I've included the description to the patch (just to save you
some time to make it yourself).

Let me know if you're OK with the one attached. If not, feel free to
pass me yours.

[-- Attachment #2: 0001-netfilter-xt_TEE-fix-routing-due-to-rt_gateway-seman.patch --]
[-- Type: text/x-diff, Size: 1159 bytes --]

>From d977416de1ceb9ec960b34c0202d711a9a6dead4 Mon Sep 17 00:00:00 2001
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 17 Oct 2012 10:18:15 +0200
Subject: [PATCH] netfilter: xt_TEE: fix routing due to rt_gateway semantic
 changes

Since (f8126f1 ipv4: Adjust semantics of rt->rt_gateway.), xt_TEE
stopped working.

The problem is that it tries to ARP lookup the original destination
address of the forwarded packet, not the address of the gateway.

Reported-by: Torsten Luettgert <ml-netfilter@enda.eu>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/xt_TEE.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index ee2e5bc..bd93e51 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -70,6 +70,7 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info)
 	fl4.daddr = info->gw.ip;
 	fl4.flowi4_tos = RT_TOS(iph->tos);
 	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_flags = FLOWI_FLAG_KNOWN_NH;
 	rt = ip_route_output_key(net, &fl4);
 	if (IS_ERR(rt))
 		return false;
-- 
1.7.10.4

[PATCH] netfilter: xt_TEE: dont use destination address found in header

[Eric Dumazet]

From-off-by: Eric Dumazet <edumazet@google.com>

Torsten Luettgert bisected TEE regression starting with commit
f8126f1d5136be1 (ipv4: Adjust semantics of rt->rt_gateway.)

The problem is that it tries to ARP-lookup the original destination
address of the forwarded packet, not the address of the gateway.

Fix this using FLOWI_FLAG_KNOWN_NH Julian added in commit
c92b96553a80c1 (ipv4: Add FLOWI_FLAG_KNOWN_NH), so that known
nexthop (info->gw.ip) has preference on resolving.

Reported-by: Torsten Luettgert <ml-netfilter@enda.eu>
Bisected-by: Torsten Luettgert <ml-netfilter@enda.eu>
Tested-by: Torsten Luettgert <ml-netfilter@enda.eu>
Cc: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/netfilter/xt_TEE.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index ee2e5bc..bd93e51 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -70,6 +70,7 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info)
 	fl4.daddr = info->gw.ip;
 	fl4.flowi4_tos = RT_TOS(iph->tos);
 	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_flags = FLOWI_FLAG_KNOWN_NH;
 	rt = ip_route_output_key(net, &fl4);
 	if (IS_ERR(rt))
 		return false;

Re: TEE broken in 3.6

[Eric Dumazet]

On Wed, 2012-10-17 at 10:23 +0200, Pablo Neira Ayuso wrote:
> On Wed, Oct 17, 2012 at 09:48:51AM +0200, Eric Dumazet wrote:
> > On Wed, 2012-10-17 at 09:45 +0200, Torsten Luettgert wrote:
> > > On Tue, 16 Oct 2012 22:00:18 +0200
> > > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > 
> > > > Could you try latest tree, with the following patch ?
> > > 
> > > Yeah, that works.
> > > 
> > > I'm still a bit sad, though, since I'll have to wait for 3.7 now (no
> > > FLOWI_FLAG_KNOWN_NH in 3.6.*), or is there any solution for those "old"
> > > kernels, too?
> > > 
> > > Regards,
> > > Torsten
> > 
> > I believe its stable material, it will reach 3.6.3 or 3.6.4 dont worry.
> 
> @Eric: I've included the description to the patch (just to save you
> some time to make it yourself).
> 
> Let me know if you're OK with the one attached. If not, feel free to
> pass me yours.

Oops, sorry I missed that, feel free to chose your version or mine !

Thanks