nat: improve nat44-ed outside address distribution

Use client address hash to pick the first outside address
instead of just address high octet, becasue it may denegerate
into stable 10/172/192, depending on nat address count.

Fix outside address distribution test to acually test the
distribution, not the algo, so previous distribution will
fail with 65 nat addresses and 100 clients:

FAIL: Outside address distribution based on source address
Traceback (most recent call last):
  File ".../test/test_nat44_ed.py", line 2048, in test_outside_address_distribution
    msg="Bad outside address distribution")
AssertionError: 156.25 not less than 0.33 : Bad outside address distribution

Type: improvement
Change-Id: I604b1294422f20d211db5614c47559557a78a193
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>

2 files changed, 15 insertions, 14 deletions

diff --git a/src/plugins/nat/nat44-ed/nat44_ed_in2out.c b/src/plugins/nat/nat44-ed/nat44_ed_in2out.c
index deec0099933..4ba51bcaea6 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed_in2out.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed_in2out.c
@@ -155,7 +155,9 @@ nat_ed_alloc_addr_and_port (snat_main_t *sm, u32 rx_fib_index,
 {
   if (vec_len (sm->addresses) > 0)
     {
-      u32 s_addr_offset = s_addr.as_u32 % vec_len (sm->addresses);
+      u32 s_addr_offset = (s_addr.as_u32 + (s_addr.as_u32 >> 8) +
+			   (s_addr.as_u32 >> 16) + (s_addr.as_u32 >> 24)) %
+			  vec_len (sm->addresses);
       snat_address_t *a, *ja = 0, *ra = 0, *ba = 0;
       int i;
 
diff --git a/test/test_nat44_ed.py b/test/test_nat44_ed.py
index 1fbfff990b6..323249e4424 100644
--- a/test/test_nat44_ed.py
+++ b/test/test_nat44_ed.py
@@ -13,6 +13,7 @@ from scapy.layers.inet import IP, TCP, UDP, ICMP, GRE
 from scapy.layers.inet import IPerror, TCPerror
 from scapy.layers.l2 import Ether
 from scapy.packet import Raw
+from statistics import variance
 from syslog_rfc5424_parser import SyslogMessage, ParseError
 from syslog_rfc5424_parser.constants import SyslogSeverity
 from util import ppp, pr, ip4_range
@@ -2313,14 +2314,17 @@ class TestNAT44ED(VppTestCase):
             raise
 
     def test_outside_address_distribution(self):
-        """Outside address distribution based on source address"""
+        """NAT44ED outside address distribution based on source address"""
 
+        addresses = 65
         x = 100
-        nat_addresses = []
 
-        for i in range(1, x):
+        nat_addresses = []
+        nat_distribution = {}
+        for i in range(1, addresses):
             a = "10.0.0.%d" % i
             nat_addresses.append(a)
+            nat_distribution[a] = set()
 
         self.nat_add_inside_interface(self.pg0)
         self.nat_add_outside_interface(self.pg1)
@@ -2359,16 +2363,11 @@ class TestNAT44ED(VppTestCase):
             self.assertTrue(info is not None)
             self.assertEqual(packet_index, info.index)
             p_sent = info.data
-            packed = socket.inet_aton(p_sent[IP].src)
-            numeric = struct.unpack("!L", packed)[0]
-            numeric = socket.htonl(numeric)
-            a = nat_addresses[(numeric - 1) % len(nat_addresses)]
-            self.assertEqual(
-                a,
-                p_recvd[IP].src,
-                "Invalid packet (src IP %s translated to %s, but expected %s)"
-                % (p_sent[IP].src, p_recvd[IP].src, a),
-            )
+            self.assertIn(p_recvd[IP].src, nat_distribution)
+            nat_distribution[p_recvd[IP].src].add(p_sent[IP].src)
+
+        var = variance(map(len, nat_distribution.values()), x / addresses)
+        self.assertLess(var, 0.33, msg="Bad outside address distribution")
 
     def test_dynamic_edge_ports(self):
         """NAT44ED dynamic translation test: edge ports"""