[U-Boot] [RFC PATCH v2 7/7] env: Automatically mark dynamic configuration info as "do not export"

[Simon Glass]

Hi Bernhard,

On 22 November 2016 at 15:30, Bernhard Nortmann
<bernhard.nortmann at web.de> wrote:
> Hi Simon!
>
>
> Am 19.11.2016 um 14:47 schrieb Simon Glass:
>>
>> Hi Bernhard,
>>
>> On 16 November 2016 at 03:30, Bernhard Nortmann
>> <bernhard.nortmann at web.de> wrote:
>>>
>>> This is an attempt to prevent such information from ending up
>>> in exported environment data, especially when doing "saveenv".
>>> (http://lists.denx.de/pipermail/u-boot/2015-September/227611.html)
>>>
>>> The patch makes use of the new setenv_transient() helper for
>>> environment variables that get updated via network configuration:
>>> BOOTP/DHCP (netboot_update_env), CDP (cdp_update_env) and
>>> link-local protocol (do_link_local).
>>>
>>> Signed-off-by: Bernhard Nortmann <bernhard.nortmann at web.de>
>>> ---
>>>
>>> Changes in v2: None
>>>
>>>   cmd/net.c | 34 +++++++++++++++++-----------------
>>>   1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> How would you make these variables non-transient? Does the transient
>> nature show up in 'printenv'?
>>
>> Regards,
>> Simon
>
>
> I think it's best to consider a practical example. I'm firing up my
> sunxi-based
> device with these patches applied on top of U-Boot v2016.11, and interrupt
> the
> autoboot.
>
> The standard way to examine variables that have non-default flags is the
> "env
> flags" command. In my case this outputs (discarding the explanations near
> the
> top):
>
> Static flags:
>         Variable Name        Variable Type        Variable Access
>         -------------        -------------        ---------------
>         eth\d?addr           MAC address          write-once
>         ipaddr               IP address           any
>         gatewayip            IP address           any
>         netmask              IP address           any
>         serverip             IP address           any
>         nvlan                decimal              any
>         vlan                 decimal              any
>         dnsip                IP address           any
>         serial#              string               write-once
>         fel_booted           boolean              system
>         fel_scriptaddr       hexadecimal          system
>
> Active flags:
>         Variable Name        Variable Type        Variable Access
>         -------------        -------------        ---------------
>         serial#              string               write-once
>         ethaddr              MAC address          write-once
>         fel_booted           boolean              system
>
> As you can see, patch 5/7 has sneaked in two new "system" flags and since
> I've
> booted using the FEL mechanism, the "fel_booted" one is in effect
> ("active").
>
> Okay, now I use the command "setenv autoload no; dhcp" to auto-configure
> networking. Let's check "env flags" again. The "Static flags" section is
> unchanged, but at the bottom there's now:
>
> Active flags:
>         Variable Name        Variable Type        Variable Access
>         -------------        -------------        ---------------
>         ipaddr               IP address           transient
>         serial#              string               write-once
>         hostname             string               transient
>         ethaddr              MAC address          write-once
>         netmask              IP address           transient
>         serverip             IP address           transient
>         fel_booted           boolean              system
>         dnsip                IP address           transient
>         gatewayip            IP address           transient
>
>>Does the transient nature show up in 'printenv'?
>
> No, it doesn't. "printenv" will give output that looks as usual / perfectly
> normal. You're also not restricted in modifying "transient" variables, for
> example I can do "ipaddr 10.11.12.13" with no problems.
>
> But: The differences will become apparent when trying to export the
> environment
> with something like "saveenv" or "env export":
> => env export 0x40000000
> ## Don't export "ipaddr"
> ## Don't export "hostname"
> ## Don't export "netmask"
> ## Don't export "serverip"
> ## Don't export "fel_booted"
> ## Don't export "dnsip"
> ## Don't export "gatewayip"
>
> U-Boot refuses to store the 'volatile' information for those variables that
> have been flagged accordingly, and informs the user about it. This is
> checked
> individually per variable, i.e. the other ones will get exported normally.
>
> At this point I decide that I want to set up a *static* IP configuration and
> make it persistent with "saveenv". Houston, we have a problem!
>
>>How would you make these variables non-transient?
>
> Well, again there's a standard U-Boot mechanism for this. The README
> mentions
> it in the description of CONFIG_ENV_FLAGS_LIST_STATIC: "To override a
> setting
> in the static list, simply add an entry for the same variable name to the
> '.flags' variable.".
>
> So we could do "setenv .flags ipaddr:i,netmask:i,gatewayip.i" to reset the
> variables to access level "any", meaning they'll get exported again.
>
> Admittedly, there's a slight inconsistency here. Due to the fact that
> setenv_transient() only sets a temporary flag, and U-Boot will rebuild the
> access flags list (from the "static" default one) when assigning ".flags",
> the "transient" flags for the networking variables will get cleared anyway -
> even when deleting ".flags".
>
> The alternative would be to mark these variables "transient" from the
> beginning
> (in the static flags), but I considered that more awkward - when taking into
> account that users might want to setup (and "saveenv") the network manually
> right from the start, and are less likely to override the values retrieved
> via
> auto-configuration (DHCP etc.).

Thanks for the explanation. It looks fine to me.

Regards,
Simon