netdev - Re: [PATCH net-next 0/4] ila: Use NF_INET_PRE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALx6S35gFbcFjCWEQsDY_DzBOrr0i7+3fdBWiF4rLmcz9HJRBg@mail.gmail.com>
Date:	Mon, 28 Sep 2015 15:27:16 -0700
From:	Tom Herbert <tom@...bertland.com>
To:	Florian Westphal <fw@...len.de>
Cc:	David Miller <davem@...emloft.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Kernel Team <kernel-team@...com>
Subject: Re: [PATCH net-next 0/4] ila: Use NF_INET_PRE_ROUTING nfhook

On Sun, Sep 27, 2015 at 1:10 AM, Florian Westphal <fw@...len.de> wrote:
> David Miller <davem@...emloft.net> wrote:
>> > This patch set addresses the issue for ILA by adding a fast locator
>> > lookup that occurs before early demux. This is done by using a hook
>> > at NF_INET_PRE_ROUTING. For the backend we implement an rhashtable
>> > that contains identifier to locator to mappings. The table also
>> > allows more specific matches that include original locator and
>> > interface.
>>
>> I really don't think we should use netfilter hooks to perform
>> operations setup outside of netfilter's normal configuration
>> mechanisms.
>
> Thanks, thats my thinking as well.
>
>> If ILA were instead configured inside of netfilter's normal mechanisms
>> then there would be full transparency about whether ILA
>> transformations are performed before or after the user's other
>> netfilter rules.  And the user would have full control over this.
>> As implemented here, they don't.
>>
>> So sorry, I'm not too keen on this and I bet if netfilter developers
>> reviewed this patch series they'd have similar objections.
>
> Seems this should/could be implemented similar to RFC6296 network
> prefix translations (see net/ipv6/netfilter/ip6t_NPT.c).

Hi Florian,

RFC6296 doesn't work because it allows an invalid checksum to be sent
on wire relative to the addresses used on the wire. That means we
would lose CHECKSUM_UNNECESSARY for ILA which is way too big of a
performance hit. This might actually be worse for some devices that
are doing NETIF_F_IP_CSUM if they are calculating the checksum based
on the addresses in packet instead of the ultimate destination address
which seems to be what RFC6296 wants (could result checksum errors).
Unfortunately we're going to be seeing this problem more and more as
there are more method that rewrite addresses without updating the
transport checksum before leaving the host (e.g. segment routing).

For devices doing CHECKSUM_COMPLETE I suspect there are also some
issues. In ip6t_npt_map_pfx it seems like if the addresses are being
rewritten in the receive path, the skb->csum should be updated
accordingly. GRO might consistently fail on bad checksums also...

In any case, I did at one point create some netfilter targets for ILA
to do the translation correctly updating the checksum. While this
provided the required functionality, I couldn't get sufficient
performance. A specialized fixed length lookup table gets most of the
performance needed for ILA.

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