[U-Boot] is there any issue with creating and using more than one hashtable?
short version: migrating target board from legacy OS to linux and, in a fixed address in flash, there is a legacy "bootline" of the form
var=value var=value ... etc etc ...
that i want to convert to u-boot compatible values such as ipaddr, gatewayip, serverip and so on.
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.
in the end, i end up with a small hashtable of u-boot compatible values, which i dump by walking through that second hashtable:
Walking populated bootline_htab_2... k: [gatewayip], d: [192.168.1.1] k: [netmask], d: [255.255.255.0] k: [ipaddr], d: [192.168.1.2]
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.
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.
thoughts? is there some weird way that values from one hashtable can "bleed" into another one? i'm baffled.
rday
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...
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.
For a test, I would like to ask you to run the following commands beofre 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?
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.
These variables get set through the U_BOOT_ENV_CALLBACK functionality, i. e. as a callback whenever the corresponfing 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 variable sof 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.
It is obvious that the U-Boot design did not anticipate a situation where an alternative hash table with different settings would be created.
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.
Best regards,
Wolfgang Denk
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
On Sat, 1 Oct 2016, Wolfgang Denk wrote:
Dear Robert,
... snip ...
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.
These variables get set through the U_BOOT_ENV_CALLBACK functionality, i. e. as a callback whenever the corresponfing 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 variable sof 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.
just to follow up, the simple solution for me would be to *not* create that second hashtable with recognized u-boot variable names like "ipaddr", "gatewayip", "serverip" unless i absolutely plan on using those new values.
an even simpler solution would be to skip the second hashtable creation altogether, walk through my first hashtable (which should have no conflicting variable names), and in one step, translate those names and add them to the current environment.
this is what i use to add/modify a new env variable -- is this the correct approach, to add it to the "env_htab" hashtable?
int add_entry_to_env(ENTRY* e) { ENTRY* ep;
printf("Adding stuff k: [%s], d: [%s] to env.\n", e->key, e->data); hsearch_r(*e, ENTER, &ep, &env_htab, 0);
return 0; }
if that's the way to do it, i'll just do away with the second hashtable and avoid the issue altogether.
rday
Dear Robert,
In message alpine.LFD.2.20.1610010456050.5186@localhost.localdomain you wrote:
244 /* If there is a callback, call it */ 245 if (htab->table[idx].entry.callback && 246 htab->table[idx].entry.callback(item.key,
...
just to follow up, the simple solution for me would be to *not* create that second hashtable with recognized u-boot variable names like "ipaddr", "gatewayip", "serverip" unless i absolutely plan on using those new values.
Well, I guess you don't need the callback funtionality for your purposes, and you will probably not pull the data from the second hash table back into the primary one, so why can you not just clear the "callback" for any entries you process?
That would suppress the unwanted actions...
this is what i use to add/modify a new env variable -- is this the correct approach, to add it to the "env_htab" hashtable?
int add_entry_to_env(ENTRY* e) { ENTRY* ep;
printf("Adding stuff k: [%s], d: [%s] to env.\n", e->key, e->data);
Insert: e->callback = NULL;
here?
Best regards,
Wolfgang Denk
On Sat, 1 Oct 2016, Wolfgang Denk wrote:
Dear Robert,
In message alpine.LFD.2.20.1610010456050.5186@localhost.localdomain you wrote:
244 /* If there is a callback, call it */ 245 if (htab->table[idx].entry.callback && 246 htab->table[idx].entry.callback(item.key,
...
just to follow up, the simple solution for me would be to *not* create that second hashtable with recognized u-boot variable names like "ipaddr", "gatewayip", "serverip" unless i absolutely plan on using those new values.
Well, I guess you don't need the callback funtionality for your purposes, and you will probably not pull the data from the second hash table back into the primary one, so why can you not just clear the "callback" for any entries you process?
That would suppress the unwanted actions...
this is what i use to add/modify a new env variable -- is this the correct approach, to add it to the "env_htab" hashtable?
int add_entry_to_env(ENTRY* e) { ENTRY* ep;
printf("Adding stuff k: [%s], d: [%s] to env.\n", e->key, e->data);Insert:
e->callback = NULL;
here?
that would be one solution, yes. i assume i would, of course, set that callback before adding the entry to the hashtable; otherwise, wouldn't do much good.
ok, i think we've settled this, thanks.
rday
On Sat, 1 Oct 2016, Wolfgang Denk wrote:
... snip ...
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.
These variables get set through the U_BOOT_ENV_CALLBACK functionality, i. e. as a callback whenever the corresponfing 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 }
... snip ...
i'm going to assume other net-related commands have this behaviour as well, yes? like ping? which would explain why ping'ing even the alleged gateway didn't work in this context.
rday
Dear Robert,
In message alpine.LFD.2.20.1610020318410.4079@localhost.localdomain you wrote:
i'm going to assume other net-related commands have this behaviour as well, yes? like ping? which would explain why ping'ing even the alleged gateway didn't work in this context.
Yes. This is common network code.
Best regards,
Wolfgang Denk
participants (2)
-
Robert P. J. Day -
Wolfgang Denk