Re: [PATCH] ixgbe:Add write memory barrier for recv pkts.

["Ananyev, Konstantin" <konstantin.ananyev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of WangDong
> Sent: Saturday, April 11, 2015 4:34 PM
> To: dev-VfR2kkLFssw@public.gmane.org
> Subject: [dpdk-dev] [PATCH] ixgbe:Add write memory barrier for recv pkts.
> 
> Like transmit packets, before update receive descriptor's tail pointer, rte_wmb() should be added after writing recv descriptor.
> 
> Signed-off-by: Dong Wang <dong.wang.pro-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 9da2c7e..d504688 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1338,6 +1338,9 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		 */
>  		rx_pkts[nb_rx++] = rxm;
>  	}
> +
> +	rte_wmb();
> +

Why do you think it is necessary?
I can't see any good reason to put wmb() here.
I would understand if, at least you'll try to insert it just before updating RDT:
 rx_id = (uint16_t) ((rx_id == 0) ?
                                     (rxq->nb_rx_desc - 1) : (rx_id - 1));
+ rte_wmb();
IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rx_id);

That is not needed IA with current implementation, but would make sense for machines with relaxed memory ordering.
Though right now DPDK IXGBE PMD is supported only on IA,  anyway.
Same for ixgbe_recv_scattered_pkts().

Konstantin


>  	rxq->rx_tail = rx_id;
> 
>  	/*
> @@ -1595,6 +1598,8 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		first_seg = NULL;
>  	}
> 
> +	rte_wmb();
> +
>  	/*
>  	 * Record index of the next RX descriptor to probe.
>  	 */
> --
> 1.9.1