Re: nftables bug: Only the first two elements of a map are used for NAT

[Pablo Neira Ayuso <pablo@netfilter.org>]

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

Hi Simon,

On Wed, Feb 08, 2017 at 07:11:17PM +0100, Simon Hanisch wrote:
> Hi,
> 
> we want to use a map for a large NAT setup, mapping subnets to a single
> IP. Thats why we wanted to use a map, but only the first two elements of
> the map are used for the NAT.
> I added two config examples to reproduce the bug. In the first example
> the NAT works fine for the network 100.64.15.0/24, in the second it does
> not. The only difference is the order of the map elements.
> We have build nft from the current master branch, last commit is
> 97a2a5bde2f03e33315eab4b76a9e69770b99351.
> 
> 
> 
> Working for 100.64.15.0/24
> 
> #!/usr/sbin/nft
> add chain nat postrouting { type nat hook postrouting priority 100 ;}
> add chain nat prerouting { type nat hook prerouting priority 0 ;}
> add map nat subnettoip { type ipv4_addr: ipv4_addr ; flags interval ; }
> add rule ip nat postrouting snat ip saddr map @subnettoip;
> add element nat subnettoip { 100.64.13.0/24 : 192.168.0.32 }
> add element nat subnettoip { 100.64.15.0/24 : 192.168.0.34 }
> add element nat subnettoip { 100.64.14.0/24 : 192.168.0.33 }
> 
> 
> Not working for 100.64.15.0/24
> 
> #!/usr/sbin/nft
> add chain nat postrouting { type nat hook postrouting priority 100 ;}
> add chain nat prerouting { type nat hook prerouting priority 0 ;}
> add map nat subnettoip { type ipv4_addr: ipv4_addr ; flags interval ; }
> add rule ip nat postrouting snat ip saddr map @subnettoip;
> add element nat subnettoip { 100.64.13.0/24 : 192.168.0.32 }
> add element nat subnettoip { 100.64.14.0/24 : 192.168.0.33 }
> add element nat subnettoip { 100.64.15.0/24 : 192.168.0.34 }

Thanks for the report, I can indeed reproduce it here.

I made a wrong assumption on an earlier patchset that aimed to fix
this. Would you please give a try to this patch to see if this solves
the problem for you?

Thanks.

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 825 bytes --]

diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 71e8fb886a73..78dfbf9588b3 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -60,11 +60,10 @@ static bool nft_rbtree_lookup(const struct net *net, const struct nft_set *set,
 		d = memcmp(this, key, set->klen);
 		if (d < 0) {
 			parent = parent->rb_left;
-			/* In case of adjacent ranges, we always see the high
-			 * part of the range in first place, before the low one.
-			 * So don't update interval if the keys are equal.
-			 */
-			if (interval && nft_rbtree_equal(set, this, interval))
+			if (interval &&
+			    nft_rbtree_equal(set, this, interval) &&
+			    nft_rbtree_interval_end(this) &&
+			    !nft_rbtree_interval_end(interval))
 				continue;
 			interval = rbe;
 		} else if (d > 0)