Reviewed-and-tested-by: Bill Fischofer <[email protected]> On Tue, Jun 20, 2017 at 2:10 PM Maxim Uvarov <[email protected]> wrote:
> Does anybody wants to add his review to this patch? According to call a > lot of people needed this patch but no review provided. > > Maxim. > > On 06/19/17 15:54, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > >> I think queue_t should be removed completely. We should just use > >> odp_queue_t for internal interfaces as well. Looking at the code, > >> using odp_queue_t would introduce 3 extra addition instructions and > >> the impact to cache does not change. We can run l2fwd benchmark and > >> compare the numbers. This would keep the code simple as well. > > > > First, this comment highlights why one direct function call provides > better modularity than unnecessary exposition of an internal mid type. With > handle_to_qentry() we can change, or even remove, the mid type without need > to touch any of the call sites. > > > > queue_entry_t *queue; > > odp_buffer_hdr_t *buf_hdr; > > > > - queue = qentry_from_int(queue_from_ext(handle)); > > + queue = handle_to_qentry(handle); > > > > > > Secondly, the internal queue pointer (or table index - format does not > matter) is needed to avoid doing the same conversion multiple times. We are > inside implementation, and it improves performance when the number of > odp_queue_t to pointer (qentry) conversions per packet is minimized. > > > > After this basic clean up is merged, I'll look into the queue interface > definition and optimize it. This current version is just a dummy, > copy-paste replacement of qentry->xxx with fn_queue->xxx(). Which is OK for > the first stage, but certainly not optimal/final solution. > > > > -Petri > > > > > >
