On 07-Jun-18 1:38 PM, Qi Zhang wrote:
The sample code demonstrate device (ethdev only) management
at multi-process envrionment. User can attach/detach a device
on primary process and see it is synced on secondary process
automatically, also user can lock a device to prevent it be
detached or unlock it to go back to default behaviour.
How to start?
./devmgm_mp --proc-type=auto
Command Line Example:
help
list
/* attach a af_packet vdev */
attach net_af_packet,iface=eth0
/* detach port 0 */
detach 0
/* attach a private af_packet vdev (secondary process only)*/
attachp net_af_packet,iface=eth0
/* detach a private device (secondary process only) */
detachp 0
/* lock port 0 */
lock 0
/* unlock port 0 */
unlock 0
Signed-off-by: Qi Zhang <qi.z.zh...@intel.com>
---
I think the "devmgm_mp" is not a descriptive enough name. What this
example demonstrates, is device hotplug. So how about naming the example
app "hotplug"? (or "mp_hotplug" to indicate that it specifically sets
out to demonstrate multiprocess hotplug)
examples/devmgm_mp/Makefile | 64 +++++++
examples/devmgm_mp/commands.c | 383 +++++++++++++++++++++++++++++++++++++++++
examples/devmgm_mp/commands.h | 10 ++
examples/devmgm_mp/main.c | 41 +++++
examples/devmgm_mp/meson.build | 11 ++
5 files changed, 509 insertions(+)
create mode 100644 examples/devmgm_mp/Makefile
create mode 100644 examples/devmgm_mp/commands.c
create mode 100644 examples/devmgm_mp/commands.h
create mode 100644 examples/devmgm_mp/main.c
create mode 100644 examples/devmgm_mp/meson.build
<snip>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <errno.h>
+#include <netinet/in.h>
+#include <termios.h>
+#ifndef __linux__
+ #ifdef __FreeBSD__
+ #include <sys/socket.h>
+ #else
+ #include <net/socket.h>
+ #endif
+#endif
This seems like a weird define. Care to elaborate why are we checking
for __linux__ not being defined?
If you're trying to differentiate between Linux and FreeBSD, there's a
readly RTE_EXEC_ENV_* config options, e.g.
#ifdef RTE_EXEC_ENV_LINUXAPP
// linux defines
#endif
#ifdef RTE_EXEC_ENV_BSDAPP
// bsd defines
#endif
or something to that effect.
+
+#include <cmdline_rdline.h>
+#include <cmdline_parse.h>
+#include <cmdline_parse_ipaddr.h>
+#include <cmdline_parse_num.h>
+#include <cmdline_parse_string.h>
+#include <cmdline.h>
+#include <rte_ethdev.h>
Generally (and as per DPDK coding guidelines), we prefer defines ordered
as follows:
1) system defines enclosed in brackets
2) DPDK defines (rte_blah) enclosed in brackets
3) private/application-specific defines enclosed in quotes.
All three groups should be separated by newline.
So, these defines should've read as:
#include <stdblah.h>
#include <sys/blah.h>
#include <rte_blah.h>
#include <rte_foo.h>
#include "cmdline_blah.h"
#include "cmdline_foo.h"
+
+/**********************************************************/
+
+struct cmd_help_result {
+ cmdline_fixed_string_t help;
+};
+
+static void cmd_help_parsed(__attribute__((unused)) void *parsed_result,
<snip>
+{
+ uint16_t port_id;
+ char dev_name[RTE_DEV_NAME_MAX_LEN];
+
+ cmdline_printf(cl, "list all etherdev\n");
+
+ RTE_ETH_FOREACH_DEV(port_id) {
+ rte_eth_dev_get_name_by_port(port_id, dev_name);
+ /* Secondary process's ethdev->state may not be
+ * updated after detach on primary process, but
+ * ethdev->data should already be reset, so
+ * use strlen(dev_name) == 0 to know the port is
+ * not used.
+ *
+ * TODO: Secondary process should be informed when a
+ * port is released on primary through mp channel.
+ */
That seems like a weird thing to leave out for TODO - it looks like an
API deficiency. Can this be automatically updated on multiprocess
hotplug sync, or somehow managed inside RTE_ETH_FOREACH_DEV?
As i understand, per-process ethdev list is not protected by any locks,
so doing this is racy. Since this is a multiprocess hotplug example app,
it should demonstrate best practices. So, either RTE_ETH_FOREACH_DEV
should be fixed to handle this case, or the application should
demonstrate how to properly synchronize access to local device list. The
latter is probably better as adding locking around ethdev device list is
outside the scope of this patchset.
+ if (strlen(dev_name) > 0)
+ cmdline_printf(cl, "%d\t%s\n", port_id, dev_name);
+ else
+ printf("empty dev_name is not expected!\n");
+ }
<snip>
+#include "commands.h"
+
+int main(int argc, char **argv)
+{
+ int ret;
+ struct cmdline *cl;
+
+ ret = rte_eal_init(argc, argv);
+ if (ret < 0)
+ rte_panic("Cannot init EAL\n");
+
+ cl = cmdline_stdin_new(main_ctx, "example> ");
+ if (cl == NULL)
+ rte_panic("Cannot create cmdline instance\n");
+ cmdline_interact(cl);
+ cmdline_stdin_exit(cl);
+
+ return 0;
Application should call rte_eal_cleanup() before exit. Otherwise, each
secondary started and stopped will leak memory.
--
Thanks,
Anatoly
