| Message ID | 20200928190334.40624-1-honnappa.nagarahalli@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/8] net/memif: do not update local copy of tail in tx function | expand |
On Mon, 28 Sep 2020 14:03:27 -0500 Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote: > In the case of S2M queues, the receiver synchronizes with the sender > (i.e. informs of the packets it has received) using ring->tail. > Hence, the sender does not need to update last_tail. > > In the case of M2S queues, the receiver uses last_tail to > keep track of the descriptors it has received. The > sender is not required to update the last_tail. Updating > the last_tail makes it a shared variable between the > transmitter and receiver affecting the performance. > > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD") > Cc: jgrajcia@cisco.com > Cc: stable@dpdk.org > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > Reviewed-by: Phil Yang <phil.yang@arm.com> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> This patch series will conflict with the pending master/slave patchset. Please let the master/slave renaming go in first.
<snip> > > On Mon, 28 Sep 2020 14:03:27 -0500 > Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote: > > > In the case of S2M queues, the receiver synchronizes with the sender > > (i.e. informs of the packets it has received) using ring->tail. > > Hence, the sender does not need to update last_tail. > > > > In the case of M2S queues, the receiver uses last_tail to keep track > > of the descriptors it has received. The sender is not required to > > update the last_tail. Updating the last_tail makes it a shared > > variable between the transmitter and receiver affecting the > > performance. > > > > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD") > > Cc: jgrajcia@cisco.com > > Cc: stable@dpdk.org > > > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > > Reviewed-by: Phil Yang <phil.yang@arm.com> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > > This patch series will conflict with the pending master/slave patchset. > Please let the master/slave renaming go in first. +1, review can go on.
Hi Jakub, Appreciate if you could review this series and provide any comments you might have. Thank you, Honnappa > -----Original Message----- > From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > Sent: Monday, September 28, 2020 2:03 PM > To: dev@dpdk.org; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; > Phil Yang <Phil.Yang@arm.com>; jgrajcia@cisco.com; ferruh.yigit@intel.com > Cc: nd <nd@arm.com>; stable@dpdk.org > Subject: [PATCH v2 1/8] net/memif: do not update local copy of tail in tx > function > > In the case of S2M queues, the receiver synchronizes with the sender (i.e. > informs of the packets it has received) using ring->tail. > Hence, the sender does not need to update last_tail. > > In the case of M2S queues, the receiver uses last_tail to keep track of the > descriptors it has received. The sender is not required to update the last_tail. > Updating the last_tail makes it a shared variable between the transmitter and > receiver affecting the performance. > > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD") > Cc: jgrajcia@cisco.com > Cc: stable@dpdk.org > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > Reviewed-by: Phil Yang <phil.yang@arm.com> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > --- > drivers/net/memif/rte_eth_memif.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/memif/rte_eth_memif.c > b/drivers/net/memif/rte_eth_memif.c > index a19c0f3e6..130099f2e 100644 > --- a/drivers/net/memif/rte_eth_memif.c > +++ b/drivers/net/memif/rte_eth_memif.c > @@ -580,12 +580,10 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, > uint16_t nb_pkts) > ring_size = 1 << mq->log2_ring_size; > mask = ring_size - 1; > > - n_free = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE) - mq- > >last_tail; > - mq->last_tail += n_free; > - > if (type == MEMIF_RING_S2M) { > slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); > - n_free = ring_size - slot + mq->last_tail; > + n_free = ring_size - slot + > + __atomic_load_n(&ring->tail, > __ATOMIC_ACQUIRE); > } else { > slot = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); > n_free = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE) > - slot; > -- > 2.17.1
> > -----Original Message----- > > From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > > Sent: Monday, September 28, 2020 2:03 PM > > To: dev@dpdk.org; Honnappa Nagarahalli > <Honnappa.Nagarahalli@arm.com>; > > Phil Yang <Phil.Yang@arm.com>; jgrajcia@cisco.com; > > ferruh.yigit@intel.com > > Cc: nd <nd@arm.com>; stable@dpdk.org > > Subject: [PATCH v2 1/8] net/memif: do not update local copy of tail in > > tx function > > > > In the case of S2M queues, the receiver synchronizes with the sender (i.e. > > informs of the packets it has received) using ring->tail. > > Hence, the sender does not need to update last_tail. > > > > In the case of M2S queues, the receiver uses last_tail to keep track > > of the descriptors it has received. The sender is not required to update the > last_tail. > > Updating the last_tail makes it a shared variable between the > > transmitter and receiver affecting the performance. Hi Honnappa, The patch series is looking good. Reviewed-by: Jakub Grajciar <jgrajcia@cisco.com>
On 10/9/2020 12:23 PM, Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco) wrote: >>> -----Original Message----- >>> From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> >>> Sent: Monday, September 28, 2020 2:03 PM >>> To: dev@dpdk.org; Honnappa Nagarahalli >> <Honnappa.Nagarahalli@arm.com>; >>> Phil Yang <Phil.Yang@arm.com>; jgrajcia@cisco.com; >>> ferruh.yigit@intel.com >>> Cc: nd <nd@arm.com>; stable@dpdk.org >>> Subject: [PATCH v2 1/8] net/memif: do not update local copy of tail in >>> tx function >>> >>> In the case of S2M queues, the receiver synchronizes with the sender (i.e. >>> informs of the packets it has received) using ring->tail. >>> Hence, the sender does not need to update last_tail. >>> >>> In the case of M2S queues, the receiver uses last_tail to keep track >>> of the descriptors it has received. The sender is not required to update the >> last_tail. >>> Updating the last_tail makes it a shared variable between the >>> transmitter and receiver affecting the performance. > > Hi Honnappa, > > The patch series is looking good. > > Reviewed-by: Jakub Grajciar <jgrajcia@cisco.com> > Series applied to dpdk-next-net/main, thanks.
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index a19c0f3e6..130099f2e 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -580,12 +580,10 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) ring_size = 1 << mq->log2_ring_size; mask = ring_size - 1; - n_free = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE) - mq->last_tail; - mq->last_tail += n_free; - if (type == MEMIF_RING_S2M) { slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); - n_free = ring_size - slot + mq->last_tail; + n_free = ring_size - slot + + __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); } else { slot = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); n_free = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE) - slot;
In the case of S2M queues, the receiver synchronizes with the sender (i.e. informs of the packets it has received) using ring->tail. Hence, the sender does not need to update last_tail. In the case of M2S queues, the receiver uses last_tail to keep track of the descriptors it has received. The sender is not required to update the last_tail. Updating the last_tail makes it a shared variable between the transmitter and receiver affecting the performance. Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD") Cc: jgrajcia@cisco.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) -- 2.17.1