On 28-Jun-18 2:52 AM, Qi Zhang wrote:
This patch cover the multi-process hotplug case when a share device
attach/detach request be issued from secondary process

device attach on secondary:
a) seconary send sync request to primary.
b) primary receive the request and attach the new device if failed
    goto i).
c) primary forward attach sync request to all secondary.
d) secondary receive request and attach device and send reply.
e) primary check the reply if all success go to j).
f) primary send attach rollback sync request to all secondary.
g) secondary receive the request and detach device and send reply.
h) primary receive the reply and detach device as rollback action.
i) send fail reply to secondary, goto k).
j) send success reply to secondary.
k) secondary process receive reply of step a) and return.

device detach on secondary:
a) secondary send sync request to primary
b) primary receive the request and perform pre-detach check, if device
    is locked, goto j).
c) primary send pre-detach sync request to all secondary.
d) secondary perform pre-detach check and send reply.
e) primary check the reply if any fail goto j).
f) primary send detach sync request to all secondary
g) secondary detach the device and send reply
h) primary detach the device.
i) send success reply to secondary, goto k).
j) send fail reply to secondary.
k) secondary process receive reply of step a) and return.

Signed-off-by: Qi Zhang <qi.z.zh...@intel.com>
---

<snip>

+       const struct rte_mp_msg *msg = &bundle->msg;
+       const struct eth_dev_mp_req *req =
+               (const struct eth_dev_mp_req *)msg->param;
+       struct eth_dev_mp_req tmp_req;
+       uint16_t port_id;
+       int ret = 0;
+
+       tmp_req = *req;
+
+       if (req->t == REQ_TYPE_ATTACH) {
+               ret = do_eth_dev_attach(req->devargs, &port_id);
+               if (!ret) {
+                       tmp_req.port_id = port_id;
+                       ret = eth_dev_request_to_secondary(&tmp_req);
+               }
+       } else if (req->t == REQ_TYPE_DETACH) {
+               if (!rte_eth_dev_is_valid_port(req->port_id))
+                       ret = -EINVAL;
+               if (!ret)
+                       ret = process_lock_callbacks(req->port_id);
+               if (!ret) {
+                       tmp_req.t = REQ_TYPE_PRE_DETACH;
+                       ret = eth_dev_request_to_secondary(&tmp_req);
+               }
+               if (!ret) {
+                       if (!tmp_req.result) {
+                               tmp_req.t = REQ_TYPE_DETACH;
+                               ret = eth_dev_request_to_secondary(&tmp_req);
+                               if (!ret)
+                                       ret = do_eth_dev_detach(req->port_id);
+                       } else {
+                               ret = tmp_req.result;
+                       }
+               }

This seems like a good opportunity to use goto, rather than checking value of ret every step of the way.

+       } else {
+               ethdev_log(ERR, "unsupported secondary to primary request\n");
+               ret = -ENOTSUP;
+       }
+
+       ret = send_response_to_secondary(&tmp_req, ret, bundle->peer);
+       if (ret)
+               ethdev_log(ERR, "failed to send response to secondary\n");
+
+       free(bundle->peer);
+       free(bundle);
+}
+
+static int
+handle_secondary_request(const struct rte_mp_msg *msg,
+                       const void *peer)
  {
-       RTE_SET_USED(msg);
-       RTE_SET_USED(peer);
-       return -ENOTSUP;
+       struct mp_reply_bundle *bundle;
+       const struct eth_dev_mp_req *req =
+               (const struct eth_dev_mp_req *)msg->param;
+       int ret = 0;
+
+       bundle = malloc(sizeof(*bundle));
+       if (bundle == NULL) {
+               ethdev_log(ERR, "not enough memory\n");
+               return send_response_to_secondary(req, -ENOMEM, peer);
+       }
+
+       bundle->msg = *msg;
+       /**
+        * We need to send reply on interrupt thread, but peer can't be
+        * parsed directly, so this is a temporal hack, need to be fixed
+        * when it is ready.
+        */
+       bundle->peer = strdup(peer);
+
+       ret = rte_eal_alarm_set(1, __handle_secondary_request, bundle);

Again, some comments explaining why this is being done on a separate thread would be great.

Otherwise,

Reviewed-by: Anatoly Burakov <anatoly.bura...@intel.com>

--
Thanks,
Anatoly

Reply via email to