The API looks solid and I'm OK with the question of flush_out_queues() being opened as a bug against RC1 since the purpose of RC1 is to have a final API. Petri, if you believe that the scope of that would be better handled as a follow-on bugfix rather than a v2 to this patch that's find.
Other than this one point, for this series: Reviewed-and-tested-by: Bill Fischofer <[email protected]> On Thu, Feb 25, 2016 at 4:19 PM, Bill Fischofer <[email protected]> wrote: > > > On Thu, Feb 25, 2016 at 5:31 AM, Petri Savolainen < > [email protected]> wrote: > >> Implemented pktout event queues. >> >> Signed-off-by: Petri Savolainen <[email protected]> >> --- >> .../linux-generic/include/odp_packet_io_internal.h | 2 + >> .../linux-generic/include/odp_queue_internal.h | 2 +- >> platform/linux-generic/odp_packet_io.c | 123 >> +++++++++++++++++++-- >> 3 files changed, 117 insertions(+), 10 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >> b/platform/linux-generic/include/odp_packet_io_internal.h >> index 0a4a55b..9297fc7 100644 >> --- a/platform/linux-generic/include/odp_packet_io_internal.h >> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >> @@ -153,6 +153,7 @@ struct pktio_entry { >> } in_queue[PKTIO_MAX_QUEUES]; >> >> struct { >> + odp_queue_t queue; >> odp_pktout_queue_t pktout; >> } out_queue[PKTIO_MAX_QUEUES]; >> }; >> @@ -254,6 +255,7 @@ int single_output_queues_config(pktio_entry_t *entry, >> int single_in_queues(pktio_entry_t *entry, odp_queue_t queues[], int >> num); >> int single_pktin_queues(pktio_entry_t *entry, odp_pktin_queue_t queues[], >> int num); >> +int single_out_queues(pktio_entry_t *entry, odp_queue_t queues[], int >> num); >> int single_pktout_queues(pktio_entry_t *entry, odp_pktout_queue_t >> queues[], >> int num); >> int single_recv_queue(pktio_entry_t *entry, int index, odp_packet_t >> packets[], >> diff --git a/platform/linux-generic/include/odp_queue_internal.h >> b/platform/linux-generic/include/odp_queue_internal.h >> index 912427f..2e352ae 100644 >> --- a/platform/linux-generic/include/odp_queue_internal.h >> +++ b/platform/linux-generic/include/odp_queue_internal.h >> @@ -77,7 +77,7 @@ struct queue_entry_s { >> odp_queue_type_t type; >> odp_queue_param_t param; >> odp_pktin_queue_t pktin; >> - odp_pktio_t pktout; >> + odp_pktout_queue_t pktout; >> char name[ODP_QUEUE_NAME_LEN]; >> uint64_t order_in; >> uint64_t order_out; >> diff --git a/platform/linux-generic/odp_packet_io.c >> b/platform/linux-generic/odp_packet_io.c >> index e3827ae..83a03d8 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -116,18 +116,33 @@ static void unlock_entry(pktio_entry_t *entry) >> odp_ticketlock_unlock(&entry->s.rxl); >> } >> >> -static void init_pktio_entry(pktio_entry_t *entry) >> +static void init_in_queues(pktio_entry_t *entry) >> { >> int i; >> >> - set_taken(entry); >> - pktio_cls_enabled_set(entry, 0); >> + for (i = 0; i < PKTIO_MAX_QUEUES; i++) { >> + entry->s.in_queue[i].queue = ODP_QUEUE_INVALID; >> + entry->s.in_queue[i].pktin = PKTIN_INVALID; >> + } >> +} >> + >> +static void init_out_queues(pktio_entry_t *entry) >> +{ >> + int i; >> >> for (i = 0; i < PKTIO_MAX_QUEUES; i++) { >> - entry->s.in_queue[i].queue = ODP_QUEUE_INVALID; >> - entry->s.in_queue[i].pktin = PKTIN_INVALID; >> + entry->s.out_queue[i].queue = ODP_QUEUE_INVALID; >> entry->s.out_queue[i].pktout = PKTOUT_INVALID; >> } >> +} >> + >> +static void init_pktio_entry(pktio_entry_t *entry) >> +{ >> + set_taken(entry); >> + pktio_cls_enabled_set(entry, 0); >> + >> + init_in_queues(entry); >> + init_out_queues(entry); >> >> pktio_classifier_init(entry); >> } >> @@ -280,6 +295,18 @@ static void destroy_in_queues(pktio_entry_t *entry, >> int num) >> } >> } >> >> +static void destroy_out_queues(pktio_entry_t *entry, int num) >> +{ >> + int i; >> + >> + for (i = 0; i < num; i++) { >> + if (entry->s.out_queue[i].queue != ODP_QUEUE_INVALID) { >> + odp_queue_destroy(entry->s.out_queue[i].queue); >> > > What happens if the out_queue is not empty? odp_queue_destroy() will fail > then and the result will be a resource leakage. The corresponding > destroy_in_queues() is guarded by the flush_in_queues() routine to ensure > that they are empty. It seems we need a corresponding flush_out_queues() > routine to ensure that the output queues are similarly empty. > > > >> + entry->s.out_queue[i].queue = ODP_QUEUE_INVALID; >> + } >> + } >> +} >> + >> static void flush_in_queues(pktio_entry_t *entry) >> { >> odp_pktin_mode_t mode; >> @@ -322,6 +349,7 @@ int odp_pktio_close(odp_pktio_t id) >> lock_entry(entry); >> >> destroy_in_queues(entry, entry->s.num_in_queue); >> + destroy_out_queues(entry, entry->s.num_out_queue); >> >> entry->s.num_in_queue = 0; >> entry->s.num_out_queue = 0; >> @@ -493,7 +521,7 @@ int pktout_enqueue(queue_entry_t *qentry, >> odp_buffer_hdr_t *buf_hdr) >> int len = 1; >> int nbr; >> >> - nbr = _odp_pktio_send(qentry->s.pktout, &pkt, len); >> + nbr = odp_pktout_send(qentry->s.pktout, &pkt, len); >> return (nbr == len ? 0 : -1); >> } >> >> @@ -513,7 +541,7 @@ int pktout_enq_multi(queue_entry_t *qentry, >> odp_buffer_hdr_t *buf_hdr[], >> for (i = 0; i < num; ++i) >> pkt_tbl[i] = >> _odp_packet_from_buffer(buf_hdr[i]->handle.handle); >> >> - nbr = _odp_pktio_send(qentry->s.pktout, pkt_tbl, num); >> + nbr = odp_pktout_send(qentry->s.pktout, pkt_tbl, num); >> return nbr; >> } >> >> @@ -1133,7 +1161,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio, >> if (mode == ODP_PKTOUT_MODE_DISABLED || mode == >> ODP_PKTOUT_MODE_TM) >> return 0; >> >> - if (mode != ODP_PKTOUT_MODE_DIRECT) { >> + if (mode != ODP_PKTOUT_MODE_DIRECT && mode != >> ODP_PKTOUT_MODE_QUEUE) { >> ODP_DBG("pktio %s: bad packet output mode\n", entry-> >> s.name); >> return -1; >> } >> @@ -1152,13 +1180,59 @@ int odp_pktout_queue_config(odp_pktio_t pktio, >> return -1; >> } >> >> + /* If re-configuring, destroy old queues */ >> + if (entry->s.num_out_queue) { >> + destroy_out_queues(entry, entry->s.num_out_queue); >> + entry->s.num_out_queue = 0; >> + } >> + >> + init_out_queues(entry); >> + >> for (i = 0; i < num_queues; i++) { >> entry->s.out_queue[i].pktout.index = i; >> - entry->s.out_queue[i].pktout.pktio = entry->s.handle; >> + entry->s.out_queue[i].pktout.pktio = pktio; >> } >> >> entry->s.num_out_queue = num_queues; >> >> + if (mode == ODP_PKTOUT_MODE_QUEUE) { >> + for (i = 0; i < num_queues; i++) { >> + odp_queue_t queue; >> + odp_queue_param_t queue_param; >> + queue_entry_t *qentry; >> + char name[ODP_QUEUE_NAME_LEN]; >> + int pktio_id = pktio_to_id(pktio); >> + >> + snprintf(name, sizeof(name), "odp-pktout-%i-%i", >> + pktio_id, i); >> + >> + odp_queue_param_init(&queue_param); >> + /* Application cannot dequeue from the queue */ >> + queue_param.deq_mode = ODP_QUEUE_OP_DISABLED; >> + >> + queue = odp_queue_create(name, &queue_param); >> + >> + if (queue == ODP_QUEUE_INVALID) { >> + ODP_DBG("pktout %s: event queue create >> failed\n", >> + entry->s.name); >> + destroy_out_queues(entry, i + 1); >> + return -1; >> + } >> + >> + qentry = queue_to_qentry(queue); >> + qentry->s.pktout.index = i; >> + qentry->s.pktout.pktio = pktio; >> + >> + /* Override default enqueue / dequeue functions */ >> + qentry->s.enqueue = queue_pktout_enq; >> + qentry->s.dequeue = pktout_dequeue; >> + qentry->s.enqueue_multi = queue_pktout_enq_multi; >> + qentry->s.dequeue_multi = pktout_deq_multi; >> + >> + entry->s.out_queue[i].queue = queue; >> + } >> + } >> + >> if (entry->s.ops->output_queues_config) >> return entry->s.ops->output_queues_config(entry, param); >> >> @@ -1216,6 +1290,37 @@ int odp_pktin_queue(odp_pktio_t pktio, >> odp_pktin_queue_t queues[], int num) >> return single_pktin_queues(entry, queues, num); >> } >> >> +int odp_pktout_event_queue(odp_pktio_t pktio, odp_queue_t queues[], int >> num) >> +{ >> + pktio_entry_t *entry; >> + odp_pktout_mode_t mode; >> + int i; >> + int num_queues; >> + >> + entry = get_pktio_entry(pktio); >> + if (entry == NULL) { >> + ODP_DBG("pktio entry %d does not exist\n", pktio); >> + return -1; >> + } >> + >> + mode = entry->s.param.out_mode; >> + >> + if (mode == ODP_PKTOUT_MODE_DISABLED) >> + return 0; >> + >> + if (mode != ODP_PKTOUT_MODE_QUEUE) >> + return -1; >> + >> + num_queues = entry->s.num_out_queue; >> + >> + if (queues && num > 0) { >> + for (i = 0; i < num && i < num_queues; i++) >> + queues[i] = entry->s.out_queue[i].queue; >> + } >> + >> + return num_queues; >> +} >> + >> int odp_pktout_queue(odp_pktio_t pktio, odp_pktout_queue_t queues[], int >> num) >> { >> pktio_entry_t *entry; >> -- >> 2.7.1 >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
