[PATCH 0/2] ixgbe: vector pmd fixes

[PATCH 0/2] ixgbe: vector pmd fixes

[Bruce Richardson]

This patch set contains small fixes for issues with the vector PMD.
The issues and the fixes for them are described in each patch individually.

Bruce Richardson (2):
  ixgbe: remove static qualifier for thread safety
  ixgbe: always perform vec RX setup if vpmd enabled

 lib/librte_pmd_ixgbe/ixgbe_rxtx.c     | 4 +++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
1.9.3

[PATCH 1/2] ixgbe: remove static qualifier for thread safety

[Bruce Richardson]

Remove the "static" prefix to the template mbuf variable in
ixgbe_rxq_vec_setup function. This will then allow different
threads to initialize different RX queues at the same time,
without one overwriting the other's data.

Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index a0d3d78..e813e43 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -730,7 +730,7 @@ static struct ixgbe_txq_ops vec_txq_ops = {
 int
 ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
 {
-	static struct rte_mbuf mb_def = {
+	struct rte_mbuf mb_def = {
 		.nb_segs = 1,
 		.data_off = RTE_PKTMBUF_HEADROOM,
 #ifdef RTE_MBUF_REFCNT
-- 
1.9.3

Re: [PATCH 1/2] ixgbe: remove static qualifier for thread safety

[Masaru Oki]

Hi,

in this code, pointer of local variable (mb_def) is returned by your changes.
mb_def should be static for each thread.

2014-10-22 19:55 GMT+09:00 Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
> Remove the "static" prefix to the template mbuf variable in
> ixgbe_rxq_vec_setup function. This will then allow different
> threads to initialize different RX queues at the same time,
> without one overwriting the other's data.
>
> Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> index a0d3d78..e813e43 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> @@ -730,7 +730,7 @@ static struct ixgbe_txq_ops vec_txq_ops = {
>  int
>  ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
>  {
> -       static struct rte_mbuf mb_def = {
> +       struct rte_mbuf mb_def = {
>                 .nb_segs = 1,
>                 .data_off = RTE_PKTMBUF_HEADROOM,
>  #ifdef RTE_MBUF_REFCNT
> --
> 1.9.3
>

Re: [PATCH 1/2] ixgbe: remove static qualifier for thread safety

[Bruce Richardson]

On Thu, Oct 23, 2014 at 08:43:39AM +0900, Masaru Oki wrote:
> Hi,
> 
> in this code, pointer of local variable (mb_def) is returned by your changes.
> mb_def should be static for each thread.

Actually, no. A copy is made of 8 bytes of the mb_def variable and stored as 
an mbuf initializer inside the rxq structure. No use of the memory occupied 
by mb_def is made outside of the function, so the value does not need to be 
static.

/Bruce
> 
> 2014-10-22 19:55 GMT+09:00 Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
> > Remove the "static" prefix to the template mbuf variable in
> > ixgbe_rxq_vec_setup function. This will then allow different
> > threads to initialize different RX queues at the same time,
> > without one overwriting the other's data.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> >  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> > index a0d3d78..e813e43 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> > @@ -730,7 +730,7 @@ static struct ixgbe_txq_ops vec_txq_ops = {
> >  int
> >  ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
> >  {
> > -       static struct rte_mbuf mb_def = {
> > +       struct rte_mbuf mb_def = {
> >                 .nb_segs = 1,
> >                 .data_off = RTE_PKTMBUF_HEADROOM,
> >  #ifdef RTE_MBUF_REFCNT
> > --
> > 1.9.3
> >

Re: [PATCH 1/2] ixgbe: remove static qualifier for thread safety

[Masaru Oki]

Oh, sorry, you are right.  I had missed first * for copy.
thank you.

2014-10-24 19:34 GMT+09:00 Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
> On Thu, Oct 23, 2014 at 08:43:39AM +0900, Masaru Oki wrote:
>> Hi,
>>
>> in this code, pointer of local variable (mb_def) is returned by your changes.
>> mb_def should be static for each thread.
>
> Actually, no. A copy is made of 8 bytes of the mb_def variable and stored as
> an mbuf initializer inside the rxq structure. No use of the memory occupied
> by mb_def is made outside of the function, so the value does not need to be
> static.
>
> /Bruce
>>
>> 2014-10-22 19:55 GMT+09:00 Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
>> > Remove the "static" prefix to the template mbuf variable in
>> > ixgbe_rxq_vec_setup function. This will then allow different
>> > threads to initialize different RX queues at the same time,
>> > without one overwriting the other's data.
>> >
>> > Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> > ---
>> >  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
>> > index a0d3d78..e813e43 100644
>> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
>> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
>> > @@ -730,7 +730,7 @@ static struct ixgbe_txq_ops vec_txq_ops = {
>> >  int
>> >  ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
>> >  {
>> > -       static struct rte_mbuf mb_def = {
>> > +       struct rte_mbuf mb_def = {
>> >                 .nb_segs = 1,
>> >                 .data_off = RTE_PKTMBUF_HEADROOM,
>> >  #ifdef RTE_MBUF_REFCNT
>> > --
>> > 1.9.3
>> >

[PATCH 2/2] ixgbe: always perform vec RX setup if vpmd enabled

[Bruce Richardson]

If the vector pmd option is turned on in the compile time config file,
then always call the vector rxq setup, since we can now use the vector
PMD for receiving jumbo frames that need chained mbufs, a.k.a scattered
packets. Up till now, this function was not being called when receiving
scattered packets, potentially leading to problems with mbufs not being
properly initialized.

Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 123b8b3..3a5a8ff 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -2191,6 +2191,9 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	 */
 	use_def_burst_func = check_rx_burst_bulk_alloc_preconditions(rxq);
 
+#ifdef RTE_IXGBE_INC_VECTOR
+	ixgbe_rxq_vec_setup(rxq);
+#endif
 	/* Check if pre-conditions are satisfied, and no Scattered Rx */
 	if (!use_def_burst_func && !dev->data->scattered_rx) {
 #ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
@@ -2203,7 +2206,6 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 		if (!ixgbe_rx_vec_condition_check(dev)) {
 			PMD_INIT_LOG(INFO, "Vector rx enabled, please make "
 				     "sure RX burst size no less than 32.");
-			ixgbe_rxq_vec_setup(rxq);
 			dev->rx_pkt_burst = ixgbe_recv_pkts_vec;
 		}
 #endif
-- 
1.9.3

Re: [PATCH 0/2] ixgbe: vector pmd fixes

[Ouyang, Changchun]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Bruce Richardson
> Sent: Wednesday, October 22, 2014 6:56 PM
> To: dev-VfR2kkLFssw@public.gmane.org
> Subject: [dpdk-dev] [PATCH 0/2] ixgbe: vector pmd fixes
> 
> This patch set contains small fixes for issues with the vector PMD.
> The issues and the fixes for them are described in each patch individually.
> 
> Bruce Richardson (2):
>   ixgbe: remove static qualifier for thread safety
>   ixgbe: always perform vec RX setup if vpmd enabled
> 
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c     | 4 +++-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> --
> 1.9.3

Acked-by: Changchun Ouyang <Changchun.ouyang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Re: [PATCH 0/2] ixgbe: vector pmd fixes

[Thomas Monjalon]

> > This patch set contains small fixes for issues with the vector PMD.
> > The issues and the fixes for them are described in each patch individually.
> > 
> > Bruce Richardson (2):
> >   ixgbe: remove static qualifier for thread safety
> >   ixgbe: always perform vec RX setup if vpmd enabled
> 
> Acked-by: Changchun Ouyang <Changchun.ouyang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Applied

Thanks
-- 
Thomas