Fix some ifup related network issues

Fix some ifup related network issues

[Juergen Borleis]

Patch 3 of 3 is more an RFC. Because we have a generic issue inside the hush.
Anybody here who is able to fix the hush?

Juergen


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] net/ifup.c: don't fail silently

[Juergen Borleis]

Since commit a162dfe50345d3461010759f8a0e79f7e388c140 the ifup command is implemented in C and runs up to two external scripts.
If one of these scripts return with an error code, the command terminates silently. This can confuse a user because there is
no hint about the reason why it fails. Add error messages to avoid this case.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 net/ifup.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ifup.c b/net/ifup.c
index 7b63136..c74120c 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -70,14 +70,18 @@ int ifup(const char *name, unsigned flags)
 	cmd_discover = asprintf("/env/network/%s-discover", name);
 
 	ret = run_command(cmd);
-	if (ret)
+	if (ret) {
+		pr_err("Running '%s' fails\n", cmd);
 		goto out;
+	}
 
 	ret = stat(cmd_discover, &s);
 	if (!ret) {
 		ret = run_command(cmd_discover);
-		if (ret)
+		if (ret) {
+			pr_err("Running '%s' fails\n", cmd);
 			goto out;
+		}
 	}
 
 	dev = get_device_by_name(name);
-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] net/ifup.c: don't fail silently

[Sascha Hauer]

On Wed, Apr 23, 2014 at 11:19:24AM +0200, Juergen Borleis wrote:
> Since commit a162dfe50345d3461010759f8a0e79f7e388c140 the ifup command is implemented in C and runs up to two external scripts.
> If one of these scripts return with an error code, the command terminates silently. This can confuse a user because there is
> no hint about the reason why it fails. Add error messages to avoid this case.
> 
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> ---
>  net/ifup.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ifup.c b/net/ifup.c
> index 7b63136..c74120c 100644
> --- a/net/ifup.c
> +++ b/net/ifup.c
> @@ -70,14 +70,18 @@ int ifup(const char *name, unsigned flags)
>  	cmd_discover = asprintf("/env/network/%s-discover", name);
>  
>  	ret = run_command(cmd);
> -	if (ret)
> +	if (ret) {
> +		pr_err("Running '%s' fails\n", cmd);

It failed, not it fails. Also, please print the error code along with
the message.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] net/ifup.c: don't fail silently

[Juergen Borleis]

Since commit a162dfe50345d3461010759f8a0e79f7e388c140 the ifup command is
implemented in C and runs up to two external scripts.
If one of these scripts return with an error code, the command terminates
silently. This can confuse a user because there is no hint about the reason
why it fails. Add error messages to avoid this case.
    
Signed-off-by: Juergen Borleis <jbe@pengutronix.de>

diff --git a/net/ifup.c b/net/ifup.c
index 7b6313629666..b259a975dc0b 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -70,14 +70,18 @@ int ifup(const char *name, unsigned flags)
 	cmd_discover = asprintf("/env/network/%s-discover", name);
 
 	ret = run_command(cmd);
-	if (ret)
+	if (ret) {
+		pr_err("Running '%s' failed with %d\n", cmd, ret);
 		goto out;
+	}
 
 	ret = stat(cmd_discover, &s);
 	if (!ret) {
 		ret = run_command(cmd_discover);
-		if (ret)
+		if (ret) {
+			pr_err("Running '%s' failed with %d\n", cmd, ret);
 			goto out;
+		}
 	}
 
 	dev = get_device_by_name(name);

-- 
Pengutronix e.K.                              | Juergen Borleis             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Peiner Str. 6-8, 31137 Hildesheim, Germany    | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] net/ifup.c: avoid setting the MAC twice

[Juergen Borleis]

The command always sets the MAC address of the interface in the code path.
For the static network configuration case it sets it again, due to the
environment variable keyword ("ethaddr") is listed again in the table.
Remove this keyword from the table, because the MAC is already set.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 net/ifup.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/ifup.c b/net/ifup.c
index c74120c..3216ce7 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -31,7 +31,6 @@ static char *vars[] = {
 	"netmask",
 	"gateway",
 	"serverip",
-	"ethaddr",
 };
 
 static int eth_set_param(struct device_d *dev, const char *param)
-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] default environment: force a specific return value

[Juergen Borleis]

Since commit a162dfe50345d3461010759f8a0e79f7e388c140 the ifup command runs
this file as a script. Due to a hush misbehave it could happen it returns
an error code by accident.

For example if the last instructions in this file are:

 if [ false ]; then
    echo "friesel"
 fi

the hush returns 1 after running this script instead of 0 and in this case
the ifup command fails.

I know, the correct fix would be to fix the hush, because it is a generic
issue...but how?

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 defaultenv/defaultenv-2-base/network/eth0 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/defaultenv/defaultenv-2-base/network/eth0 b/defaultenv/defaultenv-2-base/network/eth0
index 7e731ca..33fe7c1 100644
--- a/defaultenv/defaultenv-2-base/network/eth0
+++ b/defaultenv/defaultenv-2-base/network/eth0
@@ -14,3 +14,5 @@ serverip=
 #ethaddr=xx:xx:xx:xx:xx:xx
 
 # put code to discover eth0 (i.e. 'usb') to /env/network/eth0-discover
+
+exit 0
-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: Fix some ifup related network issues

[Sascha Hauer]

On Wed, Apr 23, 2014 at 11:19:23AM +0200, Juergen Borleis wrote:
> Patch 3 of 3 is more an RFC. Because we have a generic issue inside the hush.
> Anybody here who is able to fix the hush?

Nobody is able to fix hush, some people may be able to rewrite it ;)

Applied this series.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox