Re: [dpdk-dev] [PATCH 4/6] examples/ntb: enable an example for ntb

[Ye Xiaolong <xiaolong.ye@intel.com>]

On 06/03, Xiaoyun Li wrote:
>Enable an example for rawdev ntb. Support interactive mode to send
>file on one host and receive file from another host. The command line
>would be 'send [filepath]' and 'receive [filepath]'.
>
>But since the FIFO is not enabled right now, use rte_memcpy as the enqueue
>and dequeue functions and only support transmitting file no more than 4M.
>
>Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
>---
> drivers/raw/ntb_rawdev/ntb_rawdev.c |  25 +-
> examples/Makefile                   |   1 +
> examples/meson.build                |  21 +-
> examples/ntb/Makefile               |  68 ++++++
> examples/ntb/meson.build            |  16 ++
> examples/ntb/ntb_fwd.c              | 364 ++++++++++++++++++++++++++++
> 6 files changed, 477 insertions(+), 18 deletions(-)
> create mode 100644 examples/ntb/Makefile
> create mode 100644 examples/ntb/meson.build
> create mode 100644 examples/ntb/ntb_fwd.c
>
>diff --git a/drivers/raw/ntb_rawdev/ntb_rawdev.c b/drivers/raw/ntb_rawdev/ntb_rawdev.c
>index 35bc34c54..1824842f2 100644
>--- a/drivers/raw/ntb_rawdev/ntb_rawdev.c
>+++ b/drivers/raw/ntb_rawdev/ntb_rawdev.c
>@@ -212,11 +212,16 @@ ntb_enqueue_bufs(struct rte_rawdev *dev,
> 		 unsigned int count,
> 		 rte_rawdev_obj_t context)
> {
>-	RTE_SET_USED(dev);
>-	RTE_SET_USED(buffers);
>-	RTE_SET_USED(count);
>-	RTE_SET_USED(context);
>+	/* Not FIFO right now. Just for test memory write. */
>+	struct ntb_hw *hw = dev->dev_private;
>+	uint64_t bar_addr, size;
>+	unsigned int i;
>+
>+	bar_addr = (*hw->ntb_ops->get_peer_mw_addr)(dev, 0);

check the func pointer before calling it.

>+	size = (uint64_t)context;
> 
>+	for (i = 0; i < count; i++)
>+		rte_memcpy((void *)bar_addr, buffers[i]->buf_addr, size);
> 	return 0;
> }
> 
>@@ -226,11 +231,15 @@ ntb_dequeue_bufs(struct rte_rawdev *dev,
> 		 unsigned int count,
> 		 rte_rawdev_obj_t context)
> {
>-	RTE_SET_USED(dev);
>-	RTE_SET_USED(buffers);
>-	RTE_SET_USED(count);
>-	RTE_SET_USED(context);
>+	/* Not FIFO. Just for test memory read. */
>+	struct ntb_hw *hw = dev->dev_private;
>+	uint64_t size;
>+	unsigned int i;
>+
>+	size = (uint64_t)context;
> 
>+	for (i = 0; i < count; i++)
>+		rte_memcpy(buffers[i]->buf_addr, hw->mz[i]->addr, size);
> 	return 0;
> }
> 
>diff --git a/examples/Makefile b/examples/Makefile
>index 7562424d9..de11dd487 100644
>--- a/examples/Makefile
>+++ b/examples/Makefile
>@@ -53,6 +53,7 @@ DIRS-y += link_status_interrupt
> DIRS-$(CONFIG_RTE_LIBRTE_LPM) += load_balancer
> DIRS-y += multi_process
> DIRS-y += netmap_compat/bridge
>+DIRS-y += ntb
> DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += packet_ordering
> ifeq ($(CONFIG_RTE_ARCH_X86_64),y)
> DIRS-y += performance-thread
>diff --git a/examples/meson.build b/examples/meson.build
>index de35656d4..dda4a07a8 100644
>--- a/examples/meson.build
>+++ b/examples/meson.build
>@@ -25,16 +25,17 @@ all_examples = [
> 	'l3fwd-acl', 'l3fwd-power',
> 	'l3fwd-vf', 'link_status_interrupt',
> 	'load_balancer', 'multi_process',
>-	'netmap_compat', 'packet_ordering',
>-	'performance-thread', 'ptpclient',
>-	'qos_meter', 'qos_sched',
>-	'quota_watermark', 'rxtx_callbacks',
>-	'server_node_efd', 'service_cores',
>-	'skeleton', 'tep_termination',
>-	'timer', 'vdpa',
>-	'vhost', 'vhost_crypto',
>-	'vhost_scsi', 'vm_power_manager',
>-	'vmdq', 'vmdq_dcb',
>+	'netmap_compat', 'ntb',
>+	'packet_ordering', 'performance-thread',
>+	'ptpclient', 'qos_meter',
>+	'qos_sched', 'quota_watermark',
>+	'rxtx_callbacks', 'server_node_efd',
>+	'service_cores', 'skeleton',
>+	'tep_termination', 'timer',
>+	'vdpa', 'vhost',
>+	'vhost_crypto', 'vhost_scsi',
>+	'vm_power_manager', 'vmdq',
>+	'vmdq_dcb',
> ]
> # install all example code on install - irrespective of whether the example in
> # question is to be built as part of this build or not.
>diff --git a/examples/ntb/Makefile b/examples/ntb/Makefile
>new file mode 100644
>index 000000000..5ddd9b95f
>--- /dev/null
>+++ b/examples/ntb/Makefile
>@@ -0,0 +1,68 @@
>+# SPDX-License-Identifier: BSD-3-Clause
>+# Copyright(c) 2019 Intel Corporation
>+
>+# binary name
>+APP = ntb_fwd
>+
>+# all source are stored in SRCS-y
>+SRCS-y := ntb_fwd.c
>+
>+# Build using pkg-config variables if possible
>+$(shell pkg-config --exists libdpdk)
>+ifeq ($(.SHELLSTATUS),0)
>+
>+all: shared
>+.PHONY: shared static
>+shared: build/$(APP)-shared
>+	ln -sf $(APP)-shared build/$(APP)
>+static: build/$(APP)-static
>+	ln -sf $(APP)-static build/$(APP)
>+
>+CFLAGS += -D_FILE_OFFSET_BITS=64
>+LDFLAGS += -pthread
>+
>+PC_FILE := $(shell pkg-config --path libdpdk)
>+CFLAGS += -O3 $(shell pkg-config --cflags libdpdk)
>+LDFLAGS_SHARED = $(shell pkg-config --libs libdpdk)
>+LDFLAGS_STATIC = -Wl,-Bstatic $(shell pkg-config --static --libs libdpdk)
>+
>+build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
>+	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
>+
>+build/$(APP)-static: $(SRCS-y) Makefile $(PC_FILE) | build
>+	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_STATIC)
>+
>+build:
>+	@mkdir -p $@
>+
>+.PHONY: clean
>+clean:
>+	rm -f build/$(APP) build/$(APP)-static build/$(APP)-shared
>+	rmdir --ignore-fail-on-non-empty build
>+
>+else # Build using legacy build system
>+
>+ifeq ($(RTE_SDK),)
>+$(error "Please define RTE_SDK environment variable")
>+endif
>+
>+# Default target, can be overridden by command line or environment
>+RTE_TARGET ?= x86_64-native-linuxapp-gcc
>+
>+include $(RTE_SDK)/mk/rte.vars.mk
>+
>+ifneq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
>+$(info This application can only operate in a linuxapp environment, \
>+please change the definition of the RTE_TARGET environment variable)
>+all:
>+else
>+
>+CFLAGS += -D_FILE_OFFSET_BITS=64
>+CFLAGS += -O2
>+CFLAGS += $(WERROR_FLAGS)
>+CFLAGS += -DALLOW_EXPERIMENTAL_API
>+
>+include $(RTE_SDK)/mk/rte.extapp.mk
>+
>+endif
>+endif
>diff --git a/examples/ntb/meson.build b/examples/ntb/meson.build
>new file mode 100644
>index 000000000..9a6288f4f
>--- /dev/null
>+++ b/examples/ntb/meson.build
>@@ -0,0 +1,16 @@
>+# SPDX-License-Identifier: BSD-3-Clause
>+# Copyright(c) 2019 Intel Corporation
>+
>+# meson file, for building this example as part of a main DPDK build.
>+#
>+# To build this example as a standalone application with an already-installed
>+# DPDK instance, use 'make'
>+
>+if host_machine.system() != 'linux'
>+	build = false
>+endif
>+deps += 'rawdev'
>+cflags += ['-D_FILE_OFFSET_BITS=64']
>+sources = files(
>+	'ntb_fwd.c'
>+)
>diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c
>new file mode 100644
>index 000000000..7835d817a
>--- /dev/null
>+++ b/examples/ntb/ntb_fwd.c
>@@ -0,0 +1,364 @@
>+/* SPDX-License-Identifier: BSD-3-Clause
>+ * Copyright(c) 2019 Intel Corporation
>+ */
>+#include <stdint.h>
>+#include <stdio.h>
>+#include <unistd.h>
>+#include <signal.h>
>+#include <string.h>
>+#include <getopt.h>
>+
>+#include <cmdline_parse_string.h>
>+#include <cmdline_socket.h>
>+#include <cmdline.h>
>+#include <rte_common.h>
>+#include <rte_rawdev.h>
>+#include <rte_lcore.h>
>+
>+static uint64_t max_file_size = 0x400000;
>+static uint8_t interactive = 1;
>+static uint16_t dev_id;
>+
>+/* *** Help command with introduction. *** */
>+struct cmd_help_result {
>+	cmdline_fixed_string_t help;
>+};
>+
>+static void cmd_help_parsed(__attribute__((unused)) void *parsed_result,
>+			    struct cmdline *cl,
>+			    __attribute__((unused)) void *data)
>+{
>+	cmdline_printf(
>+		cl,
>+		"\n"
>+		"The following commands are currently available:\n\n"
>+		"Control:\n"
>+		"    quit                                      :"
>+		" Quit the application.\n"
>+		"\nFile transmit:\n"
>+		"    send [path]                               :"
>+		" Send [path] file. (No more than %lu)\n"
>+		"    recv [path]                            :"
>+		" Receive file to [path]. Make sure sending is done"
>+		" on the other side.\n",
>+		max_file_size
>+	);
>+
>+}
>+
>+cmdline_parse_token_string_t cmd_help_help =
>+	TOKEN_STRING_INITIALIZER(struct cmd_help_result, help, "help");
>+
>+cmdline_parse_inst_t cmd_help = {
>+	.f = cmd_help_parsed,
>+	.data = NULL,
>+	.help_str = "show help",
>+	.tokens = {
>+		(void *)&cmd_help_help,
>+		NULL,
>+	},
>+};
>+
>+/* *** QUIT *** */
>+struct cmd_quit_result {
>+	cmdline_fixed_string_t quit;
>+};
>+
>+static void cmd_quit_parsed(__attribute__((unused)) void *parsed_result,
>+			    struct cmdline *cl,
>+			    __attribute__((unused)) void *data)
>+{
>+	/* Stop traffic and Close port. */
>+	rte_rawdev_stop(dev_id);
>+	rte_rawdev_close(dev_id);
>+
>+	cmdline_quit(cl);
>+}
>+
>+cmdline_parse_token_string_t cmd_quit_quit =
>+		TOKEN_STRING_INITIALIZER(struct cmd_quit_result, quit, "quit");
>+
>+cmdline_parse_inst_t cmd_quit = {
>+	.f = cmd_quit_parsed,
>+	.data = NULL,
>+	.help_str = "exit application",
>+	.tokens = {
>+		(void *)&cmd_quit_quit,
>+		NULL,
>+	},
>+};
>+
>+/* *** SEND FILE PARAMETERS *** */
>+struct cmd_sendfile_result {
>+	cmdline_fixed_string_t send_string;
>+	char filepath[];
>+};
>+
>+static void
>+cmd_sendfile_parsed(void *parsed_result,
>+		    __attribute__((unused)) struct cmdline *cl,
>+		    __attribute__((unused)) void *data)
>+{
>+	struct cmd_sendfile_result *res = parsed_result;
>+	struct rte_rawdev_buf *pkts_send[1];
>+	uint64_t rsize, size, link;
>+	char *filepath;
>+	uint8_t *buff;
>+	uint32_t val;
>+	FILE *file;
>+
>+	if (!rte_rawdevs[dev_id].started)
>+		printf("Device needs to be up first. Try later.\n");

I think we can directly return here when the device is not started, instead of
continuing the execution.

>+
>+	rte_rawdev_get_attr(dev_id, "link_status", &link);
>+	if (!link)
>+		printf("Link is not up, cannot send file.\n");

return if link is down.

>+
>+	filepath = strdup(res->filepath);
>+	if (filepath == NULL) {
>+		printf("Fail to get filepath.\n");
>+		return;
>+	}
>+
>+	file = fopen(filepath, "r");

Actually I was thinking why we need the filepath, why not just
	
	file = fopen(res->filepath, "r")

Then we don't need to handle filepath free at all.

>+	if (!file) {

style 1.8.1

>+		printf("Fail to open the file.\n");

Need to free filepath in this error handling.

>+		return;
>+	}
>+
>+	fseek(file, 0, SEEK_END);
>+	size = ftell(file);
>+	fseek(file, 0, SEEK_SET);
>+
>+	/**
>+	 * No FIFO now. Only test memory. Limit sending file
>+	 * size <= max_file_size.
>+	 */
>+	if (size > max_file_size)
>+		size = max_file_size;

It's better to give the user some reminder info about the max size limition,
rather than truncate the file silently.

>+
>+	buff = (uint8_t *)malloc(size);
>+	rsize = fread(buff, size, 1, file);
>+	if (rsize != 1) {
>+		printf("Fail to read file.\n");
>+		fclose(file);

Need to free filepath and buff.

>+		return;
>+	}
>+
>+	/* Tell remote about the file size. */
>+	val = size >> 32;
>+	rte_rawdev_set_attr(dev_id, "spad14", val);
>+	val = size;
>+	rte_rawdev_set_attr(dev_id, "spad15", val);
>+
>+	pkts_send[0] = (struct rte_rawdev_buf *)malloc
>+			(sizeof(struct rte_rawdev_buf));
>+	pkts_send[0]->buf_addr = buff;
>+	rte_rawdev_enqueue_buffers(dev_id, pkts_send, 1, (void *)size);

May need to check the return value.

>+	printf("Done sending file.\n");
>+
>+	fclose(file);
>+	free((void *)filepath);

Need to free allocated pkts_send[0] and buff. 

>+}
>+
>+cmdline_parse_token_string_t cmd_send_file_send =
>+	TOKEN_STRING_INITIALIZER(struct cmd_sendfile_result, send_string,
>+				 "send");
>+cmdline_parse_token_string_t cmd_send_file_filepath =
>+	TOKEN_STRING_INITIALIZER(struct cmd_sendfile_result, filepath, NULL);
>+
>+
>+cmdline_parse_inst_t cmd_send_file = {
>+	.f = cmd_sendfile_parsed,
>+	.data = NULL,
>+	.help_str = "send <file_path>",
>+	.tokens = {
>+		(void *)&cmd_send_file_send,
>+		(void *)&cmd_send_file_filepath,
>+		NULL,
>+	},
>+};
>+
>+/* *** RECEIVE FILE PARAMETERS *** */
>+struct cmd_recvfile_result {
>+	cmdline_fixed_string_t recv_string;
>+	char filepath[];
>+};
>+
>+static void
>+cmd_recvfile_parsed(void *parsed_result,
>+		    __attribute__((unused)) struct cmdline *cl,
>+		    __attribute__((unused)) void *data)
>+{
>+	struct cmd_sendfile_result *res = parsed_result;
>+	struct rte_rawdev_buf *pkts_recv[1];
>+	uint64_t size, val;
>+	char *filepath;
>+	uint8_t *buff;
>+	FILE *file;
>+
>+	if (!rte_rawdevs[dev_id].started)
>+		printf("Device needs to be up first. Try later.\n");

return directly.

>+
>+	rte_rawdev_get_attr(dev_id, "link_status", &val);
>+	if (!val)
>+		printf("Link is not up, cannot receive file.\n");

return directly.

>+
>+	filepath = strdup(res->filepath);
>+	if (filepath == NULL) {
>+		printf("Fail to get filepath.\n");
>+		return;
>+	}
>+
>+	file = fopen(filepath, "w");
>+	if (!file) {

style 1.8.1

>+		printf("Fail to open the file.\n");
>+		return;
>+	}
>+
>+	rte_rawdev_get_attr(dev_id, "spad14", &val);
>+	size = val << 32;
>+	rte_rawdev_get_attr(dev_id, "spad15", &val);
>+	size |= val;
>+
>+	buff = (uint8_t *)malloc(size);
>+	pkts_recv[0] = (struct rte_rawdev_buf *)malloc
>+			(sizeof(struct rte_rawdev_buf));
>+	pkts_recv[0]->buf_addr = buff;
>+
>+	rte_rawdev_dequeue_buffers(dev_id, pkts_recv, 1, (void *)size);
>+
>+	fwrite(buff, size, 1, file);
>+	printf("Done receiving to file.\n");
>+
>+	fclose(file);
>+	free((void *)filepath);

free buff and pkts_recv[0] as well.

I noticed that there are two allocations, one for pkts_recv[0] and another for
pkts_recv[0]->buf_addr, How about we declare 

	struct rte_rawdev_buf pkts_recv[1];

and allocate memory for its buf_addr

	pkts_recv[0].buf_addr = malloc(size);

then we call 

	rte_rawdev_dequeue_buffers(dev_id, &pkts_recv, 1, (void *)size);

After that, we just need to free once.

	free(pkts_recv[0].buf_addr);


>+}
>+
>+cmdline_parse_token_string_t cmd_recv_file_recv =
>+	TOKEN_STRING_INITIALIZER(struct cmd_recvfile_result, recv_string,
>+				 "recv");
>+cmdline_parse_token_string_t cmd_recv_file_filepath =
>+	TOKEN_STRING_INITIALIZER(struct cmd_recvfile_result, filepath, NULL);
>+
>+
>+cmdline_parse_inst_t cmd_recv_file = {
>+	.f = cmd_recvfile_parsed,
>+	.data = NULL,
>+	.help_str = "recv <file_path>",
>+	.tokens = {
>+		(void *)&cmd_recv_file_recv,
>+		(void *)&cmd_recv_file_filepath,
>+		NULL,
>+	},
>+};
>+
>+/* list of instructions */
>+cmdline_parse_ctx_t main_ctx[] = {
>+	(cmdline_parse_inst_t *)&cmd_help,
>+	(cmdline_parse_inst_t *)&cmd_send_file,
>+	(cmdline_parse_inst_t *)&cmd_recv_file,
>+	(cmdline_parse_inst_t *)&cmd_quit,
>+	NULL,
>+};
>+
>+/* prompt function, called from main on MASTER lcore */
>+static void
>+prompt(void)
>+{
>+	struct cmdline *cl;
>+
>+	cl = cmdline_stdin_new(main_ctx, "ntb> ");
>+	if (cl == NULL)
>+		return;
>+
>+	cmdline_interact(cl);
>+	cmdline_stdin_exit(cl);
>+}
>+
>+static void
>+signal_handler(int signum)
>+{
>+	if (signum == SIGINT || signum == SIGTERM) {
>+		printf("\nSignal %d received, preparing to exit...\n", signum);
>+		signal(signum, SIG_DFL);
>+		kill(getpid(), signum);
>+	}
>+}
>+
>+static void
>+ntb_usage(const char *prgname)
>+{
>+	printf("%s [EAL options] -- [options]\n"
>+	       "-i : run in interactive mode (default value is 1)\n",
>+	       prgname);
>+}
>+
>+static int
>+parse_args(int argc, char **argv)
>+{
>+	char *prgname = argv[0], **argvopt = argv;
>+	int opt, ret;
>+
>+	/* Only support interactive mode to send/recv file first. */
>+	while ((opt = getopt(argc, argvopt, "i")) != EOF) {
>+		switch (opt) {
>+		case 'i':
>+			printf("Interactive-mode selected\n");
>+			interactive = 1;
>+			break;
>+
>+		default:
>+			ntb_usage(prgname);
>+			return -1;
>+		}
>+	}
>+
>+	if (optind >= 0)
>+		argv[optind-1] = prgname;
>+
>+	ret = optind-1;
>+	optind = 1; /* reset getopt lib */
>+	return ret;
>+}
>+
>+int
>+main(int argc, char **argv)
>+{
>+	int ret, i;
>+
>+	signal(SIGINT, signal_handler);
>+	signal(SIGTERM, signal_handler);
>+
>+	ret = rte_eal_init(argc, argv);
>+	if (ret < 0)
>+		rte_exit(EXIT_FAILURE, "Error with EAL initialization.\n");
>+
>+	/* Find 1st ntb rawdev. */
>+	for (i = 0; i < RTE_RAWDEV_MAX_DEVS; i++)
>+		if (rte_rawdevs[i].driver_name &&
>+		    (strncmp(rte_rawdevs[i].driver_name, "raw_ntb", 7) == 0) &&

Use Macro for the 7.

>+		    (rte_rawdevs[i].attached == 1))
>+			break;
>+
>+	if (i == RTE_RAWDEV_MAX_DEVS)
>+		rte_exit(EXIT_FAILURE, "Cannot find any ntb device.\n");
>+
>+	dev_id = i;
>+
>+	argc -= ret;
>+	argv += ret;
>+
>+	ret = parse_args(argc, argv);
>+	if (ret < 0)
>+		rte_exit(EXIT_FAILURE, "Invalid arguments\n");
>+
>+	rte_rawdev_start(dev_id);
>+
>+	if (interactive) {
>+		sleep(1);
>+		prompt();
>+	}
>+
>+	return 0;
>+}
>-- 
>2.17.1
>