Re: [PATCH] inet: don't call skb_orphan if tproxy happens in layer 2

Re: [PATCH] inet: don't call skb_orphan if tproxy happens in layer 2

[Florian Westphal]

Gregory Vander Schueren <gregory.vanderschueren@tessares.net> wrote:

[ cc netdev ]

> If sysctl bridge-nf-call-iptables is enabled, iptables chains are already
> traversed from the bridging code. In such case, tproxy already happened when
> reaching ip_rcv. Thus no need to call skb_orphan as this would actually undo
> tproxy.

I don't like this because it adds yet another test in fastpath, and for
a use case that has apparently never worked before.

> We noticed issues when using tproxy with net.bridge.bridge-nf-call-iptables
> enabled. In such case, ip_rcv() basically undo tproxy's job. The following
> patch proposes a fix.

I question wheter its a good idea to mix tproxy with bridges.

Tproxy relies on policy routing, but a bridge doesn't route :-)

I guess you use bridge snat mac mangling to force local delivery of
packets that are otherwise bridged?

If yes, can you use ebtables brouting instead?
This would bypass the bridge (so no iptables invocation from bridge
prerouting anymore).

We will try to get rid of nf-call-iptables eventually.

There might be (more complicated) ways to avoid this problem without
adding code in normal network path, but lets check other options first.

Re: [PATCH] inet: don't call skb_orphan if tproxy happens in layer 2

[Pablo Neira Ayuso]

On Fri, Feb 16, 2018 at 12:07:06PM +0100, Florian Westphal wrote:
> Gregory Vander Schueren <gregory.vanderschueren@tessares.net> wrote:
> 
> [ cc netdev ]
> 
> > If sysctl bridge-nf-call-iptables is enabled, iptables chains are already
> > traversed from the bridging code. In such case, tproxy already happened when
> > reaching ip_rcv. Thus no need to call skb_orphan as this would actually undo
> > tproxy.
> 
> I don't like this because it adds yet another test in fastpath, and for
> a use case that has apparently never worked before.
> 
> > We noticed issues when using tproxy with net.bridge.bridge-nf-call-iptables
> > enabled. In such case, ip_rcv() basically undo tproxy's job. The following
> > patch proposes a fix.
> 
> I question wheter its a good idea to mix tproxy with bridges.
> 
> Tproxy relies on policy routing, but a bridge doesn't route :-)
> 
> I guess you use bridge snat mac mangling to force local delivery of
> packets that are otherwise bridged?
> 
> If yes, can you use ebtables brouting instead?
> This would bypass the bridge (so no iptables invocation from bridge
> prerouting anymore).
> 
> We will try to get rid of nf-call-iptables eventually.
> 
> There might be (more complicated) ways to avoid this problem without
> adding code in normal network path, but lets check other options first.

Agreed.

If there's a fix for this, that should be away from the fast path, not
in ip_rcv().

Re: [PATCH] inet: don't call skb_orphan if tproxy happens in layer 2

[Gregory Vander Schueren]

Hi Florian & Pablo,

Thank your very much for your quick feedback.

On 02/16/2018 12:28 PM, Pablo Neira Ayuso wrote:
> On Fri, Feb 16, 2018 at 12:07:06PM +0100, Florian Westphal wrote:
>> Gregory Vander Schueren <gregory.vanderschueren@tessares.net> wrote:
>>
>> [ cc netdev ]
>>
>>> If sysctl bridge-nf-call-iptables is enabled, iptables chains are already
>>> traversed from the bridging code. In such case, tproxy already happened when
>>> reaching ip_rcv. Thus no need to call skb_orphan as this would actually undo
>>> tproxy.
>>
>> I don't like this because it adds yet another test in fastpath, and for
>> a use case that has apparently never worked before.

Agreed. I also thought this was not ideal but I did find another way to 
easily fix this.

>>> We noticed issues when using tproxy with net.bridge.bridge-nf-call-iptables
>>> enabled. In such case, ip_rcv() basically undo tproxy's job. The following
>>> patch proposes a fix.
>>
>> I question wheter its a good idea to mix tproxy with bridges.
>>
>> Tproxy relies on policy routing, but a bridge doesn't route :-)
>>
>> I guess you use bridge snat mac mangling to force local delivery of
>> packets that are otherwise bridged?

Indeed, we use DNAT MAC mangling.

>> If yes, can you use ebtables brouting instead?
>> This would bypass the bridge (so no iptables invocation from bridge
>> prerouting anymore).

We were actually pondering over the usage of MAC DNAT vs brouting. I'll 
thus follow your suggestion and use brouting instead then.

>> We will try to get rid of nf-call-iptables eventually.

Good to know!

>> There might be (more complicated) ways to avoid this problem without
>> adding code in normal network path, but lets check other options first.
> 
> Agreed.
> 
> If there's a fix for this, that should be away from the fast path, not
> in ip_rcv().
> 

-- 

------------------------------
DISCLAIMER.
This email and any files transmitted with it are confidential and intended 
solely for the use of the individual or entity to whom they are addressed. 
If you have received this email in error please notify the system manager. 
This message contains confidential information and is intended only for the 
individual named. If you are not the named addressee you should not 
disseminate, distribute or copy this e-mail. Please notify the sender 
immediately by e-mail if you have received this e-mail by mistake and 
delete this e-mail from your system. If you are not the intended recipient 
you are notified that disclosing, copying, distributing or taking any 
action in reliance on the contents of this information is strictly 
prohibited.