Finally I have some time to have a look to it. On Mon, May 16, 2016 at 3:16 PM, Bruce Richardson <bruce.richardson at intel.com> wrote: > On Mon, May 16, 2016 at 03:12:10PM +0200, Mauricio V?squez wrote: >> Hello Bruce, >> >> Although having this support does not harm anyone, I am not convinced that >> it is useful, mainly because there exists the single-thread limitation in >> other PMDs. Then, if an application has to use different kind of NICs (i.e, >> different PMDs) it has to implement the locking strategies. On the other >> hand, if an application only uses rte_rings, it could just use the >> rte_ring library. >> >> Thanks, Mauricio V >> > I agree. > If you want, please submit a patch to remove this behaviour and see > if anyone objects to it. If there are no objections, I have no problem > accepting > the patch. > > However, since this is a behaviour change to existing functionality, we may > need to implement function versionning for this for ABI compatibility. Please > take that into account when drafting any patch. >
Do you think that versioning is required in this case? If anyone is using a functionality that is not supposed to work in that way, should we care about it? I am not against versioning, I just want to know if it is worthy to do. > Regards, > /Bruce > >> On Tue, May 10, 2016 at 11:36 AM, Bruce Richardson < >> bruce.richardson at intel.com> wrote: >> >> > On Tue, May 10, 2016 at 11:13:08AM +0200, Mauricio V?squez wrote: >> > > Hello, >> > > >> > > Per-queue stats counters are defined as rte_atomic64_t, in the tx/rx >> > > functions, they are atomically increased if the rings have the multiple >> > > consumers/producer flag enabled. >> > > >> > > According to the design principles, the application should not invoke >> > those >> > > functions on the same queue on different cores, then I think that atomic >> > > increasing is not necessary. >> > > >> > > Is there something wrong with my reasoning?, If not, I am willing to >> > send a >> > > patch. >> > > >> > > Thank you very much, >> > > >> > Since the rte_rings, on which the ring pmd is obviously based, have >> > multi-producer >> > and multi-consumer support built-in, I thought it might be useful in the >> > ring >> > PMD itself to allow multiple threads to access the ring queues at the same >> > time, >> > if the underlying rings are marked as MP/MC safe. When doing enqueues and >> > dequeue >> > from the ring, the stats are either incremented atomically, or >> > non-atomically, >> > depending on the underlying queue type. >> > >> > const uint16_t nb_rx = (uint16_t)rte_ring_dequeue_burst(r->rng, >> > ptrs, nb_bufs); >> > if (r->rng->flags & RING_F_SC_DEQ) >> > r->rx_pkts.cnt += nb_rx; >> > else >> > rte_atomic64_add(&(r->rx_pkts), nb_rx); >> > >> > If people don't think this behaviour is worthwhile keeping, I'm ok with >> > removing >> > it, since all other PMDs have the restriction that the queues are >> > single-thread >> > only. >> > >> > Regards, >> > /Bruce >> > Regards, Mauricio V
