If probe fails in multi-process context, the device must removed
in other processes for consistency. This is a rollback mechanism.
However the rollback should not happen for devices which were
already probed before the current probe transaction.

When probing an already probed device, the driver may reject
with -EEXIST or update and succeed with code 0.
In order to distinguish successful new probe from re-probe,
in the function local_dev_probe(), the positive EEXIST code
is returned for the latter case.

The functions rte_dev_probe() and __handle_secondary_request()
can test for -EEXIST and +EEXIST, and skip rollback in such case.

Fixes: 244d5130719c ("eal: enable hotplug on multi-process")
Fixes: ac9e4a17370f ("eal: support attach/detach shared device from secondary")
Cc: qi.z.zh...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
---
 lib/librte_eal/common/eal_common_dev.c | 12 ++++++++++--
 lib/librte_eal/common/eal_private.h    |  2 +-
 lib/librte_eal/common/hotplug_mp.c     |  8 ++++++--
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c 
b/lib/librte_eal/common/eal_common_dev.c
index deaaea9345..2c7b1ab071 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -132,6 +132,7 @@ local_dev_probe(const char *devargs, struct rte_device 
**new_dev)
 {
        struct rte_device *dev;
        struct rte_devargs *da;
+       bool already_probed;
        int ret;
 
        *new_dev = NULL;
@@ -171,12 +172,15 @@ local_dev_probe(const char *devargs, struct rte_device 
**new_dev)
         * those devargs shouldn't be removed manually anymore.
         */
 
+       already_probed = rte_dev_is_probed(dev);
        ret = dev->bus->plug(dev);
        if (ret && !rte_dev_is_probed(dev)) { /* if hasn't ever succeeded */
                RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
                        dev->name);
                return ret;
        }
+       if (ret == 0 && already_probed)
+               ret = EEXIST; /* hint to avoid any rollback */
 
        *new_dev = dev;
        return ret;
@@ -194,6 +198,7 @@ rte_dev_probe(const char *devargs)
 {
        struct eal_dev_mp_req req;
        struct rte_device *dev;
+       bool already_probed;
        int ret;
 
        memset(&req, 0, sizeof(req));
@@ -221,8 +226,8 @@ rte_dev_probe(const char *devargs)
 
        /* primary attach the new device itself. */
        ret = local_dev_probe(devargs, &dev);
-
-       if (ret != 0 && ret != -EEXIST) {
+       already_probed = (ret == -EEXIST || ret == EEXIST);
+       if (ret < 0 && !already_probed) {
                RTE_LOG(ERR, EAL,
                        "Failed to attach device on primary process\n");
                return ret;
@@ -250,6 +255,9 @@ rte_dev_probe(const char *devargs)
        return 0;
 
 rollback:
+       if (already_probed)
+               return ret; /* skip rollback */
+
        req.t = EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK;
 
        /* primary send rollback request to secondary. */
diff --git a/lib/librte_eal/common/eal_private.h 
b/lib/librte_eal/common/eal_private.h
index 798ede553b..a01d252930 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -304,7 +304,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
  * @param new_dev
  *   new device be probed as output.
  * @return
- *   0 on success, negative on error.
+ *   >=0 on success (+EEXIST if already probed), negative on error.
  */
 int local_dev_probe(const char *devargs, struct rte_device **new_dev);
 
diff --git a/lib/librte_eal/common/hotplug_mp.c 
b/lib/librte_eal/common/hotplug_mp.c
index 69e9a16d6a..9f8ef28a3b 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -90,13 +90,15 @@ __handle_secondary_request(void *param)
        struct rte_devargs da;
        struct rte_device *dev;
        struct rte_bus *bus;
+       bool already_probed = false;
        int ret = 0;
 
        tmp_req = *req;
 
        if (req->t == EAL_DEV_REQ_TYPE_ATTACH) {
                ret = local_dev_probe(req->devargs, &dev);
-               if (ret != 0 && ret != -EEXIST) {
+               already_probed = (ret == -EEXIST || ret == EEXIST);
+               if (ret < 0 && !already_probed) {
                        RTE_LOG(ERR, EAL, "Failed to hotplug add device on 
primary\n");
                        goto finish;
                }
@@ -159,7 +161,7 @@ __handle_secondary_request(void *param)
        goto finish;
 
 rollback:
-       if (req->t == EAL_DEV_REQ_TYPE_ATTACH) {
+       if (req->t == EAL_DEV_REQ_TYPE_ATTACH && !already_probed) {
                tmp_req.t = EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK;
                eal_dev_hotplug_request_to_secondary(&tmp_req);
                local_dev_remove(dev);
@@ -238,6 +240,8 @@ static void __handle_primary_request(void *param)
        case EAL_DEV_REQ_TYPE_ATTACH:
        case EAL_DEV_REQ_TYPE_DETACH_ROLLBACK:
                ret = local_dev_probe(req->devargs, &dev);
+               if (ret > 0)
+                       ret = 0; /* return only errors */
                break;
        case EAL_DEV_REQ_TYPE_DETACH:
        case EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK:
-- 
2.20.1

Reply via email to