Re: [U-Boot] [PATCH 2/5] net: Re-check prerequisites when autoloading
On 07/04/2018 06:23 PM, Joe Hershberger wrote:
On Wed, Jul 4, 2018 at 4:20 AM, Alexander Graf wrote:
On 07/04/2018 02:36 AM, Joe Hershberger wrote:
With net autoload, we check the prerequisites for the initial command,
but the greater prerequisites when autoloading are not checked.
If we would attempt to autoload, check those prerequisites too.
If we are not expecting a serverip from the server, then don't worry
about it not being set, but don't attempt to load if it isn't.
Signed-off-by: Joe Hershberger
---
net/net.c | 20
1 file changed, 20 insertions(+)
diff --git a/net/net.c b/net/net.c
index bff3e9c5b5..42a50e60f8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -332,6 +332,16 @@ void net_auto_load(void)
const char *s = env_get("autoload");
if (s != NULL && strcmp(s, "NFS") == 0) {
+ if (net_check_prereq(NFS)) {
+/* We aren't expecting to get a serverip, so just accept the assigned IP
*/
+#ifdef CONFIG_BOOTP_SERVERIP
+ net_set_state(NETLOOP_SUCCESS);
+#else
+ printf("Cannot autoload with NFS\n");
+ net_set_state(NETLOOP_FAIL);
+#endif
I don't understand the #ifdef. In the CONFIG_BOOTP_SERVERIP case, you should
already have net_server_ip set from the variable setter, so when do you
realistically get into the case where net_check_prereq() fails here? I can
My thinking here was that if the user is in control of the serverip
and chooses not to set it, then at least populate the dhcp variables
that were successful. If we return a fail from here, even though DHCP
was successful, the result will not be saved to the env for the user.
IMHO CONFIG_BOOTP_SERVERIP is a very esoteric use case already. Let's
not interpret too much into it. Instead, I would prefer if we could just
treat it the same as the normal case as often as we can ...
Or maybe just move its functionality (do not set serverip from dhcp
command) into an environment variable.
Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/5] net: Re-check prerequisites when autoloading
On Wed, Jul 4, 2018 at 4:20 AM, Alexander Graf wrote:
> On 07/04/2018 02:36 AM, Joe Hershberger wrote:
>>
>> With net autoload, we check the prerequisites for the initial command,
>> but the greater prerequisites when autoloading are not checked.
>>
>> If we would attempt to autoload, check those prerequisites too.
>>
>> If we are not expecting a serverip from the server, then don't worry
>> about it not being set, but don't attempt to load if it isn't.
>>
>> Signed-off-by: Joe Hershberger
>> ---
>>
>> net/net.c | 20
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/net/net.c b/net/net.c
>> index bff3e9c5b5..42a50e60f8 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -332,6 +332,16 @@ void net_auto_load(void)
>> const char *s = env_get("autoload");
>> if (s != NULL && strcmp(s, "NFS") == 0) {
>> + if (net_check_prereq(NFS)) {
>> +/* We aren't expecting to get a serverip, so just accept the assigned IP
>> */
>> +#ifdef CONFIG_BOOTP_SERVERIP
>> + net_set_state(NETLOOP_SUCCESS);
>> +#else
>> + printf("Cannot autoload with NFS\n");
>> + net_set_state(NETLOOP_FAIL);
>> +#endif
>
>
> I don't understand the #ifdef. In the CONFIG_BOOTP_SERVERIP case, you should
> already have net_server_ip set from the variable setter, so when do you
> realistically get into the case where net_check_prereq() fails here? I can
My thinking here was that if the user is in control of the serverip
and chooses not to set it, then at least populate the dhcp variables
that were successful. If we return a fail from here, even though DHCP
was successful, the result will not be saved to the env for the user.
> only see that happening when serverip is not set (read: net_server_ip ==
> 0.0.0.0) in which case we should also error out in the CONFIG_BOOTP_SERVERIP
> case, no?
>
>
> Alex
>
>
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/5] net: Re-check prerequisites when autoloading
On 07/04/2018 02:36 AM, Joe Hershberger wrote:
With net autoload, we check the prerequisites for the initial command,
but the greater prerequisites when autoloading are not checked.
If we would attempt to autoload, check those prerequisites too.
If we are not expecting a serverip from the server, then don't worry
about it not being set, but don't attempt to load if it isn't.
Signed-off-by: Joe Hershberger
---
net/net.c | 20
1 file changed, 20 insertions(+)
diff --git a/net/net.c b/net/net.c
index bff3e9c5b5..42a50e60f8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -332,6 +332,16 @@ void net_auto_load(void)
const char *s = env_get("autoload");
if (s != NULL && strcmp(s, "NFS") == 0) {
+ if (net_check_prereq(NFS)) {
+/* We aren't expecting to get a serverip, so just accept the assigned IP */
+#ifdef CONFIG_BOOTP_SERVERIP
+ net_set_state(NETLOOP_SUCCESS);
+#else
+ printf("Cannot autoload with NFS\n");
+ net_set_state(NETLOOP_FAIL);
+#endif
I don't understand the #ifdef. In the CONFIG_BOOTP_SERVERIP case, you
should already have net_server_ip set from the variable setter, so when
do you realistically get into the case where net_check_prereq() fails
here? I can only see that happening when serverip is not set (read:
net_server_ip == 0.0.0.0) in which case we should also error out in the
CONFIG_BOOTP_SERVERIP case, no?
Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/5] net: Re-check prerequisites when autoloading
With net autoload, we check the prerequisites for the initial command,
but the greater prerequisites when autoloading are not checked.
If we would attempt to autoload, check those prerequisites too.
If we are not expecting a serverip from the server, then don't worry
about it not being set, but don't attempt to load if it isn't.
Signed-off-by: Joe Hershberger
---
net/net.c | 20
1 file changed, 20 insertions(+)
diff --git a/net/net.c b/net/net.c
index bff3e9c5b5..42a50e60f8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -332,6 +332,16 @@ void net_auto_load(void)
const char *s = env_get("autoload");
if (s != NULL && strcmp(s, "NFS") == 0) {
+ if (net_check_prereq(NFS)) {
+/* We aren't expecting to get a serverip, so just accept the assigned IP */
+#ifdef CONFIG_BOOTP_SERVERIP
+ net_set_state(NETLOOP_SUCCESS);
+#else
+ printf("Cannot autoload with NFS\n");
+ net_set_state(NETLOOP_FAIL);
+#endif
+ return;
+ }
/*
* Use NFS to load the bootfile.
*/
@@ -347,6 +357,16 @@ void net_auto_load(void)
net_set_state(NETLOOP_SUCCESS);
return;
}
+ if (net_check_prereq(TFTPGET)) {
+/* We aren't expecting to get a serverip, so just accept the assigned IP */
+#ifdef CONFIG_BOOTP_SERVERIP
+ net_set_state(NETLOOP_SUCCESS);
+#else
+ printf("Cannot autoload with TFTPGET\n");
+ net_set_state(NETLOOP_FAIL);
+#endif
+ return;
+ }
tftp_start(TFTPGET);
}
--
2.11.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
