On 10/17/2018 9:56 AM, Raslan Darawsheh wrote: > @@ -2082,6 +2214,16 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev) > TAP_LOG(NOTICE, "Initializing pmd_tap for %s as %s", > name, tap_name); > > + /* Register IPC feed callback */ > + if (!tap_devices_count) { > + ret = rte_mp_action_register(TAP_MP_KEY, tap_mp_sync_queues); > + if (ret < 0) { > + TAP_LOG(ERR, "%s: Failed to register IPC callback: %s", > + tuntap_name, strerror(rte_errno)); > + goto leave; > + } > + } > + tap_devices_count++; > ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac, > ETH_TUNTAP_TYPE_TAP); > > @@ -2089,6 +2231,9 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev) > if (ret == -1) { > TAP_LOG(ERR, "Failed to create pmd for %s as %s", > name, tap_name); > + if (!tap_devices_count) > + rte_mp_action_unregister(TAP_MP_KEY); > + tap_devices_count--; > tap_unit--; /* Restore the unit number */ > } > rte_kvargs_free(kvlist); Fail recovery part seems broken, it can be like [1] or [2], but both requires a new variable. I double checked the logic in prev version of the patch that uses EEXIST return values, that is also broken. Overall the challenge is in error recovery part we don't know if we enter there before or after increasing dev_count, that is why a local variable required.
If you can fix the error recovery path using EEXIST without needing a new variable, I think that is better, but if not I suggest following [2] since the logic of increase the dev_count after device successfully created makes sense to me, but both works. Thanks, ferruh [1] /* Register IPC feed callback */ if (!tap_devices_count) { ret = rte_mp_action_register(TAP_MP_KEY, tap_mp_sync_queues); if (ret < 0) { TAP_LOG(ERR, "%s: Failed to register IPC callback: %s", tuntap_name, strerror(rte_errno)); goto leave; } } tap_devices_count++; tap_devices_count_increased = 1; ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac, ETH_TUNTAP_TYPE_TAP); leave: if (ret == -1) { TAP_LOG(ERR, "Failed to create pmd for %s as %s", name, tap_name); if (tap_devices_count_increased == 1) { if (tap_devices_count == 1) rte_mp_action_unregister(TAP_MP_KEY); tap_devices_count--; } tap_unit--; /* Restore the unit number */ } rte_kvargs_free(kvlist); [2] /* Register IPC feed callback */ if (!tap_devices_count) { ret = rte_mp_action_register(TAP_MP_KEY, tap_mp_sync_queues); if (ret < 0) { TAP_LOG(ERR, "%s: Failed to register IPC callback: %s", tuntap_name, strerror(rte_errno)); goto leave; } mp_action_registered = 1; } ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac, ETH_TUNTAP_TYPE_TAP); leave: if (ret == -1) { TAP_LOG(ERR, "Failed to create pmd for %s as %s", name, tap_name); if (mp_action_registered == 1) rte_mp_action_unregister(TAP_MP_KEY); tap_unit--; /* Restore the unit number */ } else { tap_devices_count++; } rte_kvargs_free(kvlist);