[U-Boot] Read-only env variables

[Joe Hershberger]

On Wed, May 12, 2010 at 4:34 AM, Wolfgang Denk <wd at denx.de> wrote:
>> > There has been discussion about such a feature in the past. I even
>> > tried to define the user interface for such a command set, but
>> > unfortunately I didn't have enough time myself and nobody else took
>> > the bait either.
>>
>> Ok... if you can dig up the user interface you were thinking of, I can
>> compare it to what I've done and possibly tweak it.  As it is now, I
>> intend to use the acl to define which variables are preserved.  The
>> command takes no arguments and changes only the in-RAM environment.
>
> See here:
>
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/36570/focus=36861

Looks like a reasonable user interface as far as the command layout.

> Taking this one step further, we should then also rename "editenv"
> into "env edit".

Why rename one existing command?  Why not rename askenv and the rest?
I don't think I would rename any of them or I would have a clear
separation and reason for renaming some and not others.

>> > Be careful. You are defining policy here, and actually you can do all
>> > this already now.
>>
>> I want the acl to be the one defining this policy.
>
> But this is IMHO wrong. The ACl should describe properties, i. e. if a
> variable can be written or not etc.  It should NEVER try to define how
> the user deploys the properties of a variable.

It's a tool for the user to implement his chosen policy.  Instead of
writing a script to keep track of variables that have changed (that he
doesn't want to save) and having to call saveenv through a wrapper
script to process that list, it could be as simple as him adding that
to the ACL.

>> > Use "setenv ipaddr ...; saveenv" if the user wishes to change the
>> > static IP and write it to flash.
>>
>> Works great in a vacuum.
>
> ? What do you mean here?

Sorry... Nevermind.  Things are complicated by netconsole, but aren't
addressed by this anyway.

>> > Use "dhcp" (without "saveenv") to use dhcp to configure all the
>> > network settings without saving this to flash.
>>
>> In the case of dhcp, the key is that I want to be able to save some
>> unrelated change to the environment... I shouldn't be prevented from
>> doing it because I happened to configure the network already.
>
> I don't see your problem. If you want to define a default IP address
> and want to use this as "ipaddr" environment variable unless DHCP
> overwrites this value, then you can easily do that:
>
>        => setenv default_ipaddr aa.bb.cc.dd
>        => setenv default_netmask ...
>        ...
>        => setenv net_default 'setenv ipaddr ${default_ipaddr};setenv netmask ${default_netmask};...'
>        => setenv preboot run net_default

This is a reasonable approach and one that I have considered.

> How would you in your szenario, where "dhcp" can change the "ipaddr"
> environment variable, but a saveenv would not save it, set a new
> "ipaddr"?

Because the acl is editable, the user could simply change the lifetime
of the variable.  For best ease of use for this case, I've been
thinking of something like having the dhcp command mark the variables
it touches as volatile (see the bottom of this email) in the acl when
it touches them, and if they are touched by the setenv command line
interface, the volatile attribute is removed.  I wouldn't want all
variables that are marked as volatile to no longer be volatile when
accessed by setenv, so I would simply define a separate type called
auto-volatile that the dhcp command would utilize instead.  If setenv
command line sets an auto-volatile variable, then that variable loses
its volatile attribute.

> Me thinks you are adding complexity here without need, plus potential
> for lots of confusion.

It's true that you could currently choose to never call many of these
commands directly and always have wrapper scripts, but that's pretty
tedious and prevents you from having a familiar interface that comes
with using the commands directly.  I know I personally would take a
while to train myself to type "run saveenvcmd" instead of "saveenv".

While some things can be worked around, it doesn't mean they are a
easy to use.  Also, all behavior stays unchanged if you don't set the
ACL.  Like many things in u-boot, if a user is not interested, they
can just ignore the feature.

I'm curious what you think will be confusing?

>> > Well, then just do the saveenv _before_ you use DHCP. Or be careful
>>
>> Not possible for the use case... I'm using netconsole, so dhcp must be
>> first to make the console accessible... so no change made to the env
>> could ever be saved.
>
> OK, then do as described above and consider the variables set by DHCP
> as volatile and do not attempt to use these to store your default
> settings that are supposed to be persistent.

Again, I considered this possibility and have it on the back burner if
for some reason this is difficult to implement, but I don't like to
commit variables to flash that shouldn't be.

>> All of this stems from the fact that a number of env vars are magical.
>>  Things like "ipaddr" and "stdin" etc. are hooked up to internals of
>> u-boot.  In the case of stdin and family, the behavior (without
>> CONFIG_SYS_CONSOLE_IS_IN_ENV) is that those variables only affect the
>> run-time behavior and are not written to flash.  I'm certain that the
>> reason for that is the same as the issue I'm having with the network
>
> No, the reason is more likely that the one who implemented this did
> just what he actually needed, without a clear concept for such a
> global scope. The next one who needed different behaviour added
> CONFIG_SYS_CONSOLE_IS_IN_ENV. Now we have both.

I guess my point is that the same need existed for other sub-systems,
and could be implemented generically with this in a consistent way
rather than one-off hacks for each case.

>> configuration.  I think there should be a simple-to-use way to
>> indicate that a magical env var should be only magical and not in
>> flash or possibly initialized from flash but not saved.  Since I'm
>
> I disagree. Please let the end user decide what we wants to do with
> variable settings - wether he wants to save them or not.

Exactly.  I'd like to give the user a simple way to chose which he
wants to save or not.  He should not need to manage it by writing
scripts that attempt to save off variables and restore them after the
env is written to flash.  Right now the user cannot decide.

>> making an ACL, it seemed reasonable to implement that behavior
>> generically.  The list behavior idea was an attempt at making the acl
>> easier to change, since it would need to be changed (by users,
>> scripts, and u-boot code) for several different use cases.
>
> Ok, se your original intention was a scriptable "env edit" command,
> which might (or might not) be based on some internal list
> representation - but ther eis no need to introduce such a list to the
> user interface.

Fair enough.  Since the list management behavior was just to make
easier what I envisioned would be tedious, I've tabled that until I
try it out.

--------------

I've implemented the acl for type and access specification.  It
currently looks like this:

The first char in the attributes for a var in the acl is the type.  It can be:

    s - string (default)
    d - decimal
    x - hexadecimal
    b - boolean
    i - ip address
    m - ethernet mac address

The second char in the attributes for a var in the acl is the access
mode.  It can be:

    a - any (default, optional)
    r - read-only
    o - set once (this is create / change default)

I'm envisioning a third char in the attributes that is the lifetime.  It can be:

    p - precious (not removed on env default)
    v - volatile (not saved to flash)
    a - auto-volatile (not saved to flash, auto-cleared by setenv from
command line)

The fw_setenv Linux utility and the setenv command have the same
behavior and share the same code for implementing the acl.

In my initial playing with it, the experience feels like too much of
the detail that we want controlled is put in your face.  Things that
should just be a given such as the fact that ipaddr and ethaddr and a
number of other variables should have a certain type or be set-once or
whatever.  I think what will make it more reasonable to use is to have
a compiled in baseline acl, with a env var based augmentation.  That
way the user doesn't have to look at all the things that should just
always be.  It seems to be pretty much required from a usability
standpoint.

I'm also interested if you think it should be a compile time option to
support the ACL at all or if it should be added as a first class
citizen to permanently take the place of the curent hard-coded
behaviors.  I currently have it disabled at compile time (in u-boot,
not fw_env), but I also have not removed the other options it could
replace.

Best regards,
-Joe