Adding Doca Communication Channel API #190

EldarShalev · 2022-07-10T13:58:07Z

In a DPU system there is a need for a secure, network independent communication channel between the host and the DPU.
The DOCA Comm Channel provides a client-server API to provide such functionality.
The communication channel can allow the host to control services on the DPU, or to activate certain offloads.
In order to use Doca API, "doca_comm_channel.h" must be found.

Example how to use:
Host: ./sockperf pp --addr /tmp/test --doca-comm-channel
Arm: ./sockperf sr --addr /tmp/test --doca-comm-channel

swx-jenkins3 · 2022-07-10T13:59:15Z

Can one of the admins verify this patch?

agalanin-at-nvidia · 2022-07-11T12:44:58Z

configure.ac

+##########################################################################
+# check DOCA communication channel API
+#
+AC_ARG_ENABLE(


it's better to use AC_ARG_WITH here to allow specifying alternative library installation path.

Pay attention that DOCA should not be configured at the same time with --with-tls. See https://github.com/Mellanox/sockperf/blob/sockperf_v2/config/m4/tls.m4#L10
Look at potential issue in this case: https://github.com/Mellanox/sockperf/pull/190/files#diff-649a0b286b520f22da501867c812f1acbc760fe028ea70edb14b60672ba7c716R116

agalanin-at-nvidia · 2022-09-08T09:31:52Z

Please rebase on top of latest changes.

EldarShalev · 2022-09-13T12:19:53Z

Hey, Rebased on top of latest changes but DOCA API was changed drastically so still no need to review at the moment

src/sockperf.cpp

igor-ivanov · 2023-02-10T11:09:19Z

src/sockperf.cpp

@@ -2380,7 +2429,7 @@ void cleanup() {
                    }
 #endif // __windows__
                    if (s_user_params.mode == MODE_SERVER)
-                        os_unlink_unix_path(g_fds_array[ifd]->server_addr.addr_un.sun_path);
+                        unlink(g_fds_array[ifd]->server_addr.addr_un.sun_path);


what is a reason of using unlink instead of os_unlink_unix_path

igor-ivanov · 2023-02-10T11:12:14Z

src/sockperf.cpp

+#if defined(USING_DOCA_COMM_CHANNEL_API)
+typedef doca_error_t (*jobs_check)(struct doca_devinfo *);
+doca_error_t
+open_doca_device_with_pci(const struct doca_pci_bdf *value, jobs_check func, struct doca_dev **retval)


should it be static?

igor-ivanov · 2023-02-10T11:13:31Z

src/sockperf.cpp

+        size_t i;
+
+        /* Set default return value */
+        *retval = NULL;


If passed retval is NULL?

igor-ivanov · 2023-02-10T11:13:58Z

src/sockperf.cpp

+}
+
+doca_error_t
+open_doca_device_rep_with_pci(struct doca_dev *local, enum doca_dev_rep_filter filter, struct doca_pci_bdf *pci_bdf,


igor-ivanov · 2023-02-10T11:14:19Z

src/sockperf.cpp

+        doca_error_t result;
+        size_t i;
+
+        *retval = NULL;


if retval is NULL?

igor-ivanov · 2023-02-10T11:14:28Z

src/sockperf.cpp

+
+
+doca_error_t
+parse_pci_addr(char const *pci_addr, struct doca_pci_bdf *out_bdf)


igor-ivanov · 2023-02-10T11:15:13Z

src/sockperf.cpp

+{
+        unsigned int bus_bitmask = 0xFFFFFF00;
+        unsigned int dev_bitmask = 0xFFFFFFE0;
+        unsigned int func_bitmask = 0xFFFFFFF8;


please consider uint16_t

igor-ivanov · 2023-02-10T11:15:26Z

src/sockperf.cpp

+}
+
+
+int bringup_for_doca(std::unique_ptr<fds_data> &tmp)


igor-ivanov · 2023-11-27T16:10:53Z

src/server.cpp

+                    hostport.c_str());
+            rc = SOCKPERF_ERR_SOCKET;
+            break;
+        }


please check formatting

EldarShalev · 2023-11-28T08:28:17Z

src/defs.h

@@ -157,6 +156,11 @@ typedef unsigned short int sa_family_t;
 #endif // USING_XLIO_EXTRA_API
 #endif // !defined(__windows__) && !defined(__FreeBSD__) && !defined(__APPLE__)

+#ifdef USING_DOCA_COMM_CHANNEL_API
+#include "doca_cc_helper.h"
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"


I will remove it, only for compilation purpose

EldarShalev · 2023-11-28T08:33:30Z

src/sockperf.cpp

@@ -103,6 +103,8 @@
 #ifndef __windows__
 #include <dlfcn.h>
 #endif
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"


Will remove it as well, only for faster compilation

EldarShalev · 2023-12-11T09:14:12Z

src/doca_cc_helper.h

+	bool need_alloc_mem;			/* Whether need to allocate memory */
+};
+
+struct cc_data_path_ctx {


cc_ctx_fifo

EldarShalev · 2024-01-31T09:08:19Z

src/defs.h

@@ -820,9 +821,11 @@ struct user_params_t {
 #endif /* DEFINED_TLS */
 #if defined(USING_DOCA_COMM_CHANNEL_API)
    bool doca_comm_channel = false;                         /* Flag to indicate using Com Channel*/
+    bool doca_fast_path = false;                            /* Flag to indicate using fast path*/


change name:
doca_cc_zcopy / doca_cc_fifo

EldarShalev · 2024-01-31T09:11:09Z

src/doca_cc_helper.h

+	CC_FIFO_CONNECTED
+};
+
+enum cc_fifo_data_path_state {


remove this

EldarShalev · 2024-01-31T09:11:40Z

src/doca_cc_helper.h

 	CONNECTION_IN_PROGRESS,
 	CC_CONNECTED
 };
+enum cc_fifo_connection_state {
+	FIFO_CONNECTION_IN_PROGRESS,


change to start with CC
CC_FIFO_...

EldarShalev · 2024-01-31T09:12:55Z

src/doca_cc_helper.h

+	struct doca_pe *pe_underload;						/**< Progress Engine for */
+	bool underload_mode;					/**< For using different callback>*/
+	enum cc_fifo_connection_state fifo_connection_state;		 	/**< Holding state for fast path connection >*/
+	enum cc_fifo_data_path_state  fifo_data_path_state;		 		/**< Holding state for fast path data path >*/


remove cc_fifo_data_path_state

EldarShalev · 2024-01-31T09:17:41Z

src/doca_cc_helper.h

+	}
+
+	// set here sockperf buf as local->mem
+	result = doca_mmap_set_memrange(local_mem->mmap, local_mem->mem, sizeof(uint8_t) * 65507 * 2);


change to msg_size
pointer to memory
size of memory

EldarShalev · 2024-01-31T09:33:25Z

src/doca_cc_helper.h

+
+	DOCA_LOG_INFO("Message received: '%.*s'", (int)recv_msg_len, (char *)recv_msg);
+	(void)doca_buf_dec_refcount(buf, NULL);
+	doca_task_free(doca_cc_consumer_post_recv_task_as_task(task));


no need for doca_buf_dec & free, can save doca_buf under fifo_ctx and use the same buf.
need to verify when buf wasn't used during nonblock call

EldarShalev · 2024-01-31T10:00:38Z

src/input_handlers.h

+                            }
+                            if (s_user_params.mode == MODE_CLIENT && !g_pApp->m_const_params.b_client_ping_pong
+                                && !g_pApp->m_const_params.b_stream) { // latency_under_load
+                                pe_local = s_user_params.pe_underload;


double check if can removed to producer side

EldarShalev · 2024-01-31T10:11:19Z

src/input_handlers.h

+                    }
+                }  // end of doca fast path
+                // Waiting for meesage receive callback - blocking or fifo mode and task submitted successfully
+                if (s_user_params.is_blocked || (s_user_params.doca_fast_path && result == DOCA_SUCCESS)) {


submit recv
wait for recv complete
pending

need to save flag if we got eagain after task submit

EldarShalev · 2024-01-31T10:14:16Z

src/doca_cc_helper.h

 #include "os_abstract.h"

 #define MSG_SIZE 4080
+#define MAX_BUFS 10


change to 1

igor-ivanov added the draft label Jul 11, 2022

agalanin-at-nvidia reviewed Jul 11, 2022

View reviewed changes

EldarShalev force-pushed the doca_comm_channel branch 3 times, most recently from a213924 to 0b1dc18 Compare September 13, 2022 12:18

EldarShalev force-pushed the doca_comm_channel branch 2 times, most recently from e9b17b1 to 88c186d Compare November 21, 2022 12:10

EldarShalev force-pushed the doca_comm_channel branch 3 times, most recently from b45a7b6 to 6bfefa9 Compare January 2, 2023 13:05

EldarShalev force-pushed the doca_comm_channel branch from 6bfefa9 to 388d73d Compare January 9, 2023 12:53