On 09-Jul-18 4:36 AM, Qi Zhang wrote:
We are going to introduce the solution to handle hotplug in
multi-process, it includes the below scenario:
1. Attach a device from the primary
2. Detach a device from the primary
3. Attach a device from a secondary
4. Detach a device from a secondary
In the primary-secondary process model, we assume devices are shared
by default. that means attaches or detaches a device on any process
will broadcast to all other processes through mp channel then device
information will be synchronized on all processes.
Any failure during attaching/detaching process will cause inconsistent
status between processes, so proper rollback action should be considered.
This patch covers the implementation of case 1,2.
Case 3,4 will be implemented on a separate patch.
IPC scenario for Case 1, 2:
attach a device
a) primary attach the new device if failed goto h).
b) primary send attach sync request to all secondary.
c) secondary receive request and attach the device and send a reply.
d) primary check the reply if all success goes to i).
e) primary send attach rollback sync request to all secondary.
f) secondary receive the request and detach the device and send a reply.
g) primary receive the reply and detach device as rollback action.
h) attach fail
i) attach success
detach a device
a) primary send detach sync request to all secondary
b) secondary detach the device and send reply
c) primary check the reply if all success goes to f).
d) primary send detach rollback sync request to all secondary.
e) secondary receive the request and attach back device. goto g)
f) primary detach the device if success goto g), else goto d)
g) detach fail.
h) detach success.
Signed-off-by: Qi Zhang <qi.z.zh...@intel.com>
---
<snip>
+ req.t = EAL_DEV_REQ_TYPE_ATTACH;
+ strlcpy(req.busname, busname, RTE_BUS_NAME_MAX_LEN);
+ strlcpy(req.devname, devname, RTE_DEV_NAME_MAX_LEN);
+ strlcpy(req.devargs, devargs, RTE_DEV_ARGS_MAX_LEN);
+
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return -ENOTSUP;
Nitpick, but maybe do this before strlcpy?
+
+ /**
+ * attach a device from primary start from here:
+ *
+ * a) primary attach the new device if failed goto h).
+ * b) primary send attach sync request to all secondary.
+ * c) secondary receive request and attach the device and send a reply.
+ * d) primary check the reply if all success goes to i).
<snip>
+
+ memset(&req, 0, sizeof(req));
+ req.t = EAL_DEV_REQ_TYPE_DETACH;
+ strlcpy(req.busname, busname, RTE_BUS_NAME_MAX_LEN);
+ strlcpy(req.devname, devname, RTE_DEV_NAME_MAX_LEN);
+
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return -ENOTSUP;
Same nitpick, probably move this above.
+
+ /**
+ * detach a device from primary start from here:
+ *
+ * a) primary send detach sync request to all secondary
+ * b) secondary detach the device and send reply
<snip>
+ struct mp_reply_bundle *bundle = param;
+ struct rte_mp_msg *msg = &bundle->msg;
+ const struct eal_dev_mp_req *req =
+ (const struct eal_dev_mp_req *)msg->param;
+ struct rte_mp_msg mp_resp;
+ struct eal_dev_mp_req *resp =
+ (struct eal_dev_mp_req *)mp_resp.param;
+ int ret = 0;
+
+ memset(&mp_resp, 0, sizeof(mp_resp));
+
+ switch (req->t) {
+ case EAL_DEV_REQ_TYPE_ATTACH:
+ case EAL_DEV_REQ_TYPE_DETACH_ROLLBACK:
+ ret = do_dev_hotplug_add(req->busname, req->devname, "");
I'm not too familiar with devargs and hotplug, but why are we passing
empty devargs string here? Is it possible for it to be not empty?
+ break;
+ case EAL_DEV_REQ_TYPE_DETACH:
+ case EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK:
+ ret = do_dev_hotplug_remove(req->busname, req->devname);
+ break;
+ default:
+ ret = -EINVAL;
+ }
<snip>
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ ret = rte_mp_action_register(EAL_DEV_MP_ACTION_REQUEST,
+ handle_secondary_request);
+ if (ret) {
+ RTE_LOG(ERR, EAL, "Couldn't register '%s' action\n",
+ EAL_DEV_MP_ACTION_REQUEST);
+ return ret;
+ }
+ } else {
+ ret = rte_mp_action_register(EAL_DEV_MP_ACTION_REQUEST,
+ handle_primary_request);
^^ wrong indentation.
+ if (ret) {
+ RTE_LOG(ERR, EAL, "Couldn't register '%s' action\n",
+ EAL_DEV_MP_ACTION_REQUEST);
+ return ret;
+ }
+ }
<snip>
+
+#endif /* _HOTPLUG_MP_H_ */
diff --git a/lib/librte_eal/common/include/rte_bus.h
b/lib/librte_eal/common/include/rte_bus.h
index eb9eded4e..720f7c3c8 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -197,6 +197,9 @@ struct rte_bus_conf {
typedef enum rte_iova_mode (*rte_bus_get_iommu_class_t)(void);
+/* Max length for a bus name */
+#define RTE_BUS_NAME_MAX_LEN 32
Is this enforced anywhere in the bus codebase? Can we guarantee that bus
name will never be bigger than this?
+
/**
* A structure describing a generic bus.
*/
diff --git a/lib/librte_eal/common/include/rte_dev.h
b/lib/librte_eal/common/include/rte_dev.h
index 3879ff3ca..667df20f0 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -152,6 +152,9 @@ struct rte_driver {
*/
#define RTE_DEV_NAME_MAX_LEN 64
+/* Max devargs length be allowed */
+#define RTE_DEV_ARGS_MAX_LEN 128
Same - is this enforced anywhere in the codebase related to devargs?
--
Thanks,
Anatoly
