event/sw: fix queue memory leak and multi-link bug

This commit reinitializes a queue before it is reconfigured, such that
reorder buffer memory is not leaked.

This bug masked a few other problems, which this commit corrects as well:
- sw_port_link() allowed a port to link to a queue twice, such that the
  port could then successfully unlink the queue twice. Now the link
  function checks whether a port is already linked to the queue, and if so
  returns success but doesn't assign the a port a second slot in the
  queue's cq map.
- test_eventdev.c's test_eventdev_unlink() was unlinking a queue twice
  from the same port, and expecting the second unlink to succeed. Now the
  test unlinks, links, then unlinks again.
- test_eventdev.c's test_eventdev_link_get() was linking a single queue but
  expecting the unlink function to return nb_queues (where nb_queues > 1).
  The test now checks for a return value of 1.

Fixes: 5ffb2f142d95 ("event/sw: support event queues")
Fixes: 371a688fc159 ("event/sw: support linking queues to ports")
Fixes: f8f9d233ea0e ("test/eventdev: add unit tests")
Cc: stable@dpdk.org
Signed-off-by: Gage Eads <gage.eads@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index 74976c0..500387f 100644 (file)
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -34,6 +34,7 @@ sw_port_link(struct rte_eventdev *dev, void *port, const uint8_t queues[],
        RTE_SET_USED(priorities);
        for (i = 0; i < num; i++) {
                struct sw_qid *q = &sw->qids[queues[i]];
+               unsigned int j;
 
                /* check for qid map overflow */
                if (q->cq_num_mapped_cqs >= RTE_DIM(q->cq_map)) {
@@ -46,6 +47,15 @@ sw_port_link(struct rte_eventdev *dev, void *port, const uint8_t queues[],
                        break;
                }
 
+               for (j = 0; j < q->cq_num_mapped_cqs; j++) {
+                       if (q->cq_map[j] == p->id)
+                               break;
+               }
+
+               /* check if port is already linked */
+               if (j < q->cq_num_mapped_cqs)
+                       continue;
+
                if (q->type == SW_SCHED_TYPE_DIRECT) {
                        /* check directed qids only map to one port */
                        if (p->num_qids_mapped > 0) {
@@ -310,6 +320,23 @@ cleanup:
        return -EINVAL;
 }
 
+static void
+sw_queue_release(struct rte_eventdev *dev, uint8_t id)
+{
+       struct sw_evdev *sw = sw_pmd_priv(dev);
+       struct sw_qid *qid = &sw->qids[id];
+       uint32_t i;
+
+       for (i = 0; i < SW_IQS_MAX; i++)
+               iq_ring_destroy(qid->iq[i]);
+
+       if (qid->type == RTE_SCHED_TYPE_ORDERED) {
+               rte_free(qid->reorder_buffer);
+               rte_ring_free(qid->reorder_buffer_freelist);
+       }
+       memset(qid, 0, sizeof(*qid));
+}
+
 static int
 sw_queue_setup(struct rte_eventdev *dev, uint8_t queue_id,
                const struct rte_event_queue_conf *conf)
@@ -327,24 +354,11 @@ sw_queue_setup(struct rte_eventdev *dev, uint8_t queue_id,
        }
 
        struct sw_evdev *sw = sw_pmd_priv(dev);
-       return qid_init(sw, queue_id, type, conf);
-}
-
-static void
-sw_queue_release(struct rte_eventdev *dev, uint8_t id)
-{
-       struct sw_evdev *sw = sw_pmd_priv(dev);
-       struct sw_qid *qid = &sw->qids[id];
-       uint32_t i;
 
-       for (i = 0; i < SW_IQS_MAX; i++)
-               iq_ring_destroy(qid->iq[i]);
+       if (sw->qids[queue_id].initialized)
+               sw_queue_release(dev, queue_id);
 
-       if (qid->type == RTE_SCHED_TYPE_ORDERED) {
-               rte_free(qid->reorder_buffer);
-               rte_ring_free(qid->reorder_buffer_freelist);
-       }
-       memset(qid, 0, sizeof(*qid));
+       return qid_init(sw, queue_id, type, conf);
 }
 
 static void

diff --git a/test/test/test_eventdev.c b/test/test/test_eventdev.c
index 70042e5..615aeb0 100644 (file)
--- a/test/test/test_eventdev.c
+++ b/test/test/test_eventdev.c
@@ -811,6 +811,9 @@ test_eventdev_unlink(void)
        for (i = 0; i < nb_queues; i++)
                queues[i] = i;
 
+       ret = rte_event_port_link(TEST_DEV_ID, 0, NULL, NULL, 0);
+       TEST_ASSERT(ret >= 0, "Failed to link with NULL device%d",
+                                TEST_DEV_ID);
 
        ret = rte_event_port_unlink(TEST_DEV_ID, 0, queues, nb_queues);
        TEST_ASSERT(ret == nb_queues, "Failed to unlink(device%d) ret=%d",
@@ -871,9 +874,9 @@ test_eventdev_link_get(void)
        ret = rte_event_port_links_get(TEST_DEV_ID, 0, queues, priorities);
        TEST_ASSERT(ret == 1, "(%d)Wrong link get ret=%d expected=%d",
                                        TEST_DEV_ID, ret, 1);
-       /* unlink all*/
+       /* unlink the queue */
        ret = rte_event_port_unlink(TEST_DEV_ID, 0, NULL, 0);
-       TEST_ASSERT(ret == nb_queues, "Failed to unlink(device%d) ret=%d",
+       TEST_ASSERT(ret == 1, "Failed to unlink(device%d) ret=%d",
                                 TEST_DEV_ID, ret);
 
        /* 4links and 2 unlinks */