[dpdk-dev] [PATCH 1/3] examples/eventdev_pipeline: added sample app
Eads, Gage
gage.eads at intel.com
Wed May 10 18:40:52 CEST 2017
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Wednesday, May 10, 2017 9:12 AM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>
> Cc: dev at dpdk.org; Eads, Gage <gage.eads at intel.com>; Richardson, Bruce
> <bruce.richardson at intel.com>
> Subject: Re: [PATCH 1/3] examples/eventdev_pipeline: added sample app
>
> -----Original Message-----
> > Date: Fri, 21 Apr 2017 10:51:37 +0100
> > From: Harry van Haaren <harry.van.haaren at intel.com>
> > To: dev at dpdk.org
> > CC: jerin.jacob at caviumnetworks.com, Harry van Haaren
> > <harry.van.haaren at intel.com>, Gage Eads <gage.eads at intel.com>, Bruce
> > Richardson <bruce.richardson at intel.com>
> > Subject: [PATCH 1/3] examples/eventdev_pipeline: added sample app
> > X-Mailer: git-send-email 2.7.4
> >
> > This commit adds a sample app for the eventdev library.
> > The app has been tested with DPDK 17.05-rc2, hence this release (or
> > later) is recommended.
> >
<snip>
>
> > + ev[i].op = RTE_EVENT_OP_NEW;
> > + ev[i].sched_type = queue_type;
>
> The value of RTE_EVENT_QUEUE_CFG_ORDERED_ONLY !=
> RTE_SCHED_TYPE_ORDERED. So, we cannot assign .sched_type as
> queue_type.
>
> I think, one option could be to avoid translation in application is to
> - Remove RTE_EVENT_QUEUE_CFG_ALL_TYPES,
> RTE_EVENT_QUEUE_CFG_*_ONLY
> - Introduce a new RTE_EVENT_DEV_CAP_ to denote
> RTE_EVENT_QUEUE_CFG_ALL_TYPES cap ability
> - add sched_type in struct rte_event_queue_conf. If capability flag is
> not set then implementation takes sched_type value for the queue.
>
> Any thoughts?
I'm not sure this change is needed. We could create a sched_type[] array, indexed by queue ID, for assigning the event's sched type.
With the proposed approach, the sched_type assignment would still be needed for "all types"-capable PMDs, so I'm not sure it buys us anything in the application.
>
>
> > + ev[i].queue_id = qid;
> > + ev[i].event_type = RTE_EVENT_TYPE_CPU;
>
> IMO, RTE_EVENT_TYPE_ETHERNET is the better option here as it is producing
> the Ethernet packets/events.
>
> > + ev[i].sub_event_type = 0;
> > + ev[i].priority = RTE_EVENT_DEV_PRIORITY_NORMAL;
> > + ev[i].mbuf = mbufs[i];
> > + RTE_SET_USED(prio_idx);
> > + }
> > +
> > + const int nb_tx = rte_event_enqueue_burst(dev_id, port_id, ev,
> > +nb_rx);
>
> For producer pattern i.e a burst of RTE_EVENT_OP_NEW, OcteonTX can do
> burst operation unlike FORWARD case(which is one event at a time).Earlier, I
> thought I can abstract the producer pattern in PMD, but it looks like we are
> going with application driven producer model based on latest RFC.So I think,
> we can add one flag to rte_event_enqueue_burst to denote all the events are
> of type RTE_EVENT_OP_NEW as hint.SW driver can ignore this.
>
> I can send a patch for the same.
>
> Any thoughts?
>
Makes sense, though I'm a little hesitant about putting this sort of PMD-specific hint in the enqueue API. Perhaps we can use the impl_opaque field, or have the PMD inspect the event_type (if TYPE_ETHDEV, assume all packets in the burst are NEWs)?
>
> > + if (nb_tx != nb_rx) {
> > + for (i = nb_tx; i < nb_rx; i++)
> > + rte_pktmbuf_free(mbufs[i]);
> > + }
> > + enqueue_cnt[0] += nb_tx;
> > +
> > + if (unlikely(prod_stop))
>
> I think, No one updating the prod_stop
>
> > + done = 1;
> > +
> > + return 0;
> > +}
> > +
> > +static inline void
> > +schedule_devices(uint8_t dev_id, unsigned lcore_id) {
> > + if (rx_core[lcore_id] && (rx_single ||
> > + rte_atomic32_cmpset(&rx_lock, 0, 1))) {
>
> This pattern(rte_atomic32_cmpset) makes application can inject only "one
> core" worth of packets. Not enough for low-end cores. May be we need
> multiple producer options. I think, new RFC is addressing it.
>
Right, in the "wimpy" core case one can partition the Rx queues across multiple adapters, and assign the adapters to different service cores. The proposal doesn't explicitly state this, but rte_eth_rx_event_adapter_run() is not intended to be MT-safe -- so the service core implementation would need something along the lines of the cmpset if a service is affinitized to multiple service cores.
Thanks,
Gage
More information about the dev
mailing list