Re: [PATCH net-next 0/4] ila: Use NF_INET_PRE_ROUTING nfhook

2015-09-29 Thread Tom Herbert
On Mon, Sep 28, 2015 at 4:00 PM, Florian Westphal  wrote:
> Tom Herbert  wrote:
>> 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.
>
> Not following.  I did not say you should use NPT instead of ILA.
>
> [..]
>> 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.
>
> I'm not following at all.
>
> Could you explain why you can't just 'relocate' your proposed
> implementation to netfilter/ipv6?
>
Florian

I modified DNPT to perform ILA. Performance results are below. What I
see is that DNPT offers only a slight improvement over just doing
translation at LWT and not getting a hit in early demux. Top function
in perf is:

2.49%  [kernel]   [k] ip6t_do_table

so I think this performance result is mostly the overhead of netfilter
and not ILA translation. But in any case, doing a direct specialized
lookup like what we do in this patch gets us close to same performance
without ILA enabled-- low performance overhead is critical for our ILA
use cases.

Tom

No ILA, baseline
   85.72% CPU utilization
   1861945 tps
   93/163/330 50/90/99% latencies

ILA before fix (LWT on both input and output)
   83.47 CPU utilization
   16583186 tps (-11% from baseline)
   107/183/338 50/90/99% latencies

ILA after fix (hook for input)
   84.97% CPU utilization
   1833948 tps (-1.5% from baseline)
   95/164/331 50/90/99% latencies

Modify DNPT to do ILA (ip6tables -t mangle -I PREROUTING -d
2001:0:0:3::/64 -j DNPT --src-pfx 2001:0:0:3::/64 --dst-pfx
:0:0:1::/64)
   80.94% CPU utilization
   1683315 tps (-10% from baseline)
   104/179/350 50/90/99% latencies

> F.e. I see no reason why you could not use a lookup table in a netfilter
> target (or nft expression, for that matter) ... ?
>
> Thanks,
> Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-next 0/4] ila: Use NF_INET_PRE_ROUTING nfhook

2015-09-28 Thread Tom Herbert
On Sun, Sep 27, 2015 at 1:10 AM, Florian Westphal  wrote:
> David Miller  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-next 0/4] ila: Use NF_INET_PRE_ROUTING nfhook

2015-09-28 Thread Florian Westphal
Tom Herbert  wrote:
> 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.

Not following.  I did not say you should use NPT instead of ILA.

[..]
> 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.

I'm not following at all.

Could you explain why you can't just 'relocate' your proposed
implementation to netfilter/ipv6?

F.e. I see no reason why you could not use a lookup table in a netfilter
target (or nft expression, for that matter) ... ?

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


Re: [PATCH net-next 0/4] ila: Use NF_INET_PRE_ROUTING nfhook

2015-09-27 Thread Florian Westphal
David Miller  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).
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-next 0/4] ila: Use NF_INET_PRE_ROUTING nfhook

2015-09-27 Thread David Miller
From: Tom Herbert 
Date: Thu, 24 Sep 2015 09:30:20 -0700

> 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.

It is not a set of arbitray hooks to take advantage of in another
subsystem or facility.

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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html