Re: [lng-odp] [PATCHv8] linux-generic: fix odp_pktio_inq_remdef

[Maxim Uvarov]

Stuart, can you set review-by for v8?

Maxim.

On 01/19/2015 01:01 PM, Maxim Uvarov wrote:

On 01/16/2015 03:58 PM, Maxim Uvarov wrote:

Correctly remove queue from packet i/o and remove it from scheduler.

Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org>
---

v8: fixed Stuart comments and added test. Implementation of odp_pktio_inq_remdef

    also fixes segfault in existance pktio test.

Looks like I thought about something else. I should say:

because of odp_pktio_inq_remdef was not implemented before, ODP can have special conditions where odp_schedule() for closed pktio can pull from old incoming queue. In linux-generic implementation packet recv() will be called and implementation will try to place packet to destroyed queue. So there is segfault. Because current odp examples do not destroy/create packet i/o and then call scheduler we did not see that segfault. So in this patch I implemented odp_pktio_inq_rmdef and added tests case for odp_schedule

on closed packet i.o.

Maxim.

.../linux-generic/include/odp_queue_internal.h     | 10 ++++++++

platform/linux-generic/odp_packet_io.c | 29 +++++++++++++++++++++-

  platform/linux-generic/odp_schedule.c              |  5 ++++
  test/validation/odp_pktio.c                        | 17 +++++++++++++
  4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h

index d5c8e4e..dbc42c0 100644
--- a/platform/linux-generic/include/odp_queue_internal.h
+++ b/platform/linux-generic/include/odp_queue_internal.h

@@ -129,6 +129,16 @@ static inline int queue_is_destroyed(odp_queue_t handle)

        return queue->s.status == QUEUE_STATUS_DESTROYED;
  }
+
+static inline int queue_is_sched(odp_queue_t handle)
+{
+    queue_entry_t *queue;
+
+    queue = queue_to_qentry(handle);
+
+    return ((queue->s.status == QUEUE_STATUS_SCHED) &&
+        (queue->s.pktin != ODP_PKTIO_INVALID));
+}
  #ifdef __cplusplus
  }
  #endif

diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c

index cd109d2..04de756 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c

@@ -429,7 +429,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue)

    int odp_pktio_inq_remdef(odp_pktio_t id)
  {
-    return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID);
+    pktio_entry_t *pktio_entry = get_pktio_entry(id);
+    odp_queue_t queue;
+    queue_entry_t *qentry;
+
+    if (pktio_entry == NULL)
+        return -1;
+
+    lock_entry(pktio_entry);
+    queue = pktio_entry->s.inq_default;
+    qentry = queue_to_qentry(queue);
+
+    queue_lock(qentry);
+    if (qentry->s.status == QUEUE_STATUS_FREE) {
+        queue_unlock(qentry);
+        unlock_entry(pktio_entry);
+        return -1;
+    }
+
+    qentry->s.enqueue = queue_enq_dummy;
+    qentry->s.enqueue_multi = queue_enq_multi_dummy;
+    qentry->s.status = QUEUE_STATUS_NOTSCHED;
+    qentry->s.pktin = ODP_PKTIO_INVALID;
+    queue_unlock(qentry);
+
+    pktio_entry->s.inq_default = ODP_QUEUE_INVALID;
+    unlock_entry(pktio_entry);
+
+    return 0;
  }
    odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id)

diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c

index a14de4f..775b788 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c

@@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, odp_buffer_t out_buf[],

                  desc  = odp_buffer_addr(desc_buf);
                  queue = desc->queue;
  +                if (odp_queue_type(queue) ==
+                    ODP_QUEUE_TYPE_PKTIN &&
+                    !queue_is_sched(queue))
+                    continue;
+
                  num = odp_queue_deq_multi(queue,
                                sched_local.buf,
                                max_deq);
diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
index d1eb0d5..03e954a 100644
--- a/test/validation/odp_pktio.c
+++ b/test/validation/odp_pktio.c

@@ -379,6 +379,7 @@ static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts)

      pktio_txrx_multi(&pktios[0], &pktios[if_b], num_pkts);
        for (i = 0; i < num_ifaces; ++i) {
+        odp_pktio_inq_remdef(pktios[i].id);
          ret = odp_pktio_close(pktios[i].id);
          CU_ASSERT(ret == 0);
      }
@@ -472,6 +473,21 @@ static void test_odp_pktio_mac(void)
      return;
  }
  +static void test_odp_pktio_inq_remdef(void)
+{
+    odp_pktio_t pktio = create_pktio(iface_name[0]);
+    int i;
+
+    CU_ASSERT(pktio != ODP_PKTIO_INVALID);
+    CU_ASSERT(create_inq(pktio) == 0);
+    CU_ASSERT(odp_pktio_inq_remdef(pktio) == 0);
+
+    for (i = 0; i < 100; i++)
+        odp_schedule(NULL, ODP_TIME_MSEC);
+
+    CU_ASSERT(odp_pktio_close(pktio) == 0);
+}
+
  static void test_odp_pktio_open(void)
  {
      odp_pktio_t pktio;
@@ -582,6 +598,7 @@ CU_TestInfo pktio_tests[] = {
      {"pktio mtu",        test_odp_pktio_mtu},
      {"pktio promisc mode",    test_odp_pktio_promisc},
      {"pktio mac",        test_odp_pktio_mac},
+    {"pktio inq_remdef",    test_odp_pktio_inq_remdef},
      CU_TEST_INFO_NULL
  };

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp