Re: [PATCH v3 2/2] examples/vdpa: introduce a new sample for vDPA

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

On 09/21, Wang, Xiao W wrote:
>Hi Xiaolong,
>
>> -----Original Message-----
>> From: Ye, Xiaolong
>> Sent: Friday, September 21, 2018 6:28 AM
>> To: dev@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>; Bie,
>> Tiwei <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
>> Cc: Wang, Xiao W <xiao.w.wang@intel.com>; Rami Rosen
>> <roszenrami@gmail.com>; Wang, Haiyue <haiyue.wang@intel.com>; Ye,
>> Xiaolong <xiaolong.ye@intel.com>
>> Subject: [PATCH v3 2/2] examples/vdpa: introduce a new sample for vDPA
>> 
>> The vdpa sample application creates vhost-user sockets by using the
>> vDPA backend. vDPA stands for vhost Data Path Acceleration which utilizes
>> virtio ring compatible devices to serve virtio driver directly to enable
>> datapath acceleration. As vDPA driver can help to set up vhost datapath,
>> this application doesn't need to launch dedicated worker threads for vhost
>> enqueue/dequeue operations.
>> 
>> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> ---
>>  MAINTAINERS                        |   2 +
>>  doc/guides/sample_app_ug/index.rst |   1 +
>>  doc/guides/sample_app_ug/vdpa.rst  | 115 ++++++++
>>  examples/Makefile                  |   2 +-
>>  examples/vdpa/Makefile             |  32 ++
>>  examples/vdpa/main.c               | 458 +++++++++++++++++++++++++++++
>>  examples/vdpa/meson.build          |  16 +
>>  7 files changed, 625 insertions(+), 1 deletion(-)
>>  create mode 100644 doc/guides/sample_app_ug/vdpa.rst
>>  create mode 100644 examples/vdpa/Makefile
>>  create mode 100644 examples/vdpa/main.c
>>  create mode 100644 examples/vdpa/meson.build
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5967c1dd3..5656f18e8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -683,6 +683,8 @@ F: doc/guides/sample_app_ug/vhost.rst
>>  F: examples/vhost_scsi/
>>  F: doc/guides/sample_app_ug/vhost_scsi.rst
>>  F: examples/vhost_crypto/
>> +F: examples/vdpa/
>> +F: doc/guides/sample_app_ug/vdpa.rst
>> 
>>  Vhost PMD
>>  M: Maxime Coquelin <maxime.coquelin@redhat.com>
>> diff --git a/doc/guides/sample_app_ug/index.rst
>> b/doc/guides/sample_app_ug/index.rst
>> index 5bedf4f6f..74b12af85 100644
>> --- a/doc/guides/sample_app_ug/index.rst
>> +++ b/doc/guides/sample_app_ug/index.rst
>> @@ -45,6 +45,7 @@ Sample Applications User Guides
>>      vhost
>>      vhost_scsi
>>      vhost_crypto
>> +    vdpa
>>      netmap_compatibility
>>      ip_pipeline
>>      test_pipeline
>> diff --git a/doc/guides/sample_app_ug/vdpa.rst
>> b/doc/guides/sample_app_ug/vdpa.rst
>> new file mode 100644
>> index 000000000..44fe6736d
>> --- /dev/null
>> +++ b/doc/guides/sample_app_ug/vdpa.rst
>> @@ -0,0 +1,115 @@
>> +..  SPDX-License-Identifier: BSD-3-Clause
>> +    Copyright(c) 2018 Intel Corporation.
>> +
>> +Vdpa Sample Application
>> +=======================
>> +
>> +The vdpa sample application creates vhost-user sockets by using the
>> +vDPA backend. vDPA stands for vhost Data Path Acceleration which utilizes
>> +virtio ring compatible devices to serve virtio driver directly to enable
>> +datapath acceleration. As vDPA driver can help to set up vhost datapath,
>> +this application doesn't need to launch dedicated worker threads for vhost
>> +enqueue/dequeue operations.
>> +
>> +Testing steps
>
>[...]
>
>> +
>> +Then we can create 2 vdpa ports in interactive cmdline.
>> +
>> +.. code-block:: console
>> +
>> +        vdpa> list
>> +        device id       device address
>> +        0               0000:06:00.2
>> +        1               0000:06:00.3
>
>The features and queue numbers of each vDPA device could also be shown now, the doc should reflect this.

Got it.

>
>> +        vdpa> create /tmp/vdpa-socket0 0000:06:00.2
>> +        vdpa> create /tmp/vdpa-socket1 0000:06:00.3
>> +
>> +.. _vdpa_app_run_vm:
>> +
>> +Start the VMs
>> +~~~~~~~~~~~~~
>> +
>> +.. code-block:: console
>> +
>> +       qemu-system-x86_64 -cpu host -enable-kvm \
>> +       <snip>
>> +       -mem-prealloc \
>> +       -chardev socket,id=char0,path=<socket_file created in above steps> \
>> +       -netdev type=vhost-user,id=vdpa,chardev=char0 \
>> +       -device virtio-net-pci,netdev=vdpa,mac=00:aa:bb:cc:dd:ee \
>> +
>> +After the VMs launches, we can login the VMs and configure the ip, verify the
>> +network connection via ping or netperf.
>> +
>> +.. note::
>> +    Suggest to use QEMU 3.0.0 which extends vhost-user for vDPA.
>> +
>> +Live Migration
>> +~~~~~~~~~~~~~~
>> +vDPA supports cross-backend live migration, user can migrate SW vhost
>> backend
>> +VM to vDPA backend VM and vice versa. Here are the detailed steps. Assume
>> A is
>> +the source host with SW vhost VM and B is the destination host with vDPA.
>> +
>> +1. Start vdpa sample and launch a VM with exact same parameters as the VM
>> on A,
>> +   in migration-listen mode:
>> +
>> +.. code-block:: console
>> +
>> +        B: <qemu-command-line> -incoming tcp:0:4444 (or other PORT))
>> +
>> +2. Start the migration (on source host):
>> +
>> +.. code-block:: console
>> +
>> +        A: (qemu) migrate -d tcp:<B ip>:4444 (or other PORT)
>> +
>> +3. Check the status (on source host):
>> +
>> +.. code-block:: console
>> +
>> +        A: (qemu) info migrate
>> diff --git a/examples/Makefile b/examples/Makefile
>> index 481720cb6..356fcb1cd 100644
>> --- a/examples/Makefile
>> +++ b/examples/Makefile
>> @@ -65,7 +65,7 @@ ifeq ($(CONFIG_RTE_LIBRTE_HASH),y)
>>  DIRS-$(CONFIG_RTE_LIBRTE_VHOST) += tep_termination
>>  endif
>>  DIRS-$(CONFIG_RTE_LIBRTE_TIMER) += timer
>> -DIRS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost vhost_scsi
>> +DIRS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost vhost_scsi vdpa
>>  ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
>>  DIRS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_crypto
>>  endif
>> diff --git a/examples/vdpa/Makefile b/examples/vdpa/Makefile
>> new file mode 100644
>> index 000000000..42672a2bc
>> --- /dev/null
>> +++ b/examples/vdpa/Makefile
>> @@ -0,0 +1,32 @@
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright(c) 2018 Intel Corporation
>> +
>> +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")
>> +$(info This application can only operate in a linuxapp environment, \
>> +please change the definition of the RTE_TARGET environment variable)
>> +all:
>> +else
>> +
>> +# binary name
>> +APP = vdpa
>> +
>> +# all source are stored in SRCS-y
>> +SRCS-y := main.c
>> +
>> +CFLAGS += -O2 -D_FILE_OFFSET_BITS=64
>> +CFLAGS += $(WERROR_FLAGS)
>> +CFLAGS += -D_GNU_SOURCE
>> +CFLAGS += -DALLOW_EXPERIMENTAL_API
>> +
>> +include $(RTE_SDK)/mk/rte.extapp.mk
>> +
>
>[...]
>
>> +
>> +static int
>> +new_device(int vid)
>> +{
>> +	char ifname[MAX_PATH_LEN];
>> +	int i;
>> +
>> +	rte_vhost_get_ifname(vid, ifname, sizeof(ifname));
>> +	for (i = 0; i < MAX_VDPA_SAMPLE_PORTS; i++) {
>> +		if (strncmp(ifname, vports[i].ifname, MAX_PATH_LEN) == 0) {
>> +			printf("\nnew port %s, did: %d\n",
>> +					ifname, vports[i].did);
>> +			vports[i].vid = vid;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (i >= MAX_VDPA_SAMPLE_PORTS)
>> +		return -1;
>> +
>> +	return 0;
>> +}
>> +
>> +static void
>> +destroy_device(int vid)
>> +{
>> +	char ifname[MAX_PATH_LEN];
>> +	int i;
>> +
>> +	rte_vhost_get_ifname(vid, ifname, sizeof(ifname));
>> +	for (i = 0; i < MAX_VDPA_SAMPLE_PORTS; i++) {
>> +		if (strcmp(ifname, vports[i].ifname) == 0) {
>> +			printf("\ndestroy port %s, did: %d\n",
>> +					ifname, vports[i].did);
>> +			break;
>> +		}
>> +	}
>> +}
>> +
>> +static const struct vhost_device_ops vdpa_sample_devops = {
>> +	.new_device = new_device,
>> +	.destroy_device = destroy_device,
>> +};
>> +
>> +static int
>> +start_vdpa(struct vdpa_port *vport)
>> +{
>> +	int ret;
>> +	char *socket_path = vport->ifname;
>> +	int did = vport->did;
>> +
>> +	if (access(socket_path, F_OK) != -1) {
>> +		RTE_LOG(ERR, VDPA,
>> +			"%s exists, please remove it or specify another file and
>> try again.\n",
>> +			socket_path);
>> +		return -1;
>> +	}
>> +	ret = rte_vhost_driver_register(socket_path, vport->flags);
>
>It would be great if we can provide "client mode" option for this sample to create a client mode vdpa.

Sure, I'll add client mode support.

>
>> +	if (ret != 0)
>> +		rte_exit(EXIT_FAILURE,
>> +			"register driver failed: %s\n",
>> +			socket_path);
>> +
>> +	ret = rte_vhost_driver_callback_register(socket_path,
>> +			&vdpa_sample_devops);
>> +	if (ret != 0)
>> +		rte_exit(EXIT_FAILURE,
>> +			"register driver ops failed: %s\n",
>> +			socket_path);
>> +
>> +	ret = rte_vhost_driver_attach_vdpa_device(socket_path, did);
>> +	if (ret != 0)
>> +		rte_exit(EXIT_FAILURE,
>> +			"attach vdpa device failed: %s\n",
>> +			socket_path);
>> +
>> +	if (rte_vhost_driver_start(socket_path) < 0)
>> +		rte_exit(EXIT_FAILURE,
>> +			"start vhost driver failed: %s\n",
>> +			socket_path);
>> +	return 0;
>> +}
>> +
>> +static void
>> +close_vdpa(struct vdpa_port *vport)
>> +{
>> +	int ret;
>> +	char *socket_path = vport->ifname;
>> +
>> +	ret = rte_vhost_driver_detach_vdpa_device(socket_path);
>> +	if (ret != 0)
>> +		RTE_LOG(ERR, VDPA,
>> +				"attach vdpa device failed: %s\n",
>> +				socket_path);
>> +
>> +	ret = rte_vhost_driver_unregister(socket_path);
>> +	if (ret != 0)
>> +		RTE_LOG(ERR, VDPA,
>> +				"Fail to unregister vhost driver for %s.\n",
>> +				socket_path);
>> +}
>> +
>> +static void
>> +vdpa_sample_quit(void)
>> +{
>> +	int i;
>> +	for (i = 0; i < RTE_MIN(MAX_VDPA_SAMPLE_PORTS, dev_total); i++) {
>> +		if (vports[i].ifname[0] != '\0')
>> +			close_vdpa(&vports[i]);
>> +	}
>> +}
>> +
>> +static void
>> +signal_handler(int signum)
>> +{
>> +	if (signum == SIGINT || signum == SIGTERM) {
>> +		printf("\nSignal %d received, preparing to exit...\n", signum);
>> +		vdpa_sample_quit();
>> +		exit(0);
>> +	}
>> +}
>> +
>> +/* interactive cmds */
>> +
>> +/* *** 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"
>> +		"    help                                      : Show interactive instructions.\n"
>> +		"    list                                      : list all available vdpa devices.\n"
>> +		"    create <socket file> <vdev addr>          : create a new vdpa
>> port.\n"
>> +		"    quit                                      : exit vdpa sample app.\n"
>> +	);
>> +}
>> +
>> +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,
>> +	},
>> +};
>> +
>> +/* *** List all available vdpa devices *** */
>> +struct cmd_list_result {
>> +	cmdline_fixed_string_t action;
>> +};
>> +
>> +static void cmd_list_vdpa_devices_parsed(
>> +		__attribute__((unused)) void *parsed_result,
>> +		struct cmdline *cl,
>> +		__attribute__((unused)) void *data)
>> +{
>> +	int did;
>> +	uint32_t queue_num;
>> +	uint64_t features;
>> +	struct rte_vdpa_device *vdev;
>> +	struct rte_pci_addr addr;
>> +
>> +	cmdline_printf(cl, "device id\tdevice address\tqueue num\tsupported
>> features\n");
>> +	for (did = 0; did < dev_total; did++) {
>> +		vdev = rte_vdpa_get_device(did);
>> +		if (!vdev)
>> +			continue;
>> +		if (vdev->ops->get_queue_num(did, &queue_num) < 0) {
>> +			RTE_LOG(ERR, VDPA,
>> +				"failed to get vdpa queue number "
>> +				"for device id %d.\n", did);
>> +			continue;
>> +		}
>> +		if (vdev->ops->get_features(did, &features) < 0) {
>> +			RTE_LOG(ERR, VDPA,
>> +				"failed to get vdpa features "
>> +				"for device id %d.\n", did);
>> +			continue;
>> +		}
>> +		addr = vdev->addr.pci_addr;
>> +		cmdline_printf(cl, "%d\t\t"PCI_PRI_FMT"\t%d\t\t0x%lx\n", did,
>> +				addr.domain, addr.bus, addr.devid,
>> +				addr.function, queue_num, features);
>
>Use "%PRIu32"for queue_num, and use "PRIx64" instead of "%lx", otherwise you may have compile error on 32-bit machine.

Got it.

>
>> +	}
>> +}
>> +
>> +cmdline_parse_token_string_t cmd_action_list =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_list_result, action, "list");
>> +
>> +cmdline_parse_inst_t cmd_list_vdpa_devices = {
>> +	.f = cmd_list_vdpa_devices_parsed,  /* function to call */
>> +	.data = NULL,      /* 2nd arg of func */
>
>This comment doesn't provide much info.
>Remove the comment for all the cmdline_parse_inst_t instances to keep alignment.

Ok, I'll remove all useless comments.

>
>> +	.help_str = "list all available vdpa devices",
>> +	.tokens = {        /* token list, NULL terminated */
>> +		(void *)&cmd_action_list,
>> +		NULL,
>> +	},
>> +};
>> +
>> +/* *** Create new vdpa port *** */
>> +struct cmd_create_result {
>> +	cmdline_fixed_string_t action;
>> +	cmdline_fixed_string_t socket_path;
>> +	cmdline_fixed_string_t bdf;
>> +};
>> +
>> +static void cmd_create_vdpa_port_parsed(void *parsed_result,
>> +		struct cmdline *cl,
>> +		__attribute__((unused)) void *data)
>> +{
>> +	int did;
>> +	struct cmd_create_result *res = parsed_result;
>> +	struct rte_vdpa_dev_addr addr;
>> +
>> +	strncpy(vports[devcnt].ifname, res->socket_path, MAX_PATH_LEN);
>> +	if (rte_pci_addr_parse(res->bdf, &addr.pci_addr) != 0) {
>> +		cmdline_printf(cl, "Unable to parse the given bdf.\n");
>> +		return;
>> +	}
>> +	addr.type = PCI_ADDR;
>> +	did = rte_vdpa_find_device_id(&addr);
>> +	if (did < 0) {
>> +		cmdline_printf(cl, "Unable to find vdpa devide id.\n");
>> +		return;
>> +	}
>> +
>> +	vports[devcnt].did = did;
>> +
>> +	if (start_vdpa(&vports[devcnt]) == 0)
>> +		devcnt++;
>> +}
>> +
>> +cmdline_parse_token_string_t cmd_action_create =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_create_result, action,
>> "create");
>> +cmdline_parse_token_string_t cmd_socket_path =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_create_result, socket_path,
>> NULL);
>> +cmdline_parse_token_string_t cmd_bdf =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_create_result, bdf, NULL);
>> +
>> +cmdline_parse_inst_t cmd_create_vdpa_port = {
>> +	.f = cmd_create_vdpa_port_parsed,  /* function to call */
>> +	.data = NULL,      /* 2nd arg of func */
>> +	.help_str = "create a new vdpa port",
>> +	.tokens = {        /* token list, NULL terminated */
>> +		(void *)&cmd_action_create,
>> +		(void *)&cmd_socket_path,
>> +		(void *)&cmd_bdf,
>> +		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)
>> +{
>> +	vdpa_sample_quit();
>> +	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 = "quit: exit application",
>> +	.tokens = {
>> +		(void *)&cmd_quit_quit,
>> +		NULL,
>> +	},
>> +};
>> +cmdline_parse_ctx_t main_ctx[] = {
>> +	(cmdline_parse_inst_t *)&cmd_help,
>> +	(cmdline_parse_inst_t *)&cmd_list_vdpa_devices,
>> +	(cmdline_parse_inst_t *)&cmd_create_vdpa_port,
>> +	(cmdline_parse_inst_t *)&cmd_quit,
>> +	NULL,
>> +};
>> +
>> +int
>> +main(int argc, char *argv[])
>> +{
>> +	char ch;
>> +	int i;
>> +	int ret;
>> +	struct cmdline *cl;
>> +
>> +	ret = rte_eal_init(argc, argv);
>> +	if (ret < 0)
>> +		rte_exit(EXIT_FAILURE, "eal init failed\n");
>> +	argc -= ret;
>> +	argv += ret;
>> +
>> +	data_init();
>> +
>> +	signal(SIGINT, signal_handler);
>> +	signal(SIGTERM, signal_handler);
>> +
>> +	ret = parse_args(argc, argv);
>> +	if (ret < 0)
>> +		rte_exit(EXIT_FAILURE, "invalid argument\n");
>> +
>> +	if (interactive == 1) {
>> +		cl = cmdline_stdin_new(main_ctx, "vdpa> ");
>> +		if (cl == NULL)
>> +			rte_panic("Cannot create cmdline instance\n");
>> +		cmdline_interact(cl);
>> +		cmdline_stdin_exit(cl);
>> +	} else {
>> +		for (i = 0; i < RTE_MIN(MAX_VDPA_SAMPLE_PORTS, dev_total);
>> +				i++) {
>> +			vports[i].did = i;
>> +			snprintf(vports[i].ifname, MAX_PATH_LEN, "%s%d",
>> +					iface, i);
>> +
>> +			start_vdpa(&vports[i]);
>> +		}
>> +
>> +		printf("enter \'q\' to quit\n");
>> +		while (scanf("%c", &ch)) {
>> +			if (ch == 'q')
>> +				break;
>> +			while (ch != '\n') {
>> +				if (scanf("%c", &ch))
>> +					printf("%c", ch);
>> +			}
>> +			printf("enter \'q\' to quit\n");
>> +		}
>
>Do vdpa_sample_quit here to stop the vhost session and delete the vhost sockets.

It needs to call vdpa_sample_quit here.

>
>> +	}
>> +
>> +	return 0;
>> +}
>> diff --git a/examples/vdpa/meson.build b/examples/vdpa/meson.build
>> new file mode 100644
>> index 000000000..2e38a069f
>> --- /dev/null
>> +++ b/examples/vdpa/meson.build
>> @@ -0,0 +1,16 @@
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright(c) 2018 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 += 'vhost'
>> +allow_experimental_apis = true
>> +sources = files(
>> +	'main.c'
>> +)
>> \ No newline at end of file
>> --
>> 2.17.1
>
>Do not forget to add this into 18.11 release_note.

Ok, does this need to make a new separate commit?

Thanks,
Xiaolong
>
>BRs,
>Xiao
>