> -----Original Message-----
> From: McDaniel, Timothy <timothy.mcdan...@intel.com>
> Sent: Friday, September 11, 2020 3:26 PM
> Cc: dev@dpdk.org; Carrillo, Erik G <erik.g.carri...@intel.com>; Eads, Gage
> <gage.e...@intel.com>; Van Haaren, Harry <harry.van.haa...@intel.com>;
> jer...@marvell.com
> Subject: [PATCH 16/22] event/dlb2: add dequeue and its burst variants
>
> Add support for dequeue, dequeue_burst, ...
Please elaborate -- this commit message doesn't mention (e.g.) the use of
umonitor/umwait or the "sparse" dequeue function variants. I think the
average reviewer needs some more context in order to understand this change.
[...]
> +
> +static inline int
> +dlb2_process_dequeue_qes(struct dlb2_eventdev_port *ev_port,
> + struct dlb2_port *qm_port,
> + struct rte_event *events,
> + struct dlb2_dequeue_qe *qes,
> + int cnt)
> +{
> + uint8_t *qid_mappings = qm_port->qid_mappings;
> + int i, num, evq_id;
> +
> + RTE_SET_USED(ev_port); /* avoids unused variable error if stats off */
Looks like ev_port is used unconditionally later on: "evq_id =
ev_port->link[0].queue_id;"
> +
> + for (i = 0, num = 0; i < cnt; i++) {
> + struct dlb2_dequeue_qe *qe = &qes[i];
> + int sched_type_map[DLB2_NUM_HW_SCHED_TYPES] = {
> + [DLB2_SCHED_ATOMIC] = RTE_SCHED_TYPE_ATOMIC,
> + [DLB2_SCHED_UNORDERED] =
> RTE_SCHED_TYPE_PARALLEL,
> + [DLB2_SCHED_ORDERED] = RTE_SCHED_TYPE_ORDERED,
> + [DLB2_SCHED_DIRECTED] = RTE_SCHED_TYPE_ATOMIC,
> + };
> +
> + /* Fill in event information.
> + * Note that flow_id must be embedded in the data by
> + * the app, such as the mbuf RSS hash field if the data
> + * buffer is a mbuf.
> + */
> + if (unlikely(qe->error)) {
> + DLB2_LOG_ERR("QE error bit ON\n");
> + DLB2_INC_STAT(ev_port->stats.traffic.rx_drop, 1);
> + dlb2_consume_qe_immediate(qm_port, 1);
> + continue; /* Ignore */
> + }
> +
> + events[num].u64 = qe->data;
> + events[num].flow_id = qe->flow_id;
> + events[num].priority = DLB2_TO_EV_PRIO((uint8_t)qe->priority);
> + events[num].event_type = qe->u.event_type.major;
> + events[num].sub_event_type = qe->u.event_type.sub;
> + events[num].sched_type = sched_type_map[qe->sched_type];
> + events[num].impl_opaque = qe->qid_depth;
> +
> + /* qid not preserved for directed queues */
> + if (qm_port->is_directed)
> + evq_id = ev_port->link[0].queue_id;
> + else
> + evq_id = qid_mappings[qe->qid];
> +
> + events[num].queue_id = evq_id;
> + DLB2_INC_STAT(
> + ev_port->stats.queue[evq_id].qid_depth[qe-
> >qid_depth],
> + 1);
> + DLB2_INC_STAT(ev_port->stats.rx_sched_cnt[qe->sched_type],
> 1);
> + DLB2_INC_STAT(ev_port->stats.traffic.rx_ok, 1);
Move this outside the loop and increment by 'num' rather than '1'?
> + num++;
> + }
> +
> + return num;
> +}
> +
> +static inline int
> +dlb2_process_dequeue_four_qes(struct dlb2_eventdev_port *ev_port,
> + struct dlb2_port *qm_port,
> + struct rte_event *events,
> + struct dlb2_dequeue_qe *qes)
> +{
> + int sched_type_map[] = {
> + [DLB2_SCHED_ATOMIC] = RTE_SCHED_TYPE_ATOMIC,
> + [DLB2_SCHED_UNORDERED] = RTE_SCHED_TYPE_PARALLEL,
> + [DLB2_SCHED_ORDERED] = RTE_SCHED_TYPE_ORDERED,
> + [DLB2_SCHED_DIRECTED] = RTE_SCHED_TYPE_ATOMIC,
> + };
> + const int num_events = DLB2_NUM_QES_PER_CACHE_LINE;
> + uint8_t *qid_mappings = qm_port->qid_mappings;
> + __m128i sse_evt[2];
> +
> + RTE_SET_USED(ev_port); /* avoids unused variable error, if stats off */
ev_port gets passed to dlb2_process_dequeue_qes, I don't think this line is
necessary.
> +
> + /* In the unlikely case that any of the QE error bits are set, process
> + * them one at a time.
> + */
> + if (unlikely(qes[0].error || qes[1].error ||
> + qes[2].error || qes[3].error))
> + return dlb2_process_dequeue_qes(ev_port, qm_port, events,
> + qes, num_events);
Thanks,
Gage
