[dpdk-dev] [PATCH] net/mlx5: fix init on secondary process

[dpdk-dev] [PATCH] net/mlx5: fix init on secondary process

[Olivier Gournet]

Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")

Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
---
 drivers/net/mlx5/mlx5_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 130e15d..6f39965 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1308,11 +1308,13 @@ mlx5_secondary_data_setup(struct priv *priv)
 			continue;
 		primary_txq_ctrl = container_of(primary_txq,
 						struct txq_ctrl, txq);
-		txq_ctrl = rte_calloc_socket("TXQ", 1, sizeof(*txq_ctrl), 0,
+		txq_ctrl = rte_calloc_socket("TXQ", 1, sizeof(*txq_ctrl) +
+					     primary_txq->elts_n *
+					     sizeof(struct rte_mbuf *), 0,
 					     primary_txq_ctrl->socket);
 		if (txq_ctrl != NULL) {
 			if (txq_ctrl_setup(priv->dev,
-					   primary_txq_ctrl,
+					   txq_ctrl,
 					   primary_txq->elts_n,
 					   primary_txq_ctrl->socket,
 					   NULL) == 0) {
-- 
2.1.4

Re: [dpdk-dev] [PATCH] net/mlx5: fix init on secondary process

[Ferruh Yigit]

On 9/28/2016 3:24 PM, Olivier Gournet wrote:
> Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> 
> Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
> ---

Adding maintainer to the thread

Re: [dpdk-dev] [PATCH] net/mlx5: fix init on secondary process

[Adrien Mazarguil]

Hi Olivier,

Secondary process support's got overlooked during this refactoring, thanks
for the patch. However can you describe the issue you're addressing as part
of the commit log?

I think problems started when txq got mistakenly converted to
primary_txq_ctrl in 21c8bb4928c9 ("net/mlx5: split Tx queue structure"), you
may add a Fixes line for that one as well.

Otherwise, this patch looks fine to me.

On Wed, Sep 28, 2016 at 04:24:18PM +0200, Olivier Gournet wrote:
> Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> 
> Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
> ---
>  drivers/net/mlx5/mlx5_ethdev.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 130e15d..6f39965 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1308,11 +1308,13 @@ mlx5_secondary_data_setup(struct priv *priv)
>  			continue;
>  		primary_txq_ctrl = container_of(primary_txq,
>  						struct txq_ctrl, txq);
> -		txq_ctrl = rte_calloc_socket("TXQ", 1, sizeof(*txq_ctrl), 0,
> +		txq_ctrl = rte_calloc_socket("TXQ", 1, sizeof(*txq_ctrl) +
> +					     primary_txq->elts_n *
> +					     sizeof(struct rte_mbuf *), 0,
>  					     primary_txq_ctrl->socket);
>  		if (txq_ctrl != NULL) {
>  			if (txq_ctrl_setup(priv->dev,
> -					   primary_txq_ctrl,
> +					   txq_ctrl,
>  					   primary_txq->elts_n,
>  					   primary_txq_ctrl->socket,
>  					   NULL) == 0) {
> -- 
> 2.1.4

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH] net/mlx5: fix init on secondary process

[Ferruh Yigit]

Hi Olivier,

On 10/11/2016 10:45 AM, Adrien Mazarguil wrote:
> Hi Olivier,
> 
> Secondary process support's got overlooked during this refactoring, thanks
> for the patch. However can you describe the issue you're addressing as part
> of the commit log?
> 
> I think problems started when txq got mistakenly converted to
> primary_txq_ctrl in 21c8bb4928c9 ("net/mlx5: split Tx queue structure"), you
> may add a Fixes line for that one as well.
> 
> Otherwise, this patch looks fine to me.
> 

Patch doesn't apply cleanly, can you please rebase the patch on top of
latest dpdk-next-net?

Thanks,
ferruh

[dpdk-dev] [PATCH v2] net/mlx5: fix init on secondary process

[Olivier Gournet]

Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
Fixes: 21c8bb4928c9 ("net/mlx5: split Tx queue structure")

Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
---
 drivers/net/mlx5/mlx5_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index c76e754..188ade9 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1309,11 +1309,13 @@ mlx5_secondary_data_setup(struct priv *priv)
 			continue;
 		primary_txq_ctrl = container_of(primary_txq,
 						struct txq_ctrl, txq);
-		txq_ctrl = rte_calloc_socket("TXQ", 1, sizeof(*txq_ctrl), 0,
+		txq_ctrl = rte_calloc_socket("TXQ", 1, sizeof(*txq_ctrl) +
+					     (1 << primary_txq->elts_n) *
+					     sizeof(struct rte_mbuf *), 0,
 					     primary_txq_ctrl->socket);
 		if (txq_ctrl != NULL) {
 			if (txq_ctrl_setup(priv->dev,
-					   primary_txq_ctrl,
+					   txq_ctrl,
 					   1 << primary_txq->elts_n,
 					   primary_txq_ctrl->socket,
 					   NULL) == 0) {
-- 
2.1.4

Re: [dpdk-dev] [PATCH v2] net/mlx5: fix init on secondary process

[Ferruh Yigit]

Hi Adrien,

On 10/17/2016 1:56 PM, Olivier Gournet wrote:
> Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> Fixes: 21c8bb4928c9 ("net/mlx5: split Tx queue structure")
> 
> Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>

According your comment on previous version of it, I think you have your
Ack on this patch, but can you please confirm?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2] net/mlx5: fix init on secondary process

[Adrien Mazarguil]

On Mon, Oct 17, 2016 at 02:52:39PM +0100, Ferruh Yigit wrote:
> Hi Adrien,
> 
> On 10/17/2016 1:56 PM, Olivier Gournet wrote:
> > Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> > Fixes: 21c8bb4928c9 ("net/mlx5: split Tx queue structure")
> > 
> > Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
> 
> According your comment on previous version of it, I think you have your
> Ack on this patch, but can you please confirm?

Yes it's fine, thanks.

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH v2] net/mlx5: fix init on secondary process

[Bruce Richardson]

On Mon, Oct 17, 2016 at 04:18:59PM +0200, Adrien Mazarguil wrote:
> On Mon, Oct 17, 2016 at 02:52:39PM +0100, Ferruh Yigit wrote:
> > Hi Adrien,
> > 
> > On 10/17/2016 1:56 PM, Olivier Gournet wrote:
> > > Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> > > Fixes: 21c8bb4928c9 ("net/mlx5: split Tx queue structure")
> > > 
> > > Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
> > 
> > According your comment on previous version of it, I think you have your
> > Ack on this patch, but can you please confirm?
> 
> Yes it's fine, thanks.
> 
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> -- 
While this patch is acked, I'd still like a bit of detail in the commit
message describing what the problem is and how the patch fixes it. The
Chuck Norris approach of trying to stare down the code until it tells
me just isn't working for me today! :-)

Adrien or Olivier, if you can supply a brief description of what this
patch is doing and why I'll add it to the commit log on apply.

Thanks,

/Bruce

Re: [dpdk-dev] [PATCH v2] net/mlx5: fix init on secondary process

[Adrien Mazarguil]

On Wed, Oct 19, 2016 at 10:31:48AM +0100, Bruce Richardson wrote:
> On Mon, Oct 17, 2016 at 04:18:59PM +0200, Adrien Mazarguil wrote:
> > On Mon, Oct 17, 2016 at 02:52:39PM +0100, Ferruh Yigit wrote:
> > > Hi Adrien,
> > > 
> > > On 10/17/2016 1:56 PM, Olivier Gournet wrote:
> > > > Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> > > > Fixes: 21c8bb4928c9 ("net/mlx5: split Tx queue structure")
> > > > 
> > > > Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
> > > 
> > > According your comment on previous version of it, I think you have your
> > > Ack on this patch, but can you please confirm?
> > 
> > Yes it's fine, thanks.
> > 
> > Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > 
> > -- 
> While this patch is acked, I'd still like a bit of detail in the commit
> message describing what the problem is and how the patch fixes it. The
> Chuck Norris approach of trying to stare down the code until it tells
> me just isn't working for me today! :-)
> 
> Adrien or Olivier, if you can supply a brief description of what this
> patch is doing and why I'll add it to the commit log on apply.

*cough* this patch restores the original behavior of not causing a secondary
process to segfault during init.

Seriously, one needs to look at mlx5_secondary_data_setup() in both commits
to really understand what happened, my suggestion for a commit log:

The changes introduced by these commits made secondaries attempt to
reinitialize the TX queue structures of the primary instead of their own,
for which they also do not allocate enough memory, leading to crashes.

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH v2] net/mlx5: fix init on secondary process

[Bruce Richardson]

On Mon, Oct 17, 2016 at 04:18:59PM +0200, Adrien Mazarguil wrote:
> On Mon, Oct 17, 2016 at 02:52:39PM +0100, Ferruh Yigit wrote:
> > Hi Adrien,
> > 
> > On 10/17/2016 1:56 PM, Olivier Gournet wrote:
> > > Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> > > Fixes: 21c8bb4928c9 ("net/mlx5: split Tx queue structure")
> > > 
> > > Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
> > 
> > According your comment on previous version of it, I think you have your
> > Ack on this patch, but can you please confirm?
> 
> Yes it's fine, thanks.
> 
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
Applied to dpdk-next-net/rel_16_11

/Bruce