session: generate wrong thread errors instead of crashing

Change-Id: I7e59ae718d2722c49d42b22a0874e1645a191e89
Signed-off-by: Florin Coras <fcoras@cisco.com>

7 files changed, 47 insertions, 23 deletions

diff --git a/src/plugins/unittest/session_test.c b/src/plugins/unittest/session_test.c
index c504d8df15d..cc273321d32 100644
--- a/src/plugins/unittest/session_test.c
+++ b/src/plugins/unittest/session_test.c
@@ -1194,7 +1194,8 @@ session_test_rules (vlib_main_t * vm, unformat_input_t * input)
 				      &is_filtered);
   SESSION_TEST ((tc == 0), "lookup for 1.2.3.4/32 1234 5.6.7.8/16 4321 "
 		"should fail (deny rule)");
-  SESSION_TEST ((is_filtered == 1), "lookup should be filtered (deny)");
+  SESSION_TEST ((is_filtered == SESSION_LOOKUP_RESULT_FILTERED),
+		"lookup should be filtered (deny)");
 
   handle = session_lookup_local_endpoint (local_ns_index, &sep);
   SESSION_TEST ((handle == SESSION_DROP_HANDLE), "lookup for 1.2.3.4/32 1234 "
diff --git a/src/vnet/sctp/sctp_error.def b/src/vnet/sctp/sctp_error.def
index 7326301e862..b95b71a9417 100644
--- a/src/vnet/sctp/sctp_error.def
+++ b/src/vnet/sctp/sctp_error.def
@@ -13,6 +13,8 @@
  * limitations under the License.
  */
 sctp_error (NONE, "no error")
+sctp_error (WRONG_WORKER, "Wrong worker thread")
+sctp_error (FILTERED, "Packets filtered")
 sctp_error (PKTS_SENT, "Packets sent")
 sctp_error (INVALID_CONNECTION, "Invalid connection")
 sctp_error (INVALID_TAG, "Invalid verification tag")
@@ -47,5 +49,4 @@ sctp_error (EVENT_FIFO_FULL, "Events not sent for lack of event fifo space")
 sctp_error (UNKNOWN_CHUNK, "Unrecognized / unknown chunk or chunk-state mismatch")
 sctp_error (BUNDLING_VIOLATION, "Bundling not allowed")
 sctp_error (PUNT, "Packets punted")
-sctp_error (FILTERED, "Packets filtered")
 sctp_error (MAX_CONNECTIONS, "Reached max supported subconnection")
diff --git a/src/vnet/sctp/sctp_input.c b/src/vnet/sctp/sctp_input.c
index 4454f99e6bb..88e4eab754d 100644
--- a/src/vnet/sctp/sctp_input.c
+++ b/src/vnet/sctp/sctp_input.c
@@ -2114,7 +2114,7 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node,
 {
   u32 n_left_from, next_index, *from, *to_next;
   u32 my_thread_index = vm->thread_index;
-  u8 is_filtered;
+  u8 result;
   sctp_main_t *tm = vnet_get_sctp_main ();
 
   from = vlib_frame_vector_args (from_frame);
@@ -2175,7 +2175,7 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node,
 							  sctp_hdr->src_port,
 							  TRANSPORT_PROTO_SCTP,
 							  my_thread_index,
-							  &is_filtered);
+							  &result);
 	    }
 	  else
 	    {
@@ -2198,7 +2198,7 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node,
 							  sctp_hdr->src_port,
 							  TRANSPORT_PROTO_SCTP,
 							  my_thread_index,
-							  &is_filtered);
+							  &result);
 	    }
 
 	  /* Length check */
@@ -2256,10 +2256,10 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node,
 	    }
 	  else
 	    {
-	      if (is_filtered)
+	      if (result)
 		{
 		  next0 = SCTP_INPUT_NEXT_DROP;
-		  error0 = SCTP_ERROR_FILTERED;
+		  error0 = SCTP_ERROR_NONE + result;
 		}
 	      else if ((is_ip4 && tm->punt_unknown4) ||
 		       (!is_ip4 && tm->punt_unknown6))
diff --git a/src/vnet/session/session_lookup.c b/src/vnet/session/session_lookup.c
index c410dab121f..931c3d0f9e2 100644
--- a/src/vnet/session/session_lookup.c
+++ b/src/vnet/session/session_lookup.c
@@ -852,7 +852,7 @@ transport_connection_t *
 session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl,
 			       ip4_address_t * rmt, u16 lcl_port,
 			       u16 rmt_port, u8 proto, u32 thread_index,
-			       u8 * is_filtered)
+			       u8 * result)
 {
   session_table_t *st;
   session_kv4_t kv4;
@@ -871,7 +871,11 @@ session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl,
   rv = clib_bihash_search_inline_16_8 (&st->v4_session_hash, &kv4);
   if (rv == 0)
     {
-      ASSERT ((u32) (kv4.value >> 32) == thread_index);
+      if (PREDICT_FALSE ((u32) (kv4.value >> 32) != thread_index))
+	{
+	  *result = SESSION_LOOKUP_RESULT_WRONG_THREAD;
+	  return 0;
+	}
       s = session_get (kv4.value & 0xFFFFFFFFULL, thread_index);
       return tp_vfts[proto].get_connection (s->connection_index,
 					    thread_index);
@@ -891,8 +895,11 @@ session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl,
 					      rmt, lcl_port, rmt_port);
   if (session_lookup_action_index_is_valid (action_index))
     {
-      if ((*is_filtered = (action_index == SESSION_RULES_TABLE_ACTION_DROP)))
-	return 0;
+      if (action_index == SESSION_RULES_TABLE_ACTION_DROP)
+	{
+	  *result = SESSION_LOOKUP_RESULT_FILTERED;
+	  return 0;
+	}
       if ((s = session_lookup_action_to_session (action_index,
 						 FIB_PROTOCOL_IP4, proto)))
 	return tp_vfts[proto].get_listener (s->connection_index);
@@ -912,9 +919,8 @@ session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl,
 /**
  * Lookup connection with ip4 and transport layer information
  *
- * Not optimized. This is used on the fast path so it needs to be fast.
- * Thereby, duplication of code and 'hacks' allowed. Lookup logic is identical
- * to that of @ref session_lookup_connection_wt4
+ * Not optimized. Lookup logic is identical to that of
+ * @ref session_lookup_connection_wt4
  *
  * @param fib_index	index of the fib wherein the connection was received
  * @param lcl		local ip4 address
@@ -1070,7 +1076,7 @@ transport_connection_t *
 session_lookup_connection_wt6 (u32 fib_index, ip6_address_t * lcl,
 			       ip6_address_t * rmt, u16 lcl_port,
 			       u16 rmt_port, u8 proto, u32 thread_index,
-			       u8 * is_filtered)
+			       u8 * result)
 {
   session_table_t *st;
   stream_session_t *s;
@@ -1087,6 +1093,11 @@ session_lookup_connection_wt6 (u32 fib_index, ip6_address_t * lcl,
   if (rv == 0)
     {
       ASSERT ((u32) (kv6.value >> 32) == thread_index);
+      if (PREDICT_FALSE ((u32) (kv6.value >> 32) != thread_index))
+	{
+	  *result = SESSION_LOOKUP_RESULT_WRONG_THREAD;
+	  return 0;
+	}
       s = session_get (kv6.value & 0xFFFFFFFFULL, thread_index);
       return tp_vfts[proto].get_connection (s->connection_index,
 					    thread_index);
@@ -1102,8 +1113,11 @@ session_lookup_connection_wt6 (u32 fib_index, ip6_address_t * lcl,
 					      rmt, lcl_port, rmt_port);
   if (session_lookup_action_index_is_valid (action_index))
     {
-      if ((*is_filtered = (action_index == SESSION_RULES_TABLE_ACTION_DROP)))
-	return 0;
+      if (action_index == SESSION_RULES_TABLE_ACTION_DROP)
+	{
+	  *result = SESSION_LOOKUP_RESULT_FILTERED;
+	  return 0;
+	}
       if ((s = session_lookup_action_to_session (action_index,
 						 FIB_PROTOCOL_IP6, proto)))
 	return tp_vfts[proto].get_listener (s->connection_index);
diff --git a/src/vnet/session/session_lookup.h b/src/vnet/session/session_lookup.h
index 1acf923831d..212a11833a3 100644
--- a/src/vnet/session/session_lookup.h
+++ b/src/vnet/session/session_lookup.h
@@ -21,6 +21,13 @@
 #include <vnet/session/transport.h>
 #include <vnet/session/application_namespace.h>
 
+typedef enum session_lookup_result_
+{
+  SESSION_LOOKUP_RESULT_NONE,
+  SESSION_LOOKUP_RESULT_WRONG_THREAD,
+  SESSION_LOOKUP_RESULT_FILTERED
+} session_lookup_result_t;
+
 stream_session_t *session_lookup_safe4 (u32 fib_index, ip4_address_t * lcl,
 					ip4_address_t * rmt, u16 lcl_port,
 					u16 rmt_port, u8 proto);
diff --git a/src/vnet/tcp/tcp_error.def b/src/vnet/tcp/tcp_error.def
index 141ca515995..0d6c44b5402 100644
--- a/src/vnet/tcp/tcp_error.def
+++ b/src/vnet/tcp/tcp_error.def
@@ -13,6 +13,8 @@
  * limitations under the License.
  */
 tcp_error (NONE, "no error")
+tcp_error (WRONG_THREAD, "Wrong worker thread")
+tcp_error (FILTERED, "Packets filtered")
 tcp_error (LENGTH, "inconsistent ip/tcp lengths")
 tcp_error (NO_LISTENER, "no listener for dst port")
 tcp_error (LOOKUP_DROPS, "lookup drops")
@@ -40,7 +42,6 @@ tcp_error (INVALID_CONNECTION, "Invalid connection")
 tcp_error (CONNECTION_CLOSED, "Connection closed")
 tcp_error (CREATE_EXISTS, "Connection already exists")
 tcp_error (PUNT, "Packets punted")
-tcp_error (FILTERED, "Packets filtered")
 tcp_error (OPTIONS, "Could not parse options")
 tcp_error (PAWS, "PAWS check failed")
 tcp_error (RCV_WND, "Segment not in receive window")
diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c
index 4358bd30d88..82ab817430f 100644
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -3295,7 +3295,7 @@ tcp_input_trace_frame (vlib_main_t * vm, vlib_node_runtime_t * node,
 static void
 tcp_input_set_error_next (tcp_main_t * tm, u16 * next, u32 * error, u8 is_ip4)
 {
-  if (*error == TCP_ERROR_FILTERED)
+  if (*error == TCP_ERROR_FILTERED || *error == TCP_ERROR_WRONG_THREAD)
     {
       *next = TCP_INPUT_NEXT_DROP;
     }
@@ -3319,7 +3319,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error,
   int n_advance_bytes, n_data_bytes;
   transport_connection_t *tc;
   tcp_header_t *tcp;
-  u8 is_filtered = 0;
+  u8 result = 0;
 
   if (is_ip4)
     {
@@ -3345,7 +3345,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error,
       tc = session_lookup_connection_wt4 (fib_index, &ip4->dst_address,
 					  &ip4->src_address, tcp->dst_port,
 					  tcp->src_port, TRANSPORT_PROTO_TCP,
-					  thread_index, &is_filtered);
+					  thread_index, &result);
     }
   else
     {
@@ -3371,7 +3371,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error,
       tc = session_lookup_connection_wt6 (fib_index, &ip6->dst_address,
 					  &ip6->src_address, tcp->dst_port,
 					  tcp->src_port, TRANSPORT_PROTO_TCP,
-					  thread_index, &is_filtered);
+					  thread_index, &result);
     }
 
   vnet_buffer (b)->tcp.seq_number = clib_net_to_host_u32 (tcp->seq_number);
@@ -3382,7 +3382,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error,
     + n_data_bytes;
   vnet_buffer (b)->tcp.flags = 0;
 
-  *error = is_filtered ? TCP_ERROR_FILTERED : *error;
+  *error = result ? TCP_ERROR_NONE + result : *error;
 
   return tcp_get_connection_from_transport (tc);
 }