[v2,3/3] ax25: Switch to CONFIG_BOOTP_PREFER_SERVERIP

Message ID 20180614100426.40511-4-agraf@suse.de
State Superseded
Headers show
Series
  • net: Sanitize DHCP variable override
Related show

Commit Message

Alexander Graf June 14, 2018, 10:04 a.m.
The ax25-ae350 target currently uses CONFIG_BOOTP_SERVERIP which means we
ignore the DHCP provided TFTP ip address. This breaks every case where we
do now provide a serverip environment variable.

Instead, let's use the new CONFIG_BOOT_PREFER_SERVERIP option to fall back
to the DHCP provided TFTP IP if no serverip environment variable is set.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 configs/ax25-ae350_defconfig | 1 +
 include/configs/ax25-ae350.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

Comments

Joe Hershberger June 14, 2018, 4:58 p.m. | #1
On Thu, Jun 14, 2018 at 5:04 AM, Alexander Graf <agraf@suse.de> wrote:
> The ax25-ae350 target currently uses CONFIG_BOOTP_SERVERIP which means we
> ignore the DHCP provided TFTP ip address. This breaks every case where we
> do now provide a serverip environment variable.
>
> Instead, let's use the new CONFIG_BOOT_PREFER_SERVERIP option to fall back
> to the DHCP provided TFTP IP if no serverip environment variable is set.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

> ---
>  configs/ax25-ae350_defconfig | 1 +
>  include/configs/ax25-ae350.h | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig
> index fc04c87485..a328555af6 100644
> --- a/configs/ax25-ae350_defconfig
> +++ b/configs/ax25-ae350_defconfig
> @@ -40,3 +40,4 @@ CONFIG_DM_SPI=y
>  CONFIG_ATCSPI200_SPI=y
>  CONFIG_TIMER=y
>  CONFIG_ATCPIT100_TIMER=y
> +CONFIG_BOOTP_PREFER_SERVERIP=y
> diff --git a/include/configs/ax25-ae350.h b/include/configs/ax25-ae350.h
> index b1ca5ac11a..b230896734 100644
> --- a/include/configs/ax25-ae350.h
> +++ b/include/configs/ax25-ae350.h
> @@ -11,7 +11,6 @@
>   * CPU and Board Configuration Options
>   */
>  #define CONFIG_BOOTP_SEND_HOSTNAME
> -#define CONFIG_BOOTP_SERVERIP

Feel like moving this to Kconfig?

>
>  /*
>   * Miscellaneous configurable options
> --
> 2.12.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Alexander Graf June 15, 2018, 8:24 a.m. | #2
On 14.06.18 18:58, Joe Hershberger wrote:
> On Thu, Jun 14, 2018 at 5:04 AM, Alexander Graf <agraf@suse.de> wrote:
>> The ax25-ae350 target currently uses CONFIG_BOOTP_SERVERIP which means we
>> ignore the DHCP provided TFTP ip address. This breaks every case where we
>> do now provide a serverip environment variable.
>>
>> Instead, let's use the new CONFIG_BOOT_PREFER_SERVERIP option to fall back
>> to the DHCP provided TFTP IP if no serverip environment variable is set.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
> 
>> ---
>>  configs/ax25-ae350_defconfig | 1 +
>>  include/configs/ax25-ae350.h | 1 -
>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig
>> index fc04c87485..a328555af6 100644
>> --- a/configs/ax25-ae350_defconfig
>> +++ b/configs/ax25-ae350_defconfig
>> @@ -40,3 +40,4 @@ CONFIG_DM_SPI=y
>>  CONFIG_ATCSPI200_SPI=y
>>  CONFIG_TIMER=y
>>  CONFIG_ATCPIT100_TIMER=y
>> +CONFIG_BOOTP_PREFER_SERVERIP=y
>> diff --git a/include/configs/ax25-ae350.h b/include/configs/ax25-ae350.h
>> index b1ca5ac11a..b230896734 100644
>> --- a/include/configs/ax25-ae350.h
>> +++ b/include/configs/ax25-ae350.h
>> @@ -11,7 +11,6 @@
>>   * CPU and Board Configuration Options
>>   */
>>  #define CONFIG_BOOTP_SEND_HOSTNAME
>> -#define CONFIG_BOOTP_SERVERIP
> 
> Feel like moving this to Kconfig?

I would actually prefer to remove it altogether ;)


Alex
Joe Hershberger June 15, 2018, 8:08 p.m. | #3
On Fri, Jun 15, 2018 at 3:24 AM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 14.06.18 18:58, Joe Hershberger wrote:
>> On Thu, Jun 14, 2018 at 5:04 AM, Alexander Graf <agraf@suse.de> wrote:
>>> The ax25-ae350 target currently uses CONFIG_BOOTP_SERVERIP which means we
>>> ignore the DHCP provided TFTP ip address. This breaks every case where we
>>> do now provide a serverip environment variable.
>>>
>>> Instead, let's use the new CONFIG_BOOT_PREFER_SERVERIP option to fall back
>>> to the DHCP provided TFTP IP if no serverip environment variable is set.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>>
>>> ---
>>>  configs/ax25-ae350_defconfig | 1 +
>>>  include/configs/ax25-ae350.h | 1 -
>>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig
>>> index fc04c87485..a328555af6 100644
>>> --- a/configs/ax25-ae350_defconfig
>>> +++ b/configs/ax25-ae350_defconfig
>>> @@ -40,3 +40,4 @@ CONFIG_DM_SPI=y
>>>  CONFIG_ATCSPI200_SPI=y
>>>  CONFIG_TIMER=y
>>>  CONFIG_ATCPIT100_TIMER=y
>>> +CONFIG_BOOTP_PREFER_SERVERIP=y
>>> diff --git a/include/configs/ax25-ae350.h b/include/configs/ax25-ae350.h
>>> index b1ca5ac11a..b230896734 100644
>>> --- a/include/configs/ax25-ae350.h
>>> +++ b/include/configs/ax25-ae350.h
>>> @@ -11,7 +11,6 @@
>>>   * CPU and Board Configuration Options
>>>   */
>>>  #define CONFIG_BOOTP_SEND_HOSTNAME
>>> -#define CONFIG_BOOTP_SERVERIP
>>
>> Feel like moving this to Kconfig?
>
> I would actually prefer to remove it altogether ;)

I'm with you, actually... though I think the behavior should be to
always ignore the DHCP server's settings when they are on the command
line or in the environment. If you want the DHCP server's info, the
user's script can remove the variables explicitly.

>
> Alex
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Alexander Graf June 15, 2018, 8:33 p.m. | #4
On 15.06.18 22:08, Joe Hershberger wrote:
> On Fri, Jun 15, 2018 at 3:24 AM, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 14.06.18 18:58, Joe Hershberger wrote:
>>> On Thu, Jun 14, 2018 at 5:04 AM, Alexander Graf <agraf@suse.de> wrote:
>>>> The ax25-ae350 target currently uses CONFIG_BOOTP_SERVERIP which means we
>>>> ignore the DHCP provided TFTP ip address. This breaks every case where we
>>>> do now provide a serverip environment variable.
>>>>
>>>> Instead, let's use the new CONFIG_BOOT_PREFER_SERVERIP option to fall back
>>>> to the DHCP provided TFTP IP if no serverip environment variable is set.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>
>>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>>>
>>>> ---
>>>>  configs/ax25-ae350_defconfig | 1 +
>>>>  include/configs/ax25-ae350.h | 1 -
>>>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig
>>>> index fc04c87485..a328555af6 100644
>>>> --- a/configs/ax25-ae350_defconfig
>>>> +++ b/configs/ax25-ae350_defconfig
>>>> @@ -40,3 +40,4 @@ CONFIG_DM_SPI=y
>>>>  CONFIG_ATCSPI200_SPI=y
>>>>  CONFIG_TIMER=y
>>>>  CONFIG_ATCPIT100_TIMER=y
>>>> +CONFIG_BOOTP_PREFER_SERVERIP=y
>>>> diff --git a/include/configs/ax25-ae350.h b/include/configs/ax25-ae350.h
>>>> index b1ca5ac11a..b230896734 100644
>>>> --- a/include/configs/ax25-ae350.h
>>>> +++ b/include/configs/ax25-ae350.h
>>>> @@ -11,7 +11,6 @@
>>>>   * CPU and Board Configuration Options
>>>>   */
>>>>  #define CONFIG_BOOTP_SEND_HOSTNAME
>>>> -#define CONFIG_BOOTP_SERVERIP
>>>
>>> Feel like moving this to Kconfig?
>>
>> I would actually prefer to remove it altogether ;)
> 
> I'm with you, actually... though I think the behavior should be to
> always ignore the DHCP server's settings when they are on the command
> line or in the environment. If you want the DHCP server's info, the
> user's script can remove the variables explicitly.

Yeah, the only case I can think of where the current model could hurt us
is if you run "dhcp" and then call "saveenv". Because that saveenv will
contain all those glorious variables that may now override the next dhcp
request.

Btw, you wouldn't happen to work with Julia? ;)

Alex
Joe Hershberger June 15, 2018, 8:36 p.m. | #5
On Fri, Jun 15, 2018 at 3:33 PM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 15.06.18 22:08, Joe Hershberger wrote:
>> On Fri, Jun 15, 2018 at 3:24 AM, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>> On 14.06.18 18:58, Joe Hershberger wrote:
>>>> On Thu, Jun 14, 2018 at 5:04 AM, Alexander Graf <agraf@suse.de> wrote:
>>>>> The ax25-ae350 target currently uses CONFIG_BOOTP_SERVERIP which means we
>>>>> ignore the DHCP provided TFTP ip address. This breaks every case where we
>>>>> do now provide a serverip environment variable.
>>>>>
>>>>> Instead, let's use the new CONFIG_BOOT_PREFER_SERVERIP option to fall back
>>>>> to the DHCP provided TFTP IP if no serverip environment variable is set.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>
>>>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>>>>
>>>>> ---
>>>>>  configs/ax25-ae350_defconfig | 1 +
>>>>>  include/configs/ax25-ae350.h | 1 -
>>>>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig
>>>>> index fc04c87485..a328555af6 100644
>>>>> --- a/configs/ax25-ae350_defconfig
>>>>> +++ b/configs/ax25-ae350_defconfig
>>>>> @@ -40,3 +40,4 @@ CONFIG_DM_SPI=y
>>>>>  CONFIG_ATCSPI200_SPI=y
>>>>>  CONFIG_TIMER=y
>>>>>  CONFIG_ATCPIT100_TIMER=y
>>>>> +CONFIG_BOOTP_PREFER_SERVERIP=y
>>>>> diff --git a/include/configs/ax25-ae350.h b/include/configs/ax25-ae350.h
>>>>> index b1ca5ac11a..b230896734 100644
>>>>> --- a/include/configs/ax25-ae350.h
>>>>> +++ b/include/configs/ax25-ae350.h
>>>>> @@ -11,7 +11,6 @@
>>>>>   * CPU and Board Configuration Options
>>>>>   */
>>>>>  #define CONFIG_BOOTP_SEND_HOSTNAME
>>>>> -#define CONFIG_BOOTP_SERVERIP
>>>>
>>>> Feel like moving this to Kconfig?
>>>
>>> I would actually prefer to remove it altogether ;)
>>
>> I'm with you, actually... though I think the behavior should be to
>> always ignore the DHCP server's settings when they are on the command
>> line or in the environment. If you want the DHCP server's info, the
>> user's script can remove the variables explicitly.
>
> Yeah, the only case I can think of where the current model could hurt us
> is if you run "dhcp" and then call "saveenv". Because that saveenv will
> contain all those glorious variables that may now override the next dhcp
> request.

I think in that case you could explicitly clean up those variables...
I've intended to implement ephemeral variables anyway... this is one
of the reasons.

> Btw, you wouldn't happen to work with Julia? ;)

Not directly, but yes. :)

> Alex
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Alexander Graf June 15, 2018, 8:39 p.m. | #6
On 15.06.18 22:36, Joe Hershberger wrote:
> On Fri, Jun 15, 2018 at 3:33 PM, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 15.06.18 22:08, Joe Hershberger wrote:
>>> On Fri, Jun 15, 2018 at 3:24 AM, Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>>
>>>> On 14.06.18 18:58, Joe Hershberger wrote:
>>>>> On Thu, Jun 14, 2018 at 5:04 AM, Alexander Graf <agraf@suse.de> wrote:
>>>>>> The ax25-ae350 target currently uses CONFIG_BOOTP_SERVERIP which means we
>>>>>> ignore the DHCP provided TFTP ip address. This breaks every case where we
>>>>>> do now provide a serverip environment variable.
>>>>>>
>>>>>> Instead, let's use the new CONFIG_BOOT_PREFER_SERVERIP option to fall back
>>>>>> to the DHCP provided TFTP IP if no serverip environment variable is set.
>>>>>>
>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>
>>>>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>>>>>
>>>>>> ---
>>>>>>  configs/ax25-ae350_defconfig | 1 +
>>>>>>  include/configs/ax25-ae350.h | 1 -
>>>>>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig
>>>>>> index fc04c87485..a328555af6 100644
>>>>>> --- a/configs/ax25-ae350_defconfig
>>>>>> +++ b/configs/ax25-ae350_defconfig
>>>>>> @@ -40,3 +40,4 @@ CONFIG_DM_SPI=y
>>>>>>  CONFIG_ATCSPI200_SPI=y
>>>>>>  CONFIG_TIMER=y
>>>>>>  CONFIG_ATCPIT100_TIMER=y
>>>>>> +CONFIG_BOOTP_PREFER_SERVERIP=y
>>>>>> diff --git a/include/configs/ax25-ae350.h b/include/configs/ax25-ae350.h
>>>>>> index b1ca5ac11a..b230896734 100644
>>>>>> --- a/include/configs/ax25-ae350.h
>>>>>> +++ b/include/configs/ax25-ae350.h
>>>>>> @@ -11,7 +11,6 @@
>>>>>>   * CPU and Board Configuration Options
>>>>>>   */
>>>>>>  #define CONFIG_BOOTP_SEND_HOSTNAME
>>>>>> -#define CONFIG_BOOTP_SERVERIP
>>>>>
>>>>> Feel like moving this to Kconfig?
>>>>
>>>> I would actually prefer to remove it altogether ;)
>>>
>>> I'm with you, actually... though I think the behavior should be to
>>> always ignore the DHCP server's settings when they are on the command
>>> line or in the environment. If you want the DHCP server's info, the
>>> user's script can remove the variables explicitly.
>>
>> Yeah, the only case I can think of where the current model could hurt us
>> is if you run "dhcp" and then call "saveenv". Because that saveenv will
>> contain all those glorious variables that may now override the next dhcp
>> request.
> 
> I think in that case you could explicitly clean up those variables...
> I've intended to implement ephemeral variables anyway... this is one
> of the reasons.

I'll be happy to pass this one on to you then :). I am already too deep
down into rabbit holes.

>> Btw, you wouldn't happen to work with Julia? ;)
> 
> Not directly, but yes. :)

Awesome, just met her last weekend ;)


Alex

Patch

diff --git a/configs/ax25-ae350_defconfig b/configs/ax25-ae350_defconfig
index fc04c87485..a328555af6 100644
--- a/configs/ax25-ae350_defconfig
+++ b/configs/ax25-ae350_defconfig
@@ -40,3 +40,4 @@  CONFIG_DM_SPI=y
 CONFIG_ATCSPI200_SPI=y
 CONFIG_TIMER=y
 CONFIG_ATCPIT100_TIMER=y
+CONFIG_BOOTP_PREFER_SERVERIP=y
diff --git a/include/configs/ax25-ae350.h b/include/configs/ax25-ae350.h
index b1ca5ac11a..b230896734 100644
--- a/include/configs/ax25-ae350.h
+++ b/include/configs/ax25-ae350.h
@@ -11,7 +11,6 @@ 
  * CPU and Board Configuration Options
  */
 #define CONFIG_BOOTP_SEND_HOSTNAME
-#define CONFIG_BOOTP_SERVERIP
 
 /*
  * Miscellaneous configurable options