[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(-)
  

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.