Hi Thomas:
<...>

quoted

+enum eth_dev_req_type {
+	REQ_TYPE_ATTACH,
+	REQ_TYPE_PRE_DETACH,
+	REQ_TYPE_DETACH,
+	REQ_TYPE_ATTACH_ROLLBACK,
+};

These constants are missing an ethdev prefix.

OK, will fix.

quoted

+
+struct eth_dev_mp_req {
+	enum eth_dev_req_type t;
+	char devargs[MAX_DEV_ARGS_LEN];
+	uint16_t port_id;
+	int result;
+};
+
+/**
+ * this is a synchronous wrapper for secondary process send
+ * request to primary process, this is invoked when an attach
+ * or detach request issued from primary.
+ */
+int eth_dev_request_to_primary(struct eth_dev_mp_req *req);
+
+/**
+ * this is a synchronous wrapper for primary process send
+ * request to secondary process, this is invoked when an attach
+ * or detach request issued from secondary process.
+ */
+int eth_dev_request_to_secondary(struct eth_dev_mp_req *req);

Why do we need ethdev functions for IPC (mp request/response)?
How this model can reasonnably scale to other device classes (crypto,
compression, bbdev, eventdev, etc)?

Yes it will be more generic to more the multi-process device sync mechanism into eal layer.(rte_eal_hotplug_add/rte_eal_hotplug_remove)
I didn't do this is I'm not very sure if all anothers kinds of device type need this, but if you think this is a good direction and we need to enable for all devices, 
I think this could be our next step. BTW, I guess ethdev still need some IPC to sync port_id which is specific for itself, and other device type may have similar requirement.

quoted

--- /dev/null
+++ b/lib/librte_ethdev/ethdev_private.h

What is the purpose of a file ethdev_private.h?

quoted

@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2018 Intel Corporation

Are you sure about the years?

NO, will fix:)

quoted

+/**
+ * Attach a new Ethernet device in current process.
+ *
+ * @param devargs
+ *  A pointer to a strings array describing the new device
+ *  to be attached. The strings should be a pci address like
+ *  '0000:01:00.0' or virtual device name like 'net_pcap0'.

No, no. The devargs syntax is being changed, so you should not duplicate its
description here. Better to reference an unique source of doc.

OK, will check and replace with more correct description.

quoted

+ *
+ * @param port_id
+ *  A pointer to a port identifier actually attached.
+ *
+ * @return
+ *  0 on success and port_id is filled, negative on error  */ int
+do_eth_dev_attach(const char *devargs, uint16_t *port_id);

So you are duplicating rte_eth_dev_attach which is flawed in its design and
should be deprecated...

OK, just to know this, but I guess it will not be the issue, if we move the dev sync mechanism into eal layer in future right?

Regards
Qi

As you may have noticed, rte_eth_dev_attach() is calling
rte_eal_hotplug_add() which manages the EAL device.
It is wrong because the relation between an ethdev port and an EAL device is
not a 1:1 mapping.
We must manage the ethdev port as one of the possible abstractions of a
device represented by rte_device.