[dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Jean Tourrilhes]

 lib/librte_eal/linuxapp/eal/eal.c        | 14 +++++++++++---
 lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++++----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 3fb2188..5df9f6a 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -238,7 +238,8 @@ rte_eal_config_attach(void)
 	mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config),
 			PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
 	if (mem_config == MAP_FAILED)
-		rte_panic("Cannot mmap memory for rte_config\n");
+		rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
+			  errno, strerror(errno));
 
 	rte_config.mem_config = mem_config;
 }
@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
 	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] - please use '--base-virtaddr' option\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);
-	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
-		rte_panic("Cannot mmap memory for rte_config\n");
 
 	rte_config.mem_config = mem_config;
 }
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 41e0a92..b036ffc 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -1615,10 +1615,18 @@ rte_eal_hugepage_attach(void)
 				 PROT_READ, MAP_PRIVATE, fd_zero, 0);
 		if (base_addr == MAP_FAILED ||
 		    base_addr != mcfg->memseg[s].addr) {
-			RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
-				"in /dev/zero to requested address [%p]: '%s'\n",
-				(unsigned long long)mcfg->memseg[s].len,
-				mcfg->memseg[s].addr, strerror(errno));
+			if (base_addr != MAP_FAILED)
+				/* errno is stale, don't use */
+				RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
+					"in /dev/zero at [%p], got [%p] - "
+					"please use '--base-virtaddr' option\n",
+					(unsigned long long)mcfg->memseg[s].len,
+					mcfg->memseg[s].addr, base_addr);
+			else
+				RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
+					"in /dev/zero at [%p]: '%s'\n",
+					(unsigned long long)mcfg->memseg[s].len,
+					mcfg->memseg[s].addr, strerror(errno));
 			if (aslr_enabled() > 0) {
 				RTE_LOG(ERR, EAL, "It is recommended to "
 					"disable ASLR in the kernel "

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Sergio Gonzalez Monroy]

Hi Jean,

There are some format issues with the patch:

You can run scripts/check-git-log.sh to check them:
Wrong headline format:
         eal: Fix misleading error messages, errno can't be trusted.
Wrong headline uppercase:
         eal: Fix misleading error messages, errno can't be trusted.
Missing 'Fixes' tag:
         eal: Fix misleading error messages, errno can't be trusted.

The script's output highlights the different issues.


On 21/09/2016 22:10, Jean Tourrilhes wrote:
>   lib/librte_eal/linuxapp/eal/eal.c        | 14 +++++++++++---
>   lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++++----
>   2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 3fb2188..5df9f6a 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -238,7 +238,8 @@ rte_eal_config_attach(void)
>   	mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config),
>   			PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
>   	if (mem_config == MAP_FAILED)
> -		rte_panic("Cannot mmap memory for rte_config\n");
> +		rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
> +			  errno, strerror(errno));
>   
>   	rte_config.mem_config = mem_config;
>   }
> @@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
>   	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] - please use '--base-virtaddr' option\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);
> -	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
> -		rte_panic("Cannot mmap memory for rte_config\n");
>   

NIT but any reason you moved the check before closing the file 
descriptor? (not that it matters with current code as we panic anyway)

Thanks,
Sergio

>   	rte_config.mem_config = mem_config;
>   }
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 41e0a92..b036ffc 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1615,10 +1615,18 @@ rte_eal_hugepage_attach(void)
>   				 PROT_READ, MAP_PRIVATE, fd_zero, 0);
>   		if (base_addr == MAP_FAILED ||
>   		    base_addr != mcfg->memseg[s].addr) {
> -			RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
> -				"in /dev/zero to requested address [%p]: '%s'\n",
> -				(unsigned long long)mcfg->memseg[s].len,
> -				mcfg->memseg[s].addr, strerror(errno));
> +			if (base_addr != MAP_FAILED)
> +				/* errno is stale, don't use */
> +				RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
> +					"in /dev/zero at [%p], got [%p] - "
> +					"please use '--base-virtaddr' option\n",
> +					(unsigned long long)mcfg->memseg[s].len,
> +					mcfg->memseg[s].addr, base_addr);
> +			else
> +				RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
> +					"in /dev/zero at [%p]: '%s'\n",
> +					(unsigned long long)mcfg->memseg[s].len,
> +					mcfg->memseg[s].addr, strerror(errno));
>   			if (aslr_enabled() > 0) {
>   				RTE_LOG(ERR, EAL, "It is recommended to "
>   					"disable ASLR in the kernel "

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Jean Tourrilhes]

On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> Hi Jean,
> 
> There are some format issues with the patch:
> 
> You can run scripts/check-git-log.sh to check them:
> Wrong headline format:
>         eal: Fix misleading error messages, errno can't be trusted.
> Wrong headline uppercase:
>         eal: Fix misleading error messages, errno can't be trusted.
> Missing 'Fixes' tag:
>         eal: Fix misleading error messages, errno can't be trusted.
> 
> The script's output highlights the different issues.

	SOrry about that, I casually read the page on
http://dpdk.org/dev, but obviously I need to look at it again.

> On 21/09/2016 22:10, Jean Tourrilhes wrote:
> >@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
> >  	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] - please use '--base-virtaddr' option\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);
> >-	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
> >-		rte_panic("Cannot mmap memory for rte_config\n");
> 
> NIT but any reason you moved the check before closing the file descriptor?
> (not that it matters with current code as we panic anyway)

	"close()" may change "errno" according to its man page.

> Thanks,
> Sergio

	Thanks for the review !

	Jean

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jean Tourrilhes
> Sent: Monday, October 3, 2016 4:56 PM
> To: Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages,
> errno can't be trusted.
> 
> On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> > Hi Jean,
> >
> > There are some format issues with the patch:
> >
> > You can run scripts/check-git-log.sh to check them:
> > Wrong headline format:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > Wrong headline uppercase:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > Missing 'Fixes' tag:
> >         eal: Fix misleading error messages, errno can't be trusted.
> >
> > The script's output highlights the different issues.
> 
> 	SOrry about that, I casually read the page on http://dpdk.org/dev,
> but obviously I need to look at it again.

The longer more detailed version is here: "Contributing Code to DPDK":

    http://dpdk.org/doc/guides/contributing/patches.html

John

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Jean Tourrilhes]

On Mon, Oct 03, 2016 at 04:15:11PM +0000, Mcnamara, John wrote:
> 
> The longer more detailed version is here: "Contributing Code to DPDK":
> 
>     http://dpdk.org/doc/guides/contributing/patches.html
> 
> John

	Thanks a lot. I'll try to find time to look at it.

	Jean

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Thomas Monjalon]

2016-10-03 08:55, Jean Tourrilhes:
> On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> > Hi Jean,
> > 
> > There are some format issues with the patch:
> > 
> > You can run scripts/check-git-log.sh to check them:
> > Wrong headline format:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > Wrong headline uppercase:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > Missing 'Fixes' tag:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > 
> > The script's output highlights the different issues.
> 
> 	SOrry about that, I casually read the page on
> http://dpdk.org/dev, but obviously I need to look at it again.

No problem. This guide is more oriented towards regular contributors.
You come with a bug and its fix, we can make some effort to format
the patch :)

The title could be "mem: fix hugepage mapping error messages"

> > On 21/09/2016 22:10, Jean Tourrilhes wrote:
> > >@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
> > >  	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] - please use '--base-virtaddr' option\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);
> > >-	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
> > >-		rte_panic("Cannot mmap memory for rte_config\n");
> > 
> > NIT but any reason you moved the check before closing the file descriptor?
> > (not that it matters with current code as we panic anyway)
> 
> 	"close()" may change "errno" according to its man page.

Sergio, do you have more comments?
Should we wait another version or is it OK?
Maybe you'd prefer to rework it yourself?

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Sergio Gonzalez Monroy]

On 03/10/2016 21:46, Thomas Monjalon wrote:
> 2016-10-03 08:55, Jean Tourrilhes:
>> On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
>>> Hi Jean,
>>>
>>> There are some format issues with the patch:
>>>
>>> You can run scripts/check-git-log.sh to check them:
>>> Wrong headline format:
>>>          eal: Fix misleading error messages, errno can't be trusted.
>>> Wrong headline uppercase:
>>>          eal: Fix misleading error messages, errno can't be trusted.
>>> Missing 'Fixes' tag:
>>>          eal: Fix misleading error messages, errno can't be trusted.
>>>
>>> The script's output highlights the different issues.
>> 	SOrry about that, I casually read the page on
>> http://dpdk.org/dev, but obviously I need to look at it again.
> No problem. This guide is more oriented towards regular contributors.
> You come with a bug and its fix, we can make some effort to format
> the patch :)
>
> The title could be "mem: fix hugepage mapping error messages"
>
>>> On 21/09/2016 22:10, Jean Tourrilhes wrote:
>>>> @@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
>>>>   	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] - please use '--base-virtaddr' option\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);
>>>> -	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
>>>> -		rte_panic("Cannot mmap memory for rte_config\n");
>>> NIT but any reason you moved the check before closing the file descriptor?
>>> (not that it matters with current code as we panic anyway)
>> 	"close()" may change "errno" according to its man page.
> Sergio, do you have more comments?

Nop.

> Should we wait another version or is it OK?
> Maybe you'd prefer to rework it yourself?

AFAICS it really is just the commit message formatting, so maybe you can 
fix that when applying it?

Sergio

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Thomas Monjalon]

2016-10-04 09:03, Sergio Gonzalez Monroy:
> On 03/10/2016 21:46, Thomas Monjalon wrote:
> > 2016-10-03 08:55, Jean Tourrilhes:
> >> On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> >>> Hi Jean,
> >>>
> >>> There are some format issues with the patch:
> >>>
> >>> You can run scripts/check-git-log.sh to check them:
> >>> Wrong headline format:
> >>>          eal: Fix misleading error messages, errno can't be trusted.
> >>> Wrong headline uppercase:
> >>>          eal: Fix misleading error messages, errno can't be trusted.
> >>> Missing 'Fixes' tag:
> >>>          eal: Fix misleading error messages, errno can't be trusted.
> >>>
> >>> The script's output highlights the different issues.
> >> 	SOrry about that, I casually read the page on
> >> http://dpdk.org/dev, but obviously I need to look at it again.
> > No problem. This guide is more oriented towards regular contributors.
> > You come with a bug and its fix, we can make some effort to format
> > the patch :)
> >
> > The title could be "mem: fix hugepage mapping error messages"
[...]
> > Sergio, do you have more comments?
> 
> Nop.
> 
> > Should we wait another version or is it OK?
> > Maybe you'd prefer to rework it yourself?
> 
> AFAICS it really is just the commit message formatting, so maybe you can 
> fix that when applying it?

Yes but I've just noticed that the Signed-off-by line is missing.
Please Jean, could you add this required line with your explanations,
rebase on fresh head and resend please?

[dpdk-dev] [PATCH v2 1/3] mem: fix hugepage mapping error messages

[Jean Tourrilhes]

Running secondary is tricky due to the need to map the memory region
at the right place in VM, which is whatever primary has chosen. If the
base address for primary happens to by already mapped in the
secondary, we will hit precisely these error messages (depending if we
fail on the config region or the hugepages). This is why there is
already a comment about ASLR.

The issue is that in most cases, remapping does not happen and "errno"
is not changed and therefore stale. In our case, we got a "permission
denied", which sent us down the wrong track. It's such a common error
for secondary that I feel this error message should be unambiguous and
helpful.
The call to close was also moved because close() may override errno.

Signed-off-by: Jean Tourrilhes <jt@labs.hpe.com>
---
 lib/librte_eal/linuxapp/eal/eal.c        | 14 +++++++++++---
 lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++++----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 3fb2188..5df9f6a 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -238,7 +238,8 @@ rte_eal_config_attach(void)
 	mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config),
 			PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
 	if (mem_config == MAP_FAILED)
-		rte_panic("Cannot mmap memory for rte_config\n");
+		rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
+			  errno, strerror(errno));
 
 	rte_config.mem_config = mem_config;
 }
@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
 	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] - please use '--base-virtaddr' option\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);
-	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
-		rte_panic("Cannot mmap memory for rte_config\n");
 
 	rte_config.mem_config = mem_config;
 }
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 41e0a92..b036ffc 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -1615,10 +1615,18 @@ rte_eal_hugepage_attach(void)
 				 PROT_READ, MAP_PRIVATE, fd_zero, 0);
 		if (base_addr == MAP_FAILED ||
 		    base_addr != mcfg->memseg[s].addr) {
-			RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
-				"in /dev/zero to requested address [%p]: '%s'\n",
-				(unsigned long long)mcfg->memseg[s].len,
-				mcfg->memseg[s].addr, strerror(errno));
+			if (base_addr != MAP_FAILED)
+				/* errno is stale, don't use */
+				RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
+					"in /dev/zero at [%p], got [%p] - "
+					"please use '--base-virtaddr' option\n",
+					(unsigned long long)mcfg->memseg[s].len,
+					mcfg->memseg[s].addr, base_addr);
+			else
+				RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
+					"in /dev/zero at [%p]: '%s'\n",
+					(unsigned long long)mcfg->memseg[s].len,
+					mcfg->memseg[s].addr, strerror(errno));
 			if (aslr_enabled() > 0) {
 				RTE_LOG(ERR, EAL, "It is recommended to "
 					"disable ASLR in the kernel "

Re: [dpdk-dev] [PATCH v2 1/3] mem: fix hugepage mapping error messages

[Sergio Gonzalez Monroy]

On 04/10/2016 18:17, Jean Tourrilhes wrote:
> Running secondary is tricky due to the need to map the memory region
> at the right place in VM, which is whatever primary has chosen. If the
> base address for primary happens to by already mapped in the
> secondary, we will hit precisely these error messages (depending if we
> fail on the config region or the hugepages). This is why there is
> already a comment about ASLR.
>
> The issue is that in most cases, remapping does not happen and "errno"
> is not changed and therefore stale. In our case, we got a "permission
> denied", which sent us down the wrong track. It's such a common error
> for secondary that I feel this error message should be unambiguous and
> helpful.
> The call to close was also moved because close() may override errno.
>
> Signed-off-by: Jean Tourrilhes <jt@labs.hpe.com>
> ---
>   lib/librte_eal/linuxapp/eal/eal.c        | 14 +++++++++++---
>   lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++++----
>   2 files changed, 23 insertions(+), 7 deletions(-)

Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Re: [dpdk-dev] [PATCH v2 1/3] mem: fix hugepage mapping error messages

[Thomas Monjalon]

2016-10-04 20:07, Sergio Gonzalez Monroy:
> On 04/10/2016 18:17, Jean Tourrilhes wrote:
> > Running secondary is tricky due to the need to map the memory region
> > at the right place in VM, which is whatever primary has chosen. If the
> > base address for primary happens to by already mapped in the
> > secondary, we will hit precisely these error messages (depending if we
> > fail on the config region or the hugepages). This is why there is
> > already a comment about ASLR.
> >
> > The issue is that in most cases, remapping does not happen and "errno"
> > is not changed and therefore stale. In our case, we got a "permission
> > denied", which sent us down the wrong track. It's such a common error
> > for secondary that I feel this error message should be unambiguous and
> > helpful.
> > The call to close was also moved because close() may override errno.
> >
> > Signed-off-by: Jean Tourrilhes <jt@labs.hpe.com>
> > ---
> >   lib/librte_eal/linuxapp/eal/eal.c        | 14 +++++++++++---
> >   lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++++----
> >   2 files changed, 23 insertions(+), 7 deletions(-)
> 
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Applied, thanks
A rebase was necessary because of this patch: http://dpdk.org/commit/c00ae961
Please check everything is OK.

Re: [dpdk-dev] [PATCH v2 1/3] mem: fix hugepage mapping error messages

[Jean Tourrilhes]

On Wed, Oct 05, 2016 at 11:51:48AM +0200, Thomas Monjalon wrote:
> 
> Applied, thanks
> A rebase was necessary because of this patch: http://dpdk.org/commit/c00ae961
> Please check everything is OK.

	Tested today's master. Working as expected.
	Thanks !

	Jean

Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.

[Jean Tourrilhes]

On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> Hi Jean,
> 
> NIT but any reason you moved the check before closing the file descriptor?
> (not that it matters with current code as we panic anyway)
> 
> Thanks,
> Sergio

	More details, as I admit I was terse
	Running secondary is tricky due to the need to map the memory
region at the right place, which is whatever primary has chosen. If
the base address for primary happens to by already mapped in the
secondary, we will hit precisely this error message (well, in a few
case we might hit the other one). This is why there is already
a comment about ASLR.
	A colleague of mine hit that message and was misled by errno
claiming "permission denied", which sent him down the wrong
track. It's such a common error for secondary that I feel this error
message should be unambiguous and helpful.
	Regards,

	Jean