[dpdk-dev] [PATCH 0/2] ixgbe vector rx/tx changes

[dpdk-dev] [PATCH 0/2] ixgbe vector rx/tx changes

[Eric Kinzie]

Clear values specific to ixgbe vector RX during queue reset.

I've also include a patch that adds a memory barrier before writing the
rx/tx tail pointer registers in ixgbe_rxtx_vec.c.  The non-vector code
has such barriers which looks right to me.  Comments?

Eric Kinzie (2):
  ixgbe: vector rx rearm after queue reset
  ixgbe: add memory barriers in vector rx/tx

 drivers/net/ixgbe/ixgbe_rxtx.c     |    4 ++++
 drivers/net/ixgbe/ixgbe_rxtx_vec.c |    3 +++
 2 files changed, 7 insertions(+)

-- 
1.7.10.4

[dpdk-dev] [PATCH 1/2] ixgbe: vector rx rearm after queue reset

[Eric Kinzie]

zero values in ixgbe_reset_rx_queue() used by vector receive so that
rearming the rx queue happens at the right time.  Not doing so can in
some cases result in the software inadvertently setting the card's rx
tail pointer equal to the head pointer, which indicates that there are
no descriptors available.  This causes receive to stop indefinitely
on that queue.

Fixes: 01fa1d6215fa ("ixgbe: unify Rx setup")

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 3ace8a8..1e840b6 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2261,6 +2261,10 @@ ixgbe_reset_rx_queue(struct ixgbe_adapter *adapter, struct ixgbe_rx_queue *rxq)
 	rxq->nb_rx_hold = 0;
 	rxq->pkt_first_seg = NULL;
 	rxq->pkt_last_seg = NULL;
+#ifdef RTE_IXGBE_INC_VECTOR
+	rxq->rxrearm_nb = 0;
+	rxq->rxrearm_start = 0;
+#endif
 }
 
 int
-- 
1.7.10.4

[dpdk-dev] [PATCH 2/2] ixgbe: add memory barriers in vector rx/tx

[Eric Kinzie]

Add write memory barrier before writing tail pointer.

Fixes c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 drivers/net/ixgbe/ixgbe_rxtx_vec.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
index abd10f6..b601de8 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
@@ -123,6 +123,7 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
 			     (rxq->nb_rx_desc - 1) : (rxq->rxrearm_start - 1));
 
 	/* Update the tail pointer on the NIC */
+	rte_wmb();
 	IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rx_id);
 }
 
@@ -645,6 +646,8 @@ ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 	txq->tx_tail = tx_id;
 
+	/* update tail pointer */
+	rte_wmb();
 	IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, txq->tx_tail);
 
 	return nb_pkts;
-- 
1.7.10.4

Re: [dpdk-dev] [PATCH 2/2] ixgbe: add memory barriers in vector rx/tx

[Ananyev, Konstantin]

Hi Eric,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Eric Kinzie
> Sent: Thursday, June 25, 2015 7:26 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/2] ixgbe: add memory barriers in vector rx/tx
> 
> Add write memory barrier before writing tail pointer.
> 
> Fixes c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
> 
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> index abd10f6..b601de8 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> @@ -123,6 +123,7 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
>  			     (rxq->nb_rx_desc - 1) : (rxq->rxrearm_start - 1));
> 
>  	/* Update the tail pointer on the NIC */
> +	rte_wmb();
>  	IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rx_id);
>  }
> 
> @@ -645,6 +646,8 @@ ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
> 
>  	txq->tx_tail = tx_id;
> 
> +	/* update tail pointer */
> +	rte_wmb();
>  	IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, txq->tx_tail);
> 
>  	return nb_pkts;


There were several discussions about that subject already:
why fence is not necessary here for IA and why we don't want to put it here:
That I suppose was the last one:
http://dpdk.org/ml/archives/dev/2015-April/016463.html
As I can see, Dong already submitted patches for  that:
http://dpdk.org/dev/patchwork/patch/5884/ 
Though I didn't look at it closely yet.
Konstantin


> --
> 1.7.10.4

Re: [dpdk-dev] [PATCH 1/2] ixgbe: vector rx rearm after queue reset

[Thomas Monjalon]

2015-06-25 11:25, Eric Kinzie:
> zero values in ixgbe_reset_rx_queue() used by vector receive so that
> rearming the rx queue happens at the right time.  Not doing so can in
> some cases result in the software inadvertently setting the card's rx
> tail pointer equal to the head pointer, which indicates that there are
> no descriptors available.  This causes receive to stop indefinitely
> on that queue.
> 
> Fixes: 01fa1d6215fa ("ixgbe: unify Rx setup")
> 
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
[...]
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2261,6 +2261,10 @@ ixgbe_reset_rx_queue(struct ixgbe_adapter *adapter, struct ixgbe_rx_queue *rxq)
>  	rxq->nb_rx_hold = 0;
>  	rxq->pkt_first_seg = NULL;
>  	rxq->pkt_last_seg = NULL;
> +#ifdef RTE_IXGBE_INC_VECTOR
> +	rxq->rxrearm_nb = 0;
> +	rxq->rxrearm_start = 0;
> +#endif

A similar patch has been applied:
	http://dpdk.org/browse/dpdk/commit/?id=48e967695ba78
Thanks

Re: [dpdk-dev] [PATCH 2/2] ixgbe: add memory barriers in vector rx/tx

[Thomas Monjalon]

2015-06-29 11:28, Ananyev, Konstantin:
> Hi Eric,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Eric Kinzie
> > Sent: Thursday, June 25, 2015 7:26 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 2/2] ixgbe: add memory barriers in vector rx/tx
> > 
> > Add write memory barrier before writing tail pointer.
> > 
> > Fixes c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
> > 
> > Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_rxtx_vec.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> > index abd10f6..b601de8 100644
> > --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> > +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> > @@ -123,6 +123,7 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
> >  			     (rxq->nb_rx_desc - 1) : (rxq->rxrearm_start - 1));
> > 
> >  	/* Update the tail pointer on the NIC */
> > +	rte_wmb();
> >  	IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rx_id);
> >  }
> > 
> > @@ -645,6 +646,8 @@ ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
> > 
> >  	txq->tx_tail = tx_id;
> > 
> > +	/* update tail pointer */
> > +	rte_wmb();
> >  	IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, txq->tx_tail);
> > 
> >  	return nb_pkts;
> 
> 
> There were several discussions about that subject already:
> why fence is not necessary here for IA and why we don't want to put it here:
> That I suppose was the last one:
> http://dpdk.org/ml/archives/dev/2015-April/016463.html
> As I can see, Dong already submitted patches for  that:
> http://dpdk.org/dev/patchwork/patch/5884/ 
> Though I didn't look at it closely yet.

It will be a good idea to re-open the topic of the cross-arch memory barriers
at the beginning of the 2.2 cycle.
Thanks

Re: [dpdk-dev] [PATCH 2/2] ixgbe: add memory barriers in vector rx/tx

[Thomas Monjalon]

2015-08-03 17:08, Thomas Monjalon:
> 2015-06-29 11:28, Ananyev, Konstantin:
> > There were several discussions about that subject already:
> > why fence is not necessary here for IA and why we don't want to put it here:
> > That I suppose was the last one:
> > http://dpdk.org/ml/archives/dev/2015-April/016463.html
> > As I can see, Dong already submitted patches for  that:
> > http://dpdk.org/dev/patchwork/patch/5884/ 
> > Though I didn't look at it closely yet.
> 
> It will be a good idea to re-open the topic of the cross-arch memory barriers
> at the beginning of the 2.2 cycle.
> Thanks

Any update on this topic?
The patch is still "New" in patchwork.