diff mbox series

[1/2] net: Add option to prefer bootp/dhcp serverip

Message ID 20180606123202.53631-1-agraf@suse.de
State New
Headers show
Series [1/2] net: Add option to prefer bootp/dhcp serverip | expand

Commit Message

Alexander Graf June 6, 2018, 12:32 p.m. UTC
Currently we can choose between 2 different types of behavior for the
serverip variable:

  1) Always overwrite it with the DHCP server IP address (default)
  2) Ignore what the DHCP server says (CONFIG_BOOTP_SERVERIP)

This patch adds a 3rd option:

  3) Use serverip from DHCP if no serverip is given
     (CONFIG_BOOTP_PREFER_SERVERIP)

With this new option, we can have the default case that a boot file gets
loaded from the DHCP provided TFTP server work while allowing users to
specify their own serverip variable to explicitly use a different tftp
server.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 README      | 5 +++++
 cmd/Kconfig | 9 +++++++++
 net/bootp.c | 7 ++++++-
 3 files changed, 20 insertions(+), 1 deletion(-)

Comments

Rick Chen June 7, 2018, 1:54 a.m. UTC | #1
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Wednesday, June 06, 2018 8:32 PM
> To: u-boot@lists.denx.de
> Cc: Rick Jian-Zhi Chen(陳建志); Joe Hershberger; Simon Glass
> Subject: [PATCH 1/2] net: Add option to prefer bootp/dhcp serverip
>
> Currently we can choose between 2 different types of behavior for the serverip
> variable:
>
>   1) Always overwrite it with the DHCP server IP address (default)
>   2) Ignore what the DHCP server says (CONFIG_BOOTP_SERVERIP)
>
> This patch adds a 3rd option:
>
>   3) Use serverip from DHCP if no serverip is given
>      (CONFIG_BOOTP_PREFER_SERVERIP)
>
> With this new option, we can have the default case that a boot file gets loaded
> from the DHCP provided TFTP server work while allowing users to specify their
> own serverip variable to explicitly use a different tftp server.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  README      | 5 +++++
>  cmd/Kconfig | 9 +++++++++
>  net/bootp.c | 7 ++++++-
>  3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/README b/README
> index fb331f910d..d8a99281ca 100644
> --- a/README
> +++ b/README
> @@ -1511,10 +1511,15 @@ The following options need to be configured:
>               CONFIG_BOOTP_TIMEOFFSET
>               CONFIG_BOOTP_VENDOREX
>               CONFIG_BOOTP_MAY_FAIL
> +             CONFIG_BOOTP_PREFER_SERVERIP
>
>               CONFIG_BOOTP_SERVERIP - TFTP server will be the serverip
>               environment variable, not the BOOTP server.
>
> +             CONFIG_BOOTP_PREFER_SERVERIP - TFTP server will be the
> +             serverip environment variable if previously unset, otherwise
> +             the DHCP provided serverip is used.
> +
>               CONFIG_BOOTP_MAY_FAIL - If the DHCP server is not found
>               after the configured retry count, the call will fail
>               instead of starting over.  This can be used to fail over diff --git
> a/cmd/Kconfig b/cmd/Kconfig index e283cb9a8a..e77a4131b3 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1121,6 +1121,15 @@ config BOOTP_HOSTNAME
>       help
>         The name may or may not be qualified with the local domain name.
>
> +config BOOTP_PREFER_SERVERIP
> +     bool "Leave serverip variable in place if existing"
> +     default n
> +     depends on CMD_BOOTP
> +     help
> +       By default a BOOTP/DHCP reply will overwrite the tftp target ip
> +       address. With this option enabled, it will leave it alone if
> +       already specified, but populate it if no serverip is specified.
> +
>  config BOOTP_SUBNETMASK
>       bool "Request & store 'netmask' from BOOTP/DHCP server"
>       default y
> diff --git a/net/bootp.c b/net/bootp.c
> index 9d7cb5d30c..91de4cd426 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -147,9 +147,14 @@ static void store_net_params(struct bootp_hdr *bp)
> {  #if !defined(CONFIG_BOOTP_SERVERIP)
>       struct in_addr tmp_ip;
> +     bool overwrite_serverip = true;
> +
> +#if defined(CONFIG_BOOTP_PREFER_SERVERIP)
> +     overwrite_serverip = false;
> +#endif
>
>       net_copy_ip(&tmp_ip, &bp->bp_siaddr);
> -     if (tmp_ip.s_addr != 0)
> +     if (tmp_ip.s_addr != 0 && (overwrite_serverip ||
> +!net_server_ip.s_addr))
>               net_copy_ip(&net_server_ip, &bp->bp_siaddr);
>       memcpy(net_server_ethaddr,
>              ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);
> --
> 2.12.3

Hi Alex

I have apply those two patchs and verify
U-Boot-1-2-net-Add-option-to-prefer-bootp-dhcp-serverip.patch
U-Boot-2-2-ax25-Switch-to-CONFIG_BOOTP_PREFER_SERVERIP.patch

But it still fail in dhcp command as below

case 1
serverip is null

RISC-V # set serverip
RISC-V # env print
baudrate=38400
bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
bbl-ae350.bin;go 0x0
bootdelay=3
bootfile=pxelinux.0
ethact=mac@e0100000
fdtcontroladdr=3fedf290
fileaddr=600000
filesize=1bb7d34
stderr=serial@f0300000
stdin=serial@f0300000
stdout=serial@f0300000

Environment size: 304/8188 bytes
RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
DHCP client bound to address 10.0.4.191 (4603 ms)
Using mac@e0100000 device
TFTP from server 255.255.255.255; our IP address is 10.0.4.191;
sending through gateway 10.0.4.254
Filename 'pxelinux.0'.
Load address: 0x600000
Loading: *
TFTP error: 'File not found' (1)
Not retrying...

TFTP error: 'File not found' (1)
Not retrying...

case 2
serverip has value

RISC-V # setenv serverip 10.0.4.97 ;

RISC-V # dhcp 0x600000 boomimage-310y-ag101p.bin
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
DHCP client bound to address 10.0.4.191 (4592 ms)
Using mac@e0100000 device
TFTP from server 10.0.4.97; our IP address is 10.0.4.191
Filename 'pxelinux.0'.
Load address: 0x600000
Loading: *
TFTP error: 'File not found' (1)
Not retrying...

TFTP error: 'File not found' (1)
Not retrying...
RISC-V #



Rick
Joe Hershberger June 12, 2018, 7:59 p.m. UTC | #2
On Wed, Jun 6, 2018 at 7:32 AM, Alexander Graf <agraf@suse.de> wrote:
> Currently we can choose between 2 different types of behavior for the
> serverip variable:
>
>   1) Always overwrite it with the DHCP server IP address (default)
>   2) Ignore what the DHCP server says (CONFIG_BOOTP_SERVERIP)
>
> This patch adds a 3rd option:
>
>   3) Use serverip from DHCP if no serverip is given
>      (CONFIG_BOOTP_PREFER_SERVERIP)
>
> With this new option, we can have the default case that a boot file gets
> loaded from the DHCP provided TFTP server work while allowing users to
> specify their own serverip variable to explicitly use a different tftp
> server.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  README      | 5 +++++
>  cmd/Kconfig | 9 +++++++++
>  net/bootp.c | 7 ++++++-
>  3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/README b/README
> index fb331f910d..d8a99281ca 100644
> --- a/README
> +++ b/README
> @@ -1511,10 +1511,15 @@ The following options need to be configured:
>                 CONFIG_BOOTP_TIMEOFFSET
>                 CONFIG_BOOTP_VENDOREX
>                 CONFIG_BOOTP_MAY_FAIL
> +               CONFIG_BOOTP_PREFER_SERVERIP
>
>                 CONFIG_BOOTP_SERVERIP - TFTP server will be the serverip
>                 environment variable, not the BOOTP server.
>
> +               CONFIG_BOOTP_PREFER_SERVERIP - TFTP server will be the
> +               serverip environment variable if previously unset, otherwise

I think you mean "previously set" here.

Also, I don't think we are putting descriptions in the README if the
help is already in the Kconfig. Right, Tom?

> +               the DHCP provided serverip is used.
> +
>                 CONFIG_BOOTP_MAY_FAIL - If the DHCP server is not found
>                 after the configured retry count, the call will fail
>                 instead of starting over.  This can be used to fail over
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index e283cb9a8a..e77a4131b3 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1121,6 +1121,15 @@ config BOOTP_HOSTNAME
>         help
>           The name may or may not be qualified with the local domain name.
>
> +config BOOTP_PREFER_SERVERIP
> +       bool "Leave serverip variable in place if existing"

I think this would be clearer if it were "serverip variable in the
environment takes precedent over DHCP server IP."

> +       default n
> +       depends on CMD_BOOTP
> +       help
> +         By default a BOOTP/DHCP reply will overwrite the tftp target ip

Use the actual variable name here... "serverip"

> +         address. With this option enabled, it will leave it alone if

Too many "it"s.

"With this option enabled, the 'serverip' variable in the environment
takes precedence over DHCP server IP and will only be set by the DHCP
server if not already set in the environment."

> +         already specified, but populate it if no serverip is specified.
> +
>  config BOOTP_SUBNETMASK
>         bool "Request & store 'netmask' from BOOTP/DHCP server"
>         default y
> diff --git a/net/bootp.c b/net/bootp.c
> index 9d7cb5d30c..91de4cd426 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -147,9 +147,14 @@ static void store_net_params(struct bootp_hdr *bp)
>  {
>  #if !defined(CONFIG_BOOTP_SERVERIP)
>         struct in_addr tmp_ip;
> +       bool overwrite_serverip = true;
> +
> +#if defined(CONFIG_BOOTP_PREFER_SERVERIP)
> +       overwrite_serverip = false;
> +#endif
>
>         net_copy_ip(&tmp_ip, &bp->bp_siaddr);
> -       if (tmp_ip.s_addr != 0)
> +       if (tmp_ip.s_addr != 0 && (overwrite_serverip || !net_server_ip.s_addr))
>                 net_copy_ip(&net_server_ip, &bp->bp_siaddr);
>         memcpy(net_server_ethaddr,
>                ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);
> --
> 2.12.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Joe Hershberger June 12, 2018, 7:59 p.m. UTC | #3
On Wed, Jun 6, 2018 at 8:54 PM, Rick Chen <rickchen36@gmail.com> wrote:
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Wednesday, June 06, 2018 8:32 PM
>> To: u-boot@lists.denx.de
>> Cc: Rick Jian-Zhi Chen(陳建志); Joe Hershberger; Simon Glass
>> Subject: [PATCH 1/2] net: Add option to prefer bootp/dhcp serverip
>>
>> Currently we can choose between 2 different types of behavior for the serverip
>> variable:
>>
>>   1) Always overwrite it with the DHCP server IP address (default)
>>   2) Ignore what the DHCP server says (CONFIG_BOOTP_SERVERIP)
>>
>> This patch adds a 3rd option:
>>
>>   3) Use serverip from DHCP if no serverip is given
>>      (CONFIG_BOOTP_PREFER_SERVERIP)
>>
>> With this new option, we can have the default case that a boot file gets loaded
>> from the DHCP provided TFTP server work while allowing users to specify their
>> own serverip variable to explicitly use a different tftp server.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  README      | 5 +++++
>>  cmd/Kconfig | 9 +++++++++
>>  net/bootp.c | 7 ++++++-
>>  3 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/README b/README
>> index fb331f910d..d8a99281ca 100644
>> --- a/README
>> +++ b/README
>> @@ -1511,10 +1511,15 @@ The following options need to be configured:
>>               CONFIG_BOOTP_TIMEOFFSET
>>               CONFIG_BOOTP_VENDOREX
>>               CONFIG_BOOTP_MAY_FAIL
>> +             CONFIG_BOOTP_PREFER_SERVERIP
>>
>>               CONFIG_BOOTP_SERVERIP - TFTP server will be the serverip
>>               environment variable, not the BOOTP server.
>>
>> +             CONFIG_BOOTP_PREFER_SERVERIP - TFTP server will be the
>> +             serverip environment variable if previously unset, otherwise
>> +             the DHCP provided serverip is used.
>> +
>>               CONFIG_BOOTP_MAY_FAIL - If the DHCP server is not found
>>               after the configured retry count, the call will fail
>>               instead of starting over.  This can be used to fail over diff --git
>> a/cmd/Kconfig b/cmd/Kconfig index e283cb9a8a..e77a4131b3 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -1121,6 +1121,15 @@ config BOOTP_HOSTNAME
>>       help
>>         The name may or may not be qualified with the local domain name.
>>
>> +config BOOTP_PREFER_SERVERIP
>> +     bool "Leave serverip variable in place if existing"
>> +     default n
>> +     depends on CMD_BOOTP
>> +     help
>> +       By default a BOOTP/DHCP reply will overwrite the tftp target ip
>> +       address. With this option enabled, it will leave it alone if
>> +       already specified, but populate it if no serverip is specified.
>> +
>>  config BOOTP_SUBNETMASK
>>       bool "Request & store 'netmask' from BOOTP/DHCP server"
>>       default y
>> diff --git a/net/bootp.c b/net/bootp.c
>> index 9d7cb5d30c..91de4cd426 100644
>> --- a/net/bootp.c
>> +++ b/net/bootp.c
>> @@ -147,9 +147,14 @@ static void store_net_params(struct bootp_hdr *bp)
>> {  #if !defined(CONFIG_BOOTP_SERVERIP)
>>       struct in_addr tmp_ip;
>> +     bool overwrite_serverip = true;
>> +
>> +#if defined(CONFIG_BOOTP_PREFER_SERVERIP)
>> +     overwrite_serverip = false;
>> +#endif
>>
>>       net_copy_ip(&tmp_ip, &bp->bp_siaddr);
>> -     if (tmp_ip.s_addr != 0)
>> +     if (tmp_ip.s_addr != 0 && (overwrite_serverip ||
>> +!net_server_ip.s_addr))
>>               net_copy_ip(&net_server_ip, &bp->bp_siaddr);
>>       memcpy(net_server_ethaddr,
>>              ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);
>> --
>> 2.12.3
>
> Hi Alex
>
> I have apply those two patchs and verify
> U-Boot-1-2-net-Add-option-to-prefer-bootp-dhcp-serverip.patch
> U-Boot-2-2-ax25-Switch-to-CONFIG_BOOTP_PREFER_SERVERIP.patch
>
> But it still fail in dhcp command as below
>
> case 1
> serverip is null
>
> RISC-V # set serverip
> RISC-V # env print
> baudrate=38400
> bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
> bbl-ae350.bin;go 0x0
> bootdelay=3
> bootfile=pxelinux.0
> ethact=mac@e0100000
> fdtcontroladdr=3fedf290
> fileaddr=600000
> filesize=1bb7d34
> stderr=serial@f0300000
> stdin=serial@f0300000
> stdout=serial@f0300000
>
> Environment size: 304/8188 bytes
> RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin

You are explicitly setting the server IP in the DHCP command line, so
why would you expect the DHCP server IP to be used?

> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4
> DHCP client bound to address 10.0.4.191 (4603 ms)
> Using mac@e0100000 device
> TFTP from server 255.255.255.255; our IP address is 10.0.4.191;

This broadcast address is clearly not right. It should have been what
you had in the dhcp command. That should be assigned in net/tftp.c:
757...

>>                         tftp_remote_ip = string_to_ip(net_boot_file_name);

So something must be wrong with that somehow.

> sending through gateway 10.0.4.254
> Filename 'pxelinux.0'.

Why is this filename still set? That's from the environment and is set
by a env callback handler. That should happen before netboot_common()
which should be overwriting net_boot_file_name with the command line
filename.

> Load address: 0x600000
> Loading: *
> TFTP error: 'File not found' (1)
> Not retrying...
>
> TFTP error: 'File not found' (1)
> Not retrying...

Could you print out the env again? Did the server IP get set?

Also, you not finding a file assumes your TFTP server has a pxelinux.0
file on it... I don't think we have any reason from what you've
presented that it's the case. Especially since that's not the file you
specified on the command line.

>
> case 2
> serverip has value
>
> RISC-V # setenv serverip 10.0.4.97 ;
>
> RISC-V # dhcp 0x600000 boomimage-310y-ag101p.bin
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4
> DHCP client bound to address 10.0.4.191 (4592 ms)
> Using mac@e0100000 device
> TFTP from server 10.0.4.97; our IP address is 10.0.4.191

You are getting the correct IP used as the server, so it seems that
this patch is working (though I don't know that your DHCP server isn't
the same as the hard-coded address).

> Filename 'pxelinux.0'.
> Load address: 0x600000
> Loading: *
> TFTP error: 'File not found' (1)
> Not retrying...
>
> TFTP error: 'File not found' (1)
> Not retrying...
> RISC-V #

What version of U-Boot are you using? This behavior doesn't match what
I'm seeing in the code.

-Joe
Rick Chen June 13, 2018, 1:42 a.m. UTC | #4
2018-06-13 3:59 GMT+08:00 Joe Hershberger <joe.hershberger@ni.com>:
> On Wed, Jun 6, 2018 at 8:54 PM, Rick Chen <rickchen36@gmail.com> wrote:
>>> From: Alexander Graf [mailto:agraf@suse.de]
>>> Sent: Wednesday, June 06, 2018 8:32 PM
>>> To: u-boot@lists.denx.de
>>> Cc: Rick Jian-Zhi Chen(陳建志); Joe Hershberger; Simon Glass
>>> Subject: [PATCH 1/2] net: Add option to prefer bootp/dhcp serverip
>>>
>>> Currently we can choose between 2 different types of behavior for the serverip
>>> variable:
>>>
>>>   1) Always overwrite it with the DHCP server IP address (default)
>>>   2) Ignore what the DHCP server says (CONFIG_BOOTP_SERVERIP)
>>>
>>> This patch adds a 3rd option:
>>>
>>>   3) Use serverip from DHCP if no serverip is given
>>>      (CONFIG_BOOTP_PREFER_SERVERIP)
>>>
>>> With this new option, we can have the default case that a boot file gets loaded
>>> from the DHCP provided TFTP server work while allowing users to specify their
>>> own serverip variable to explicitly use a different tftp server.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  README      | 5 +++++
>>>  cmd/Kconfig | 9 +++++++++
>>>  net/bootp.c | 7 ++++++-
>>>  3 files changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/README b/README
>>> index fb331f910d..d8a99281ca 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -1511,10 +1511,15 @@ The following options need to be configured:
>>>               CONFIG_BOOTP_TIMEOFFSET
>>>               CONFIG_BOOTP_VENDOREX
>>>               CONFIG_BOOTP_MAY_FAIL
>>> +             CONFIG_BOOTP_PREFER_SERVERIP
>>>
>>>               CONFIG_BOOTP_SERVERIP - TFTP server will be the serverip
>>>               environment variable, not the BOOTP server.
>>>
>>> +             CONFIG_BOOTP_PREFER_SERVERIP - TFTP server will be the
>>> +             serverip environment variable if previously unset, otherwise
>>> +             the DHCP provided serverip is used.
>>> +
>>>               CONFIG_BOOTP_MAY_FAIL - If the DHCP server is not found
>>>               after the configured retry count, the call will fail
>>>               instead of starting over.  This can be used to fail over diff --git
>>> a/cmd/Kconfig b/cmd/Kconfig index e283cb9a8a..e77a4131b3 100644
>>> --- a/cmd/Kconfig
>>> +++ b/cmd/Kconfig
>>> @@ -1121,6 +1121,15 @@ config BOOTP_HOSTNAME
>>>       help
>>>         The name may or may not be qualified with the local domain name.
>>>
>>> +config BOOTP_PREFER_SERVERIP
>>> +     bool "Leave serverip variable in place if existing"
>>> +     default n
>>> +     depends on CMD_BOOTP
>>> +     help
>>> +       By default a BOOTP/DHCP reply will overwrite the tftp target ip
>>> +       address. With this option enabled, it will leave it alone if
>>> +       already specified, but populate it if no serverip is specified.
>>> +
>>>  config BOOTP_SUBNETMASK
>>>       bool "Request & store 'netmask' from BOOTP/DHCP server"
>>>       default y
>>> diff --git a/net/bootp.c b/net/bootp.c
>>> index 9d7cb5d30c..91de4cd426 100644
>>> --- a/net/bootp.c
>>> +++ b/net/bootp.c
>>> @@ -147,9 +147,14 @@ static void store_net_params(struct bootp_hdr *bp)
>>> {  #if !defined(CONFIG_BOOTP_SERVERIP)
>>>       struct in_addr tmp_ip;
>>> +     bool overwrite_serverip = true;
>>> +
>>> +#if defined(CONFIG_BOOTP_PREFER_SERVERIP)
>>> +     overwrite_serverip = false;
>>> +#endif
>>>
>>>       net_copy_ip(&tmp_ip, &bp->bp_siaddr);
>>> -     if (tmp_ip.s_addr != 0)
>>> +     if (tmp_ip.s_addr != 0 && (overwrite_serverip ||
>>> +!net_server_ip.s_addr))
>>>               net_copy_ip(&net_server_ip, &bp->bp_siaddr);
>>>       memcpy(net_server_ethaddr,
>>>              ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);
>>> --
>>> 2.12.3
>>
>> Hi Alex
>>
>> I have apply those two patchs and verify
>> U-Boot-1-2-net-Add-option-to-prefer-bootp-dhcp-serverip.patch
>> U-Boot-2-2-ax25-Switch-to-CONFIG_BOOTP_PREFER_SERVERIP.patch
>>
>> But it still fail in dhcp command as below
>>
>> case 1
>> serverip is null
>>
>> RISC-V # set serverip
>> RISC-V # env print
>> baudrate=38400
>> bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
>> bbl-ae350.bin;go 0x0
>> bootdelay=3
>> bootfile=pxelinux.0
>> ethact=mac@e0100000
>> fdtcontroladdr=3fedf290
>> fileaddr=600000
>> filesize=1bb7d34
>> stderr=serial@f0300000
>> stdin=serial@f0300000
>> stdout=serial@f0300000
>>
>> Environment size: 304/8188 bytes
>> RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
>
> You are explicitly setting the server IP in the DHCP command line, so
> why would you expect the DHCP server IP to be used?
>
>> BOOTP broadcast 1
>> BOOTP broadcast 2
>> BOOTP broadcast 3
>> BOOTP broadcast 4
>> DHCP client bound to address 10.0.4.191 (4603 ms)
>> Using mac@e0100000 device
>> TFTP from server 255.255.255.255; our IP address is 10.0.4.191;
>
> This broadcast address is clearly not right. It should have been what
> you had in the dhcp command. That should be assigned in net/tftp.c:
> 757...
>
>>>                         tftp_remote_ip = string_to_ip(net_boot_file_name);
>
> So something must be wrong with that somehow.
>
>> sending through gateway 10.0.4.254
>> Filename 'pxelinux.0'.
>
> Why is this filename still set? That's from the environment and is set
> by a env callback handler. That should happen before netboot_common()
> which should be overwriting net_boot_file_name with the command line
> filename.
>
>> Load address: 0x600000
>> Loading: *
>> TFTP error: 'File not found' (1)
>> Not retrying...
>>
>> TFTP error: 'File not found' (1)
>> Not retrying...
>
> Could you print out the env again? Did the server IP get set?
>
> Also, you not finding a file assumes your TFTP server has a pxelinux.0
> file on it... I don't think we have any reason from what you've
> presented that it's the case. Especially since that's not the file you
> specified on the command line.
>
>>
>> case 2
>> serverip has value
>>
>> RISC-V # setenv serverip 10.0.4.97 ;
>>
>> RISC-V # dhcp 0x600000 boomimage-310y-ag101p.bin
>> BOOTP broadcast 1
>> BOOTP broadcast 2
>> BOOTP broadcast 3
>> BOOTP broadcast 4
>> DHCP client bound to address 10.0.4.191 (4592 ms)
>> Using mac@e0100000 device
>> TFTP from server 10.0.4.97; our IP address is 10.0.4.191
>
> You are getting the correct IP used as the server, so it seems that
> this patch is working (though I don't know that your DHCP server isn't
> the same as the hard-coded address).
>
>> Filename 'pxelinux.0'.
>> Load address: 0x600000
>> Loading: *
>> TFTP error: 'File not found' (1)
>> Not retrying...
>>
>> TFTP error: 'File not found' (1)
>> Not retrying...
>> RISC-V #
>
> What version of U-Boot are you using? This behavior doesn't match what
> I'm seeing in the code.
>
> -Joe

Hi Joe

I sync to the latest U-Boot
and do the following experiments with ax25-ae350_defconfig

==================
The latest U-Boot
serverip is null
dhcp command pass
==================

U-Boot 2018.07-rc1-00071-g7868909 (Jun 13 2018 - 09:12:38 +0800)

DRAM:  1 GiB
No arch specific invalidate_icache_all available!
Flash: 64 MiB
MMC:   mmc@f0e00000: 0
Loading Environment from SPI Flash... SF: Detected mx25u1635e with
page size 256 Bytes, erase size 4 KiB, total 2 MiB
OK
In:    serial@f0300000
Out:   serial@f0300000
Err:   serial@f0300000
Net:   no alias for ethernet0

Warning: mac@e0100000 (eth0) using random MAC address - ca:96:e2:64:71:ac
eth0: mac@e0100000
Hit any key to stop autoboot:  0
RISC-V #
RISC-V # env print
baudrate=38400
bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
bbl-ae350.bin;go 0x0
bootdelay=3
ethact=mac@e0100000
fdtcontroladdr=3fee6290
fileaddr=600000
filesize=1bb7d34
serverip=10.0.4.97
stderr=serial@f0300000
stdin=serial@f0300000
stdout=serial@f0300000

Environment size: 303/8188 bytes
RISC-V # setenv serverip
RISC-V #
RISC-V # env print
baudrate=38400
bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
bbl-ae350.bin;go 0x0
bootdelay=3
ethact=mac@e0100000
fdtcontroladdr=3fee6290
fileaddr=600000
filesize=1bb7d34
stderr=serial@f0300000
stdin=serial@f0300000
stdout=serial@f0300000

Environment size: 282/8188 bytes
RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
DHCP client bound to address 10.0.4.171 (4602 ms)
Using mac@e0100000 device
TFTP from server 10.0.4.97; our IP address is 10.0.4.171
Filename 'boomimage-310y-ag101p.bin'.
Load address: 0x600000
Loading: #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         ########################################
         197.3 KiB/s
done
Bytes transferred = 13938796 (d4b06c hex)
RISC-V #

==================================================
The latest U-Boot apply Alex's 2 patchs
[PATCH 1/2] net: Add option to prefer bootp/dhcp serverip
[PATCH 2/2] ax25: Switch to CONFIG_BOOTP_PREFER_SERVERIP

No matter serverip is null or have server ip address
dhcp command fail
==================================================

U-Boot 2018.07-rc1-00072-ge8732dd-dirty (Jun 13 2018 - 09:26:15 +0800)

DRAM:  1 GiB
No arch specific invalidate_icache_all available!
Flash: 64 MiB
MMC:   mmc@f0e00000: 0
Loading Environment from SPI Flash... SF: Detected mx25u1635e with
page size 256 Bytes, erase size 4 KiB, total 2 MiB
OK
In:    serial@f0300000
Out:   serial@f0300000
Err:   serial@f0300000
Net:   no alias for ethernet0

Warning: mac@e0100000 (eth0) using random MAC address - ea:97:27:2b:a0:7c
eth0: mac@e0100000
Hit any key to stop autoboot:  0
RISC-V #
RISC-V # env print
baudrate=38400
bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
bbl-ae350.bin;go 0x0
bootdelay=3
ethact=mac@e0100000
fdtcontroladdr=3fee6290
fileaddr=600000
filesize=1bb7d34
serverip=10.0.4.97
stderr=serial@f0300000
stdin=serial@f0300000
stdout=serial@f0300000

Environment size: 303/8188 bytes
RISC-V #
RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
DHCP client bound to address 10.0.4.172 (4626 ms)
Using mac@e0100000 device
TFTP from server 10.0.4.97; our IP address is 10.0.4.172
Filename 'pxelinux.0'.
Load address: 0x600000
Loading: *
TFTP error: 'File not found' (1)
Not retrying...

TFTP error: 'File not found' (1)
Not retrying...



RISC-V # setenv serverip
RISC-V # env print
baudrate=38400
bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
bbl-ae350.bin;go 0x0
bootdelay=3
bootfile=pxelinux.0
ethact=mac@e0100000
fdtcontroladdr=3fee6290
fileaddr=600000
filesize=1bb7d34
stderr=serial@f0300000
stdin=serial@f0300000
stdout=serial@f0300000

Environment size: 304/8188 bytes
RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
DHCP client bound to address 10.0.4.172 (4604 ms)
Using mac@e0100000 device
TFTP from server 255.255.255.255; our IP address is 10.0.4.172;
sending through gateway 10.0.4.254
Filename 'pxelinux.0'.
Load address: 0x600000
Loading: *
TFTP error: 'File not found' (1)
Not retrying...

TFTP error: 'File not found' (1)
Not retrying...
RISC-V #

Rick
Rick Chen June 13, 2018, 8:46 a.m. UTC | #5
2018-06-13 9:42 GMT+08:00 Rick Chen <rickchen36@gmail.com>:
> 2018-06-13 3:59 GMT+08:00 Joe Hershberger <joe.hershberger@ni.com>:
>> On Wed, Jun 6, 2018 at 8:54 PM, Rick Chen <rickchen36@gmail.com> wrote:
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Wednesday, June 06, 2018 8:32 PM
>>>> To: u-boot@lists.denx.de
>>>> Cc: Rick Jian-Zhi Chen(陳建志); Joe Hershberger; Simon Glass
>>>> Subject: [PATCH 1/2] net: Add option to prefer bootp/dhcp serverip
>>>>
>>>> Currently we can choose between 2 different types of behavior for the serverip
>>>> variable:
>>>>
>>>>   1) Always overwrite it with the DHCP server IP address (default)
>>>>   2) Ignore what the DHCP server says (CONFIG_BOOTP_SERVERIP)
>>>>
>>>> This patch adds a 3rd option:
>>>>
>>>>   3) Use serverip from DHCP if no serverip is given
>>>>      (CONFIG_BOOTP_PREFER_SERVERIP)
>>>>
>>>> With this new option, we can have the default case that a boot file gets loaded
>>>> from the DHCP provided TFTP server work while allowing users to specify their
>>>> own serverip variable to explicitly use a different tftp server.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>>  README      | 5 +++++
>>>>  cmd/Kconfig | 9 +++++++++
>>>>  net/bootp.c | 7 ++++++-
>>>>  3 files changed, 20 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/README b/README
>>>> index fb331f910d..d8a99281ca 100644
>>>> --- a/README
>>>> +++ b/README
>>>> @@ -1511,10 +1511,15 @@ The following options need to be configured:
>>>>               CONFIG_BOOTP_TIMEOFFSET
>>>>               CONFIG_BOOTP_VENDOREX
>>>>               CONFIG_BOOTP_MAY_FAIL
>>>> +             CONFIG_BOOTP_PREFER_SERVERIP
>>>>
>>>>               CONFIG_BOOTP_SERVERIP - TFTP server will be the serverip
>>>>               environment variable, not the BOOTP server.
>>>>
>>>> +             CONFIG_BOOTP_PREFER_SERVERIP - TFTP server will be the
>>>> +             serverip environment variable if previously unset, otherwise
>>>> +             the DHCP provided serverip is used.
>>>> +
>>>>               CONFIG_BOOTP_MAY_FAIL - If the DHCP server is not found
>>>>               after the configured retry count, the call will fail
>>>>               instead of starting over.  This can be used to fail over diff --git
>>>> a/cmd/Kconfig b/cmd/Kconfig index e283cb9a8a..e77a4131b3 100644
>>>> --- a/cmd/Kconfig
>>>> +++ b/cmd/Kconfig
>>>> @@ -1121,6 +1121,15 @@ config BOOTP_HOSTNAME
>>>>       help
>>>>         The name may or may not be qualified with the local domain name.
>>>>
>>>> +config BOOTP_PREFER_SERVERIP
>>>> +     bool "Leave serverip variable in place if existing"
>>>> +     default n
>>>> +     depends on CMD_BOOTP
>>>> +     help
>>>> +       By default a BOOTP/DHCP reply will overwrite the tftp target ip
>>>> +       address. With this option enabled, it will leave it alone if
>>>> +       already specified, but populate it if no serverip is specified.
>>>> +
>>>>  config BOOTP_SUBNETMASK
>>>>       bool "Request & store 'netmask' from BOOTP/DHCP server"
>>>>       default y
>>>> diff --git a/net/bootp.c b/net/bootp.c
>>>> index 9d7cb5d30c..91de4cd426 100644
>>>> --- a/net/bootp.c
>>>> +++ b/net/bootp.c
>>>> @@ -147,9 +147,14 @@ static void store_net_params(struct bootp_hdr *bp)
>>>> {  #if !defined(CONFIG_BOOTP_SERVERIP)
>>>>       struct in_addr tmp_ip;
>>>> +     bool overwrite_serverip = true;
>>>> +
>>>> +#if defined(CONFIG_BOOTP_PREFER_SERVERIP)
>>>> +     overwrite_serverip = false;
>>>> +#endif
>>>>
>>>>       net_copy_ip(&tmp_ip, &bp->bp_siaddr);
>>>> -     if (tmp_ip.s_addr != 0)
>>>> +     if (tmp_ip.s_addr != 0 && (overwrite_serverip ||
>>>> +!net_server_ip.s_addr))
>>>>               net_copy_ip(&net_server_ip, &bp->bp_siaddr);
>>>>       memcpy(net_server_ethaddr,
>>>>              ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);
>>>> --
>>>> 2.12.3
>>>
>>> Hi Alex
>>>
>>> I have apply those two patchs and verify
>>> U-Boot-1-2-net-Add-option-to-prefer-bootp-dhcp-serverip.patch
>>> U-Boot-2-2-ax25-Switch-to-CONFIG_BOOTP_PREFER_SERVERIP.patch
>>>
>>> But it still fail in dhcp command as below
>>>
>>> case 1
>>> serverip is null
>>>
>>> RISC-V # set serverip
>>> RISC-V # env print
>>> baudrate=38400
>>> bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
>>> bbl-ae350.bin;go 0x0
>>> bootdelay=3
>>> bootfile=pxelinux.0
>>> ethact=mac@e0100000
>>> fdtcontroladdr=3fedf290
>>> fileaddr=600000
>>> filesize=1bb7d34
>>> stderr=serial@f0300000
>>> stdin=serial@f0300000
>>> stdout=serial@f0300000
>>>
>>> Environment size: 304/8188 bytes
>>> RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
>>
>> You are explicitly setting the server IP in the DHCP command line, so
>> why would you expect the DHCP server IP to be used?
>>
>>> BOOTP broadcast 1
>>> BOOTP broadcast 2
>>> BOOTP broadcast 3
>>> BOOTP broadcast 4
>>> DHCP client bound to address 10.0.4.191 (4603 ms)
>>> Using mac@e0100000 device
>>> TFTP from server 255.255.255.255; our IP address is 10.0.4.191;
>>
>> This broadcast address is clearly not right. It should have been what
>> you had in the dhcp command. That should be assigned in net/tftp.c:
>> 757...
>>
>>>>                         tftp_remote_ip = string_to_ip(net_boot_file_name);
>>
>> So something must be wrong with that somehow.
>>
>>> sending through gateway 10.0.4.254
>>> Filename 'pxelinux.0'.
>>
>> Why is this filename still set? That's from the environment and is set
>> by a env callback handler. That should happen before netboot_common()
>> which should be overwriting net_boot_file_name with the command line
>> filename.
>>
>>> Load address: 0x600000
>>> Loading: *
>>> TFTP error: 'File not found' (1)
>>> Not retrying...
>>>
>>> TFTP error: 'File not found' (1)
>>> Not retrying...
>>
>> Could you print out the env again? Did the server IP get set?
>>
>> Also, you not finding a file assumes your TFTP server has a pxelinux.0
>> file on it... I don't think we have any reason from what you've
>> presented that it's the case. Especially since that's not the file you
>> specified on the command line.
>>
>>>
>>> case 2
>>> serverip has value
>>>
>>> RISC-V # setenv serverip 10.0.4.97 ;
>>>
>>> RISC-V # dhcp 0x600000 boomimage-310y-ag101p.bin
>>> BOOTP broadcast 1
>>> BOOTP broadcast 2
>>> BOOTP broadcast 3
>>> BOOTP broadcast 4
>>> DHCP client bound to address 10.0.4.191 (4592 ms)
>>> Using mac@e0100000 device
>>> TFTP from server 10.0.4.97; our IP address is 10.0.4.191
>>
>> You are getting the correct IP used as the server, so it seems that
>> this patch is working (though I don't know that your DHCP server isn't
>> the same as the hard-coded address).
>>
>>> Filename 'pxelinux.0'.
>>> Load address: 0x600000
>>> Loading: *
>>> TFTP error: 'File not found' (1)
>>> Not retrying...
>>>
>>> TFTP error: 'File not found' (1)
>>> Not retrying...
>>> RISC-V #
>>
>> What version of U-Boot are you using? This behavior doesn't match what
>> I'm seeing in the code.
>>
>> -Joe
>
> Hi Joe
>
> I sync to the latest U-Boot
> and do the following experiments with ax25-ae350_defconfig
>
> ==================
> The latest U-Boot
> serverip is null
> dhcp command pass
> ==================
>
> U-Boot 2018.07-rc1-00071-g7868909 (Jun 13 2018 - 09:12:38 +0800)
>
> DRAM:  1 GiB
> No arch specific invalidate_icache_all available!
> Flash: 64 MiB
> MMC:   mmc@f0e00000: 0
> Loading Environment from SPI Flash... SF: Detected mx25u1635e with
> page size 256 Bytes, erase size 4 KiB, total 2 MiB
> OK
> In:    serial@f0300000
> Out:   serial@f0300000
> Err:   serial@f0300000
> Net:   no alias for ethernet0
>
> Warning: mac@e0100000 (eth0) using random MAC address - ca:96:e2:64:71:ac
> eth0: mac@e0100000
> Hit any key to stop autoboot:  0
> RISC-V #
> RISC-V # env print
> baudrate=38400
> bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
> bbl-ae350.bin;go 0x0
> bootdelay=3
> ethact=mac@e0100000
> fdtcontroladdr=3fee6290
> fileaddr=600000
> filesize=1bb7d34
> serverip=10.0.4.97
> stderr=serial@f0300000
> stdin=serial@f0300000
> stdout=serial@f0300000
>
> Environment size: 303/8188 bytes
> RISC-V # setenv serverip
> RISC-V #
> RISC-V # env print
> baudrate=38400
> bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
> bbl-ae350.bin;go 0x0
> bootdelay=3
> ethact=mac@e0100000
> fdtcontroladdr=3fee6290
> fileaddr=600000
> filesize=1bb7d34
> stderr=serial@f0300000
> stdin=serial@f0300000
> stdout=serial@f0300000
>
> Environment size: 282/8188 bytes
> RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4
> DHCP client bound to address 10.0.4.171 (4602 ms)
> Using mac@e0100000 device
> TFTP from server 10.0.4.97; our IP address is 10.0.4.171
> Filename 'boomimage-310y-ag101p.bin'.
> Load address: 0x600000
> Loading: #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          ########################################
>          197.3 KiB/s
> done
> Bytes transferred = 13938796 (d4b06c hex)
> RISC-V #
>
> ==================================================
> The latest U-Boot apply Alex's 2 patchs
> [PATCH 1/2] net: Add option to prefer bootp/dhcp serverip
> [PATCH 2/2] ax25: Switch to CONFIG_BOOTP_PREFER_SERVERIP
>
> No matter serverip is null or have server ip address
> dhcp command fail
> ==================================================
>
> U-Boot 2018.07-rc1-00072-ge8732dd-dirty (Jun 13 2018 - 09:26:15 +0800)
>
> DRAM:  1 GiB
> No arch specific invalidate_icache_all available!
> Flash: 64 MiB
> MMC:   mmc@f0e00000: 0
> Loading Environment from SPI Flash... SF: Detected mx25u1635e with
> page size 256 Bytes, erase size 4 KiB, total 2 MiB
> OK
> In:    serial@f0300000
> Out:   serial@f0300000
> Err:   serial@f0300000
> Net:   no alias for ethernet0
>
> Warning: mac@e0100000 (eth0) using random MAC address - ea:97:27:2b:a0:7c
> eth0: mac@e0100000
> Hit any key to stop autoboot:  0
> RISC-V #
> RISC-V # env print
> baudrate=38400
> bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
> bbl-ae350.bin;go 0x0
> bootdelay=3
> ethact=mac@e0100000
> fdtcontroladdr=3fee6290
> fileaddr=600000
> filesize=1bb7d34
> serverip=10.0.4.97
> stderr=serial@f0300000
> stdin=serial@f0300000
> stdout=serial@f0300000
>
> Environment size: 303/8188 bytes
> RISC-V #
> RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4
> DHCP client bound to address 10.0.4.172 (4626 ms)
> Using mac@e0100000 device
> TFTP from server 10.0.4.97; our IP address is 10.0.4.172
> Filename 'pxelinux.0'.
> Load address: 0x600000
> Loading: *
> TFTP error: 'File not found' (1)
> Not retrying...
>
> TFTP error: 'File not found' (1)
> Not retrying...
>
>
>
> RISC-V # setenv serverip
> RISC-V # env print
> baudrate=38400
> bootcmd=fatload mmc 0:1 0x20000000 ae350_64.dtb;fatload mmc 0:1 0x0
> bbl-ae350.bin;go 0x0
> bootdelay=3
> bootfile=pxelinux.0
> ethact=mac@e0100000
> fdtcontroladdr=3fee6290
> fileaddr=600000
> filesize=1bb7d34
> stderr=serial@f0300000
> stdin=serial@f0300000
> stdout=serial@f0300000
>
> Environment size: 304/8188 bytes
> RISC-V # dhcp 0x600000 10.0.4.97:boomimage-310y-ag101p.bin
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4
> DHCP client bound to address 10.0.4.172 (4604 ms)
> Using mac@e0100000 device
> TFTP from server 255.255.255.255; our IP address is 10.0.4.172;
> sending through gateway 10.0.4.254
> Filename 'pxelinux.0'.
> Load address: 0x600000
> Loading: *
> TFTP error: 'File not found' (1)
> Not retrying...
>
> TFTP error: 'File not found' (1)
> Not retrying...
> RISC-V #
>
> Rick


Hi Joe and Alex


After trace, I found that in store_net_params( )
if CONFIG_BOOTP_SERVERIP is not define,
net_boot_file_name will be replaced by bp->bp_file

copy_filename(net_boot_file_name, bp->bp_file,sizeof(net_boot_file_name));

That is why the file name will become pxelinux.0, after remove
CONFIG_BOOTP_SERVERIP.

But I try to find where pxelinux.0 came from.
It seems come from packet payload in dhcp_handler( ).


Rick
Joe Hershberger June 13, 2018, 10:41 p.m. UTC | #6
On Wed, Jun 13, 2018 at 3:46 AM, Rick Chen <rickchen36@gmail.com> wrote:
> Hi Joe and Alex
>
>
> After trace, I found that in store_net_params( )
> if CONFIG_BOOTP_SERVERIP is not define,
> net_boot_file_name will be replaced by bp->bp_file
>
> copy_filename(net_boot_file_name, bp->bp_file,sizeof(net_boot_file_name));
>
> That is why the file name will become pxelinux.0, after remove
> CONFIG_BOOTP_SERVERIP.
>
> But I try to find where pxelinux.0 came from.
> It seems come from packet payload in dhcp_handler( ).
>

OK, that makes sense. It seems this will need to change as part of
this patch. Thanks for tracking it down.

>
> Rick
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
diff mbox series

Patch

diff --git a/README b/README
index fb331f910d..d8a99281ca 100644
--- a/README
+++ b/README
@@ -1511,10 +1511,15 @@  The following options need to be configured:
 		CONFIG_BOOTP_TIMEOFFSET
 		CONFIG_BOOTP_VENDOREX
 		CONFIG_BOOTP_MAY_FAIL
+		CONFIG_BOOTP_PREFER_SERVERIP
 
 		CONFIG_BOOTP_SERVERIP - TFTP server will be the serverip
 		environment variable, not the BOOTP server.
 
+		CONFIG_BOOTP_PREFER_SERVERIP - TFTP server will be the
+		serverip environment variable if previously unset, otherwise
+		the DHCP provided serverip is used.
+
 		CONFIG_BOOTP_MAY_FAIL - If the DHCP server is not found
 		after the configured retry count, the call will fail
 		instead of starting over.  This can be used to fail over
diff --git a/cmd/Kconfig b/cmd/Kconfig
index e283cb9a8a..e77a4131b3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1121,6 +1121,15 @@  config BOOTP_HOSTNAME
 	help
 	  The name may or may not be qualified with the local domain name.
 
+config BOOTP_PREFER_SERVERIP
+	bool "Leave serverip variable in place if existing"
+	default n
+	depends on CMD_BOOTP
+	help
+	  By default a BOOTP/DHCP reply will overwrite the tftp target ip
+	  address. With this option enabled, it will leave it alone if
+	  already specified, but populate it if no serverip is specified.
+
 config BOOTP_SUBNETMASK
 	bool "Request & store 'netmask' from BOOTP/DHCP server"
 	default y
diff --git a/net/bootp.c b/net/bootp.c
index 9d7cb5d30c..91de4cd426 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -147,9 +147,14 @@  static void store_net_params(struct bootp_hdr *bp)
 {
 #if !defined(CONFIG_BOOTP_SERVERIP)
 	struct in_addr tmp_ip;
+	bool overwrite_serverip = true;
+
+#if defined(CONFIG_BOOTP_PREFER_SERVERIP)
+	overwrite_serverip = false;
+#endif
 
 	net_copy_ip(&tmp_ip, &bp->bp_siaddr);
-	if (tmp_ip.s_addr != 0)
+	if (tmp_ip.s_addr != 0 && (overwrite_serverip || !net_server_ip.s_addr))
 		net_copy_ip(&net_server_ip, &bp->bp_siaddr);
 	memcpy(net_server_ethaddr,
 	       ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);