diff mbox

[RFC] Remove queue types ODP_QUEUE_TYPE_PKTIN and PKTOUT

Message ID 1412972575800.73127@caviumnetworks.com
State New
Headers show

Commit Message

Rosenboim, Leonid Oct. 10, 2014, 8:22 p.m. UTC
Bill.


The idea of separating the odp_queue_type_t in two, makes total sense, where the scheduling (ordered, atomic, unorderer)  type is orthogonal to anticipated use (pktio -> app; app -> pktio; app -> app)  is a good one, and ha sbeen on my mind for a while.


While we are discussion the API in google-docs, I whish people did not propose changes to the same APIs in the form of proposed patches. Having two discussion threads for the same subject matter might be rather confusing.


Have a pleasant trip,

- Leo

Comments

Mike Holmes Oct. 10, 2014, 8:39 p.m. UTC | #1
On 10 October 2014 16:22, Rosenboim, Leonid <
Leonid.Rosenboim@caviumnetworks.com> wrote:

>  Bill.
>
>
>  The idea of separating the odp_queue_type_t in two, makes total sense,
> where the scheduling (ordered, atomic, unorderer)  type is orthogonal to
> anticipated use (pktio -> app; app -> pktio; app -> app)  is a good one,
> and ha sbeen on my mind for a while.
>
>
>  While we are discussion the API in google-docs, I whish people did not
> propose changes to the same APIs in the form of proposed patches. Having
> two discussion threads for the same subject matter might be rather
> confusing.
>

A number of folks have pointed out that the dual track of docs and code
development has some limitations, we have some plans floated on the list to
help, any other ideas that help us keep the code progressing but avoiding
these conflicts with the docs appreciated.



>
>  Have a pleasant trip,
>
> - Leo
>
>
>
>  ------------------------------
> *From:* Bill Fischofer <bill.fischofer@linaro.org>
> *Sent:* Friday, October 10, 2014 12:39 PM
> *To:* Rosenboim, Leonid
> *Cc:* Stuart Haslam; lng-odp@lists.linaro.org; Kapoor, Prasun; Jacob,
> Jerin; Manoharan, Balasubramanian
> *Subject:* Re: [lng-odp] [RFC PATCH] Remove queue types
> ODP_QUEUE_TYPE_PKTIN and PKTOUT
>
>  We had discussed moving PKTIN/PKTOUT from the odp_queue_type to a new
> odp_queue_class, with the rationale being that these really were orthogonal
> concepts, not eliminating it.  I agree with Leo that we need to be mindful
> of HW-based implementations in the design of all ODP APIs.  It's the main
> reason why we do these designs in the open so that the various
> implementation stake holders can provide feedback on API feasibility and
> opportunities for acceleration, along with recommendations concerning the
> appropriate level of abstraction needed to accomplish these goals.
>
> I will be revising the queue design doc to reflect this.  I will be
> traveling on business next Tuesday but Mike will be hosting the ODP team
> call where this should be discussed in more detail.
>
>  Bill
>
> On Fri, Oct 10, 2014 at 1:28 PM, Rosenboim, Leonid <
> Leonid.Rosenboim@caviumnetworks.com> wrote:
>
>> Stuart,
>>
>> The declaration of a queue as PKTIN, PKTOUT during creation time is NOT
>> an implementation detail, and was in my opinion deliberate and changing
>> this will have serious consequences for our ability to support this API on
>> Octeon SoCs.
>>
>> Octeon has queues implemented in hardware, and there are two distinct
>> types of queues supported in the hardware of these chips: One type of queue
>> is for scheduling incoming packets, timer events and generic events
>> initiated by an application or packets in transit (SSO). The other type of
>> queues are only used for packet output (PKO). For mapping an ODP queue to
>> one of these, the implementation needs to know the queue's intended use to
>> select and reserve a resource from the correct resource type.
>>
>> It seems your comprehension of the API is entirely focused on a software
>> based implementation, without considering advanced network processors, and
>> the ability of the ODP API to abstract these hardware mechanisms. The
>> objective of enabling the mapping of network processor packet processing
>> hardware to ODP APIs is an objective clearly set forth in ODP charter, and
>> should not be neglected.
>>
>> Here are some documents in the public domain containing further
>> information.
>>
>> http://university.caviumnetworks.com/downloads/Mini_version_of_Prog_Guide_EDU_July_2010.pdf
>> .
>> hactive.googlecode.com/files/CN50XX-HRM-V0.99E.pdf
>>
>> Detailed documentation on the latest SoC models and complete and updated
>> programmers guide is available under NDA.
>>
>> - Leo
>> ________________________________________
>> From: lng-odp-bounces@lists.linaro.org <lng-odp-bounces@lists.linaro.org>
>> on behalf of Stuart Haslam <stuart.haslam@arm.com>
>> Sent: Thursday, October 09, 2014 1:33 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [RFC PATCH] Remove queue types ODP_QUEUE_TYPE_PKTIN
>> and      PKTOUT
>>
>> The PKTIN and PKTOUT queue types are currently used to identify queues
>> as being attached to a pktio interface, but this is an implementation
>> detail that has leaked into the API.
>>
>> Instead of using the queue type to modify the behaviour of a queue
>> endpoint to send/recv packets over a pktio interface, add hooks to
>> change this behaviour when a queue is set as a pktio input or output
>> queue.
>>
>> Signed-off-by: Stuart Haslam <stuart.haslam@arm.com>
>> ---
>> Sending as an RFC for now as I've only done limited testing so far.
>> Also, I'm not sure if other platforms depend on these queue types, I
>> expect they don't but please shout if this is not the case.
>>
>> Tested only with SCHED queues with odp_pktio on linux-generic.
>>
>>  example/generator/odp_generator.c                  |  2 +-
>>  example/ipsec/odp_ipsec.c                          |  4 +-
>>  example/l2fwd/odp_l2fwd.c                          |  2 +-
>>  example/packet/odp_pktio.c                         |  2 +-
>>  platform/linux-generic/include/api/odp_queue.h     |  2 -
>>  .../linux-generic/include/odp_queue_internal.h     |  3 ++
>>  platform/linux-generic/odp_packet_io.c             | 10 ++--
>>  platform/linux-generic/odp_queue.c                 | 61
>> ++++++++++++----------
>>  platform/linux-generic/odp_schedule.c              |  4 +-
>>  9 files changed, 45 insertions(+), 45 deletions(-)
>>
>> diff --git a/example/generator/odp_generator.c
>> b/example/generator/odp_generator.c
>> index 6055324..d566a19 100644
>> --- a/example/generator/odp_generator.c
>> +++ b/example/generator/odp_generator.c
>> @@ -473,7 +473,7 @@ static void *gen_recv_thread(void *arg)
>>         qparam.sched.group = ODP_SCHED_GROUP_DEFAULT;
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>> &qparam);
>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>> &qparam);
>>         if (inq_def == ODP_QUEUE_INVALID) {
>>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
>> thr);
>>                 return NULL;
>> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
>> index ec6c87a..ba90242 100644
>> --- a/example/ipsec/odp_ipsec.c
>> +++ b/example/ipsec/odp_ipsec.c
>> @@ -291,7 +291,7 @@ odp_queue_t polled_odp_queue_create(const char *name,
>>
>>         my_queue = odp_queue_create(name, my_type, param);
>>
>> -       if ((ODP_QUEUE_TYPE_SCHED == type) || (ODP_QUEUE_TYPE_PKTIN ==
>> type)) {
>> +       if (ODP_QUEUE_TYPE_SCHED == type) {
>>                 poll_queues[num_polled_queues++] = my_queue;
>>                 printf("%s: adding %d\n", __func__, my_queue);
>>         }
>> @@ -572,7 +572,7 @@ void initialize_intf(char *intf)
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>
>> -       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
>> +       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
>>         if (ODP_QUEUE_INVALID == inq_def) {
>>                 ODP_ERR("Error: pktio queue creation failed for %s\n",
>> intf);
>>                 exit(EXIT_FAILURE);
>> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
>> index 8aa0ba0..4f08dd2 100644
>> --- a/example/l2fwd/odp_l2fwd.c
>> +++ b/example/l2fwd/odp_l2fwd.c
>> @@ -165,7 +165,7 @@ static odp_pktio_t queue_mode_init_params(void *arg,
>> odp_buffer_pool_t pool)
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>
>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>> &qparam);
>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>> &qparam);
>>         if (inq_def == ODP_QUEUE_INVALID) {
>>                 ODP_ERR("  Error: pktio queue creation failed");
>>                 return ODP_PKTIO_INVALID;
>> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
>> index 145ae47..afb2cd2 100644
>> --- a/example/packet/odp_pktio.c
>> +++ b/example/packet/odp_pktio.c
>> @@ -151,7 +151,7 @@ static void *pktio_queue_thread(void *arg)
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>
>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>> &qparam);
>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>> &qparam);
>>         if (inq_def == ODP_QUEUE_INVALID) {
>>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
>> thr);
>>                 return NULL;
>> diff --git a/platform/linux-generic/include/api/odp_queue.h
>> b/platform/linux-generic/include/api/odp_queue.h
>> index 5e083f1..38ff272 100644
>> --- a/platform/linux-generic/include/api/odp_queue.h
>> +++ b/platform/linux-generic/include/api/odp_queue.h
>> @@ -42,8 +42,6 @@ typedef int odp_queue_type_t;
>>
>>  #define ODP_QUEUE_TYPE_SCHED  0  /**< Scheduled queue */
>>  #define ODP_QUEUE_TYPE_POLL   1  /**< Not scheduled queue */
>> -#define ODP_QUEUE_TYPE_PKTIN  2  /**< Packet input queue */
>> -#define ODP_QUEUE_TYPE_PKTOUT 3  /**< Packet output queue */
>>
>>  /**
>>   * ODP schedule priority
>> diff --git a/platform/linux-generic/include/odp_queue_internal.h
>> b/platform/linux-generic/include/odp_queue_internal.h
>> index 8b6c517..5329d4f 100644
>> --- a/platform/linux-generic/include/odp_queue_internal.h
>> +++ b/platform/linux-generic/include/odp_queue_internal.h
>> @@ -95,6 +95,9 @@ void queue_unlock(queue_entry_t *queue);
>>  odp_buffer_t queue_sched_buf(odp_queue_t queue);
>>  int queue_sched_atomic(odp_queue_t handle);
>>
>> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t id);
>> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t id);
>> +
>>  static inline uint32_t queue_to_id(odp_queue_t handle)
>>  {
>>         return handle - 1;
>> diff --git a/platform/linux-generic/odp_packet_io.c
>> b/platform/linux-generic/odp_packet_io.c
>> index 0c30f0f..fbf23ff 100644
>> --- a/platform/linux-generic/odp_packet_io.c
>> +++ b/platform/linux-generic/odp_packet_io.c
>> @@ -65,13 +65,13 @@ int odp_pktio_init_global(void)
>>                 snprintf(name, sizeof(name), "%i-pktio_outq_default",
>> (int)id);
>>                 name[ODP_QUEUE_NAME_LEN-1] = '\0';
>>
>> -               qid = odp_queue_create(name, ODP_QUEUE_TYPE_PKTOUT, NULL);
>> +               qid = odp_queue_create(name, ODP_QUEUE_TYPE_SCHED, NULL);
>>                 if (qid == ODP_QUEUE_INVALID)
>>                         return -1;
>>                 pktio_entry->s.outq_default = qid;
>>
>>                 queue_entry = queue_to_qentry(qid);
>> -               queue_entry->s.pktout = id;
>> +               queue_set_pktout(queue_entry, id);
>>         }
>>
>>         return 0;
>> @@ -325,16 +325,12 @@ int odp_pktio_inq_setdef(odp_pktio_t id,
>> odp_queue_t queue)
>>         if (pktio_entry == NULL || qentry == NULL)
>>                 return -1;
>>
>> -       if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
>> -               return -1;
>> -
>>         lock_entry(pktio_entry);
>>         pktio_entry->s.inq_default = queue;
>>         unlock_entry(pktio_entry);
>>
>>         queue_lock(qentry);
>> -       qentry->s.pktin = id;
>> -       qentry->s.status = QUEUE_STATUS_SCHED;
>> +       queue_set_pktin(qentry, id);
>>         queue_unlock(qentry);
>>
>>         odp_schedule_queue(queue, qentry->s.param.sched.prio);
>> diff --git a/platform/linux-generic/odp_queue.c
>> b/platform/linux-generic/odp_queue.c
>> index 1318bcd..1f0e33a 100644
>> --- a/platform/linux-generic/odp_queue.c
>> +++ b/platform/linux-generic/odp_queue.c
>> @@ -53,6 +53,8 @@ static void queue_init(queue_entry_t *queue, const char
>> *name,
>>  {
>>         strncpy(queue->s.name, name, ODP_QUEUE_NAME_LEN - 1);
>>         queue->s.type = type;
>> +       queue->s.pktin  = ODP_PKTIO_INVALID;
>> +       queue->s.pktout = ODP_PKTIO_INVALID;
>>
>>         if (param) {
>>                 memcpy(&queue->s.param, param, sizeof(odp_queue_param_t));
>> @@ -64,27 +66,10 @@ static void queue_init(queue_entry_t *queue, const
>> char *name,
>>                 queue->s.param.sched.group = ODP_SCHED_GROUP_DEFAULT;
>>         }
>>
>> -       switch (type) {
>> -       case ODP_QUEUE_TYPE_PKTIN:
>> -               queue->s.enqueue = pktin_enqueue;
>> -               queue->s.dequeue = pktin_dequeue;
>> -               queue->s.enqueue_multi = pktin_enq_multi;
>> -               queue->s.dequeue_multi = pktin_deq_multi;
>> -               break;
>> -       case ODP_QUEUE_TYPE_PKTOUT:
>> -               queue->s.enqueue = pktout_enqueue;
>> -               queue->s.dequeue = pktout_dequeue;
>> -               queue->s.enqueue_multi = pktout_enq_multi;
>> -               queue->s.dequeue_multi = pktout_deq_multi;
>> -               break;
>> -       default:
>> -               queue->s.enqueue = queue_enq;
>> -               queue->s.dequeue = queue_deq;
>> -               queue->s.enqueue_multi = queue_enq_multi;
>> -               queue->s.dequeue_multi = queue_deq_multi;
>> -               break;
>> -       }
>> -
>> +       queue->s.enqueue = queue_enq;
>> +       queue->s.dequeue = queue_deq;
>> +       queue->s.enqueue_multi = queue_enq_multi;
>> +       queue->s.dequeue_multi = queue_deq_multi;
>>         queue->s.head = NULL;
>>         queue->s.tail = NULL;
>>         queue->s.sched_buf = ODP_BUFFER_INVALID;
>> @@ -162,8 +147,7 @@ odp_queue_t odp_queue_create(const char *name,
>> odp_queue_type_t type,
>>                 if (queue->s.status == QUEUE_STATUS_FREE) {
>>                         queue_init(queue, name, type, param);
>>
>> -                       if (type == ODP_QUEUE_TYPE_SCHED ||
>> -                           type == ODP_QUEUE_TYPE_PKTIN)
>> +                       if (type == ODP_QUEUE_TYPE_SCHED)
>>                                 queue->s.status = QUEUE_STATUS_NOTSCHED;
>>                         else
>>                                 queue->s.status = QUEUE_STATUS_READY;
>> @@ -175,8 +159,7 @@ odp_queue_t odp_queue_create(const char *name,
>> odp_queue_type_t type,
>>                 UNLOCK(&queue->s.lock);
>>         }
>>
>> -       if (handle != ODP_QUEUE_INVALID &&
>> -           (type == ODP_QUEUE_TYPE_SCHED || type ==
>> ODP_QUEUE_TYPE_PKTIN)) {
>> +       if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
>>                 odp_buffer_t buf;
>>
>>                 buf = odp_schedule_buffer_alloc(handle);
>> @@ -353,8 +337,7 @@ odp_buffer_hdr_t *queue_deq(queue_entry_t *queue)
>>
>>         if (queue->s.head == NULL) {
>>                 /* Already empty queue */
>> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
>> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
>> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>>         } else {
>>                 buf_hdr       = queue->s.head;
>> @@ -381,8 +364,7 @@ int queue_deq_multi(queue_entry_t *queue,
>> odp_buffer_hdr_t *buf_hdr[], int num)
>>
>>         if (queue->s.head == NULL) {
>>                 /* Already empty queue */
>> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
>> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
>> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>>         } else {
>>                 odp_buffer_hdr_t *hdr = queue->s.head;
>> @@ -453,3 +435,24 @@ void queue_unlock(queue_entry_t *queue)
>>  {
>>         UNLOCK(&queue->s.lock);
>>  }
>> +
>> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t pktio_id)
>> +{
>> +       queue->s.pktin = pktio_id;
>> +       queue->s.enqueue = pktin_enqueue;
>> +       queue->s.dequeue = pktin_dequeue;
>> +       queue->s.enqueue_multi = pktin_enq_multi;
>> +       queue->s.dequeue_multi = pktin_deq_multi;
>> +       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
>> +               queue->s.status = QUEUE_STATUS_SCHED;
>> +}
>> +
>> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t pktio_id)
>> +{
>> +       queue->s.pktout = pktio_id;
>> +       queue->s.enqueue = pktout_enqueue;
>> +       queue->s.dequeue = pktout_dequeue;
>> +       queue->s.enqueue_multi = pktout_enq_multi;
>> +       queue->s.dequeue_multi = pktout_deq_multi;
>> +}
>> +
>> diff --git a/platform/linux-generic/odp_schedule.c
>> b/platform/linux-generic/odp_schedule.c
>> index 1bf819b..5742196 100644
>> --- a/platform/linux-generic/odp_schedule.c
>> +++ b/platform/linux-generic/odp_schedule.c
>> @@ -301,8 +301,8 @@ static int schedule(odp_queue_t *out_queue,
>> odp_buffer_t out_buf[],
>>                                         /* Remove empty queue from
>> scheduling,
>>                                          * except packet input queues
>>                                          */
>> -                                       if (odp_queue_type(queue) ==
>> -                                           ODP_QUEUE_TYPE_PKTIN)
>> +                                       queue_entry_t *qentry =
>> queue_to_qentry(queue);
>> +                                       if (qentry->s.pktin !=
>> ODP_PKTIO_INVALID)
>>                                                 odp_queue_enq(pri_q,
>> desc_buf);
>>
>>                                         continue;
>> --
>> 1.9.1
>>
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
Bill Fischofer Oct. 10, 2014, 8:43 p.m. UTC | #2
On reflection, I'm thinking we might want to generalize this further,
building on suggestions from both Leo and observations from Bala, Stuart,
and others.

A queue supports two main operations: enq and deq, and the problem is that
a single "type" doesn't really capture this distinction. Instead we might
want to do is describe who is doing these operations for any given queue.
Right now we define "types" of POLL, SCHED, PKTIN, and PKTOUT, but that
doesn't really capture things well.  POLL is a way of saying the
application will be doing deq calls for this queue, while SCHED says a
scheduler will be doing these.  By implication PKTOUT means that a
odp_pktio_t is doing these deq calls.  Similarly PKTIN might be thought of
as saying that an odp_pktio_t will be doing enq operations on the queue.

But let's look at the range of possibilities.  For any given queue (either
application-managed or implementation-managed) we have a number of possible
actors for either enq or deq.  The application can be the one doing the enq
or deq operations and that's usually the case for enq operations we've
considered in most use cases but only applies for "polled queue" for deq
operations. Other possible actors on queues include PKTIO objects,
SCHEDulers, various offload engines, classifiers, policers sand traffic
shapers (the latter ones being something we haven't talked about yet).

So maybe it might be cleaner to define an odp_queue_actor_e that enumerates
these various actors and then split odp_queue_type into odp_queue_enq_type
and odp_queue_deq_type.  That would completely characterize how queues are
connected and who is expected to be performing enqs and deqs on them.

Opinions?

Bill

On Fri, Oct 10, 2014 at 3:22 PM, Rosenboim, Leonid <
Leonid.Rosenboim@caviumnetworks.com> wrote:

>  Bill.
>
>
>  The idea of separating the odp_queue_type_t in two, makes total sense,
> where the scheduling (ordered, atomic, unorderer)  type is orthogonal to
> anticipated use (pktio -> app; app -> pktio; app -> app)  is a good one,
> and ha sbeen on my mind for a while.
>
>
>  While we are discussion the API in google-docs, I whish people did not
> propose changes to the same APIs in the form of proposed patches. Having
> two discussion threads for the same subject matter might be rather
> confusing.
>
>
>  Have a pleasant trip,
>
> - Leo
>
>
>
>  ------------------------------
> *From:* Bill Fischofer <bill.fischofer@linaro.org>
> *Sent:* Friday, October 10, 2014 12:39 PM
> *To:* Rosenboim, Leonid
> *Cc:* Stuart Haslam; lng-odp@lists.linaro.org; Kapoor, Prasun; Jacob,
> Jerin; Manoharan, Balasubramanian
> *Subject:* Re: [lng-odp] [RFC PATCH] Remove queue types
> ODP_QUEUE_TYPE_PKTIN and PKTOUT
>
>  We had discussed moving PKTIN/PKTOUT from the odp_queue_type to a new
> odp_queue_class, with the rationale being that these really were orthogonal
> concepts, not eliminating it.  I agree with Leo that we need to be mindful
> of HW-based implementations in the design of all ODP APIs.  It's the main
> reason why we do these designs in the open so that the various
> implementation stake holders can provide feedback on API feasibility and
> opportunities for acceleration, along with recommendations concerning the
> appropriate level of abstraction needed to accomplish these goals.
>
> I will be revising the queue design doc to reflect this.  I will be
> traveling on business next Tuesday but Mike will be hosting the ODP team
> call where this should be discussed in more detail.
>
>  Bill
>
> On Fri, Oct 10, 2014 at 1:28 PM, Rosenboim, Leonid <
> Leonid.Rosenboim@caviumnetworks.com> wrote:
>
>> Stuart,
>>
>> The declaration of a queue as PKTIN, PKTOUT during creation time is NOT
>> an implementation detail, and was in my opinion deliberate and changing
>> this will have serious consequences for our ability to support this API on
>> Octeon SoCs.
>>
>> Octeon has queues implemented in hardware, and there are two distinct
>> types of queues supported in the hardware of these chips: One type of queue
>> is for scheduling incoming packets, timer events and generic events
>> initiated by an application or packets in transit (SSO). The other type of
>> queues are only used for packet output (PKO). For mapping an ODP queue to
>> one of these, the implementation needs to know the queue's intended use to
>> select and reserve a resource from the correct resource type.
>>
>> It seems your comprehension of the API is entirely focused on a software
>> based implementation, without considering advanced network processors, and
>> the ability of the ODP API to abstract these hardware mechanisms. The
>> objective of enabling the mapping of network processor packet processing
>> hardware to ODP APIs is an objective clearly set forth in ODP charter, and
>> should not be neglected.
>>
>> Here are some documents in the public domain containing further
>> information.
>>
>> http://university.caviumnetworks.com/downloads/Mini_version_of_Prog_Guide_EDU_July_2010.pdf
>> .
>> hactive.googlecode.com/files/CN50XX-HRM-V0.99E.pdf
>>
>> Detailed documentation on the latest SoC models and complete and updated
>> programmers guide is available under NDA.
>>
>> - Leo
>> ________________________________________
>> From: lng-odp-bounces@lists.linaro.org <lng-odp-bounces@lists.linaro.org>
>> on behalf of Stuart Haslam <stuart.haslam@arm.com>
>> Sent: Thursday, October 09, 2014 1:33 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [RFC PATCH] Remove queue types ODP_QUEUE_TYPE_PKTIN
>> and      PKTOUT
>>
>> The PKTIN and PKTOUT queue types are currently used to identify queues
>> as being attached to a pktio interface, but this is an implementation
>> detail that has leaked into the API.
>>
>> Instead of using the queue type to modify the behaviour of a queue
>> endpoint to send/recv packets over a pktio interface, add hooks to
>> change this behaviour when a queue is set as a pktio input or output
>> queue.
>>
>> Signed-off-by: Stuart Haslam <stuart.haslam@arm.com>
>> ---
>> Sending as an RFC for now as I've only done limited testing so far.
>> Also, I'm not sure if other platforms depend on these queue types, I
>> expect they don't but please shout if this is not the case.
>>
>> Tested only with SCHED queues with odp_pktio on linux-generic.
>>
>>  example/generator/odp_generator.c                  |  2 +-
>>  example/ipsec/odp_ipsec.c                          |  4 +-
>>  example/l2fwd/odp_l2fwd.c                          |  2 +-
>>  example/packet/odp_pktio.c                         |  2 +-
>>  platform/linux-generic/include/api/odp_queue.h     |  2 -
>>  .../linux-generic/include/odp_queue_internal.h     |  3 ++
>>  platform/linux-generic/odp_packet_io.c             | 10 ++--
>>  platform/linux-generic/odp_queue.c                 | 61
>> ++++++++++++----------
>>  platform/linux-generic/odp_schedule.c              |  4 +-
>>  9 files changed, 45 insertions(+), 45 deletions(-)
>>
>> diff --git a/example/generator/odp_generator.c
>> b/example/generator/odp_generator.c
>> index 6055324..d566a19 100644
>> --- a/example/generator/odp_generator.c
>> +++ b/example/generator/odp_generator.c
>> @@ -473,7 +473,7 @@ static void *gen_recv_thread(void *arg)
>>         qparam.sched.group = ODP_SCHED_GROUP_DEFAULT;
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>> &qparam);
>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>> &qparam);
>>         if (inq_def == ODP_QUEUE_INVALID) {
>>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
>> thr);
>>                 return NULL;
>> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
>> index ec6c87a..ba90242 100644
>> --- a/example/ipsec/odp_ipsec.c
>> +++ b/example/ipsec/odp_ipsec.c
>> @@ -291,7 +291,7 @@ odp_queue_t polled_odp_queue_create(const char *name,
>>
>>         my_queue = odp_queue_create(name, my_type, param);
>>
>> -       if ((ODP_QUEUE_TYPE_SCHED == type) || (ODP_QUEUE_TYPE_PKTIN ==
>> type)) {
>> +       if (ODP_QUEUE_TYPE_SCHED == type) {
>>                 poll_queues[num_polled_queues++] = my_queue;
>>                 printf("%s: adding %d\n", __func__, my_queue);
>>         }
>> @@ -572,7 +572,7 @@ void initialize_intf(char *intf)
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>
>> -       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
>> +       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
>>         if (ODP_QUEUE_INVALID == inq_def) {
>>                 ODP_ERR("Error: pktio queue creation failed for %s\n",
>> intf);
>>                 exit(EXIT_FAILURE);
>> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
>> index 8aa0ba0..4f08dd2 100644
>> --- a/example/l2fwd/odp_l2fwd.c
>> +++ b/example/l2fwd/odp_l2fwd.c
>> @@ -165,7 +165,7 @@ static odp_pktio_t queue_mode_init_params(void *arg,
>> odp_buffer_pool_t pool)
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>
>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>> &qparam);
>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>> &qparam);
>>         if (inq_def == ODP_QUEUE_INVALID) {
>>                 ODP_ERR("  Error: pktio queue creation failed");
>>                 return ODP_PKTIO_INVALID;
>> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
>> index 145ae47..afb2cd2 100644
>> --- a/example/packet/odp_pktio.c
>> +++ b/example/packet/odp_pktio.c
>> @@ -151,7 +151,7 @@ static void *pktio_queue_thread(void *arg)
>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>> (int)pktio);
>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>
>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>> &qparam);
>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>> &qparam);
>>         if (inq_def == ODP_QUEUE_INVALID) {
>>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
>> thr);
>>                 return NULL;
>> diff --git a/platform/linux-generic/include/api/odp_queue.h
>> b/platform/linux-generic/include/api/odp_queue.h
>> index 5e083f1..38ff272 100644
>> --- a/platform/linux-generic/include/api/odp_queue.h
>> +++ b/platform/linux-generic/include/api/odp_queue.h
>> @@ -42,8 +42,6 @@ typedef int odp_queue_type_t;
>>
>>  #define ODP_QUEUE_TYPE_SCHED  0  /**< Scheduled queue */
>>  #define ODP_QUEUE_TYPE_POLL   1  /**< Not scheduled queue */
>> -#define ODP_QUEUE_TYPE_PKTIN  2  /**< Packet input queue */
>> -#define ODP_QUEUE_TYPE_PKTOUT 3  /**< Packet output queue */
>>
>>  /**
>>   * ODP schedule priority
>> diff --git a/platform/linux-generic/include/odp_queue_internal.h
>> b/platform/linux-generic/include/odp_queue_internal.h
>> index 8b6c517..5329d4f 100644
>> --- a/platform/linux-generic/include/odp_queue_internal.h
>> +++ b/platform/linux-generic/include/odp_queue_internal.h
>> @@ -95,6 +95,9 @@ void queue_unlock(queue_entry_t *queue);
>>  odp_buffer_t queue_sched_buf(odp_queue_t queue);
>>  int queue_sched_atomic(odp_queue_t handle);
>>
>> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t id);
>> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t id);
>> +
>>  static inline uint32_t queue_to_id(odp_queue_t handle)
>>  {
>>         return handle - 1;
>> diff --git a/platform/linux-generic/odp_packet_io.c
>> b/platform/linux-generic/odp_packet_io.c
>> index 0c30f0f..fbf23ff 100644
>> --- a/platform/linux-generic/odp_packet_io.c
>> +++ b/platform/linux-generic/odp_packet_io.c
>> @@ -65,13 +65,13 @@ int odp_pktio_init_global(void)
>>                 snprintf(name, sizeof(name), "%i-pktio_outq_default",
>> (int)id);
>>                 name[ODP_QUEUE_NAME_LEN-1] = '\0';
>>
>> -               qid = odp_queue_create(name, ODP_QUEUE_TYPE_PKTOUT, NULL);
>> +               qid = odp_queue_create(name, ODP_QUEUE_TYPE_SCHED, NULL);
>>                 if (qid == ODP_QUEUE_INVALID)
>>                         return -1;
>>                 pktio_entry->s.outq_default = qid;
>>
>>                 queue_entry = queue_to_qentry(qid);
>> -               queue_entry->s.pktout = id;
>> +               queue_set_pktout(queue_entry, id);
>>         }
>>
>>         return 0;
>> @@ -325,16 +325,12 @@ int odp_pktio_inq_setdef(odp_pktio_t id,
>> odp_queue_t queue)
>>         if (pktio_entry == NULL || qentry == NULL)
>>                 return -1;
>>
>> -       if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
>> -               return -1;
>> -
>>         lock_entry(pktio_entry);
>>         pktio_entry->s.inq_default = queue;
>>         unlock_entry(pktio_entry);
>>
>>         queue_lock(qentry);
>> -       qentry->s.pktin = id;
>> -       qentry->s.status = QUEUE_STATUS_SCHED;
>> +       queue_set_pktin(qentry, id);
>>         queue_unlock(qentry);
>>
>>         odp_schedule_queue(queue, qentry->s.param.sched.prio);
>> diff --git a/platform/linux-generic/odp_queue.c
>> b/platform/linux-generic/odp_queue.c
>> index 1318bcd..1f0e33a 100644
>> --- a/platform/linux-generic/odp_queue.c
>> +++ b/platform/linux-generic/odp_queue.c
>> @@ -53,6 +53,8 @@ static void queue_init(queue_entry_t *queue, const char
>> *name,
>>  {
>>         strncpy(queue->s.name, name, ODP_QUEUE_NAME_LEN - 1);
>>         queue->s.type = type;
>> +       queue->s.pktin  = ODP_PKTIO_INVALID;
>> +       queue->s.pktout = ODP_PKTIO_INVALID;
>>
>>         if (param) {
>>                 memcpy(&queue->s.param, param, sizeof(odp_queue_param_t));
>> @@ -64,27 +66,10 @@ static void queue_init(queue_entry_t *queue, const
>> char *name,
>>                 queue->s.param.sched.group = ODP_SCHED_GROUP_DEFAULT;
>>         }
>>
>> -       switch (type) {
>> -       case ODP_QUEUE_TYPE_PKTIN:
>> -               queue->s.enqueue = pktin_enqueue;
>> -               queue->s.dequeue = pktin_dequeue;
>> -               queue->s.enqueue_multi = pktin_enq_multi;
>> -               queue->s.dequeue_multi = pktin_deq_multi;
>> -               break;
>> -       case ODP_QUEUE_TYPE_PKTOUT:
>> -               queue->s.enqueue = pktout_enqueue;
>> -               queue->s.dequeue = pktout_dequeue;
>> -               queue->s.enqueue_multi = pktout_enq_multi;
>> -               queue->s.dequeue_multi = pktout_deq_multi;
>> -               break;
>> -       default:
>> -               queue->s.enqueue = queue_enq;
>> -               queue->s.dequeue = queue_deq;
>> -               queue->s.enqueue_multi = queue_enq_multi;
>> -               queue->s.dequeue_multi = queue_deq_multi;
>> -               break;
>> -       }
>> -
>> +       queue->s.enqueue = queue_enq;
>> +       queue->s.dequeue = queue_deq;
>> +       queue->s.enqueue_multi = queue_enq_multi;
>> +       queue->s.dequeue_multi = queue_deq_multi;
>>         queue->s.head = NULL;
>>         queue->s.tail = NULL;
>>         queue->s.sched_buf = ODP_BUFFER_INVALID;
>> @@ -162,8 +147,7 @@ odp_queue_t odp_queue_create(const char *name,
>> odp_queue_type_t type,
>>                 if (queue->s.status == QUEUE_STATUS_FREE) {
>>                         queue_init(queue, name, type, param);
>>
>> -                       if (type == ODP_QUEUE_TYPE_SCHED ||
>> -                           type == ODP_QUEUE_TYPE_PKTIN)
>> +                       if (type == ODP_QUEUE_TYPE_SCHED)
>>                                 queue->s.status = QUEUE_STATUS_NOTSCHED;
>>                         else
>>                                 queue->s.status = QUEUE_STATUS_READY;
>> @@ -175,8 +159,7 @@ odp_queue_t odp_queue_create(const char *name,
>> odp_queue_type_t type,
>>                 UNLOCK(&queue->s.lock);
>>         }
>>
>> -       if (handle != ODP_QUEUE_INVALID &&
>> -           (type == ODP_QUEUE_TYPE_SCHED || type ==
>> ODP_QUEUE_TYPE_PKTIN)) {
>> +       if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
>>                 odp_buffer_t buf;
>>
>>                 buf = odp_schedule_buffer_alloc(handle);
>> @@ -353,8 +337,7 @@ odp_buffer_hdr_t *queue_deq(queue_entry_t *queue)
>>
>>         if (queue->s.head == NULL) {
>>                 /* Already empty queue */
>> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
>> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
>> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>>         } else {
>>                 buf_hdr       = queue->s.head;
>> @@ -381,8 +364,7 @@ int queue_deq_multi(queue_entry_t *queue,
>> odp_buffer_hdr_t *buf_hdr[], int num)
>>
>>         if (queue->s.head == NULL) {
>>                 /* Already empty queue */
>> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
>> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
>> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>>         } else {
>>                 odp_buffer_hdr_t *hdr = queue->s.head;
>> @@ -453,3 +435,24 @@ void queue_unlock(queue_entry_t *queue)
>>  {
>>         UNLOCK(&queue->s.lock);
>>  }
>> +
>> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t pktio_id)
>> +{
>> +       queue->s.pktin = pktio_id;
>> +       queue->s.enqueue = pktin_enqueue;
>> +       queue->s.dequeue = pktin_dequeue;
>> +       queue->s.enqueue_multi = pktin_enq_multi;
>> +       queue->s.dequeue_multi = pktin_deq_multi;
>> +       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
>> +               queue->s.status = QUEUE_STATUS_SCHED;
>> +}
>> +
>> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t pktio_id)
>> +{
>> +       queue->s.pktout = pktio_id;
>> +       queue->s.enqueue = pktout_enqueue;
>> +       queue->s.dequeue = pktout_dequeue;
>> +       queue->s.enqueue_multi = pktout_enq_multi;
>> +       queue->s.dequeue_multi = pktout_deq_multi;
>> +}
>> +
>> diff --git a/platform/linux-generic/odp_schedule.c
>> b/platform/linux-generic/odp_schedule.c
>> index 1bf819b..5742196 100644
>> --- a/platform/linux-generic/odp_schedule.c
>> +++ b/platform/linux-generic/odp_schedule.c
>> @@ -301,8 +301,8 @@ static int schedule(odp_queue_t *out_queue,
>> odp_buffer_t out_buf[],
>>                                         /* Remove empty queue from
>> scheduling,
>>                                          * except packet input queues
>>                                          */
>> -                                       if (odp_queue_type(queue) ==
>> -                                           ODP_QUEUE_TYPE_PKTIN)
>> +                                       queue_entry_t *qentry =
>> queue_to_qentry(queue);
>> +                                       if (qentry->s.pktin !=
>> ODP_PKTIO_INVALID)
>>                                                 odp_queue_enq(pri_q,
>> desc_buf);
>>
>>                                         continue;
>> --
>> 1.9.1
>>
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
Bill Fischofer Oct. 11, 2014, 10:10 p.m. UTC | #3
The Queue API Doc has been updated to reflect requested clarifications
based on comments received so far, and it also incorporates a number of
different proposals for how to deal with the queue_type issues that have
been identified.  The current doc proposes that instead of an
odp_queue_type we characterize each queue by two odp_queue_entpoint_t
items, one for enq operations and one for deq operations.  A proposed set
of enums for this is shown along with a table of how the cross-product of
each of these (enq, deq) would map into the various queue usages ODP
supports, or might wish to support in the future.  This seems to be a
pretty straightforward extension to what we're doing that should help
eliminate a lot of ambiguity about who can do what with various queues.

Bill

On Fri, Oct 10, 2014 at 3:43 PM, Bill Fischofer <bill.fischofer@linaro.org>
wrote:

> On reflection, I'm thinking we might want to generalize this further,
> building on suggestions from both Leo and observations from Bala, Stuart,
> and others.
>
> A queue supports two main operations: enq and deq, and the problem is that
> a single "type" doesn't really capture this distinction. Instead we might
> want to do is describe who is doing these operations for any given queue.
> Right now we define "types" of POLL, SCHED, PKTIN, and PKTOUT, but that
> doesn't really capture things well.  POLL is a way of saying the
> application will be doing deq calls for this queue, while SCHED says a
> scheduler will be doing these.  By implication PKTOUT means that a
> odp_pktio_t is doing these deq calls.  Similarly PKTIN might be thought of
> as saying that an odp_pktio_t will be doing enq operations on the queue.
>
> But let's look at the range of possibilities.  For any given queue (either
> application-managed or implementation-managed) we have a number of possible
> actors for either enq or deq.  The application can be the one doing the enq
> or deq operations and that's usually the case for enq operations we've
> considered in most use cases but only applies for "polled queue" for deq
> operations. Other possible actors on queues include PKTIO objects,
> SCHEDulers, various offload engines, classifiers, policers sand traffic
> shapers (the latter ones being something we haven't talked about yet).
>
> So maybe it might be cleaner to define an odp_queue_actor_e that
> enumerates these various actors and then split odp_queue_type into
> odp_queue_enq_type and odp_queue_deq_type.  That would completely
> characterize how queues are connected and who is expected to be performing
> enqs and deqs on them.
>
> Opinions?
>
> Bill
>
> On Fri, Oct 10, 2014 at 3:22 PM, Rosenboim, Leonid <
> Leonid.Rosenboim@caviumnetworks.com> wrote:
>
>>  Bill.
>>
>>
>>  The idea of separating the odp_queue_type_t in two, makes total sense,
>> where the scheduling (ordered, atomic, unorderer)  type is orthogonal to
>> anticipated use (pktio -> app; app -> pktio; app -> app)  is a good one,
>> and ha sbeen on my mind for a while.
>>
>>
>>  While we are discussion the API in google-docs, I whish people did not
>> propose changes to the same APIs in the form of proposed patches. Having
>> two discussion threads for the same subject matter might be rather
>> confusing.
>>
>>
>>  Have a pleasant trip,
>>
>> - Leo
>>
>>
>>
>>  ------------------------------
>> *From:* Bill Fischofer <bill.fischofer@linaro.org>
>> *Sent:* Friday, October 10, 2014 12:39 PM
>> *To:* Rosenboim, Leonid
>> *Cc:* Stuart Haslam; lng-odp@lists.linaro.org; Kapoor, Prasun; Jacob,
>> Jerin; Manoharan, Balasubramanian
>> *Subject:* Re: [lng-odp] [RFC PATCH] Remove queue types
>> ODP_QUEUE_TYPE_PKTIN and PKTOUT
>>
>>  We had discussed moving PKTIN/PKTOUT from the odp_queue_type to a new
>> odp_queue_class, with the rationale being that these really were orthogonal
>> concepts, not eliminating it.  I agree with Leo that we need to be mindful
>> of HW-based implementations in the design of all ODP APIs.  It's the main
>> reason why we do these designs in the open so that the various
>> implementation stake holders can provide feedback on API feasibility and
>> opportunities for acceleration, along with recommendations concerning the
>> appropriate level of abstraction needed to accomplish these goals.
>>
>> I will be revising the queue design doc to reflect this.  I will be
>> traveling on business next Tuesday but Mike will be hosting the ODP team
>> call where this should be discussed in more detail.
>>
>>  Bill
>>
>> On Fri, Oct 10, 2014 at 1:28 PM, Rosenboim, Leonid <
>> Leonid.Rosenboim@caviumnetworks.com> wrote:
>>
>>> Stuart,
>>>
>>> The declaration of a queue as PKTIN, PKTOUT during creation time is NOT
>>> an implementation detail, and was in my opinion deliberate and changing
>>> this will have serious consequences for our ability to support this API on
>>> Octeon SoCs.
>>>
>>> Octeon has queues implemented in hardware, and there are two distinct
>>> types of queues supported in the hardware of these chips: One type of queue
>>> is for scheduling incoming packets, timer events and generic events
>>> initiated by an application or packets in transit (SSO). The other type of
>>> queues are only used for packet output (PKO). For mapping an ODP queue to
>>> one of these, the implementation needs to know the queue's intended use to
>>> select and reserve a resource from the correct resource type.
>>>
>>> It seems your comprehension of the API is entirely focused on a software
>>> based implementation, without considering advanced network processors, and
>>> the ability of the ODP API to abstract these hardware mechanisms. The
>>> objective of enabling the mapping of network processor packet processing
>>> hardware to ODP APIs is an objective clearly set forth in ODP charter, and
>>> should not be neglected.
>>>
>>> Here are some documents in the public domain containing further
>>> information.
>>>
>>> http://university.caviumnetworks.com/downloads/Mini_version_of_Prog_Guide_EDU_July_2010.pdf
>>> .
>>> hactive.googlecode.com/files/CN50XX-HRM-V0.99E.pdf
>>>
>>> Detailed documentation on the latest SoC models and complete and updated
>>> programmers guide is available under NDA.
>>>
>>> - Leo
>>> ________________________________________
>>> From: lng-odp-bounces@lists.linaro.org <lng-odp-bounces@lists.linaro.org>
>>> on behalf of Stuart Haslam <stuart.haslam@arm.com>
>>> Sent: Thursday, October 09, 2014 1:33 PM
>>> To: lng-odp@lists.linaro.org
>>> Subject: [lng-odp] [RFC PATCH] Remove queue types ODP_QUEUE_TYPE_PKTIN
>>> and      PKTOUT
>>>
>>> The PKTIN and PKTOUT queue types are currently used to identify queues
>>> as being attached to a pktio interface, but this is an implementation
>>> detail that has leaked into the API.
>>>
>>> Instead of using the queue type to modify the behaviour of a queue
>>> endpoint to send/recv packets over a pktio interface, add hooks to
>>> change this behaviour when a queue is set as a pktio input or output
>>> queue.
>>>
>>> Signed-off-by: Stuart Haslam <stuart.haslam@arm.com>
>>> ---
>>> Sending as an RFC for now as I've only done limited testing so far.
>>> Also, I'm not sure if other platforms depend on these queue types, I
>>> expect they don't but please shout if this is not the case.
>>>
>>> Tested only with SCHED queues with odp_pktio on linux-generic.
>>>
>>>  example/generator/odp_generator.c                  |  2 +-
>>>  example/ipsec/odp_ipsec.c                          |  4 +-
>>>  example/l2fwd/odp_l2fwd.c                          |  2 +-
>>>  example/packet/odp_pktio.c                         |  2 +-
>>>  platform/linux-generic/include/api/odp_queue.h     |  2 -
>>>  .../linux-generic/include/odp_queue_internal.h     |  3 ++
>>>  platform/linux-generic/odp_packet_io.c             | 10 ++--
>>>  platform/linux-generic/odp_queue.c                 | 61
>>> ++++++++++++----------
>>>  platform/linux-generic/odp_schedule.c              |  4 +-
>>>  9 files changed, 45 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/example/generator/odp_generator.c
>>> b/example/generator/odp_generator.c
>>> index 6055324..d566a19 100644
>>> --- a/example/generator/odp_generator.c
>>> +++ b/example/generator/odp_generator.c
>>> @@ -473,7 +473,7 @@ static void *gen_recv_thread(void *arg)
>>>         qparam.sched.group = ODP_SCHED_GROUP_DEFAULT;
>>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>>> (int)pktio);
>>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>>> &qparam);
>>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>>> &qparam);
>>>         if (inq_def == ODP_QUEUE_INVALID) {
>>>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
>>> thr);
>>>                 return NULL;
>>> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
>>> index ec6c87a..ba90242 100644
>>> --- a/example/ipsec/odp_ipsec.c
>>> +++ b/example/ipsec/odp_ipsec.c
>>> @@ -291,7 +291,7 @@ odp_queue_t polled_odp_queue_create(const char *name,
>>>
>>>         my_queue = odp_queue_create(name, my_type, param);
>>>
>>> -       if ((ODP_QUEUE_TYPE_SCHED == type) || (ODP_QUEUE_TYPE_PKTIN ==
>>> type)) {
>>> +       if (ODP_QUEUE_TYPE_SCHED == type) {
>>>                 poll_queues[num_polled_queues++] = my_queue;
>>>                 printf("%s: adding %d\n", __func__, my_queue);
>>>         }
>>> @@ -572,7 +572,7 @@ void initialize_intf(char *intf)
>>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>>> (int)pktio);
>>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>>
>>> -       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
>>> +       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
>>>         if (ODP_QUEUE_INVALID == inq_def) {
>>>                 ODP_ERR("Error: pktio queue creation failed for %s\n",
>>> intf);
>>>                 exit(EXIT_FAILURE);
>>> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
>>> index 8aa0ba0..4f08dd2 100644
>>> --- a/example/l2fwd/odp_l2fwd.c
>>> +++ b/example/l2fwd/odp_l2fwd.c
>>> @@ -165,7 +165,7 @@ static odp_pktio_t queue_mode_init_params(void *arg,
>>> odp_buffer_pool_t pool)
>>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>>> (int)pktio);
>>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>>
>>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>>> &qparam);
>>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>>> &qparam);
>>>         if (inq_def == ODP_QUEUE_INVALID) {
>>>                 ODP_ERR("  Error: pktio queue creation failed");
>>>                 return ODP_PKTIO_INVALID;
>>> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
>>> index 145ae47..afb2cd2 100644
>>> --- a/example/packet/odp_pktio.c
>>> +++ b/example/packet/odp_pktio.c
>>> @@ -151,7 +151,7 @@ static void *pktio_queue_thread(void *arg)
>>>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
>>> (int)pktio);
>>>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>>>
>>> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
>>> &qparam);
>>> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
>>> &qparam);
>>>         if (inq_def == ODP_QUEUE_INVALID) {
>>>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
>>> thr);
>>>                 return NULL;
>>> diff --git a/platform/linux-generic/include/api/odp_queue.h
>>> b/platform/linux-generic/include/api/odp_queue.h
>>> index 5e083f1..38ff272 100644
>>> --- a/platform/linux-generic/include/api/odp_queue.h
>>> +++ b/platform/linux-generic/include/api/odp_queue.h
>>> @@ -42,8 +42,6 @@ typedef int odp_queue_type_t;
>>>
>>>  #define ODP_QUEUE_TYPE_SCHED  0  /**< Scheduled queue */
>>>  #define ODP_QUEUE_TYPE_POLL   1  /**< Not scheduled queue */
>>> -#define ODP_QUEUE_TYPE_PKTIN  2  /**< Packet input queue */
>>> -#define ODP_QUEUE_TYPE_PKTOUT 3  /**< Packet output queue */
>>>
>>>  /**
>>>   * ODP schedule priority
>>> diff --git a/platform/linux-generic/include/odp_queue_internal.h
>>> b/platform/linux-generic/include/odp_queue_internal.h
>>> index 8b6c517..5329d4f 100644
>>> --- a/platform/linux-generic/include/odp_queue_internal.h
>>> +++ b/platform/linux-generic/include/odp_queue_internal.h
>>> @@ -95,6 +95,9 @@ void queue_unlock(queue_entry_t *queue);
>>>  odp_buffer_t queue_sched_buf(odp_queue_t queue);
>>>  int queue_sched_atomic(odp_queue_t handle);
>>>
>>> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t id);
>>> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t id);
>>> +
>>>  static inline uint32_t queue_to_id(odp_queue_t handle)
>>>  {
>>>         return handle - 1;
>>> diff --git a/platform/linux-generic/odp_packet_io.c
>>> b/platform/linux-generic/odp_packet_io.c
>>> index 0c30f0f..fbf23ff 100644
>>> --- a/platform/linux-generic/odp_packet_io.c
>>> +++ b/platform/linux-generic/odp_packet_io.c
>>> @@ -65,13 +65,13 @@ int odp_pktio_init_global(void)
>>>                 snprintf(name, sizeof(name), "%i-pktio_outq_default",
>>> (int)id);
>>>                 name[ODP_QUEUE_NAME_LEN-1] = '\0';
>>>
>>> -               qid = odp_queue_create(name, ODP_QUEUE_TYPE_PKTOUT,
>>> NULL);
>>> +               qid = odp_queue_create(name, ODP_QUEUE_TYPE_SCHED, NULL);
>>>                 if (qid == ODP_QUEUE_INVALID)
>>>                         return -1;
>>>                 pktio_entry->s.outq_default = qid;
>>>
>>>                 queue_entry = queue_to_qentry(qid);
>>> -               queue_entry->s.pktout = id;
>>> +               queue_set_pktout(queue_entry, id);
>>>         }
>>>
>>>         return 0;
>>> @@ -325,16 +325,12 @@ int odp_pktio_inq_setdef(odp_pktio_t id,
>>> odp_queue_t queue)
>>>         if (pktio_entry == NULL || qentry == NULL)
>>>                 return -1;
>>>
>>> -       if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
>>> -               return -1;
>>> -
>>>         lock_entry(pktio_entry);
>>>         pktio_entry->s.inq_default = queue;
>>>         unlock_entry(pktio_entry);
>>>
>>>         queue_lock(qentry);
>>> -       qentry->s.pktin = id;
>>> -       qentry->s.status = QUEUE_STATUS_SCHED;
>>> +       queue_set_pktin(qentry, id);
>>>         queue_unlock(qentry);
>>>
>>>         odp_schedule_queue(queue, qentry->s.param.sched.prio);
>>> diff --git a/platform/linux-generic/odp_queue.c
>>> b/platform/linux-generic/odp_queue.c
>>> index 1318bcd..1f0e33a 100644
>>> --- a/platform/linux-generic/odp_queue.c
>>> +++ b/platform/linux-generic/odp_queue.c
>>> @@ -53,6 +53,8 @@ static void queue_init(queue_entry_t *queue, const
>>> char *name,
>>>  {
>>>         strncpy(queue->s.name, name, ODP_QUEUE_NAME_LEN - 1);
>>>         queue->s.type = type;
>>> +       queue->s.pktin  = ODP_PKTIO_INVALID;
>>> +       queue->s.pktout = ODP_PKTIO_INVALID;
>>>
>>>         if (param) {
>>>                 memcpy(&queue->s.param, param,
>>> sizeof(odp_queue_param_t));
>>> @@ -64,27 +66,10 @@ static void queue_init(queue_entry_t *queue, const
>>> char *name,
>>>                 queue->s.param.sched.group = ODP_SCHED_GROUP_DEFAULT;
>>>         }
>>>
>>> -       switch (type) {
>>> -       case ODP_QUEUE_TYPE_PKTIN:
>>> -               queue->s.enqueue = pktin_enqueue;
>>> -               queue->s.dequeue = pktin_dequeue;
>>> -               queue->s.enqueue_multi = pktin_enq_multi;
>>> -               queue->s.dequeue_multi = pktin_deq_multi;
>>> -               break;
>>> -       case ODP_QUEUE_TYPE_PKTOUT:
>>> -               queue->s.enqueue = pktout_enqueue;
>>> -               queue->s.dequeue = pktout_dequeue;
>>> -               queue->s.enqueue_multi = pktout_enq_multi;
>>> -               queue->s.dequeue_multi = pktout_deq_multi;
>>> -               break;
>>> -       default:
>>> -               queue->s.enqueue = queue_enq;
>>> -               queue->s.dequeue = queue_deq;
>>> -               queue->s.enqueue_multi = queue_enq_multi;
>>> -               queue->s.dequeue_multi = queue_deq_multi;
>>> -               break;
>>> -       }
>>> -
>>> +       queue->s.enqueue = queue_enq;
>>> +       queue->s.dequeue = queue_deq;
>>> +       queue->s.enqueue_multi = queue_enq_multi;
>>> +       queue->s.dequeue_multi = queue_deq_multi;
>>>         queue->s.head = NULL;
>>>         queue->s.tail = NULL;
>>>         queue->s.sched_buf = ODP_BUFFER_INVALID;
>>> @@ -162,8 +147,7 @@ odp_queue_t odp_queue_create(const char *name,
>>> odp_queue_type_t type,
>>>                 if (queue->s.status == QUEUE_STATUS_FREE) {
>>>                         queue_init(queue, name, type, param);
>>>
>>> -                       if (type == ODP_QUEUE_TYPE_SCHED ||
>>> -                           type == ODP_QUEUE_TYPE_PKTIN)
>>> +                       if (type == ODP_QUEUE_TYPE_SCHED)
>>>                                 queue->s.status = QUEUE_STATUS_NOTSCHED;
>>>                         else
>>>                                 queue->s.status = QUEUE_STATUS_READY;
>>> @@ -175,8 +159,7 @@ odp_queue_t odp_queue_create(const char *name,
>>> odp_queue_type_t type,
>>>                 UNLOCK(&queue->s.lock);
>>>         }
>>>
>>> -       if (handle != ODP_QUEUE_INVALID &&
>>> -           (type == ODP_QUEUE_TYPE_SCHED || type ==
>>> ODP_QUEUE_TYPE_PKTIN)) {
>>> +       if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED)
>>> {
>>>                 odp_buffer_t buf;
>>>
>>>                 buf = odp_schedule_buffer_alloc(handle);
>>> @@ -353,8 +337,7 @@ odp_buffer_hdr_t *queue_deq(queue_entry_t *queue)
>>>
>>>         if (queue->s.head == NULL) {
>>>                 /* Already empty queue */
>>> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
>>> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
>>> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>>>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>>>         } else {
>>>                 buf_hdr       = queue->s.head;
>>> @@ -381,8 +364,7 @@ int queue_deq_multi(queue_entry_t *queue,
>>> odp_buffer_hdr_t *buf_hdr[], int num)
>>>
>>>         if (queue->s.head == NULL) {
>>>                 /* Already empty queue */
>>> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
>>> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
>>> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>>>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>>>         } else {
>>>                 odp_buffer_hdr_t *hdr = queue->s.head;
>>> @@ -453,3 +435,24 @@ void queue_unlock(queue_entry_t *queue)
>>>  {
>>>         UNLOCK(&queue->s.lock);
>>>  }
>>> +
>>> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t pktio_id)
>>> +{
>>> +       queue->s.pktin = pktio_id;
>>> +       queue->s.enqueue = pktin_enqueue;
>>> +       queue->s.dequeue = pktin_dequeue;
>>> +       queue->s.enqueue_multi = pktin_enq_multi;
>>> +       queue->s.dequeue_multi = pktin_deq_multi;
>>> +       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
>>> +               queue->s.status = QUEUE_STATUS_SCHED;
>>> +}
>>> +
>>> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t pktio_id)
>>> +{
>>> +       queue->s.pktout = pktio_id;
>>> +       queue->s.enqueue = pktout_enqueue;
>>> +       queue->s.dequeue = pktout_dequeue;
>>> +       queue->s.enqueue_multi = pktout_enq_multi;
>>> +       queue->s.dequeue_multi = pktout_deq_multi;
>>> +}
>>> +
>>> diff --git a/platform/linux-generic/odp_schedule.c
>>> b/platform/linux-generic/odp_schedule.c
>>> index 1bf819b..5742196 100644
>>> --- a/platform/linux-generic/odp_schedule.c
>>> +++ b/platform/linux-generic/odp_schedule.c
>>> @@ -301,8 +301,8 @@ static int schedule(odp_queue_t *out_queue,
>>> odp_buffer_t out_buf[],
>>>                                         /* Remove empty queue from
>>> scheduling,
>>>                                          * except packet input queues
>>>                                          */
>>> -                                       if (odp_queue_type(queue) ==
>>> -                                           ODP_QUEUE_TYPE_PKTIN)
>>> +                                       queue_entry_t *qentry =
>>> queue_to_qentry(queue);
>>> +                                       if (qentry->s.pktin !=
>>> ODP_PKTIO_INVALID)
>>>                                                 odp_queue_enq(pri_q,
>>> desc_buf);
>>>
>>>                                         continue;
>>> --
>>> 1.9.1
>>>
>>>
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>>
>
Bill Fischofer Oct. 12, 2014, 11:08 a.m. UTC | #4
Current DRAFT API docs are all linked from the ODP Web site at
http://www.opendataplane.org/documentation/

The referenced ODP Queue API doc is:
https://docs.google.com/a/linaro.org/document/d/1h0g7OEQst_PlOauNIHKmIyqK9Bofiv9L-bn9slRaBZ8/edit

On Sun, Oct 12, 2014 at 12:37 AM, Gilad Ben Yossef <giladb@ezchip.com>
wrote:

>
>
> This is getting quite confusing – is there a location where one would be
> able to watch/track these document? I realize they are on Google Docs
> somewhere but is there a location with link to current versions of them?
>
>
>
> Thanks,
>
> Gilad
>
>
>
> *Gilad Ben-Yossef*
>
> Software Architect
>
> EZchip Technologies Ltd.
>
> 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
>
> Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483
> Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
>
> Email: giladb@ezchip.com, Web: http://www.ezchip.com
>
>
>
> *"Ethernet always wins."*
>
>         — Andy Bechtolsheim
>
>
>
> *From:* lng-odp-bounces@lists.linaro.org [mailto:
> lng-odp-bounces@lists.linaro.org] *On Behalf Of *Bill Fischofer
> *Sent:* Sunday, October 12, 2014 1:11 AM
> *To:* Rosenboim, Leonid
> *Cc:* Kapoor, Prasun; Manoharan, Balasubramanian; lng-odp@lists.linaro.org
>
> *Subject:* Re: [lng-odp] [RFC PATCH] Remove queue types
> ODP_QUEUE_TYPE_PKTIN and PKTOUT
>
>
>
> The Queue API Doc has been updated to reflect requested clarifications
> based on comments received so far, and it also incorporates a number of
> different proposals for how to deal with the queue_type issues that have
> been identified.  The current doc proposes that instead of an
> odp_queue_type we characterize each queue by two odp_queue_entpoint_t
> items, one for enq operations and one for deq operations.  A proposed set
> of enums for this is shown along with a table of how the cross-product of
> each of these (enq, deq) would map into the various queue usages ODP
> supports, or might wish to support in the future.  This seems to be a
> pretty straightforward extension to what we're doing that should help
> eliminate a lot of ambiguity about who can do what with various queues.
>
>
>
> Bill
>
>
>
> On Fri, Oct 10, 2014 at 3:43 PM, Bill Fischofer <bill.fischofer@linaro.org>
> wrote:
>
> On reflection, I'm thinking we might want to generalize this further,
> building on suggestions from both Leo and observations from Bala, Stuart,
> and others.
>
>
>
> A queue supports two main operations: enq and deq, and the problem is that
> a single "type" doesn't really capture this distinction. Instead we might
> want to do is describe who is doing these operations for any given queue.
> Right now we define "types" of POLL, SCHED, PKTIN, and PKTOUT, but that
> doesn't really capture things well.  POLL is a way of saying the
> application will be doing deq calls for this queue, while SCHED says a
> scheduler will be doing these.  By implication PKTOUT means that a
> odp_pktio_t is doing these deq calls.  Similarly PKTIN might be thought of
> as saying that an odp_pktio_t will be doing enq operations on the queue.
>
>
>
> But let's look at the range of possibilities.  For any given queue (either
> application-managed or implementation-managed) we have a number of possible
> actors for either enq or deq.  The application can be the one doing the enq
> or deq operations and that's usually the case for enq operations we've
> considered in most use cases but only applies for "polled queue" for deq
> operations. Other possible actors on queues include PKTIO objects,
> SCHEDulers, various offload engines, classifiers, policers sand traffic
> shapers (the latter ones being something we haven't talked about yet).
>
>
>
> So maybe it might be cleaner to define an odp_queue_actor_e that
> enumerates these various actors and then split odp_queue_type into
> odp_queue_enq_type and odp_queue_deq_type.  That would completely
> characterize how queues are connected and who is expected to be performing
> enqs and deqs on them.
>
>
>
> Opinions?
>
>
>
> Bill
>
>
>
> On Fri, Oct 10, 2014 at 3:22 PM, Rosenboim, Leonid <
> Leonid.Rosenboim@caviumnetworks.com> wrote:
>
> Bill.
>
>
>
> The idea of separating the odp_queue_type_t in two, makes total sense,
> where the scheduling (ordered, atomic, unorderer)  type is orthogonal to
> anticipated use (pktio -> app; app -> pktio; app -> app)  is a good one,
> and ha sbeen on my mind for a while.
>
>
>
> While we are discussion the API in google-docs, I whish people did not
> propose changes to the same APIs in the form of proposed patches. Having
> two discussion threads for the same subject matter might be rather
> confusing.
>
>
>
> Have a pleasant trip,
>
> - Leo
>
>
>
>
>   ------------------------------
>
> *From:* Bill Fischofer <bill.fischofer@linaro.org>
> *Sent:* Friday, October 10, 2014 12:39 PM
> *To:* Rosenboim, Leonid
> *Cc:* Stuart Haslam; lng-odp@lists.linaro.org; Kapoor, Prasun; Jacob,
> Jerin; Manoharan, Balasubramanian
> *Subject:* Re: [lng-odp] [RFC PATCH] Remove queue types
> ODP_QUEUE_TYPE_PKTIN and PKTOUT
>
>
>
> We had discussed moving PKTIN/PKTOUT from the odp_queue_type to a new
> odp_queue_class, with the rationale being that these really were orthogonal
> concepts, not eliminating it.  I agree with Leo that we need to be mindful
> of HW-based implementations in the design of all ODP APIs.  It's the main
> reason why we do these designs in the open so that the various
> implementation stake holders can provide feedback on API feasibility and
> opportunities for acceleration, along with recommendations concerning the
> appropriate level of abstraction needed to accomplish these goals.
>
>
>
> I will be revising the queue design doc to reflect this.  I will be
> traveling on business next Tuesday but Mike will be hosting the ODP team
> call where this should be discussed in more detail.
>
>
>
> Bill
>
>
>
> On Fri, Oct 10, 2014 at 1:28 PM, Rosenboim, Leonid <
> Leonid.Rosenboim@caviumnetworks.com> wrote:
>
> Stuart,
>
> The declaration of a queue as PKTIN, PKTOUT during creation time is NOT an
> implementation detail, and was in my opinion deliberate and changing this
> will have serious consequences for our ability to support this API on
> Octeon SoCs.
>
> Octeon has queues implemented in hardware, and there are two distinct
> types of queues supported in the hardware of these chips: One type of queue
> is for scheduling incoming packets, timer events and generic events
> initiated by an application or packets in transit (SSO). The other type of
> queues are only used for packet output (PKO). For mapping an ODP queue to
> one of these, the implementation needs to know the queue's intended use to
> select and reserve a resource from the correct resource type.
>
> It seems your comprehension of the API is entirely focused on a software
> based implementation, without considering advanced network processors, and
> the ability of the ODP API to abstract these hardware mechanisms. The
> objective of enabling the mapping of network processor packet processing
> hardware to ODP APIs is an objective clearly set forth in ODP charter, and
> should not be neglected.
>
> Here are some documents in the public domain containing further
> information.
>
> http://university.caviumnetworks.com/downloads/Mini_version_of_Prog_Guide_EDU_July_2010.pdf
> .
> hactive.googlecode.com/files/CN50XX-HRM-V0.99E.pdf
>
> Detailed documentation on the latest SoC models and complete and updated
> programmers guide is available under NDA.
>
> - Leo
> ________________________________________
> From: lng-odp-bounces@lists.linaro.org <lng-odp-bounces@lists.linaro.org>
> on behalf of Stuart Haslam <stuart.haslam@arm.com>
> Sent: Thursday, October 09, 2014 1:33 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [RFC PATCH] Remove queue types ODP_QUEUE_TYPE_PKTIN
> and      PKTOUT
>
>
> The PKTIN and PKTOUT queue types are currently used to identify queues
> as being attached to a pktio interface, but this is an implementation
> detail that has leaked into the API.
>
> Instead of using the queue type to modify the behaviour of a queue
> endpoint to send/recv packets over a pktio interface, add hooks to
> change this behaviour when a queue is set as a pktio input or output
> queue.
>
> Signed-off-by: Stuart Haslam <stuart.haslam@arm.com>
> ---
> Sending as an RFC for now as I've only done limited testing so far.
> Also, I'm not sure if other platforms depend on these queue types, I
> expect they don't but please shout if this is not the case.
>
> Tested only with SCHED queues with odp_pktio on linux-generic.
>
>  example/generator/odp_generator.c                  |  2 +-
>  example/ipsec/odp_ipsec.c                          |  4 +-
>  example/l2fwd/odp_l2fwd.c                          |  2 +-
>  example/packet/odp_pktio.c                         |  2 +-
>  platform/linux-generic/include/api/odp_queue.h     |  2 -
>  .../linux-generic/include/odp_queue_internal.h     |  3 ++
>  platform/linux-generic/odp_packet_io.c             | 10 ++--
>  platform/linux-generic/odp_queue.c                 | 61
> ++++++++++++----------
>  platform/linux-generic/odp_schedule.c              |  4 +-
>  9 files changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> index 6055324..d566a19 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -473,7 +473,7 @@ static void *gen_recv_thread(void *arg)
>         qparam.sched.group = ODP_SCHED_GROUP_DEFAULT;
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
> &qparam);
> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
> &qparam);
>         if (inq_def == ODP_QUEUE_INVALID) {
>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
> thr);
>                 return NULL;
> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
> index ec6c87a..ba90242 100644
> --- a/example/ipsec/odp_ipsec.c
> +++ b/example/ipsec/odp_ipsec.c
> @@ -291,7 +291,7 @@ odp_queue_t polled_odp_queue_create(const char *name,
>
>         my_queue = odp_queue_create(name, my_type, param);
>
> -       if ((ODP_QUEUE_TYPE_SCHED == type) || (ODP_QUEUE_TYPE_PKTIN ==
> type)) {
> +       if (ODP_QUEUE_TYPE_SCHED == type) {
>                 poll_queues[num_polled_queues++] = my_queue;
>                 printf("%s: adding %d\n", __func__, my_queue);
>         }
> @@ -572,7 +572,7 @@ void initialize_intf(char *intf)
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>
> -       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
> +       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
>         if (ODP_QUEUE_INVALID == inq_def) {
>                 ODP_ERR("Error: pktio queue creation failed for %s\n",
> intf);
>                 exit(EXIT_FAILURE);
> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
> index 8aa0ba0..4f08dd2 100644
> --- a/example/l2fwd/odp_l2fwd.c
> +++ b/example/l2fwd/odp_l2fwd.c
> @@ -165,7 +165,7 @@ static odp_pktio_t queue_mode_init_params(void *arg,
> odp_buffer_pool_t pool)
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>
> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
> &qparam);
> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
> &qparam);
>         if (inq_def == ODP_QUEUE_INVALID) {
>                 ODP_ERR("  Error: pktio queue creation failed");
>                 return ODP_PKTIO_INVALID;
> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
> index 145ae47..afb2cd2 100644
> --- a/example/packet/odp_pktio.c
> +++ b/example/packet/odp_pktio.c
> @@ -151,7 +151,7 @@ static void *pktio_queue_thread(void *arg)
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>
> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
> &qparam);
> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
> &qparam);
>         if (inq_def == ODP_QUEUE_INVALID) {
>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
> thr);
>                 return NULL;
> diff --git a/platform/linux-generic/include/api/odp_queue.h
> b/platform/linux-generic/include/api/odp_queue.h
> index 5e083f1..38ff272 100644
> --- a/platform/linux-generic/include/api/odp_queue.h
> +++ b/platform/linux-generic/include/api/odp_queue.h
> @@ -42,8 +42,6 @@ typedef int odp_queue_type_t;
>
>  #define ODP_QUEUE_TYPE_SCHED  0  /**< Scheduled queue */
>  #define ODP_QUEUE_TYPE_POLL   1  /**< Not scheduled queue */
> -#define ODP_QUEUE_TYPE_PKTIN  2  /**< Packet input queue */
> -#define ODP_QUEUE_TYPE_PKTOUT 3  /**< Packet output queue */
>
>  /**
>   * ODP schedule priority
> diff --git a/platform/linux-generic/include/odp_queue_internal.h
> b/platform/linux-generic/include/odp_queue_internal.h
> index 8b6c517..5329d4f 100644
> --- a/platform/linux-generic/include/odp_queue_internal.h
> +++ b/platform/linux-generic/include/odp_queue_internal.h
> @@ -95,6 +95,9 @@ void queue_unlock(queue_entry_t *queue);
>  odp_buffer_t queue_sched_buf(odp_queue_t queue);
>  int queue_sched_atomic(odp_queue_t handle);
>
> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t id);
> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t id);
> +
>  static inline uint32_t queue_to_id(odp_queue_t handle)
>  {
>         return handle - 1;
> diff --git a/platform/linux-generic/odp_packet_io.c
> b/platform/linux-generic/odp_packet_io.c
> index 0c30f0f..fbf23ff 100644
> --- a/platform/linux-generic/odp_packet_io.c
> +++ b/platform/linux-generic/odp_packet_io.c
> @@ -65,13 +65,13 @@ int odp_pktio_init_global(void)
>                 snprintf(name, sizeof(name), "%i-pktio_outq_default",
> (int)id);
>                 name[ODP_QUEUE_NAME_LEN-1] = '\0';
>
> -               qid = odp_queue_create(name, ODP_QUEUE_TYPE_PKTOUT, NULL);
> +               qid = odp_queue_create(name, ODP_QUEUE_TYPE_SCHED, NULL);
>                 if (qid == ODP_QUEUE_INVALID)
>                         return -1;
>                 pktio_entry->s.outq_default = qid;
>
>                 queue_entry = queue_to_qentry(qid);
> -               queue_entry->s.pktout = id;
> +               queue_set_pktout(queue_entry, id);
>         }
>
>         return 0;
> @@ -325,16 +325,12 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t
> queue)
>         if (pktio_entry == NULL || qentry == NULL)
>                 return -1;
>
> -       if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
> -               return -1;
> -
>         lock_entry(pktio_entry);
>         pktio_entry->s.inq_default = queue;
>         unlock_entry(pktio_entry);
>
>         queue_lock(qentry);
> -       qentry->s.pktin = id;
> -       qentry->s.status = QUEUE_STATUS_SCHED;
> +       queue_set_pktin(qentry, id);
>         queue_unlock(qentry);
>
>         odp_schedule_queue(queue, qentry->s.param.sched.prio);
> diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> index 1318bcd..1f0e33a 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -53,6 +53,8 @@ static void queue_init(queue_entry_t *queue, const char
> *name,
>  {
>         strncpy(queue->s.name, name, ODP_QUEUE_NAME_LEN - 1);
>         queue->s.type = type;
> +       queue->s.pktin  = ODP_PKTIO_INVALID;
> +       queue->s.pktout = ODP_PKTIO_INVALID;
>
>         if (param) {
>                 memcpy(&queue->s.param, param, sizeof(odp_queue_param_t));
> @@ -64,27 +66,10 @@ static void queue_init(queue_entry_t *queue, const
> char *name,
>                 queue->s.param.sched.group = ODP_SCHED_GROUP_DEFAULT;
>         }
>
> -       switch (type) {
> -       case ODP_QUEUE_TYPE_PKTIN:
> -               queue->s.enqueue = pktin_enqueue;
> -               queue->s.dequeue = pktin_dequeue;
> -               queue->s.enqueue_multi = pktin_enq_multi;
> -               queue->s.dequeue_multi = pktin_deq_multi;
> -               break;
> -       case ODP_QUEUE_TYPE_PKTOUT:
> -               queue->s.enqueue = pktout_enqueue;
> -               queue->s.dequeue = pktout_dequeue;
> -               queue->s.enqueue_multi = pktout_enq_multi;
> -               queue->s.dequeue_multi = pktout_deq_multi;
> -               break;
> -       default:
> -               queue->s.enqueue = queue_enq;
> -               queue->s.dequeue = queue_deq;
> -               queue->s.enqueue_multi = queue_enq_multi;
> -               queue->s.dequeue_multi = queue_deq_multi;
> -               break;
> -       }
> -
> +       queue->s.enqueue = queue_enq;
> +       queue->s.dequeue = queue_deq;
> +       queue->s.enqueue_multi = queue_enq_multi;
> +       queue->s.dequeue_multi = queue_deq_multi;
>         queue->s.head = NULL;
>         queue->s.tail = NULL;
>         queue->s.sched_buf = ODP_BUFFER_INVALID;
> @@ -162,8 +147,7 @@ odp_queue_t odp_queue_create(const char *name,
> odp_queue_type_t type,
>                 if (queue->s.status == QUEUE_STATUS_FREE) {
>                         queue_init(queue, name, type, param);
>
> -                       if (type == ODP_QUEUE_TYPE_SCHED ||
> -                           type == ODP_QUEUE_TYPE_PKTIN)
> +                       if (type == ODP_QUEUE_TYPE_SCHED)
>                                 queue->s.status = QUEUE_STATUS_NOTSCHED;
>                         else
>                                 queue->s.status = QUEUE_STATUS_READY;
> @@ -175,8 +159,7 @@ odp_queue_t odp_queue_create(const char *name,
> odp_queue_type_t type,
>                 UNLOCK(&queue->s.lock);
>         }
>
> -       if (handle != ODP_QUEUE_INVALID &&
> -           (type == ODP_QUEUE_TYPE_SCHED || type ==
> ODP_QUEUE_TYPE_PKTIN)) {
> +       if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
>                 odp_buffer_t buf;
>
>                 buf = odp_schedule_buffer_alloc(handle);
> @@ -353,8 +337,7 @@ odp_buffer_hdr_t *queue_deq(queue_entry_t *queue)
>
>         if (queue->s.head == NULL) {
>                 /* Already empty queue */
> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>         } else {
>                 buf_hdr       = queue->s.head;
> @@ -381,8 +364,7 @@ int queue_deq_multi(queue_entry_t *queue,
> odp_buffer_hdr_t *buf_hdr[], int num)
>
>         if (queue->s.head == NULL) {
>                 /* Already empty queue */
> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>         } else {
>                 odp_buffer_hdr_t *hdr = queue->s.head;
> @@ -453,3 +435,24 @@ void queue_unlock(queue_entry_t *queue)
>  {
>         UNLOCK(&queue->s.lock);
>  }
> +
> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t pktio_id)
> +{
> +       queue->s.pktin = pktio_id;
> +       queue->s.enqueue = pktin_enqueue;
> +       queue->s.dequeue = pktin_dequeue;
> +       queue->s.enqueue_multi = pktin_enq_multi;
> +       queue->s.dequeue_multi = pktin_deq_multi;
> +       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
> +               queue->s.status = QUEUE_STATUS_SCHED;
> +}
> +
> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t pktio_id)
> +{
> +       queue->s.pktout = pktio_id;
> +       queue->s.enqueue = pktout_enqueue;
> +       queue->s.dequeue = pktout_dequeue;
> +       queue->s.enqueue_multi = pktout_enq_multi;
> +       queue->s.dequeue_multi = pktout_deq_multi;
> +}
> +
> diff --git a/platform/linux-generic/odp_schedule.c
> b/platform/linux-generic/odp_schedule.c
> index 1bf819b..5742196 100644
> --- a/platform/linux-generic/odp_schedule.c
> +++ b/platform/linux-generic/odp_schedule.c
> @@ -301,8 +301,8 @@ static int schedule(odp_queue_t *out_queue,
> odp_buffer_t out_buf[],
>                                         /* Remove empty queue from
> scheduling,
>                                          * except packet input queues
>                                          */
> -                                       if (odp_queue_type(queue) ==
> -                                           ODP_QUEUE_TYPE_PKTIN)
> +                                       queue_entry_t *qentry =
> queue_to_qentry(queue);
> +                                       if (qentry->s.pktin !=
> ODP_PKTIO_INVALID)
>                                                 odp_queue_enq(pri_q,
> desc_buf);
>
>                                         continue;
> --
> 1.9.1
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
>
>
>
Bill Fischofer Oct. 12, 2014, 11:13 a.m. UTC | #5
Good points, however using those terms we still have output queues that
come back to SW (e.g., offload block responses, loopbacks, or SW
pipelines).  I'm all for simplicity and flexibility.  Good topics for
Tuesday's call.

Bill

On Sun, Oct 12, 2014 at 2:28 AM, Gilad Ben Yossef <giladb@ezchip.com> wrote:

>
>
>
>
> The distinction between input queues and output queues is not about who
> does the queue/de-queue.  It is about the nature of what is in the queue:
>
>
>
> Input queue items represents  work to be done. Items there await
> processing.
>
> Output queues items are work that is already done. Items there await I/O
> outside the system.
>
>
>
> Input queues are consumed by processing units – which are either CPUs
> running software or HW blocks.
>
> Output queues are consumed by I/O units – network interfaces, DMA over
> PCIe bus, IPC etc.
>
>
>
> This is the distinction that we have **today** – and it is the right one.
> I'd really like to keep it.
>
>
>
> Why not have per queue:
>
>
>
> Type – either INPUT_QUEUE and OUTPUT_QUEUE.
>
>
>
> Content – or of flags PACKET_QUEUE (i.e. "I can be associated with a
> pkt_io"), TIMER_QUEUE (i.e. "I can be associated with a  timer"). Note that
> whether a platforms allows a single queue to carry both  packets and timer
> events is an implementation detail.
>
>
>
> Schedule - ATOMIC, ORDERED, PLAIN or NONE (a.k.a POLL) for input queues,
> DEFAULT for output queues (we'll probably define some sort of output
> scheduler for traffic management sometime post 1.0 …)
>
>
>
> A platform can define the legal combos – e.g. INPUT queues marked PACKET
> can only be ATOMIC or ORDERED for one platform, not allow an OUTPUT queue
> that is not of type PACKET etc.. but if your platform can say accept both
> TIMER and PACKET on the same INPUT queue you are free to allow it.
>
>
>
> Both input and output queues maybe queued to by processing units (SW or HW
> accelerator blocks) – for input queue the meaning is: "here's some work to
> do", for output queues the meaning is: "here a packet to send out".
>
>
>
> We associate each pkt_io with at least one queue of either INPUT or OUTPUT.
>
>
>
> It's pretty much what we have today, barring some semantic changes –
> unless I'm mistaken it doesn't call for any major changes for any example
> or platform.
>
> Is there someone that can't live with this?
>
>
>
> Gilad
>
>
>
>
>
>
>
> *Gilad Ben-Yossef*
>
> Software Architect
>
> EZchip Technologies Ltd.
>
> 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
>
> Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483
> Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
>
> Email: giladb@ezchip.com, Web: http://www.ezchip.com
>
>
>
> *"Ethernet always wins."*
>
>         — Andy Bechtolsheim
>
>
>
> *From:* lng-odp-bounces@lists.linaro.org [mailto:
> lng-odp-bounces@lists.linaro.org] *On Behalf Of *Bill Fischofer
> *Sent:* Sunday, October 12, 2014 1:11 AM
> *To:* Rosenboim, Leonid
> *Cc:* Kapoor, Prasun; Manoharan, Balasubramanian; lng-odp@lists.linaro.org
> *Subject:* Re: [lng-odp] [RFC PATCH] Remove queue types
> ODP_QUEUE_TYPE_PKTIN and PKTOUT
>
>
>
> The Queue API Doc has been updated to reflect requested clarifications
> based on comments received so far, and it also incorporates a number of
> different proposals for how to deal with the queue_type issues that have
> been identified.  The current doc proposes that instead of an
> odp_queue_type we characterize each queue by two odp_queue_entpoint_t
> items, one for enq operations and one for deq operations.  A proposed set
> of enums for this is shown along with a table of how the cross-product of
> each of these (enq, deq) would map into the various queue usages ODP
> supports, or might wish to support in the future.  This seems to be a
> pretty straightforward extension to what we're doing that should help
> eliminate a lot of ambiguity about who can do what with various queues.
>
>
>
> Bill
>
>
>
> On Fri, Oct 10, 2014 at 3:43 PM, Bill Fischofer <bill.fischofer@linaro.org>
> wrote:
>
> On reflection, I'm thinking we might want to generalize this further,
> building on suggestions from both Leo and observations from Bala, Stuart,
> and others.
>
>
>
> A queue supports two main operations: enq and deq, and the problem is that
> a single "type" doesn't really capture this distinction. Instead we might
> want to do is describe who is doing these operations for any given queue.
> Right now we define "types" of POLL, SCHED, PKTIN, and PKTOUT, but that
> doesn't really capture things well.  POLL is a way of saying the
> application will be doing deq calls for this queue, while SCHED says a
> scheduler will be doing these.  By implication PKTOUT means that a
> odp_pktio_t is doing these deq calls.  Similarly PKTIN might be thought of
> as saying that an odp_pktio_t will be doing enq operations on the queue.
>
>
>
> But let's look at the range of possibilities.  For any given queue (either
> application-managed or implementation-managed) we have a number of possible
> actors for either enq or deq.  The application can be the one doing the enq
> or deq operations and that's usually the case for enq operations we've
> considered in most use cases but only applies for "polled queue" for deq
> operations. Other possible actors on queues include PKTIO objects,
> SCHEDulers, various offload engines, classifiers, policers sand traffic
> shapers (the latter ones being something we haven't talked about yet).
>
>
>
> So maybe it might be cleaner to define an odp_queue_actor_e that
> enumerates these various actors and then split odp_queue_type into
> odp_queue_enq_type and odp_queue_deq_type.  That would completely
> characterize how queues are connected and who is expected to be performing
> enqs and deqs on them.
>
>
>
> Opinions?
>
>
>
> Bill
>
>
>
> On Fri, Oct 10, 2014 at 3:22 PM, Rosenboim, Leonid <
> Leonid.Rosenboim@caviumnetworks.com> wrote:
>
> Bill.
>
>
>
> The idea of separating the odp_queue_type_t in two, makes total sense,
> where the scheduling (ordered, atomic, unorderer)  type is orthogonal to
> anticipated use (pktio -> app; app -> pktio; app -> app)  is a good one,
> and ha sbeen on my mind for a while.
>
>
>
> While we are discussion the API in google-docs, I whish people did not
> propose changes to the same APIs in the form of proposed patches. Having
> two discussion threads for the same subject matter might be rather
> confusing.
>
>
>
> Have a pleasant trip,
>
> - Leo
>
>
>
>
>   ------------------------------
>
> *From:* Bill Fischofer <bill.fischofer@linaro.org>
> *Sent:* Friday, October 10, 2014 12:39 PM
> *To:* Rosenboim, Leonid
> *Cc:* Stuart Haslam; lng-odp@lists.linaro.org; Kapoor, Prasun; Jacob,
> Jerin; Manoharan, Balasubramanian
> *Subject:* Re: [lng-odp] [RFC PATCH] Remove queue types
> ODP_QUEUE_TYPE_PKTIN and PKTOUT
>
>
>
> We had discussed moving PKTIN/PKTOUT from the odp_queue_type to a new
> odp_queue_class, with the rationale being that these really were orthogonal
> concepts, not eliminating it.  I agree with Leo that we need to be mindful
> of HW-based implementations in the design of all ODP APIs.  It's the main
> reason why we do these designs in the open so that the various
> implementation stake holders can provide feedback on API feasibility and
> opportunities for acceleration, along with recommendations concerning the
> appropriate level of abstraction needed to accomplish these goals.
>
>
>
> I will be revising the queue design doc to reflect this.  I will be
> traveling on business next Tuesday but Mike will be hosting the ODP team
> call where this should be discussed in more detail.
>
>
>
> Bill
>
>
>
> On Fri, Oct 10, 2014 at 1:28 PM, Rosenboim, Leonid <
> Leonid.Rosenboim@caviumnetworks.com> wrote:
>
> Stuart,
>
> The declaration of a queue as PKTIN, PKTOUT during creation time is NOT an
> implementation detail, and was in my opinion deliberate and changing this
> will have serious consequences for our ability to support this API on
> Octeon SoCs.
>
> Octeon has queues implemented in hardware, and there are two distinct
> types of queues supported in the hardware of these chips: One type of queue
> is for scheduling incoming packets, timer events and generic events
> initiated by an application or packets in transit (SSO). The other type of
> queues are only used for packet output (PKO). For mapping an ODP queue to
> one of these, the implementation needs to know the queue's intended use to
> select and reserve a resource from the correct resource type.
>
> It seems your comprehension of the API is entirely focused on a software
> based implementation, without considering advanced network processors, and
> the ability of the ODP API to abstract these hardware mechanisms. The
> objective of enabling the mapping of network processor packet processing
> hardware to ODP APIs is an objective clearly set forth in ODP charter, and
> should not be neglected.
>
> Here are some documents in the public domain containing further
> information.
>
> http://university.caviumnetworks.com/downloads/Mini_version_of_Prog_Guide_EDU_July_2010.pdf
> .
> hactive.googlecode.com/files/CN50XX-HRM-V0.99E.pdf
>
> Detailed documentation on the latest SoC models and complete and updated
> programmers guide is available under NDA.
>
> - Leo
> ________________________________________
> From: lng-odp-bounces@lists.linaro.org <lng-odp-bounces@lists.linaro.org>
> on behalf of Stuart Haslam <stuart.haslam@arm.com>
> Sent: Thursday, October 09, 2014 1:33 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [RFC PATCH] Remove queue types ODP_QUEUE_TYPE_PKTIN
> and      PKTOUT
>
>
> The PKTIN and PKTOUT queue types are currently used to identify queues
> as being attached to a pktio interface, but this is an implementation
> detail that has leaked into the API.
>
> Instead of using the queue type to modify the behaviour of a queue
> endpoint to send/recv packets over a pktio interface, add hooks to
> change this behaviour when a queue is set as a pktio input or output
> queue.
>
> Signed-off-by: Stuart Haslam <stuart.haslam@arm.com>
> ---
> Sending as an RFC for now as I've only done limited testing so far.
> Also, I'm not sure if other platforms depend on these queue types, I
> expect they don't but please shout if this is not the case.
>
> Tested only with SCHED queues with odp_pktio on linux-generic.
>
>  example/generator/odp_generator.c                  |  2 +-
>  example/ipsec/odp_ipsec.c                          |  4 +-
>  example/l2fwd/odp_l2fwd.c                          |  2 +-
>  example/packet/odp_pktio.c                         |  2 +-
>  platform/linux-generic/include/api/odp_queue.h     |  2 -
>  .../linux-generic/include/odp_queue_internal.h     |  3 ++
>  platform/linux-generic/odp_packet_io.c             | 10 ++--
>  platform/linux-generic/odp_queue.c                 | 61
> ++++++++++++----------
>  platform/linux-generic/odp_schedule.c              |  4 +-
>  9 files changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> index 6055324..d566a19 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -473,7 +473,7 @@ static void *gen_recv_thread(void *arg)
>         qparam.sched.group = ODP_SCHED_GROUP_DEFAULT;
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
> &qparam);
> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
> &qparam);
>         if (inq_def == ODP_QUEUE_INVALID) {
>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
> thr);
>                 return NULL;
> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
> index ec6c87a..ba90242 100644
> --- a/example/ipsec/odp_ipsec.c
> +++ b/example/ipsec/odp_ipsec.c
> @@ -291,7 +291,7 @@ odp_queue_t polled_odp_queue_create(const char *name,
>
>         my_queue = odp_queue_create(name, my_type, param);
>
> -       if ((ODP_QUEUE_TYPE_SCHED == type) || (ODP_QUEUE_TYPE_PKTIN ==
> type)) {
> +       if (ODP_QUEUE_TYPE_SCHED == type) {
>                 poll_queues[num_polled_queues++] = my_queue;
>                 printf("%s: adding %d\n", __func__, my_queue);
>         }
> @@ -572,7 +572,7 @@ void initialize_intf(char *intf)
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>
> -       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
> +       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
>         if (ODP_QUEUE_INVALID == inq_def) {
>                 ODP_ERR("Error: pktio queue creation failed for %s\n",
> intf);
>                 exit(EXIT_FAILURE);
> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
> index 8aa0ba0..4f08dd2 100644
> --- a/example/l2fwd/odp_l2fwd.c
> +++ b/example/l2fwd/odp_l2fwd.c
> @@ -165,7 +165,7 @@ static odp_pktio_t queue_mode_init_params(void *arg,
> odp_buffer_pool_t pool)
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>
> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
> &qparam);
> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
> &qparam);
>         if (inq_def == ODP_QUEUE_INVALID) {
>                 ODP_ERR("  Error: pktio queue creation failed");
>                 return ODP_PKTIO_INVALID;
> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
> index 145ae47..afb2cd2 100644
> --- a/example/packet/odp_pktio.c
> +++ b/example/packet/odp_pktio.c
> @@ -151,7 +151,7 @@ static void *pktio_queue_thread(void *arg)
>         snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def",
> (int)pktio);
>         inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
>
> -       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN,
> &qparam);
> +       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED,
> &qparam);
>         if (inq_def == ODP_QUEUE_INVALID) {
>                 ODP_ERR("  [%02i] Error: pktio queue creation failed\n",
> thr);
>                 return NULL;
> diff --git a/platform/linux-generic/include/api/odp_queue.h
> b/platform/linux-generic/include/api/odp_queue.h
> index 5e083f1..38ff272 100644
> --- a/platform/linux-generic/include/api/odp_queue.h
> +++ b/platform/linux-generic/include/api/odp_queue.h
> @@ -42,8 +42,6 @@ typedef int odp_queue_type_t;
>
>  #define ODP_QUEUE_TYPE_SCHED  0  /**< Scheduled queue */
>  #define ODP_QUEUE_TYPE_POLL   1  /**< Not scheduled queue */
> -#define ODP_QUEUE_TYPE_PKTIN  2  /**< Packet input queue */
> -#define ODP_QUEUE_TYPE_PKTOUT 3  /**< Packet output queue */
>
>  /**
>   * ODP schedule priority
> diff --git a/platform/linux-generic/include/odp_queue_internal.h
> b/platform/linux-generic/include/odp_queue_internal.h
> index 8b6c517..5329d4f 100644
> --- a/platform/linux-generic/include/odp_queue_internal.h
> +++ b/platform/linux-generic/include/odp_queue_internal.h
> @@ -95,6 +95,9 @@ void queue_unlock(queue_entry_t *queue);
>  odp_buffer_t queue_sched_buf(odp_queue_t queue);
>  int queue_sched_atomic(odp_queue_t handle);
>
> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t id);
> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t id);
> +
>  static inline uint32_t queue_to_id(odp_queue_t handle)
>  {
>         return handle - 1;
> diff --git a/platform/linux-generic/odp_packet_io.c
> b/platform/linux-generic/odp_packet_io.c
> index 0c30f0f..fbf23ff 100644
> --- a/platform/linux-generic/odp_packet_io.c
> +++ b/platform/linux-generic/odp_packet_io.c
> @@ -65,13 +65,13 @@ int odp_pktio_init_global(void)
>                 snprintf(name, sizeof(name), "%i-pktio_outq_default",
> (int)id);
>                 name[ODP_QUEUE_NAME_LEN-1] = '\0';
>
> -               qid = odp_queue_create(name, ODP_QUEUE_TYPE_PKTOUT, NULL);
> +               qid = odp_queue_create(name, ODP_QUEUE_TYPE_SCHED, NULL);
>                 if (qid == ODP_QUEUE_INVALID)
>                         return -1;
>                 pktio_entry->s.outq_default = qid;
>
>                 queue_entry = queue_to_qentry(qid);
> -               queue_entry->s.pktout = id;
> +               queue_set_pktout(queue_entry, id);
>         }
>
>         return 0;
> @@ -325,16 +325,12 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t
> queue)
>         if (pktio_entry == NULL || qentry == NULL)
>                 return -1;
>
> -       if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
> -               return -1;
> -
>         lock_entry(pktio_entry);
>         pktio_entry->s.inq_default = queue;
>         unlock_entry(pktio_entry);
>
>         queue_lock(qentry);
> -       qentry->s.pktin = id;
> -       qentry->s.status = QUEUE_STATUS_SCHED;
> +       queue_set_pktin(qentry, id);
>         queue_unlock(qentry);
>
>         odp_schedule_queue(queue, qentry->s.param.sched.prio);
> diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> index 1318bcd..1f0e33a 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -53,6 +53,8 @@ static void queue_init(queue_entry_t *queue, const char
> *name,
>  {
>         strncpy(queue->s.name, name, ODP_QUEUE_NAME_LEN - 1);
>         queue->s.type = type;
> +       queue->s.pktin  = ODP_PKTIO_INVALID;
> +       queue->s.pktout = ODP_PKTIO_INVALID;
>
>         if (param) {
>                 memcpy(&queue->s.param, param, sizeof(odp_queue_param_t));
> @@ -64,27 +66,10 @@ static void queue_init(queue_entry_t *queue, const
> char *name,
>                 queue->s.param.sched.group = ODP_SCHED_GROUP_DEFAULT;
>         }
>
> -       switch (type) {
> -       case ODP_QUEUE_TYPE_PKTIN:
> -               queue->s.enqueue = pktin_enqueue;
> -               queue->s.dequeue = pktin_dequeue;
> -               queue->s.enqueue_multi = pktin_enq_multi;
> -               queue->s.dequeue_multi = pktin_deq_multi;
> -               break;
> -       case ODP_QUEUE_TYPE_PKTOUT:
> -               queue->s.enqueue = pktout_enqueue;
> -               queue->s.dequeue = pktout_dequeue;
> -               queue->s.enqueue_multi = pktout_enq_multi;
> -               queue->s.dequeue_multi = pktout_deq_multi;
> -               break;
> -       default:
> -               queue->s.enqueue = queue_enq;
> -               queue->s.dequeue = queue_deq;
> -               queue->s.enqueue_multi = queue_enq_multi;
> -               queue->s.dequeue_multi = queue_deq_multi;
> -               break;
> -       }
> -
> +       queue->s.enqueue = queue_enq;
> +       queue->s.dequeue = queue_deq;
> +       queue->s.enqueue_multi = queue_enq_multi;
> +       queue->s.dequeue_multi = queue_deq_multi;
>         queue->s.head = NULL;
>         queue->s.tail = NULL;
>         queue->s.sched_buf = ODP_BUFFER_INVALID;
> @@ -162,8 +147,7 @@ odp_queue_t odp_queue_create(const char *name,
> odp_queue_type_t type,
>                 if (queue->s.status == QUEUE_STATUS_FREE) {
>                         queue_init(queue, name, type, param);
>
> -                       if (type == ODP_QUEUE_TYPE_SCHED ||
> -                           type == ODP_QUEUE_TYPE_PKTIN)
> +                       if (type == ODP_QUEUE_TYPE_SCHED)
>                                 queue->s.status = QUEUE_STATUS_NOTSCHED;
>                         else
>                                 queue->s.status = QUEUE_STATUS_READY;
> @@ -175,8 +159,7 @@ odp_queue_t odp_queue_create(const char *name,
> odp_queue_type_t type,
>                 UNLOCK(&queue->s.lock);
>         }
>
> -       if (handle != ODP_QUEUE_INVALID &&
> -           (type == ODP_QUEUE_TYPE_SCHED || type ==
> ODP_QUEUE_TYPE_PKTIN)) {
> +       if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
>                 odp_buffer_t buf;
>
>                 buf = odp_schedule_buffer_alloc(handle);
> @@ -353,8 +337,7 @@ odp_buffer_hdr_t *queue_deq(queue_entry_t *queue)
>
>         if (queue->s.head == NULL) {
>                 /* Already empty queue */
> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>         } else {
>                 buf_hdr       = queue->s.head;
> @@ -381,8 +364,7 @@ int queue_deq_multi(queue_entry_t *queue,
> odp_buffer_hdr_t *buf_hdr[], int num)
>
>         if (queue->s.head == NULL) {
>                 /* Already empty queue */
> -               if (queue->s.status == QUEUE_STATUS_SCHED &&
> -                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
> +               if (queue->s.status == QUEUE_STATUS_SCHED)
>                         queue->s.status = QUEUE_STATUS_NOTSCHED;
>         } else {
>                 odp_buffer_hdr_t *hdr = queue->s.head;
> @@ -453,3 +435,24 @@ void queue_unlock(queue_entry_t *queue)
>  {
>         UNLOCK(&queue->s.lock);
>  }
> +
> +void queue_set_pktin(queue_entry_t *queue, odp_pktio_t pktio_id)
> +{
> +       queue->s.pktin = pktio_id;
> +       queue->s.enqueue = pktin_enqueue;
> +       queue->s.dequeue = pktin_dequeue;
> +       queue->s.enqueue_multi = pktin_enq_multi;
> +       queue->s.dequeue_multi = pktin_deq_multi;
> +       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
> +               queue->s.status = QUEUE_STATUS_SCHED;
> +}
> +
> +void queue_set_pktout(queue_entry_t *queue, odp_pktio_t pktio_id)
> +{
> +       queue->s.pktout = pktio_id;
> +       queue->s.enqueue = pktout_enqueue;
> +       queue->s.dequeue = pktout_dequeue;
> +       queue->s.enqueue_multi = pktout_enq_multi;
> +       queue->s.dequeue_multi = pktout_deq_multi;
> +}
> +
> diff --git a/platform/linux-generic/odp_schedule.c
> b/platform/linux-generic/odp_schedule.c
> index 1bf819b..5742196 100644
> --- a/platform/linux-generic/odp_schedule.c
> +++ b/platform/linux-generic/odp_schedule.c
> @@ -301,8 +301,8 @@ static int schedule(odp_queue_t *out_queue,
> odp_buffer_t out_buf[],
>                                         /* Remove empty queue from
> scheduling,
>                                          * except packet input queues
>                                          */
> -                                       if (odp_queue_type(queue) ==
> -                                           ODP_QUEUE_TYPE_PKTIN)
> +                                       queue_entry_t *qentry =
> queue_to_qentry(queue);
> +                                       if (qentry->s.pktin !=
> ODP_PKTIO_INVALID)
>                                                 odp_queue_enq(pri_q,
> desc_buf);
>
>                                         continue;
> --
> 1.9.1
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
>
>
>
diff mbox

Patch

diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
index 6055324..d566a19 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -473,7 +473,7 @@  static void *gen_recv_thread(void *arg)
        qparam.sched.group = ODP_SCHED_GROUP_DEFAULT;
        snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def", (int)pktio);
        inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';
-       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
+       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
        if (inq_def == ODP_QUEUE_INVALID) {
                ODP_ERR("  [%02i] Error: pktio queue creation failed\n", thr);
                return NULL;
diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
index ec6c87a..ba90242 100644
--- a/example/ipsec/odp_ipsec.c
+++ b/example/ipsec/odp_ipsec.c
@@ -291,7 +291,7 @@  odp_queue_t polled_odp_queue_create(const char *name,

        my_queue = odp_queue_create(name, my_type, param);

-       if ((ODP_QUEUE_TYPE_SCHED == type) || (ODP_QUEUE_TYPE_PKTIN == type)) {
+       if (ODP_QUEUE_TYPE_SCHED == type) {
                poll_queues[num_polled_queues++] = my_queue;
                printf("%s: adding %d\n", __func__, my_queue);
        }
@@ -572,7 +572,7 @@  void initialize_intf(char *intf)
        snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def", (int)pktio);
        inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';

-       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
+       inq_def = QUEUE_CREATE(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
        if (ODP_QUEUE_INVALID == inq_def) {
                ODP_ERR("Error: pktio queue creation failed for %s\n", intf);
                exit(EXIT_FAILURE);
diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
index 8aa0ba0..4f08dd2 100644
--- a/example/l2fwd/odp_l2fwd.c
+++ b/example/l2fwd/odp_l2fwd.c
@@ -165,7 +165,7 @@  static odp_pktio_t queue_mode_init_params(void *arg, odp_buffer_pool_t pool)
        snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def", (int)pktio);
        inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';

-       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
+       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
        if (inq_def == ODP_QUEUE_INVALID) {
                ODP_ERR("  Error: pktio queue creation failed");
                return ODP_PKTIO_INVALID;
diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
index 145ae47..afb2cd2 100644
--- a/example/packet/odp_pktio.c
+++ b/example/packet/odp_pktio.c
@@ -151,7 +151,7 @@  static void *pktio_queue_thread(void *arg)
        snprintf(inq_name, sizeof(inq_name), "%i-pktio_inq_def", (int)pktio);
        inq_name[ODP_QUEUE_NAME_LEN - 1] = '\0';

-       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_PKTIN, &qparam);
+       inq_def = odp_queue_create(inq_name, ODP_QUEUE_TYPE_SCHED, &qparam);
        if (inq_def == ODP_QUEUE_INVALID) {
                ODP_ERR("  [%02i] Error: pktio queue creation failed\n", thr);
                return NULL;
diff --git a/platform/linux-generic/include/api/odp_queue.h b/platform/linux-generic/include/api/odp_queue.h
index 5e083f1..38ff272 100644
--- a/platform/linux-generic/include/api/odp_queue.h
+++ b/platform/linux-generic/include/api/odp_queue.h
@@ -42,8 +42,6 @@  typedef int odp_queue_type_t;

 #define ODP_QUEUE_TYPE_SCHED  0  /**< Scheduled queue */
 #define ODP_QUEUE_TYPE_POLL   1  /**< Not scheduled queue */
-#define ODP_QUEUE_TYPE_PKTIN  2  /**< Packet input queue */
-#define ODP_QUEUE_TYPE_PKTOUT 3  /**< Packet output queue */

 /**
  * ODP schedule priority
diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h
index 8b6c517..5329d4f 100644
--- a/platform/linux-generic/include/odp_queue_internal.h
+++ b/platform/linux-generic/include/odp_queue_internal.h
@@ -95,6 +95,9 @@  void queue_unlock(queue_entry_t *queue);
 odp_buffer_t queue_sched_buf(odp_queue_t queue);
 int queue_sched_atomic(odp_queue_t handle);

+void queue_set_pktin(queue_entry_t *queue, odp_pktio_t id);
+void queue_set_pktout(queue_entry_t *queue, odp_pktio_t id);
+
 static inline uint32_t queue_to_id(odp_queue_t handle)
 {
        return handle - 1;
diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
index 0c30f0f..fbf23ff 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -65,13 +65,13 @@  int odp_pktio_init_global(void)
                snprintf(name, sizeof(name), "%i-pktio_outq_default", (int)id);
                name[ODP_QUEUE_NAME_LEN-1] = '\0';

-               qid = odp_queue_create(name, ODP_QUEUE_TYPE_PKTOUT, NULL);
+               qid = odp_queue_create(name, ODP_QUEUE_TYPE_SCHED, NULL);
                if (qid == ODP_QUEUE_INVALID)
                        return -1;
                pktio_entry->s.outq_default = qid;

                queue_entry = queue_to_qentry(qid);
-               queue_entry->s.pktout = id;
+               queue_set_pktout(queue_entry, id);
        }

        return 0;
@@ -325,16 +325,12 @@  int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue)
        if (pktio_entry == NULL || qentry == NULL)
                return -1;

-       if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
-               return -1;
-
        lock_entry(pktio_entry);
        pktio_entry->s.inq_default = queue;
        unlock_entry(pktio_entry);

        queue_lock(qentry);
-       qentry->s.pktin = id;
-       qentry->s.status = QUEUE_STATUS_SCHED;
+       queue_set_pktin(qentry, id);
        queue_unlock(qentry);

        odp_schedule_queue(queue, qentry->s.param.sched.prio);
diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c
index 1318bcd..1f0e33a 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -53,6 +53,8 @@  static void queue_init(queue_entry_t *queue, const char *name,
 {
        strncpy(queue->s.name<http://s.name>, name, ODP_QUEUE_NAME_LEN - 1);
        queue->s.type = type;
+       queue->s.pktin  = ODP_PKTIO_INVALID;
+       queue->s.pktout = ODP_PKTIO_INVALID;

        if (param) {
                memcpy(&queue->s.param, param, sizeof(odp_queue_param_t));
@@ -64,27 +66,10 @@  static void queue_init(queue_entry_t *queue, const char *name,
                queue->s.param.sched.group = ODP_SCHED_GROUP_DEFAULT;
        }

-       switch (type) {
-       case ODP_QUEUE_TYPE_PKTIN:
-               queue->s.enqueue = pktin_enqueue;
-               queue->s.dequeue = pktin_dequeue;
-               queue->s.enqueue_multi = pktin_enq_multi;
-               queue->s.dequeue_multi = pktin_deq_multi;
-               break;
-       case ODP_QUEUE_TYPE_PKTOUT:
-               queue->s.enqueue = pktout_enqueue;
-               queue->s.dequeue = pktout_dequeue;
-               queue->s.enqueue_multi = pktout_enq_multi;
-               queue->s.dequeue_multi = pktout_deq_multi;
-               break;
-       default:
-               queue->s.enqueue = queue_enq;
-               queue->s.dequeue = queue_deq;
-               queue->s.enqueue_multi = queue_enq_multi;
-               queue->s.dequeue_multi = queue_deq_multi;
-               break;
-       }
-
+       queue->s.enqueue = queue_enq;
+       queue->s.dequeue = queue_deq;
+       queue->s.enqueue_multi = queue_enq_multi;
+       queue->s.dequeue_multi = queue_deq_multi;
        queue->s.head = NULL;
        queue->s.tail = NULL;
        queue->s.sched_buf = ODP_BUFFER_INVALID;
@@ -162,8 +147,7 @@  odp_queue_t odp_queue_create(const char *name, odp_queue_type_t type,
                if (queue->s.status == QUEUE_STATUS_FREE) {
                        queue_init(queue, name, type, param);

-                       if (type == ODP_QUEUE_TYPE_SCHED ||
-                           type == ODP_QUEUE_TYPE_PKTIN)
+                       if (type == ODP_QUEUE_TYPE_SCHED)
                                queue->s.status = QUEUE_STATUS_NOTSCHED;
                        else
                                queue->s.status = QUEUE_STATUS_READY;
@@ -175,8 +159,7 @@  odp_queue_t odp_queue_create(const char *name, odp_queue_type_t type,
                UNLOCK(&queue->s.lock);
        }

-       if (handle != ODP_QUEUE_INVALID &&
-           (type == ODP_QUEUE_TYPE_SCHED || type == ODP_QUEUE_TYPE_PKTIN)) {
+       if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
                odp_buffer_t buf;

                buf = odp_schedule_buffer_alloc(handle);
@@ -353,8 +337,7 @@  odp_buffer_hdr_t *queue_deq(queue_entry_t *queue)

        if (queue->s.head == NULL) {
                /* Already empty queue */
-               if (queue->s.status == QUEUE_STATUS_SCHED &&
-                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
+               if (queue->s.status == QUEUE_STATUS_SCHED)
                        queue->s.status = QUEUE_STATUS_NOTSCHED;
        } else {
                buf_hdr       = queue->s.head;
@@ -381,8 +364,7 @@  int queue_deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[], int num)

        if (queue->s.head == NULL) {
                /* Already empty queue */
-               if (queue->s.status == QUEUE_STATUS_SCHED &&
-                   queue->s.type != ODP_QUEUE_TYPE_PKTIN)
+               if (queue->s.status == QUEUE_STATUS_SCHED)
                        queue->s.status = QUEUE_STATUS_NOTSCHED;
        } else {
                odp_buffer_hdr_t *hdr = queue->s.head;
@@ -453,3 +435,24 @@  void queue_unlock(queue_entry_t *queue)
 {
        UNLOCK(&queue->s.lock);
 }
+
+void queue_set_pktin(queue_entry_t *queue, odp_pktio_t pktio_id)
+{
+       queue->s.pktin = pktio_id;
+       queue->s.enqueue = pktin_enqueue;
+       queue->s.dequeue = pktin_dequeue;
+       queue->s.enqueue_multi = pktin_enq_multi;
+       queue->s.dequeue_multi = pktin_deq_multi;
+       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
+               queue->s.status = QUEUE_STATUS_SCHED;
+}
+
+void queue_set_pktout(queue_entry_t *queue, odp_pktio_t pktio_id)
+{
+       queue->s.pktout = pktio_id;
+       queue->s.enqueue = pktout_enqueue;
+       queue->s.dequeue = pktout_dequeue;
+       queue->s.enqueue_multi = pktout_enq_multi;
+       queue->s.dequeue_multi = pktout_deq_multi;
+}
+
diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c
index 1bf819b..5742196 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -301,8 +301,8 @@  static int schedule(odp_queue_t *out_queue, odp_buffer_t out_buf[],
                                        /* Remove empty queue from scheduling,
                                         * except packet input queues
                                         */
-                                       if (odp_queue_type(queue) ==
-                                           ODP_QUEUE_TYPE_PKTIN)
+                                       queue_entry_t *qentry = queue_to_qentry(queue);
+                                       if (qentry->s.pktin != ODP_PKTIO_INVALID)
                                                odp_queue_enq(pri_q, desc_buf);

                                        continue;