[v2,2/2] eal/freebsd: add config reattach

Message ID 658a39af271ae33f9df20678da0655216646702c.1561635211.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2,1/2] eal/freebsd: fix missing write to internal config |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Anatoly Burakov June 27, 2019, 11:33 a.m. UTC
  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(-)
  

Comments

David Marchand June 27, 2019, 11:46 a.m. UTC | #1
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!
  
Thomas Monjalon July 1, 2019, 4:02 p.m. UTC | #2
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
  

Patch

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: