IP fragmentation buffer chains, part 2.

Change-Id: I8d1072cf9ff9f502302fd906c5590e0f3698dc60
Signed-off-by: Ole Troan <ot@cisco.com>

2 files changed, 75 insertions, 31 deletions

diff --git a/src/vnet/ip/ip_frag.c b/src/vnet/ip/ip_frag.c
index 667a92bbfa5..630948723cd 100644
--- a/src/vnet/ip/ip_frag.c
+++ b/src/vnet/ip/ip_frag.c
@@ -128,7 +128,6 @@ ip4_frag_do_fragment (vlib_main_t * vm, u32 from_bi, u32 ** buffer,
       if (!vlib_buffer_alloc (vm, &to_bi, 1))
 	{
 	  *error = IP_FRAG_ERROR_MEMORY;
-	  /* XXX: Free already allocated buffers? */
 	  return;
 	}
       vec_add1 (*buffer, to_bi);
@@ -152,12 +151,22 @@ ip4_frag_do_fragment (vlib_main_t * vm, u32 from_bi, u32 ** buffer,
       to_ip4 = vlib_buffer_get_current (to_b) + offset;
       to_data = (void *) (to_ip4 + 1);
 
-      /* Spin through buffer chain copying data */
-      // XXX: Make sure we don't overflow source buffer!!!
-      if (len > left_in_from_buffer)
+      /* Spin through from buffers filling up the to buffer */
+      u16 to_ptr = 0;
+      u16 bytes_to_copy, left_in_to_buffer = len;
+      while (1)
 	{
-	  clib_memcpy (to_data, from_data + ptr, left_in_from_buffer);
-
+	  /* Figure out how many bytes we can safely copy */
+	  bytes_to_copy = left_in_to_buffer <= left_in_from_buffer ?
+	    left_in_to_buffer : left_in_from_buffer;
+	  clib_memcpy (to_data + to_ptr, from_data + ptr, bytes_to_copy);
+	  left_in_to_buffer -= bytes_to_copy;
+	  ptr += bytes_to_copy;
+	  left_in_from_buffer -= bytes_to_copy;
+	  if (left_in_to_buffer == 0)
+	    break;
+
+	  ASSERT (left_in_from_buffer == 0);
 	  /* Move buffer */
 	  if (!(from_b->flags & VLIB_BUFFER_NEXT_PRESENT))
 	    {
@@ -166,18 +175,11 @@ ip4_frag_do_fragment (vlib_main_t * vm, u32 from_bi, u32 ** buffer,
 	    }
 	  from_b = vlib_get_buffer (vm, from_b->next_buffer);
 	  from_data = (u8 *) vlib_buffer_get_current (from_b);
-	  clib_memcpy (to_data + left_in_from_buffer, from_data,
-		       len - left_in_from_buffer);
-	  ptr = len - left_in_from_buffer;
-	  left_in_from_buffer =
-	    from_b->current_length - (len - left_in_from_buffer);
-	}
-      else
-	{
-	  clib_memcpy (to_data, from_data + ptr, len);
-	  left_in_from_buffer -= len;
-	  ptr += len;
+	  ptr = 0;
+	  left_in_from_buffer = from_b->current_length;
+	  to_ptr += bytes_to_copy;
 	}
+
       to_b->current_length = offset + len + sizeof (ip4_header_t);
 
       to_ip4->fragment_id = ip_frag_id;
@@ -209,8 +211,6 @@ ip4_frag_do_fragment (vlib_main_t * vm, u32 from_bi, u32 ** buffer,
       rem -= len;
       fo += len;
     }
-  /* Free original packet chain */
-  vlib_buffer_free_one (vm, from_bi);
 }
 
 void
@@ -287,6 +287,8 @@ ip4_frag (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_frame_t * frame)
 
 	  if (error0 == IP_FRAG_ERROR_NONE)
 	    {
+	      /* Free original buffer chain */
+	      vlib_buffer_free_one (vm, pi0);
 	      frag_sent += vec_len (buffer);
 	      small_packets += (vec_len (buffer) == 1);
 	    }
diff --git a/test/test_ipip.py b/test/test_ipip.py
index 582ab5be84c..8d99f83e6c2 100644
--- a/test/test_ipip.py
+++ b/test/test_ipip.py
@@ -3,7 +3,7 @@
 
 import unittest
 from scapy.layers.inet6 import IPv6, Ether, IP, UDP
-from scapy.all import fragment
+from scapy.all import fragment, RandShort
 from framework import VppTestCase, VppTestRunner
 from vpp_ip_route import VppIpRoute, VppRoutePath, DpoProto
 from socket import AF_INET, AF_INET6, inet_pton
@@ -60,6 +60,18 @@ class TestIPIP(VppTestCase):
     def validate(self, rx, expected):
         self.assertEqual(rx, expected.__class__(str(expected)))
 
+    def generate_frags(self, payload_length, fragment_size):
+        p_ether = Ether(src=self.pg1.remote_mac, dst=self.pg1.local_mac)
+        p_payload = UDP(sport=1234, dport=1234) / self.payload(payload_length)
+        p_ip4 = IP(src="1.2.3.4", dst=self.pg0.remote_ip4)
+        outer_ip4 = (p_ether / IP(src=self.pg1.remote_ip4,
+                                  id=RandShort(),
+                                  dst=self.pg0.local_ip4) / p_ip4 / p_payload)
+        frags = fragment(outer_ip4, fragment_size)
+        p4_reply = (p_ip4 / p_payload)
+        p4_reply.ttl -= 1
+        return frags, p4_reply
+
     def test_ipip4(self):
         """ ip{v4,v6} over ip4 test """
         p_ether = Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac)
@@ -142,29 +154,48 @@ class TestIPIP(VppTestCase):
         for p in rx:
             self.validate(p[1], p6_reply)
 
+        #
         # Fragmentation / Reassembly and Re-fragmentation
+        #
         rv = self.vapi.ip_reassembly_enable_disable(
             sw_if_index=self.pg1.sw_if_index,
             enable_ip4=1)
-        # Decapsulation
-        p_ether = Ether(src=self.pg1.remote_mac, dst=self.pg1.local_mac)
-        p_payload = UDP(sport=1234, dport=1234) / self.payload(3123)
-        p_ip4 = IP(src="1.2.3.4", dst=self.pg0.remote_ip4)
-        outer_ip4 = (p_ether / IP(src=self.pg1.remote_ip4,
-                     dst=self.pg0.local_ip4) / p_ip4 / p_payload)
-        frags = fragment(outer_ip4, 1400)
-        p4_reply = (p_ip4 / p_payload)
-        p4_reply.ttl -= 1
 
+        # Send lots of fragments, verify reassembled packet
+        frags, p4_reply = self.generate_frags(3131, 1400)
+        f = []
+        for i in range(0, 1000):
+            f.extend(frags)
+        self.pg1.add_stream(f)
         self.pg_enable_capture()
-        self.pg1.add_stream(frags)
         self.pg_start()
-        rx = self.pg0.get_capture(1)
+        rx = self.pg0.get_capture(1000)
+
         for p in rx:
             self.validate(p[1], p4_reply)
 
+        f = []
+        r = []
+        for i in range(1, 90):
+            frags, p4_reply = self.generate_frags(i * 100, 1000)
+            f.extend(frags)
+            r.extend(p4_reply)
+        self.pg_enable_capture()
+        self.pg1.add_stream(f)
+        self.pg_start()
+        rx = self.pg0.get_capture(89)
+        i = 0
+        for p in rx:
+            self.validate(p[1], r[i])
+            i += 1
+
         # Now try with re-fragmentation
+        #
+        # Send fragments to tunnel head-end, for the tunnel head end
+        # to reassemble and then refragment
+        #
         self.vapi.sw_interface_set_mtu(self.pg0.sw_if_index, [576, 0, 0, 0])
+        frags, p4_reply = self.generate_frags(3123, 1200)
         self.pg_enable_capture()
         self.pg1.add_stream(frags)
         self.pg_start()
@@ -174,6 +205,17 @@ class TestIPIP(VppTestCase):
         p4_reply.id = 256
         self.validate(reass_pkt, p4_reply)
 
+        self.vapi.sw_interface_set_mtu(self.pg0.sw_if_index, [1600, 0, 0, 0])
+        frags, p4_reply = self.generate_frags(3123, 1200)
+        self.pg_enable_capture()
+        self.pg1.add_stream(frags)
+        self.pg_start()
+        rx = self.pg0.get_capture(2)
+        reass_pkt = reassemble(rx)
+        p4_reply.ttl -= 1
+        p4_reply.id = 512
+        self.validate(reass_pkt, p4_reply)
+
     def test_ipip6(self):
         """ ip{v4,v6} over ip6 test """
         p_ether = Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac)