DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] eal/freebsd: fix missing write to internal config
@ 2019-06-25 15:50 Anatoly Burakov
  2019-06-25 15:50 ` [dpdk-dev] [PATCH 2/2] eal/freebsd: add config reattach Anatoly Burakov
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Anatoly Burakov @ 2019-06-25 15:50 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable

When init is complete, EAL is supposed to update internal config
to indicate that initialization is complete. Add missing write.

Fixes: a99c96e96ad3 ("eal: add internal flag of init completed")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/freebsd/eal/eal.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index 4eaa53195..8c399c799 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -566,6 +566,8 @@ rte_eal_mcfg_complete(void)
 	/* ALL shared mem_config related INIT DONE */
 	if (rte_config.process_type == RTE_PROC_PRIMARY)
 		rte_config.mem_config->magic = RTE_MAGIC;
+
+	internal_config.init_complete = 1;
 }
 
 /* return non-zero if hugepages are enabled. */
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH 2/2] eal/freebsd: add config reattach
  2019-06-25 15:50 [dpdk-dev] [PATCH 1/2] eal/freebsd: fix missing write to internal config Anatoly Burakov
@ 2019-06-25 15:50 ` Anatoly Burakov
  2019-06-26 12:03   ` [dpdk-dev] [dpdk-stable] " David Marchand
  2019-06-26 12:03 ` [dpdk-dev] [dpdk-stable] [PATCH 1/2] eal/freebsd: fix missing write to internal config David Marchand
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Anatoly Burakov @ 2019-06-25 15:50 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable

Linux EAL will attach the shared config at an arbitrary address,
find out where the shared config is mapped in the primary, and
then will reattach it at that exact address.

FreeBSD version doesn't seem to go for that extra reattach step,
which makes one wonder how did it ever work in the first place.

Fix the FreeBSD init to also reattach shared config to the exact
same place the primary process has it.

Fixes: 764bf26873b9 ("add FreeBSD support")
Cc: bruce.richardson@intel.com
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/freebsd/eal/eal.c | 36 ++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index 8c399c799..ce7a5f91d 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -280,6 +280,41 @@ rte_eal_config_attach(void)
 	rte_config.mem_config = rte_mem_cfg_addr;
 }
 
+/* reattach the shared config at exact memory location primary process has it */
+static void
+rte_eal_config_reattach(void)
+{
+	struct rte_mem_config *mem_config;
+	void *rte_mem_cfg_addr;
+
+	if (internal_config.no_shconf)
+		return;
+
+	/* save the address primary process has mapped shared config to */
+	rte_mem_cfg_addr =
+			(void *)(uintptr_t)rte_config.mem_config->mem_cfg_addr;
+
+	/* unmap original config */
+	munmap(rte_config.mem_config, sizeof(struct rte_mem_config));
+
+	/* remap the config at proper address */
+	mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
+			sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED,
+			mem_cfg_fd, 0);
+	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
+		if (mem_config != MAP_FAILED)
+			/* errno is stale, don't use */
+			rte_panic("Cannot mmap memory for rte_config at [%p], got [%p]\n",
+				  rte_mem_cfg_addr, mem_config);
+		else
+			rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
+				  errno, strerror(errno));
+	}
+	close(mem_cfg_fd);
+
+	rte_config.mem_config = mem_config;
+}
+
 /* Detect if we are a primary or a secondary process */
 enum rte_proc_type_t
 eal_proc_type_detect(void)
@@ -318,6 +353,7 @@ rte_config_init(void)
 	case RTE_PROC_SECONDARY:
 		rte_eal_config_attach();
 		rte_eal_mcfg_wait_complete(rte_config.mem_config);
+		rte_eal_config_reattach();
 		break;
 	case RTE_PROC_AUTO:
 	case RTE_PROC_INVALID:
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] eal/freebsd: add config reattach
  2019-06-25 15:50 ` [dpdk-dev] [PATCH 2/2] eal/freebsd: add config reattach Anatoly Burakov
@ 2019-06-26 12:03   ` David Marchand
  2019-06-26 12:21     ` David Marchand
  2019-06-26 12:49     ` Burakov, Anatoly
  0 siblings, 2 replies; 11+ messages in thread
From: David Marchand @ 2019-06-26 12:03 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Bruce Richardson, dpdk stable

On Tue, Jun 25, 2019 at 5:51 PM Anatoly Burakov <anatoly.burakov@intel.com>
wrote:

> Linux EAL will attach the shared config at an arbitrary address,
> find out where the shared config is mapped in the primary, and
> then will reattach it at that exact address.
>
> FreeBSD version doesn't seem to go for that extra reattach step,
> which makes one wonder how did it ever work in the first place.
>
> Fix the FreeBSD init to also reattach shared config to the exact
> same place the primary process has it.
>
> Fixes: 764bf26873b9 ("add FreeBSD support")
> Cc: bruce.richardson@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  lib/librte_eal/freebsd/eal/eal.c | 36 ++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/lib/librte_eal/freebsd/eal/eal.c
> b/lib/librte_eal/freebsd/eal/eal.c
> index 8c399c799..ce7a5f91d 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -280,6 +280,41 @@ rte_eal_config_attach(void)
>         rte_config.mem_config = rte_mem_cfg_addr;
>  }
>
> +/* reattach the shared config at exact memory location primary process
> has it */
> +static void
> +rte_eal_config_reattach(void)
> +{
> +       struct rte_mem_config *mem_config;
> +       void *rte_mem_cfg_addr;
> +
> +       if (internal_config.no_shconf)
> +               return;
> +
> +       /* save the address primary process has mapped shared config to */
> +       rte_mem_cfg_addr =
> +                       (void
> *)(uintptr_t)rte_config.mem_config->mem_cfg_addr;
>

It should be within the 80 columns limit on a single line.

+
> +       /* unmap original config */
> +       munmap(rte_config.mem_config, sizeof(struct rte_mem_config));
>

Hum, the previous mapping is PROT_WRITE while unneeded, right?


+
> +       /* remap the config at proper address */
> +       mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
> +                       sizeof(*mem_config), PROT_READ | PROT_WRITE,
> MAP_SHARED,
> +                       mem_cfg_fd, 0);
>

mem_cfg_fd should have been closed in rte_eal_config_attach().
So it should fail here.


+       if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
> +               if (mem_config != MAP_FAILED)
> +                       /* errno is stale, don't use */
> +                       rte_panic("Cannot mmap memory for rte_config at
> [%p], got [%p]\n",
> +                                 rte_mem_cfg_addr, mem_config);
> +               else
> +                       rte_panic("Cannot mmap memory for rte_config!
> error %i (%s)\n",
> +                                 errno, strerror(errno));
> +       }
> +       close(mem_cfg_fd);
> +
> +       rte_config.mem_config = mem_config;
> +}
> +
>  /* Detect if we are a primary or a secondary process */
>  enum rte_proc_type_t
>  eal_proc_type_detect(void)
> @@ -318,6 +353,7 @@ rte_config_init(void)
>         case RTE_PROC_SECONDARY:
>                 rte_eal_config_attach();
>                 rte_eal_mcfg_wait_complete(rte_config.mem_config);
> +               rte_eal_config_reattach();
>                 break;
>         case RTE_PROC_AUTO:
>         case RTE_PROC_INVALID:
> --
> 2.17.1
>


Is there a reason why FreeBSD EAL does not support the virtaddr hint like
for Linux EAL?


-- 
David Marchand

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 1/2] eal/freebsd: fix missing write to internal config
  2019-06-25 15:50 [dpdk-dev] [PATCH 1/2] eal/freebsd: fix missing write to internal config Anatoly Burakov
  2019-06-25 15:50 ` [dpdk-dev] [PATCH 2/2] eal/freebsd: add config reattach Anatoly Burakov
@ 2019-06-26 12:03 ` David Marchand
  2019-06-27 11:33 ` [dpdk-dev] [PATCH v2 " Anatoly Burakov
  2019-06-27 11:33 ` [dpdk-dev] [PATCH v2 2/2] eal/freebsd: add config reattach Anatoly Burakov
  3 siblings, 0 replies; 11+ messages in thread
From: David Marchand @ 2019-06-26 12:03 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Bruce Richardson, dpdk stable

On Tue, Jun 25, 2019 at 5:51 PM Anatoly Burakov <anatoly.burakov@intel.com>
wrote:

> When init is complete, EAL is supposed to update internal config
> to indicate that initialization is complete. Add missing write.
>
> Fixes: a99c96e96ad3 ("eal: add internal flag of init completed")
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  lib/librte_eal/freebsd/eal/eal.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_eal/freebsd/eal/eal.c
> b/lib/librte_eal/freebsd/eal/eal.c
> index 4eaa53195..8c399c799 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -566,6 +566,8 @@ rte_eal_mcfg_complete(void)
>         /* ALL shared mem_config related INIT DONE */
>         if (rte_config.process_type == RTE_PROC_PRIMARY)
>                 rte_config.mem_config->magic = RTE_MAGIC;
> +
> +       internal_config.init_complete = 1;
>  }
>
>  /* return non-zero if hugepages are enabled. */
> --
> 2.17.1
>

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] eal/freebsd: add config reattach
  2019-06-26 12:03   ` [dpdk-dev] [dpdk-stable] " David Marchand
@ 2019-06-26 12:21     ` David Marchand
  2019-06-26 12:50       ` Burakov, Anatoly
  2019-06-26 12:49     ` Burakov, Anatoly
  1 sibling, 1 reply; 11+ messages in thread
From: David Marchand @ 2019-06-26 12:21 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Bruce Richardson, dpdk stable, Arnon Warshavsky

On Wed, Jun 26, 2019 at 2:03 PM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Tue, Jun 25, 2019 at 5:51 PM Anatoly Burakov <anatoly.burakov@intel.com>
> wrote:
>
>> Linux EAL will attach the shared config at an arbitrary address,
>> find out where the shared config is mapped in the primary, and
>> then will reattach it at that exact address.
>>
>> FreeBSD version doesn't seem to go for that extra reattach step,
>> which makes one wonder how did it ever work in the first place.
>>
>> Fix the FreeBSD init to also reattach shared config to the exact
>> same place the primary process has it.
>>
>> Fixes: 764bf26873b9 ("add FreeBSD support")
>> Cc: bruce.richardson@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>  lib/librte_eal/freebsd/eal/eal.c | 36 ++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>
>> diff --git a/lib/librte_eal/freebsd/eal/eal.c
>> b/lib/librte_eal/freebsd/eal/eal.c
>> index 8c399c799..ce7a5f91d 100644
>> --- a/lib/librte_eal/freebsd/eal/eal.c
>> +++ b/lib/librte_eal/freebsd/eal/eal.c
>> @@ -280,6 +280,41 @@ rte_eal_config_attach(void)
>>         rte_config.mem_config = rte_mem_cfg_addr;
>>  }
>>
>> +/* reattach the shared config at exact memory location primary process
>> has it */
>> +static void
>> +rte_eal_config_reattach(void)
>> +{
>> +       struct rte_mem_config *mem_config;
>> +       void *rte_mem_cfg_addr;
>> +
>> +       if (internal_config.no_shconf)
>> +               return;
>> +
>> +       /* save the address primary process has mapped shared config to */
>> +       rte_mem_cfg_addr =
>> +                       (void
>> *)(uintptr_t)rte_config.mem_config->mem_cfg_addr;
>>
>
> It should be within the 80 columns limit on a single line.
>
> +
>> +       /* unmap original config */
>> +       munmap(rte_config.mem_config, sizeof(struct rte_mem_config));
>>
>
> Hum, the previous mapping is PROT_WRITE while unneeded, right?
>
>
> +
>> +       /* remap the config at proper address */
>> +       mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
>> +                       sizeof(*mem_config), PROT_READ | PROT_WRITE,
>> MAP_SHARED,
>> +                       mem_cfg_fd, 0);
>>
>
> mem_cfg_fd should have been closed in rte_eal_config_attach().
> So it should fail here.
>
>
> +       if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
>> +               if (mem_config != MAP_FAILED)
>> +                       /* errno is stale, don't use */
>> +                       rte_panic("Cannot mmap memory for rte_config at
>> [%p], got [%p]\n",
>> +                                 rte_mem_cfg_addr, mem_config);
>> +               else
>> +                       rte_panic("Cannot mmap memory for rte_config!
>> error %i (%s)\n",
>> +                                 errno, strerror(errno));
>> +       }
>> +       close(mem_cfg_fd);
>> +
>> +       rte_config.mem_config = mem_config;
>> +}
>> +
>>  /* Detect if we are a primary or a secondary process */
>>  enum rte_proc_type_t
>>  eal_proc_type_detect(void)
>> @@ -318,6 +353,7 @@ rte_config_init(void)
>>         case RTE_PROC_SECONDARY:
>>                 rte_eal_config_attach();
>>                 rte_eal_mcfg_wait_complete(rte_config.mem_config);
>> +               rte_eal_config_reattach();
>>                 break;
>>         case RTE_PROC_AUTO:
>>         case RTE_PROC_INVALID:
>> --
>> 2.17.1
>>
>
>
> Is there a reason why FreeBSD EAL does not support the virtaddr hint like
> for Linux EAL?
>

Not sure you noticed, but Arnon also had sent a patch touching the config
mapping parts.
http://patchwork.dpdk.org/patch/54583/


-- 
David Marchand

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] eal/freebsd: add config reattach
  2019-06-26 12:03   ` [dpdk-dev] [dpdk-stable] " David Marchand
  2019-06-26 12:21     ` David Marchand
@ 2019-06-26 12:49     ` Burakov, Anatoly
  1 sibling, 0 replies; 11+ messages in thread
From: Burakov, Anatoly @ 2019-06-26 12:49 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Bruce Richardson, dpdk stable

On 26-Jun-19 1:03 PM, David Marchand wrote:
> On Tue, Jun 25, 2019 at 5:51 PM Anatoly Burakov <anatoly.burakov@intel.com>
> wrote:
> 
>> Linux EAL will attach the shared config at an arbitrary address,
>> find out where the shared config is mapped in the primary, and
>> then will reattach it at that exact address.
>>
>> FreeBSD version doesn't seem to go for that extra reattach step,
>> which makes one wonder how did it ever work in the first place.
>>
>> Fix the FreeBSD init to also reattach shared config to the exact
>> same place the primary process has it.
>>
>> Fixes: 764bf26873b9 ("add FreeBSD support")
>> Cc: bruce.richardson@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>   lib/librte_eal/freebsd/eal/eal.c | 36 ++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/lib/librte_eal/freebsd/eal/eal.c
>> b/lib/librte_eal/freebsd/eal/eal.c
>> index 8c399c799..ce7a5f91d 100644
>> --- a/lib/librte_eal/freebsd/eal/eal.c
>> +++ b/lib/librte_eal/freebsd/eal/eal.c
>> @@ -280,6 +280,41 @@ rte_eal_config_attach(void)
>>          rte_config.mem_config = rte_mem_cfg_addr;
>>   }
>>
>> +/* reattach the shared config at exact memory location primary process
>> has it */
>> +static void
>> +rte_eal_config_reattach(void)
>> +{
>> +       struct rte_mem_config *mem_config;
>> +       void *rte_mem_cfg_addr;
>> +
>> +       if (internal_config.no_shconf)
>> +               return;
>> +
>> +       /* save the address primary process has mapped shared config to */
>> +       rte_mem_cfg_addr =
>> +                       (void
>> *)(uintptr_t)rte_config.mem_config->mem_cfg_addr;
>>
> 
> It should be within the 80 columns limit on a single line.

Nope, tried it - doesn't fit.

> 
> +
>> +       /* unmap original config */
>> +       munmap(rte_config.mem_config, sizeof(struct rte_mem_config));
>>
> 
> Hum, the previous mapping is PROT_WRITE while unneeded, right?

Yes, good catch.

> 
> 
> +
>> +       /* remap the config at proper address */
>> +       mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
>> +                       sizeof(*mem_config), PROT_READ | PROT_WRITE,
>> MAP_SHARED,
>> +                       mem_cfg_fd, 0);
>>
> 
> mem_cfg_fd should have been closed in rte_eal_config_attach().
> So it should fail here.

Hmm, wonder why it didn't. I'll investigate and fix if necessary.

> 
> 
> +       if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
>> +               if (mem_config != MAP_FAILED)
>> +                       /* errno is stale, don't use */
>> +                       rte_panic("Cannot mmap memory for rte_config at
>> [%p], got [%p]\n",
>> +                                 rte_mem_cfg_addr, mem_config);
>> +               else
>> +                       rte_panic("Cannot mmap memory for rte_config!
>> error %i (%s)\n",
>> +                                 errno, strerror(errno));
>> +       }
>> +       close(mem_cfg_fd);
>> +
>> +       rte_config.mem_config = mem_config;
>> +}
>> +
>>   /* Detect if we are a primary or a secondary process */
>>   enum rte_proc_type_t
>>   eal_proc_type_detect(void)
>> @@ -318,6 +353,7 @@ rte_config_init(void)
>>          case RTE_PROC_SECONDARY:
>>                  rte_eal_config_attach();
>>                  rte_eal_mcfg_wait_complete(rte_config.mem_config);
>> +               rte_eal_config_reattach();
>>                  break;
>>          case RTE_PROC_AUTO:
>>          case RTE_PROC_INVALID:
>> --
>> 2.17.1
>>
> 
> 
> Is there a reason why FreeBSD EAL does not support the virtaddr hint like
> for Linux EAL?
> 

No real reason. Implementing that is probably out of scope for this 
patchset though.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] eal/freebsd: add config reattach
  2019-06-26 12:21     ` David Marchand
@ 2019-06-26 12:50       ` Burakov, Anatoly
  0 siblings, 0 replies; 11+ messages in thread
From: Burakov, Anatoly @ 2019-06-26 12:50 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Bruce Richardson, dpdk stable, Arnon Warshavsky

On 26-Jun-19 1:21 PM, David Marchand wrote:
> On Wed, Jun 26, 2019 at 2:03 PM David Marchand <david.marchand@redhat.com>
> wrote:
> 
>>
>>
>> On Tue, Jun 25, 2019 at 5:51 PM Anatoly Burakov <anatoly.burakov@intel.com>
>> wrote:
>>
>>> Linux EAL will attach the shared config at an arbitrary address,
>>> find out where the shared config is mapped in the primary, and
>>> then will reattach it at that exact address.
>>>
>>> FreeBSD version doesn't seem to go for that extra reattach step,
>>> which makes one wonder how did it ever work in the first place.
>>>
>>> Fix the FreeBSD init to also reattach shared config to the exact
>>> same place the primary process has it.
>>>
>>> Fixes: 764bf26873b9 ("add FreeBSD support")
>>> Cc: bruce.richardson@intel.com
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>> ---

<...>

> Not sure you noticed, but Arnon also had sent a patch touching the config
> mapping parts.
> http://patchwork.dpdk.org/patch/54583/
> 

Yes, i've seen those, didn't get a chance to look at them yet. I'll 
rebase when necessary.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] eal/freebsd: fix missing write to internal config
  2019-06-25 15:50 [dpdk-dev] [PATCH 1/2] eal/freebsd: fix missing write to internal config Anatoly Burakov
  2019-06-25 15:50 ` [dpdk-dev] [PATCH 2/2] eal/freebsd: add config reattach Anatoly Burakov
  2019-06-26 12:03 ` [dpdk-dev] [dpdk-stable] [PATCH 1/2] eal/freebsd: fix missing write to internal config David Marchand
@ 2019-06-27 11:33 ` Anatoly Burakov
  2019-06-27 11:33 ` [dpdk-dev] [PATCH v2 2/2] eal/freebsd: add config reattach Anatoly Burakov
  3 siblings, 0 replies; 11+ messages in thread
From: Anatoly Burakov @ 2019-06-27 11:33 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, david.marchand, stable

When init is complete, EAL is supposed to update internal config
to indicate that initialization is complete. Add missing write.

Fixes: a99c96e96ad3 ("eal: add internal flag of init completed")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/freebsd/eal/eal.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index 97633a55a..3e15af792 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -594,6 +594,8 @@ rte_eal_mcfg_complete(void)
 	/* ALL shared mem_config related INIT DONE */
 	if (rte_config.process_type == RTE_PROC_PRIMARY)
 		rte_config.mem_config->magic = RTE_MAGIC;
+
+	internal_config.init_complete = 1;
 }
 
 /* return non-zero if hugepages are enabled. */
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] eal/freebsd: add config reattach
  2019-06-25 15:50 [dpdk-dev] [PATCH 1/2] eal/freebsd: fix missing write to internal config Anatoly Burakov
                   ` (2 preceding siblings ...)
  2019-06-27 11:33 ` [dpdk-dev] [PATCH v2 " Anatoly Burakov
@ 2019-06-27 11:33 ` Anatoly Burakov
  2019-06-27 11:46   ` David Marchand
  3 siblings, 1 reply; 11+ messages in thread
From: Anatoly Burakov @ 2019-06-27 11:33 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, david.marchand, stable

Linux EAL will attach the shared config at an arbitrary address,
find out where the shared config is mapped in the primary, and
then will reattach it at that exact address.

FreeBSD version doesn't seem to go for that extra reattach step,
which makes one wonder how did it ever work in the first place.

Fix the FreeBSD init to also reattach shared config to the exact
same place the primary process has it.

Fixes: 764bf26873b9 ("add FreeBSD support")
Cc: bruce.richardson@intel.com
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2:
    - Rebase on top of latest master
    - Fix fd handling
    - Fix mmap PROT flags on first map

 lib/librte_eal/freebsd/eal/eal.c | 49 ++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index 3e15af792..fac43b017 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -288,10 +288,11 @@ rte_eal_config_attach(void)
 	}
 
 	rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
-				PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
-	close(mem_cfg_fd);
-	mem_cfg_fd = -1;
+				PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
+	/* don't close the fd here, it will be closed on reattach */
 	if (rte_mem_cfg_addr == MAP_FAILED) {
+		close(mem_cfg_fd);
+		mem_cfg_fd = -1;
 		RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n",
 			errno, strerror(errno));
 		return -1;
@@ -302,6 +303,46 @@ rte_eal_config_attach(void)
 	return 0;
 }
 
+/* reattach the shared config at exact memory location primary process has it */
+static int
+rte_eal_config_reattach(void)
+{
+	struct rte_mem_config *mem_config;
+	void *rte_mem_cfg_addr;
+
+	if (internal_config.no_shconf)
+		return 0;
+
+	/* save the address primary process has mapped shared config to */
+	rte_mem_cfg_addr =
+			(void *)(uintptr_t)rte_config.mem_config->mem_cfg_addr;
+
+	/* unmap original config */
+	munmap(rte_config.mem_config, sizeof(struct rte_mem_config));
+
+	/* remap the config at proper address */
+	mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
+			sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED,
+			mem_cfg_fd, 0);
+	close(mem_cfg_fd);
+	mem_cfg_fd = -1;
+
+	if (mem_config == MAP_FAILED) {
+		RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n",
+				errno, strerror(errno));
+		return -1;
+	} else if (mem_config != rte_mem_cfg_addr) {
+		/* errno is stale, don't use */
+		RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]\n",
+			  rte_mem_cfg_addr, mem_config);
+		return -1;
+	}
+
+	rte_config.mem_config = mem_config;
+
+	return 0;
+}
+
 /* Detect if we are a primary or a secondary process */
 enum rte_proc_type_t
 eal_proc_type_detect(void)
@@ -342,6 +383,8 @@ rte_config_init(void)
 		if (rte_eal_config_attach() < 0)
 			return -1;
 		rte_eal_mcfg_wait_complete(rte_config.mem_config);
+		if (rte_eal_config_reattach() < 0)
+			return -1;
 		break;
 	case RTE_PROC_AUTO:
 	case RTE_PROC_INVALID:
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] eal/freebsd: add config reattach
  2019-06-27 11:33 ` [dpdk-dev] [PATCH v2 2/2] eal/freebsd: add config reattach Anatoly Burakov
@ 2019-06-27 11:46   ` David Marchand
  2019-07-01 16:02     ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
  0 siblings, 1 reply; 11+ messages in thread
From: David Marchand @ 2019-06-27 11:46 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Bruce Richardson, dpdk stable

On Thu, Jun 27, 2019 at 1:33 PM Anatoly Burakov <anatoly.burakov@intel.com>
wrote:

> Linux EAL will attach the shared config at an arbitrary address,
> find out where the shared config is mapped in the primary, and
> then will reattach it at that exact address.
>
> FreeBSD version doesn't seem to go for that extra reattach step,
> which makes one wonder how did it ever work in the first place.
>
> Fix the FreeBSD init to also reattach shared config to the exact
> same place the primary process has it.
>
> Fixes: 764bf26873b9 ("add FreeBSD support")
> Cc: bruce.richardson@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v2:
>     - Rebase on top of latest master
>     - Fix fd handling
>     - Fix mmap PROT flags on first map
>
>  lib/librte_eal/freebsd/eal/eal.c | 49 ++++++++++++++++++++++++++++++--
>  1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/freebsd/eal/eal.c
> b/lib/librte_eal/freebsd/eal/eal.c
> index 3e15af792..fac43b017 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -288,10 +288,11 @@ rte_eal_config_attach(void)
>         }
>
>         rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
> -                               PROT_READ | PROT_WRITE, MAP_SHARED,
> mem_cfg_fd, 0);
> -       close(mem_cfg_fd);
> -       mem_cfg_fd = -1;
> +                               PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
> +       /* don't close the fd here, it will be closed on reattach */
>         if (rte_mem_cfg_addr == MAP_FAILED) {
> +               close(mem_cfg_fd);
> +               mem_cfg_fd = -1;
>                 RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config!
> error %i (%s)\n",
>                         errno, strerror(errno));
>                 return -1;
> @@ -302,6 +303,46 @@ rte_eal_config_attach(void)
>         return 0;
>  }
>
> +/* reattach the shared config at exact memory location primary process
> has it */
> +static int
> +rte_eal_config_reattach(void)
> +{
> +       struct rte_mem_config *mem_config;
> +       void *rte_mem_cfg_addr;
> +
> +       if (internal_config.no_shconf)
> +               return 0;
> +
> +       /* save the address primary process has mapped shared config to */
> +       rte_mem_cfg_addr =
> +                       (void
> *)(uintptr_t)rte_config.mem_config->mem_cfg_addr;
> +
> +       /* unmap original config */
> +       munmap(rte_config.mem_config, sizeof(struct rte_mem_config));
> +
> +       /* remap the config at proper address */
> +       mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
> +                       sizeof(*mem_config), PROT_READ | PROT_WRITE,
> MAP_SHARED,
> +                       mem_cfg_fd, 0);
> +       close(mem_cfg_fd);
> +       mem_cfg_fd = -1;
> +
> +       if (mem_config == MAP_FAILED) {
> +               RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config!
> error %i (%s)\n",
> +                               errno, strerror(errno));
> +               return -1;
> +       } else if (mem_config != rte_mem_cfg_addr) {
> +               /* errno is stale, don't use */
> +               RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at
> [%p], got [%p]\n",
> +                         rte_mem_cfg_addr, mem_config);
> +               return -1;
> +       }
>

Ah, it looks better than this double if we have in linux eal.c :-)
+1

+
> +       rte_config.mem_config = mem_config;
> +
> +       return 0;
> +}
> +
>  /* Detect if we are a primary or a secondary process */
>  enum rte_proc_type_t
>  eal_proc_type_detect(void)
> @@ -342,6 +383,8 @@ rte_config_init(void)
>                 if (rte_eal_config_attach() < 0)
>                         return -1;
>                 rte_eal_mcfg_wait_complete(rte_config.mem_config);
> +               if (rte_eal_config_reattach() < 0)
> +                       return -1;
>                 break;
>         case RTE_PROC_AUTO:
>         case RTE_PROC_INVALID:
> --
> 2.17.1
>

Reviewed-by: David Marchand <david.marchand@redhat.com>

Thanks!

-- 
David Marchand

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/2] eal/freebsd: add config reattach
  2019-06-27 11:46   ` David Marchand
@ 2019-07-01 16:02     ` Thomas Monjalon
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2019-07-01 16:02 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: stable, David Marchand, dev, Bruce Richardson

27/06/2019 13:46, David Marchand:
> On Thu, Jun 27, 2019 at 1:33 PM Anatoly Burakov <anatoly.burakov@intel.com>
> wrote:
> 
> > Linux EAL will attach the shared config at an arbitrary address,
> > find out where the shared config is mapped in the primary, and
> > then will reattach it at that exact address.
> >
> > FreeBSD version doesn't seem to go for that extra reattach step,
> > which makes one wonder how did it ever work in the first place.
> >
> > Fix the FreeBSD init to also reattach shared config to the exact
> > same place the primary process has it.
> >
> > Fixes: 764bf26873b9 ("add FreeBSD support")
> > Cc: bruce.richardson@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com>

Applied, thanks




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-07-01 16:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-25 15:50 [dpdk-dev] [PATCH 1/2] eal/freebsd: fix missing write to internal config Anatoly Burakov
2019-06-25 15:50 ` [dpdk-dev] [PATCH 2/2] eal/freebsd: add config reattach Anatoly Burakov
2019-06-26 12:03   ` [dpdk-dev] [dpdk-stable] " David Marchand
2019-06-26 12:21     ` David Marchand
2019-06-26 12:50       ` Burakov, Anatoly
2019-06-26 12:49     ` Burakov, Anatoly
2019-06-26 12:03 ` [dpdk-dev] [dpdk-stable] [PATCH 1/2] eal/freebsd: fix missing write to internal config David Marchand
2019-06-27 11:33 ` [dpdk-dev] [PATCH v2 " Anatoly Burakov
2019-06-27 11:33 ` [dpdk-dev] [PATCH v2 2/2] eal/freebsd: add config reattach Anatoly Burakov
2019-06-27 11:46   ` David Marchand
2019-07-01 16:02     ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).