Hi Anatoly,

On 09/07/2019 16:12, Burakov, Anatoly wrote:
On 09-Jul-19 4:07 PM, David Hunt wrote:
From: Marcin Hajkowski <marcinx.hajkow...@intel.com>

This patch implements a separate FIFO for each cpu core to improve the
previous functionality where anyone with access to the FIFO could affect
any core on the system. By using appropriate permissions, fifo interfaces
can be configured to only affect the particular cores.

Because each FIFO is per core, the following fields have been removed
from the command JSON format: core_list, resource_id, name.

Signed-off-by: Lukasz Krakowiak <lukaszx.krakow...@intel.com>
Signed-off-by: Lukasz Gosiewski <lukaszx.gosiew...@intel.com>
Signed-off-by: Marcin Hajkowski <marcinx.hajkow...@intel.com>
Tested-by: David Hunt <david.h...@intel.com>

---
v2:
* updated handling vm_name (use proper buff size)
* rebase to master changes

v3:
* improvement to coding style

v4:
* rebase to tip of master

v5:
* merged docs into same patch as the code, as per mailing list policy
* made changes out of review by Anatoly.
---

<snip>

-    if (setup_host_channel_info(&chan_info, 0) < 0) {
-        rte_free(chan_info);
-        return 0;
+        ret = fifo_path(socket_path, sizeof(socket_path), i);
+        if (ret < 0)
+            goto error;
+
+        ret = mkfifo(socket_path, 0660);
+        RTE_LOG(DEBUG, CHANNEL_MANAGER, "TRY CREATE fifo '%s'\n",
+            socket_path);
+        if ((errno != EEXIST) && (ret < 0)) {
+            RTE_LOG(ERR, CHANNEL_MANAGER, "Cannot create fifo '%s' error: "
+                    "%s\n", socket_path, strerror(errno));
+            goto error;
+        }
+        chan_info = rte_malloc(NULL, sizeof(*chan_info), 0);
+        if (chan_info == NULL) {
+            RTE_LOG(ERR, CHANNEL_MANAGER, "Error allocating memory for "
+                    "channel '%s'\n", socket_path);
+            goto error;
+        }
+        chan_infos[i] = chan_info;
+        rte_strlcpy(chan_info->channel_path, socket_path,
+                sizeof(chan_info->channel_path));
+
+        if (setup_host_channel_info(&chan_info, i) < 0) {
+            rte_free(chan_info);
+            chan_infos[i] = NULL;
+            goto error;
+        }
+        num_channels_enabled++;
      }
-    num_channels_enabled++;
        return num_channels_enabled;
+error:
+    /* Clean up the channels opened before we hit an error. */
+    for (i = 0; i < RTE_MAX_LCORE; i++) {

You're going up to RTE_MAX_LCORE here, but up to ci->core_count in the original loop. Is that intentional?

Other than that,

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


Just to be totally clean, I've fixed the loop limit, and respun as v6. :)

Thanks for the review.

Rgds,

Dave.



Reply via email to