On Sat, 1 Oct 2016, Wolfgang Denk wrote:
Dear Robert,
In message alpine.LFD.2.20.1609301101260.11297@localhost.localdomain you wrote:
in misc_init_r(), i create a new hashtable to just grab the contents of that bootline as is. i then create a *second* hashtable and selectively move over just those keys i want from the first hashtable, possibly renaming them in the process.
I'm not sure how you implemented this "move over" thing...
oh, nothing fancy, it's manual ... if i find an initial string of "g=a.b.c.d", i translate that to the u-boot required "gatewayip" variable name, and add that to the second hashtable. here's a snippet of the routine that populates the second hash table given an ENTRY corresponding to an original "var=value" -- you can see that it just checks possible initial keys, and writes the transformed value to the second table -- pure brute force, and it seems to work just fine:
populate_htab_2(ENTRY* e) { const char* k = e->key; ENTRY* entry_ptr;
if (!strcmp(k, "h")) { ENTRY he = { .key = "serverip", .data = e->data }; printf("found h, translating to serverip.\n"); hsearch_r(he, ENTER, &entry_ptr, &bootline_htab_2, 0); }
if (!strcmp(k, "g")) { ENTRY ge = { .key = "gatewayip", .data = e->data }; printf("found g, translating to gatewayip.\n"); hsearch_r(ge, ENTER, &entry_ptr, &bootline_htab_2, 0); }
... snip ...
now here's the thing ... after all that, i manually check whether the CRC for the env table in persistent storage is valid and, if it is, i do *nothing* with all that hashtable content -- that was all wasted cycles, but it should not do any harm.
but it does, because this is what happens:
TFTP from server 10.35.5.37; our IP address is 192.168.1.2; sending through gateway 192.168.1.1
the server address is correct, but the IP address and gateway are totally wrong -- they should have been untouched and been completely different values; instead, they now equal the values in that second hashtable, which i chose to do nothing with.
one thing i want to emphasize about the above -- the example i was working with pulled out and transformed just the ip address and gateway loading them into the second hashtable, and those are the values that are now "incorrect." there was no server setting in that original data, hence "serverip" was not loaded into the second hashtable, so it's fine. it seems that only u-boot variables i put in the second hashtable somehow "bleed" into my working environment, whatever that might mean.
For a test, I would like to ask you to run the following commands before the TFTP command:
printenv serverip;setenv serverip ${serverip};printenv serverip printenv ipaddr;setenv ipaddr ${ipaddr};printenv ipaddr printenv gatewayip;setenv gatewayip ${gatewayip};printenv gatewayip
Does this change anything?
i'll have to wait until later to try this, but i'm pretty sure i tried the above yesterday, and everything looked fine ... i would "print" the variables, they seemed fine, i'd "env save", i'd reset, check again, all looked good, and yet ... same problem.
wait, maybe i didn't explicitlt "setenv" in the middle, because it didn't seem necessary. that might be something i didn't test.
is there some weird issue with re-entrancy here? if i print "ipaddr" and "gatewayip", they seem correct. if i "md" the environment in persistent storage, again, they seem correct. and yet, when TFTP kicks in, it seems to pick up the values from that hashtable.
Actually the TFTP will not access the environment directly to determine the boot parameters like bootfile, ipaddr, gatewayip, netmask, serverip, etc.; instead, it uses internal variables. So your environment settings for "ipaddr" and "gatewayip" may contain totally different values than the variables net_ip resp. net_gateway which get used by the TFTP code.
ah ... would this also apply to other net-related commands like ping? because, weirdly, even ping doesn't seem to work, unable to ping even its own gateway, but if that command is *also* quietly picking up the incorrect "gatewayip" setting from that second hashtable, that would also explain its failure.
These variables get set through the U_BOOT_ENV_CALLBACK functionality, i. e. as a callback whenever the corresponding variable gets set. "setting" here means that the value in the hash table gets changed - see function _compare_and_overwrite_entry() in "hashtable.c":
244 /* If there is a callback, call it */ 245 if (htab->table[idx].entry.callback && 246 htab->table[idx].entry.callback(item.key, 247 item.data, env_op_overwrite, flag)) { 248 debug("callback() rejected setting variable " 249 "%s, skipping it!\n", item.key); 250 __set_errno(EINVAL); 251 *retval = NULL; 252 return 0; 253 }
So when you insert variables of these registered names into your alternative hash table using the common code, then the respective callbacks will fire in the same way as if you had changed the environment settings through a setenv command.
oh ... crap ... that would appear to be the issue, then.
It is obvious that the U-Boot design did not anticipate a situation where an alternative hash table with different settings would be created.
i will admit i didn't see any documentation that told me explicitly that i *could* have additional hashtables; i just assumed, hey, it's just another data structure, it has some useful functionality, i'll just create another couple of them to save me some coding.
i *did* try to verify that there was no shared data; things like making sure "strdup" was used to avoid sharing things like strings. i never thought of checking the callback structure.
A quick but ugly workaround could be to re-set the variables to the values stored in the environment by running something like
setenv ipaddr ${ipaddr}
etc.
Hope this helps.
massively, thanks. it just means that my attempt at cleverness in using a couple more hashtables wasn't as clever as i thought.
rday