​We are in the progress to implement a new queue interface which is more
modulized.

Kevin

2017-05-23 16:22 GMT+08:00 Savolainen, Petri (Nokia - FI/Espoo) <
[email protected]>:

>
> > diff --git a/platform/linux-generic/include/odp_queue_internal.h
> > b/platform/linux-generic/include/odp_queue_internal.h
> > index 560f826e..cbe065dc 100644
> > --- a/platform/linux-generic/include/odp_queue_internal.h
> > +++ b/platform/linux-generic/include/odp_queue_internal.h
> > @@ -19,16 +19,24 @@ extern "C" {
> >  #endif
> >
> >  #include <odp/api/queue.h>
> > -#include <odp_forward_typedefs_internal.h>
> > -#include <odp_schedule_if.h>
> > -#include <odp_buffer_internal.h>
> > -#include <odp_align_internal.h>
> > +#include <odp/api/std_types.h>
> > +#include <odp/api/buffer.h>
> >  #include <odp/api/packet_io.h>
> >  #include <odp/api/align.h>
> >  #include <odp/api/hints.h>
> >  #include <odp/api/ticketlock.h>
> > +
> >  #include <odp_config_internal.h>
> >
> > +#include <odp_align_internal.h>
> > +#include <odp_buffer_internal.h>
> > +#include <odp_forward_typedefs_internal.h>
> > +#include <odp_schedule_if.h>
> > +#ifdef ODP_SCHEDULE_SCALABLE
> > +#include <odp_schedule_scalable.h>
> > +#include <odp_schedule_scalable_ordered.h>
> > +#endif
> > +
> >  #define QUEUE_MULTI_MAX CONFIG_BURST_SIZE
> >
> >  #define QUEUE_STATUS_FREE         0
> > @@ -37,8 +45,6 @@ extern "C" {
> >  #define QUEUE_STATUS_NOTSCHED     3
> >  #define QUEUE_STATUS_SCHED        4
> >
> > -
> > -/* forward declaration */
> >  union queue_entry_u;
> >
> >  typedef int (*enq_func_t)(union queue_entry_u *, odp_buffer_hdr_t *);
> > @@ -49,6 +55,47 @@ typedef int (*enq_multi_func_t)(union queue_entry_u *,
> >  typedef      int (*deq_multi_func_t)(union queue_entry_u *,
> >                               odp_buffer_hdr_t **, int);
> >
> > +#ifdef ODP_SCHEDULE_SCALABLE
>
> The queue interface need to be still improved, so that there's no
> conditional compiling like this piece of code here.
>
>
> > +
> > +struct queue_entry_s {
> > +     sched_elem_t     sched_elem;
> > +
> > +     odp_ticketlock_t lock ODP_ALIGNED_CACHE;
> > +     int              status;
> > +
> > +     enq_func_t       enqueue ODP_ALIGNED_CACHE;
> > +     deq_func_t       dequeue;
> > +     enq_multi_func_t enqueue_multi;
> > +     deq_multi_func_t dequeue_multi;
> > +
> > +     uint32_t           index;
> > +     odp_queue_t        handle;
> > +     odp_queue_type_t   type;
> > +     odp_queue_param_t  param;
> > +     odp_pktin_queue_t  pktin;
> > +     odp_pktout_queue_t pktout;
> > +     char               name[ODP_QUEUE_NAME_LEN];
> > +};
> > +
> > +int _odp_queue_deq(sched_elem_t *q, odp_buffer_hdr_t *buf_hdr[], int
> > num);
> > +int _odp_queue_deq_sc(sched_elem_t *q, odp_event_t *evp, int num);
> > +int _odp_queue_deq_mc(sched_elem_t *q, odp_event_t *evp, int num);
> > +
> > +/* Round up memory size to next cache line size to
> > + * align all memory addresses on cache line boundary.
> > + */
> > +static inline void *shm_pool_alloc_align(_odp_ishm_pool_t *pool,
> uint32_t
> > size)
> > +{
> > +     void *addr;
> > +
> > +     addr = _odp_ishm_pool_alloc(pool, ROUNDUP_CACHE_LINE(size));
> > +     ODP_ASSERT(((uintptr_t)addr & (ODP_CACHE_LINE_SIZE - 1)) == 0);
> > +
> > +     return addr;
> > +}
> > +
> > +#else
>
>
>
>
> > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-
> > generic/odp_packet_io.c
> > index d8cae15c..717e7e33 100644
> > --- a/platform/linux-generic/odp_packet_io.c
> > +++ b/platform/linux-generic/odp_packet_io.c
> > @@ -5,23 +5,25 @@
> >   */
> >  #include <odp_posix_extensions.h>
> >
> > -#include <odp/api/packet_io.h>
> > -#include <odp_packet_io_internal.h>
> > -#include <odp_packet_io_queue.h>
> >  #include <odp/api/packet.h>
> > -#include <odp_packet_internal.h>
> > -#include <odp_internal.h>
> > +#include <odp/api/packet_io.h>
> >  #include <odp/api/spinlock.h>
> >  #include <odp/api/ticketlock.h>
> >  #include <odp/api/shared_memory.h>
> > -#include <odp_packet_socket.h>
> > +#include <odp/api/time.h>
> > +
> > +#include <odp_internal.h>
> >  #include <odp_config_internal.h>
> > -#include <odp_queue_internal.h>
> > -#include <odp_schedule_if.h>
> > -#include <odp_classification_internal.h>
> >  #include <odp_debug_internal.h>
> > +
> > +#include <odp_classification_internal.h>
> > +#include <odp_queue_internal.h>
> >  #include <odp_packet_io_ipc_internal.h>
> > -#include <odp/api/time.h>
> > +#include <odp_packet_io_internal.h>
> > +#include <odp_packet_io_queue.h>
> > +#include <odp_packet_internal.h>
> > +#include <odp_packet_socket.h>
> > +#include <odp_schedule_if.h>
> >
> >  #include <string.h>
> >  #include <inttypes.h>
> > @@ -472,7 +474,6 @@ int odp_pktio_start(odp_pktio_t hdl)
> >                               return -1;
> >                       }
> >               }
> > -
> >               sched_fn->pktio_start(pktio_to_id(hdl), num, index);
> >       }
> >
> > @@ -554,7 +555,6 @@ static inline int pktin_recv_buf(odp_pktin_queue_t
> > queue,
> >       odp_packet_t packets[num];
> >       odp_packet_hdr_t *pkt_hdr;
> >       odp_buffer_hdr_t *buf_hdr;
> > -     odp_buffer_t buf;
> >       int i;
> >       int pkts;
> >       int num_rx = 0;
> > @@ -564,9 +564,7 @@ static inline int pktin_recv_buf(odp_pktin_queue_t
> > queue,
> >       for (i = 0; i < pkts; i++) {
> >               pkt = packets[i];
> >               pkt_hdr = odp_packet_hdr(pkt);
> > -             buf = _odp_packet_to_buffer(pkt);
> > -             buf_hdr = buf_hdl_to_hdr(buf);
> > -
> > +             buf_hdr =
> > buf_hdl_to_hdr(_odp_packet_to_buffer(pkt));
>
>
> Do not include unnecessary style changes like this one (and the one above)
> into this patch. These are just cosmetic and obscure the   functional
> change you are trying to achieve. These waste reviewer time to figure out
> why this change was needed - just to realize that it was not actually
> needed.
>
> Also e.g. in these cases, I like the original style better.
>
>
>
> >               if (pkt_hdr->p.input_flags.dst_queue) {
> >                       queue_entry_t *dst_queue;
> >                       int ret;
> > @@ -584,11 +582,12 @@ static inline int pktin_recv_buf(odp_pktin_queue_t
> > queue,
> >
> >  int pktout_enqueue(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr)
> >  {
> > -     odp_packet_t pkt = _odp_packet_from_buffer(buf_hdr-
> > >handle.handle);
> > +     odp_packet_t pkt;
> >       int len = 1;
> >       int nbr;
> >
> > -     nbr = odp_pktout_send(qentry->s.pktout, &pkt, len);
> > +     pkt = _odp_packet_from_buffer(buf_hdr->handle.handle);
>
> Again, remove all these style-only-changes from the patch.
>
> > +     nbr = odp_pktout_send(queue_get_pktout(qentry), &pkt, len);
>
>
> This is the first functional code line change in this file. When you
> remove extra style changes, it's easier to see that this file is updated
> with a new queue interface.
>
> -Petri
>
>
>


-- 
Thanks,
Kevin

Reply via email to