Handle buffer alloc failure in vlib_buffer_add_data

It's not OK to crash due to a transient buffer allocation failure.

Return 1 if the requested operation failed, otherwise 0.

Buffer index parameter change to a value-result, so the caller can
differentiate between partial and complete allocation failure: callers
which request an initial allocation (inbound bi = ~0) need to check
the (out) value to decide whether or not to call vlib_buffer_free(...).

Change-Id: I03029d7f2714c17dca4630dfd95a1eb578b68384
Signed-off-by: Dave Barach <dave@barachs.net>

5 files changed, 81 insertions, 42 deletions

diff --git a/src/vlib/buffer.c b/src/vlib/buffer.c
index d024aba1e0f..5370d501293 100644
--- a/src/vlib/buffer.c
+++ b/src/vlib/buffer.c
@@ -705,16 +705,16 @@ vlib_packet_template_get_packet (vlib_main_t * vm,
 }
 
 /* Append given data to end of buffer, possibly allocating new buffers. */
-u32
+int
 vlib_buffer_add_data (vlib_main_t * vm,
 		      vlib_buffer_free_list_index_t free_list_index,
-		      u32 buffer_index, void *data, u32 n_data_bytes)
+		      u32 * buffer_index, void *data, u32 n_data_bytes)
 {
   u32 n_buffer_bytes, n_left, n_left_this_buffer, bi;
   vlib_buffer_t *b;
   void *d;
 
-  bi = buffer_index;
+  bi = *buffer_index;
   if (bi == ~0
       && 1 != vlib_buffer_alloc_from_free_list (vm, &bi, 1, free_list_index))
     goto out_of_buffers;
@@ -756,11 +756,12 @@ vlib_buffer_add_data (vlib_main_t * vm,
       b = vlib_get_buffer (vm, b->next_buffer);
     }
 
-  return bi;
+  *buffer_index = bi;
+  return 0;
 
 out_of_buffers:
-  clib_error ("out of buffers");
-  return bi;
+  clib_warning ("out of buffers");
+  return 1;
 }
 
 u16
diff --git a/src/vlib/buffer_funcs.h b/src/vlib/buffer_funcs.h
index 54fc1f61598..b561a91c394 100644
--- a/src/vlib/buffer_funcs.h
+++ b/src/vlib/buffer_funcs.h
@@ -672,9 +672,9 @@ vlib_buffer_free_list_buffer_size (vlib_main_t * vm,
 }
 
 /* Append given data to end of buffer, possibly allocating new buffers. */
-u32 vlib_buffer_add_data (vlib_main_t * vm,
+int vlib_buffer_add_data (vlib_main_t * vm,
 			  vlib_buffer_free_list_index_t free_list_index,
-			  u32 buffer_index, void *data, u32 n_data_bytes);
+			  u32 * buffer_index, void *data, u32 n_data_bytes);
 
 /* duplicate all buffers in chain */
 always_inline vlib_buffer_t *
diff --git a/src/vnet/ip/icmp6.h b/src/vnet/ip/icmp6.h
index 1cac8240466..628d225eac4 100644
--- a/src/vnet/ip/icmp6.h
+++ b/src/vnet/ip/icmp6.h
@@ -46,7 +46,8 @@
   _ (PACKET_TOO_BIG_SENT, "packet too big response sent")		\
   _ (TTL_EXPIRE_SENT, "hop limit exceeded response sent")		\
   _ (PARAM_PROBLEM_SENT, "parameter problem response sent")		\
-  _ (DROP, "error message dropped")
+  _ (DROP, "error message dropped")					\
+  _ (ALLOC_FAILURE, "buffer allocation failure")
 
 
 typedef enum
diff --git a/src/vnet/ip/ip6_neighbor.c b/src/vnet/ip/ip6_neighbor.c
index f9acfeff389..fde3a7e71d4 100755
--- a/src/vnet/ip/ip6_neighbor.c
+++ b/src/vnet/ip/ip6_neighbor.c
@@ -1690,11 +1690,15 @@ icmp6_router_solicitation (vlib_main_t * vm,
 		      u16 payload_length =
 			sizeof (icmp6_router_advertisement_header_t);
 
-		      vlib_buffer_add_data (vm,
-					    vlib_buffer_get_free_list_index
-					    (p0), bi0, (void *) &rh,
-					    sizeof
-					    (icmp6_router_advertisement_header_t));
+		      if (vlib_buffer_add_data
+			  (vm, vlib_buffer_get_free_list_index
+			   (p0), &bi0, (void *) &rh,
+			   sizeof (icmp6_router_advertisement_header_t)))
+			{
+			  /* buffer allocation failed, drop the pkt */
+			  error0 = ICMP6_ERROR_ALLOC_FAILURE;
+			  goto drop0;
+			}
 
 		      if (radv_info->adv_link_layer_address)
 			{
@@ -1709,11 +1713,15 @@ icmp6_router_solicitation (vlib_main_t * vm,
 			  clib_memcpy (&h.ethernet_address[0],
 				       eth_if0->address, 6);
 
-			  vlib_buffer_add_data (vm,
-						vlib_buffer_get_free_list_index
-						(p0), bi0, (void *) &h,
-						sizeof
-						(icmp6_neighbor_discovery_ethernet_link_layer_address_option_t));
+			  if (vlib_buffer_add_data
+			      (vm, vlib_buffer_get_free_list_index
+			       (p0), &bi0, (void *) &h,
+			       sizeof
+			       (icmp6_neighbor_discovery_ethernet_link_layer_address_option_t)))
+			    {
+			      error0 = ICMP6_ERROR_ALLOC_FAILURE;
+			      goto drop0;
+			    }
 
 			  payload_length +=
 			    sizeof
@@ -1734,11 +1742,15 @@ icmp6_router_solicitation (vlib_main_t * vm,
 			  payload_length +=
 			    sizeof (icmp6_neighbor_discovery_mtu_option_t);
 
-			  vlib_buffer_add_data (vm,
-						vlib_buffer_get_free_list_index
-						(p0), bi0, (void *) &h,
-						sizeof
-						(icmp6_neighbor_discovery_mtu_option_t));
+			  if (vlib_buffer_add_data
+			      (vm, vlib_buffer_get_free_list_index
+			       (p0), &bi0, (void *) &h,
+			       sizeof
+			       (icmp6_neighbor_discovery_mtu_option_t)))
+			    {
+			      error0 = ICMP6_ERROR_ALLOC_FAILURE;
+			      goto drop0;
+			    }
 			}
 
 		      /* add advertised prefix options  */
@@ -1787,10 +1799,15 @@ icmp6_router_solicitation (vlib_main_t * vm,
 
                             payload_length += sizeof( icmp6_neighbor_discovery_prefix_information_option_t);
 
-                            vlib_buffer_add_data (vm,
-					    vlib_buffer_get_free_list_index (p0),
-                                                  bi0,
-                                                  (void *)&h, sizeof(icmp6_neighbor_discovery_prefix_information_option_t));
+                            if (vlib_buffer_add_data
+                                (vm, vlib_buffer_get_free_list_index (p0),
+                                 &bi0,
+                                 (void *)&h,
+                                 sizeof(icmp6_neighbor_discovery_prefix_information_option_t)))
+                              {
+                                error0 = ICMP6_ERROR_ALLOC_FAILURE;
+                                goto drop0;
+                              }
 
                           }
                       }));
@@ -1875,6 +1892,7 @@ icmp6_router_solicitation (vlib_main_t * vm,
 		}
 	    }
 
+	drop0:
 	  p0->error = error_node->errors[error0];
 
 	  if (error0 != ICMP6_ERROR_NONE)
@@ -2761,6 +2779,7 @@ ip6_neighbor_send_mldpv2_report (u32 sw_if_index)
   n_allocated = vlib_buffer_alloc (vm, &bo0, n_to_alloc);
   if (PREDICT_FALSE (n_allocated == 0))
     {
+    alloc_fail:
       clib_warning ("buffer allocation failure");
       return;
     }
@@ -2827,9 +2846,13 @@ ip6_neighbor_send_mldpv2_report (u32 sw_if_index)
 
     num_addr_records++;
 
-    vlib_buffer_add_data
-      (vm, vlib_buffer_get_free_list_index (b0), bo0,
-       (void *)&rr, sizeof(icmp6_multicast_address_record_t));
+    if(vlib_buffer_add_data
+      (vm, vlib_buffer_get_free_list_index (b0), &bo0,
+       (void *)&rr, sizeof(icmp6_multicast_address_record_t)))
+      {
+        vlib_buffer_free (vm, &bo0, 1);
+        goto alloc_fail;
+      }
 
     payload_length += sizeof( icmp6_multicast_address_record_t);
   }));
diff --git a/src/vnet/srp/node.c b/src/vnet/srp/node.c
index cec014f987d..cf9bd31c513 100644
--- a/src/vnet/srp/node.c
+++ b/src/vnet/srp/node.c
@@ -328,16 +328,24 @@ srp_topology_packet (vlib_main_t * vm, u32 sw_if_index, u8 ** contents)
 					      vec_len (*contents) - STRUCT_OFFSET_OF (srp_generic_control_header_t, control)));
 
   {
-    vlib_frame_t * f = vlib_get_frame_to_node (vm, hi->output_node_index);
+    vlib_frame_t * f; 
     vlib_buffer_t * b;
-    u32 * to_next = vlib_frame_vector_args (f);
-    u32 bi;
-
-    bi = vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX,
-			       /* buffer to append to */ ~0,
-			       *contents, vec_len (*contents));
+    u32 * to_next;
+    u32 bi = ~0;
+
+    if (vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX,
+                              /* buffer to append to */ &bi,
+                              *contents, vec_len (*contents)))
+      {
+        /* complete or partial buffer allocation failure */
+        if (bi != ~0)
+          vlib_buffer_free (vm, &bi, 1);
+        return SRP_ERROR_CONTROL_PACKETS_PROCESSED;
+      }
     b = vlib_get_buffer (vm, bi);
     vnet_buffer (b)->sw_if_index[VLIB_RX] = vnet_buffer (b)->sw_if_index[VLIB_TX] = sw_if_index;
+    f = vlib_get_frame_to_node (vm, hi->output_node_index);
+    to_next = vlib_frame_vector_args (f);
     to_next[0] = bi;
     f->n_vectors = 1;
     vlib_put_frame_to_node (vm, hi->output_node_index, f);
@@ -609,7 +617,7 @@ static void tx_ips_packet (srp_interface_t * si,
   vnet_hw_interface_t * hi = vnet_get_hw_interface (vnm, si->rings[tx_ring].hw_if_index);
   vlib_frame_t * f;
   vlib_buffer_t * b;
-  u32 * to_next, bi;
+  u32 * to_next, bi = ~0;
 
   if (! vnet_sw_interface_is_admin_up (vnm, hi->sw_if_index))
     return;
@@ -620,9 +628,15 @@ static void tx_ips_packet (srp_interface_t * si,
     = ~ip_csum_fold (ip_incremental_checksum (0, &i->control,
 					      sizeof (i[0]) - STRUCT_OFFSET_OF (srp_ips_header_t, control)));
 
-  bi = vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX,
-			     /* buffer to append to */ ~0,
-			     i, sizeof (i[0]));
+  if (vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX,
+                            /* buffer to append to */ &bi,
+                            i, sizeof (i[0])))
+    {
+      /* complete or partial allocation failure */
+      if (bi != ~0)
+        vlib_buffer_free (vm, &bi, 1);
+      return;
+    }
 
   /* FIXME trace. */
   if (0)