On 12/17/2014 05:45 PM, Stuart Haslam wrote:
On Wed, Dec 17, 2014 at 01:56:56PM +0000, Maxim Uvarov wrote:
odp_pktio_inq_remdef() calls odp_pktio_inq_setdef() with
ODP_QUEUE_INVALID. odp_pktio_inq_setdef should set up
ODP_QUEUE_INVALID to pktio instead of returning with error.

Signed-off-by: Maxim Uvarov <[email protected]>
---
  platform/linux-generic/odp_packet_io.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 3ca8100..4e54658 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -367,9 +367,16 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue)
        pktio_entry_t *pktio_entry = get_pktio_entry(id);
        queue_entry_t *qentry;
- if (pktio_entry == NULL || queue == ODP_QUEUE_INVALID)
+       if (pktio_entry == NULL)
                return -1;
+ if (queue == ODP_QUEUE_INVALID) {
+               lock_entry(pktio_entry);
+               pktio_entry->s.inq_default = ODP_QUEUE_INVALID;
+               unlock_entry(pktio_entry);
You could've avoided this duplicated chunk of code by just shuffling the
existing code around and adding an early return for an INVALID queue.

+               return 0;
+       }
+
        qentry = queue_to_qentry(queue);
if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
--
1.8.5.1.163.gd7aced9

My main concern though is that setdef and remdef are not symmetric.
setdef not only sets the queue as the default for the pktio (which is
only used when the application calls getdef) but also sets the pktin for
the queue and schedules it. With this change, after calling remdef the
previous default inq is still being scheduled, is that what the caller
expected and how do they now remove it from the scheduler if required?

In that case current odp_pktio_inq_remdef() is wrong.

It has to:
1.  take default  queue:
queue = pktio_entry->s.inq_default;

2. get entry:
qentry = queue_to_qentry(queue);

3. Remove it from scheduler:
qentry->s.status = QUEUE_STATUS_FREE or QUEUE_STATUS_DESTROYED (need to check code, unclear).

4. Return.

Right?

Maxim.


_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to