Changeset 1407294 for lib

Timestamp:

03/18/15 16:31:49 (6 years ago)

Author:

Richard Sanger <rsangerarj@…>

Branches:

4.0.1-hotfixes, cachetimestamps, develop, dpdk-ndag, etsilive, libtrace4, master, ndag_format, pfring, rc-4.0.1, rc-4.0.2, rc-4.0.3, rc-4.0.4, ringdecrementfix, ringperformance, ringtimestampfixes

Children:

963b13b

Parents:

6c84681

Message:

Remove HASHER_HARDWARE and doc/code tidies

We don't want to expose this option to the user as it was only used internally.
As it happens we can completely remove it if needed.

Remove error handling from start thread and fix gcc warning in start thread.

Location:

lib

Files:

• format_dpdk.c (modified) (1 diff)
• format_linux_common.c (modified) (1 diff)
• libtrace_parallel.h (modified) (4 diffs)
• trace_parallel.c (modified) (15 diffs)

lib/format_dpdk.c

@@ -879 +879 @@
                         toeplitz_create_bikey(FORMAT(libtrace)->rss_key);
                         return 0;
-                case HASHER_HARDWARE:
                 case HASHER_CUSTOM:
                         // We don't support these

lib/format_linux_common.c

@@ -527 +527 @@
                                         return 0;
                                 case HASHER_CUSTOM:
-                                case HASHER_HARDWARE:
                                         return -1;
                         }

lib/libtrace_parallel.h

@@ -284 +284 @@
 };
 
+/** The hasher types avaliable to libtrace application.
+ * These can be selected using trace_set_hasher().
+ */
 enum hasher_types {
-        /**
-         * Balance load across CPUs best as possible, this is basically to say do
-         * not care about hash. This might still might be implemented
-         * using a hash or round robin etc. under the hood depending on the format
+        /** Balance load across per-packet threads as best as possible, this is
+         * basically to say I do not care about where packets are sent. This
+         * might still might be implemented using a hash or round robin etc.
+         * depending on the format and libtrace configuration.
          */
         HASHER_BALANCE,
 
-        /** Use a hash which is bi-directional for TCP flows, that is packets with
-         * the same hash are sent to the same thread. All non TCP packets will be
-         * sent to the same thread. UDP may or may not be sent to separate
-         * threads like TCP, this depends on the format support.
+        /** Use a hash which is bi-directional for TCP and UDP flows, that is
+         * packets with the same 5-tuple are sent to the same per-packet thread.
+         * All non TCP/UDP packets will be sent to the same thread.
+         *
+         * @note it is possible that UDP packets may not be spread across
+         * per-packet threads, depending upon the format support. In this case
+         * they would be directed to a single per-packet thread.
          */
         HASHER_BIDIRECTIONAL,
 
-        /**
-         * Use a hash which is uni-directional across TCP flows, that means the
-         * opposite directions of the same 5 tuple might end up on separate cores.
-         * Otherwise is identical to HASHER_BIDIRECTIONAL
+        /** Use a hash which is uni-directional across TCP and UDP flows, this
+         * means the opposing directions of the same 5-tuple might end up on
+         * different per-packet threads.
+         * Otherwise this is identical to HASHER_BIDIRECTIONAL
          */
         HASHER_UNIDIRECTIONAL,
 
         /**
-         * Always use the user supplied hasher, this currently disables native
-         * support and is likely significantly slower.
-         */
-        HASHER_CUSTOM,
-
-        /**
-         * This is not a valid option, used internally only!!! TODO remove
-         * Set by the format if the hashing is going to be done in hardware
-         */
-        HASHER_HARDWARE
+         * Always use the user supplied hasher, this disables native
+         * support in and is likely significantly slower.
+         */
+        HASHER_CUSTOM
 };
 
@@ -676 +676 @@
 /// @}
 
-
-/** TODO DOXS
+/** Set the hasher function for a parallel trace.
+ *
+ * @param[in] trace The parallel trace to apply the hasher to
+ * @param[in] type The type of hashing to apply, see enum hasher_types
+ * @param[in] hasher A hasher function to use [Optional]
+ * @param[in] data Data passed to the hasher function [Optional]
+ *
+ * @return 0 if successful otherwise -1 on error
+ *
+ * The hasher function in a parallel trace can be used to control which
+ * per-packet thread a packets is processed by.
+ *
+ * HASHER_BALANCE is the default and will dispatch packets as fast as possible
+ * to all threads arbitrarily. As such when called the hasher and
+ * data parameters must be set to NULL.
+ *
+ * HASHER_CUSTOM will force the libtrace to use the user defined function. As
+ * such the hasher parameter must be supplied.
+ *
+ * With other defined hasher types we will try to push the hashing into the format
+ * by default. In this case the hasher parameter is optional and will be
+ * preferred over the default supplied by libtrace.
+ *
+ * @note When supplying a hasher function it should be thread-safe as it could
+ * be run in parallel by libtrace. Ideally this should rely upon no state, other
+ * than some form of seed value supplied in data.
  */
 DLLEXPORT int trace_set_hasher(libtrace_t *trace, enum hasher_types type, fn_hasher hasher, void *data);
@@ -685 +709 @@
  * as such making use of built-in types is important.
  *
- * User specific results should be defined as values greater than RESULT_USER(1000)
+ * Custom result types users should be defined as RESULT_USER(1000) or greater.
  *
  */
@@ -731 +755 @@
                                     int type);
 
-/** Check if a dedicated hasher thread is being used
+/** Check if a dedicated hasher thread is being used.
  *
  * @return True if the trace has dedicated hasher thread otherwise false.
+ *
+ * This is valid once the trace is running after calling trace_pstart().
  */
 DLLEXPORT bool trace_has_dedicated_hasher(libtrace_t * libtrace);

lib/trace_parallel.c

@@ -657 +657 @@
 
         pthread_exit(NULL);
-};
+}
 
 /**
@@ -1314 +1314 @@
 
 /**
+ * @return the number of CPU cores on the machine. -1 if unknown.
+ */
+SIMPLE_FUNCTION static int get_nb_cores() {
+        // TODO add BSD support
+        return sysconf(_SC_NPROCESSORS_ONLN);
+}
+
+/**
  * Verifies the configuration and sets default values for any values not
  * specified by the user.
  */
 static void verify_configuration(libtrace_t *libtrace) {
-        bool require_hasher = false;
-
-        /* Might we need a dedicated hasher thread? */
-        if (libtrace->hasher && libtrace->hasher_type != HASHER_HARDWARE) {
-                require_hasher = true;
-        }
 
         if (libtrace->config.hasher_queue_size <= 0)
@@ -1329 +1331 @@
 
         if (libtrace->config.perpkt_threads <= 0) {
-                // TODO add BSD support
-                libtrace->perpkt_thread_count = sysconf(_SC_NPROCESSORS_ONLN);
+                libtrace->perpkt_thread_count = get_nb_cores();
                 if (libtrace->perpkt_thread_count <= 0)
                         // Lets just use one
@@ -1354 +1355 @@
                 libtrace->combiner = combiner_unordered;
 
-
         /* Figure out if we are using a dedicated hasher thread? */
-        if (require_hasher && libtrace->perpkt_thread_count > 1) {
+        if (libtrace->hasher && libtrace->perpkt_thread_count > 1) {
                 libtrace->hasher_thread.type = THREAD_HASHER;
         }
@@ -1382 +1382 @@
                        int perpkt_num,
                        const char *name) {
-        int ret;
+        pthread_attr_t attrib;
+        cpu_set_t cpus;
+        int ret, i;
         assert(t->type == THREAD_EMPTY);
         t->trace = trace;
@@ -1389 +1391 @@
         t->type = type;
         t->state = THREAD_RUNNING;
-        ret = pthread_create(&t->tid, NULL, start_routine, (void *) trace);
+
+        CPU_ZERO(&cpus);
+        for (i = 0; i < get_nb_cores(); i++)
+                CPU_SET(i, &cpus);
+        pthread_attr_init(&attrib);
+        pthread_attr_setaffinity_np(&attrib, sizeof(cpus), &cpus);
+
+        ret = pthread_create(&t->tid, &attrib, start_routine, (void *) trace);
+        pthread_attr_destroy(&attrib);
         if (ret != 0) {
                 libtrace_zero_thread(t);
-                trace_set_err(trace, ret, "Failed to create a thread");
+                trace_set_err(trace, ret, "Failed to create a thread of type=%d\n", type);
                 return -1;
         }
@@ -1568 +1578 @@
                                    THREAD_HASHER, hasher_entry, -1,
                                    "hasher-thread");
-                if (ret != 0) {
-                        trace_set_err(libtrace, errno, "trace_pstart "
-                                      "failed to start a hasher thread.");
+                if (ret != 0)
                         goto cleanup_started;
-                }
                 libtrace->pread = trace_pread_packet_hasher_thread;
         } else {
@@ -1592 +1599 @@
                                    THREAD_PERPKT, perpkt_threads_entry, i,
                                    name);
-                if (ret != 0) {
-                        trace_set_err(libtrace, errno, "trace_pstart "
-                                      "failed to start a perpkt thread.");
+                if (ret != 0)
                         goto cleanup_threads;
-                }
         }
 
@@ -1606 +1610 @@
                                    THREAD_REPORTER, reporter_entry, -1,
                                    "reporter_thread");
-                if (ret != 0) {
-                        trace_set_err(libtrace, errno, "trace_pstart "
-                                      "failed to start reporter thread.");
+                if (ret != 0)
                         goto cleanup_threads;
-                }
         }
 
@@ -1618 +1619 @@
                                    THREAD_KEEPALIVE, keepalive_entry, -1,
                                    "keepalive_thread");
-                if (ret != 0) {
-                        trace_set_err(libtrace, errno, "trace_pstart "
-                                      "failed to start keepalive thread.");
+                if (ret != 0)
                         goto cleanup_threads;
-                }
         }
 
@@ -1760 +1758 @@
                         libtrace_message_t message = {0};
                         message.code = MESSAGE_DO_PAUSE;
-                        trace_message_thread(libtrace, &libtrace->perpkt_threads[i], &message);
+                        ASSERT_RET(trace_message_thread(libtrace, &libtrace->perpkt_threads[i], &message), != -1);
                         if(trace_has_dedicated_hasher(libtrace)) {
                                 // The hasher has stopped and other threads have messages waiting therefore
@@ -1870 +1868 @@
 }
 
-/**
- * Set the hasher type along with a selected function, if hardware supports
- * that generic type of hashing it will be used otherwise the supplied
- * hasher function will be used and passed data when called.
- *
- * @return 0 if successful otherwise -1 on error
- */
 DLLEXPORT int trace_set_hasher(libtrace_t *trace, enum hasher_types type, fn_hasher hasher, void *data) {
         int ret = -1;
-        if (type == HASHER_HARDWARE || (type == HASHER_CUSTOM && !hasher) || (type == HASHER_BALANCE && hasher)) {
+        if ((type == HASHER_CUSTOM && !hasher) || (type == HASHER_BALANCE && hasher)) {
                 return -1;
         }
@@ -1900 +1891 @@
 
         if (ret == -1) {
-                // We have to deal with this ourself
-                // This most likely means single threaded reading of the trace
+                /* We have to deal with this ourself */
                 if (!hasher) {
                         switch (type)
@@ -1918 +1908 @@
                                         toeplitz_init_config(trace->hasher_data, 0);
                                         return 0;
-                                case HASHER_HARDWARE:
-                                        return -1;
                         }
                         return -1;
                 }
         } else {
-                // The hardware is dealing with this yay
-                trace->hasher_type = HASHER_HARDWARE;
+                /* If the hasher is hardware we zero out the hasher and hasher
+                 * data fields - only if we need a hasher do we do this */
+                trace->hasher = NULL;
+                trace->hasher_data = NULL;
         }
 
@@ -2057 +2047 @@
 {
         int i;
-        int missed;
+        int missed = 0;
         if (message->sender == NULL)
                 message->sender = get_thread_descriptor(libtrace);