<div dir="ltr">Thank you for applying the commits!<br>I started implementing support for RFC 3396, hopefully it will be ready soon.<br><br>Martin<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 29 Jun 2020 at 08:40, Denys Vlasenko <<a href="mailto:vda.linux@googlemail.com">vda.linux@googlemail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Applied, thanks!!!!<br>
<br>
On Tue, Jun 23, 2020 at 3:41 PM Martin Lewis <<a href="mailto:martin.lewis.x84@gmail.com" target="_blank">martin.lewis.x84@gmail.com</a>> wrote:<br>
><br>
> fill_envp now iterates over the packet only once instead of a few hundred times<br>
> using the new option scanner.<br>
><br>
> Signed-off-by: Martin Lewis <<a href="mailto:martin.lewis.x84@gmail.com" target="_blank">martin.lewis.x84@gmail.com</a>><br>
> ---<br>
> networking/udhcp/dhcpc.c | 201 ++++++++++++++++++++---------------------------<br>
> 1 file changed, 87 insertions(+), 114 deletions(-)<br>
><br>
> diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c<br>
> index 102178a4f..2669428e1 100644<br>
> --- a/networking/udhcp/dhcpc.c<br>
> +++ b/networking/udhcp/dhcpc.c<br>
> @@ -386,59 +386,81 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_<br>
> return ret;<br>
> }<br>
><br>
> +static void putenvp(llist_t **envp, char *new_opt)<br>
> +{<br>
> + putenv(new_opt);<br>
> + llist_add_to(envp, new_opt);<br>
> +}<br>
> +<br>
> +static int is_unknown_opt(uint8_t code, const struct dhcp_optflag **dh, const char **opt_name)<br>
> +{<br>
> + *opt_name = "";<br>
> +<br>
> + /* Find the option:<br>
> + * dhcp_optflags is sorted so we stop searching when dh->code >= code, which is faster<br>
> + * than iterating over the entire array.<br>
> + * Options which don't have a match in dhcp_option_strings[], e.g DHCP_REQUESTED_IP,<br>
> + * are located after the sorted array, so these entries will never be reached<br>
> + * and they'll count as unknown options.<br>
> + */<br>
> + for (*dh = dhcp_optflags; (*dh)->code && (*dh)->code < code; (*dh)++);<br>
> +<br>
> + if ((*dh)->code == code)<br>
> + *opt_name = nth_string(dhcp_option_strings, (*dh - dhcp_optflags));<br>
> +<br>
> + return (*dh)->code > code;<br>
> +}<br>
> +<br>
> /* put all the parameters into the environment */<br>
> -static char **fill_envp(struct dhcp_packet *packet)<br>
> +static llist_t *fill_envp(struct dhcp_packet *packet)<br>
> {<br>
> - int envc;<br>
> - int i;<br>
> - char **envp, **curr;<br>
> - const char *opt_name;<br>
> - uint8_t *temp;<br>
> - uint8_t overload = 0;<br>
> -<br>
> -#define BITMAP unsigned<br>
> -#define BBITS (sizeof(BITMAP) * 8)<br>
> -#define BMASK(i) (1 << (i & (sizeof(BITMAP) * 8 - 1)))<br>
> -#define FOUND_OPTS(i) (found_opts[(unsigned)i / BBITS])<br>
> - BITMAP found_opts[256 / BBITS];<br>
> -<br>
> - memset(found_opts, 0, sizeof(found_opts));<br>
> -<br>
> - /* We need 7 elements for:<br>
> - * "interface=IFACE"<br>
> - * "ip=N.N.N.N" from packet->yiaddr<br>
> - * "giaddr=IP" from packet->gateway_nip (unless 0)<br>
> - * "siaddr=IP" from packet->siaddr_nip (unless 0)<br>
> - * "boot_file=FILE" from packet->file (unless overloaded)<br>
> - * "sname=SERVER_HOSTNAME" from packet->sname (unless overloaded)<br>
> - * terminating NULL<br>
> - */<br>
> - envc = 7;<br>
> - /* +1 element for each option, +2 for subnet option: */<br>
> - if (packet) {<br>
> - /* note: do not search for "pad" (0) and "end" (255) options */<br>
> -//TODO: change logic to scan packet _once_<br>
> - for (i = 1; i < 255; i++) {<br>
> - temp = udhcp_get_option(packet, i);<br>
> - if (temp) {<br>
> - if (i == DHCP_OPTION_OVERLOAD)<br>
> - overload |= *temp;<br>
> - else if (i == DHCP_SUBNET)<br>
> - envc++; /* for $mask */<br>
> - envc++;<br>
> - /*if (i != DHCP_MESSAGE_TYPE)*/<br>
> - FOUND_OPTS(i) |= BMASK(i);<br>
> - }<br>
> - }<br>
> - }<br>
> - curr = envp = xzalloc(sizeof(envp[0]) * envc);<br>
> + char *new_opt;<br>
> + uint8_t *optptr;<br>
> + const struct dhcp_optflag *dh;<br>
> + struct dhcp_scan_state scan_state;<br>
> + const char *opt_name = "";<br>
> + llist_t *envp = NULL;<br>
><br>
> - *curr = xasprintf("interface=%s", client_data.interface);<br>
> - putenv(*curr++);<br>
> + new_opt = xasprintf("interface=%s", client_data.interface);<br>
> + putenvp(&envp, new_opt);<br>
><br>
> if (!packet)<br>
> return envp;<br>
><br>
> + init_scan_state(packet, &scan_state);<br>
> +<br>
> + /* Iterate over the packet options.<br>
> + * Handle each option based on whether it's an unknown / known option.<br>
> + * There may be (although unlikely) duplicate options. For now, only the last<br>
> + * appearing option will be stored in the environment, and all duplicates<br>
> + * are freed properly.<br>
> + * Long options may be implemented in the future (see RFC 3396) if needed.<br>
> + */<br>
> + while ((optptr = udhcp_scan_options(packet, &scan_state)) != NULL) {<br>
> + uint8_t code = optptr[OPT_CODE];<br>
> + uint8_t len = optptr[OPT_LEN];<br>
> + uint8_t *data = optptr + OPT_DATA;<br>
> +<br>
> + if (is_unknown_opt(code, &dh, &opt_name)) {<br>
> + unsigned ofs;<br>
> +<br>
> + new_opt = xmalloc(sizeof("optNNN=") + 1 + len*2);<br>
> + ofs = sprintf(new_opt, "opt%u=", code);<br>
> + *bin2hex(new_opt + ofs, (char *)data, len) = '\0';<br>
> + putenvp(&envp, new_opt);<br>
> + } else {<br>
> + new_opt = xmalloc_optname_optval(data, dh, opt_name);<br>
> + putenvp(&envp, new_opt);<br>
> +<br>
> + if (code == DHCP_SUBNET && len == 4) {<br>
> + uint32_t subnet;<br>
> + move_from_unaligned32(subnet, data);<br>
> + new_opt = xasprintf("mask=%u", mton(subnet));<br>
> + putenvp(&envp, new_opt);<br>
> + }<br>
> + }<br>
> + }<br>
> +<br>
> /* Export BOOTP fields. Fields we don't (yet?) export:<br>
> * uint8_t op; // always BOOTREPLY<br>
> * uint8_t htype; // hardware address type. 1 = 10mb ethernet<br>
> @@ -452,77 +474,31 @@ static char **fill_envp(struct dhcp_packet *packet)<br>
> * uint8_t chaddr[16]; // link-layer client hardware address (MAC)<br>
> */<br>
> /* Most important one: yiaddr as $ip */<br>
> - *curr = xmalloc(sizeof("ip=255.255.255.255"));<br>
> - sprint_nip(*curr, "ip=", (uint8_t *) &packet->yiaddr);<br>
> - putenv(*curr++);<br>
> + new_opt = xmalloc(sizeof("ip=255.255.255.255"));<br>
> + sprint_nip(new_opt, "ip=", (uint8_t *) &packet->yiaddr);<br>
> + putenvp(&envp, new_opt);<br>
> +<br>
> if (packet->siaddr_nip) {<br>
> /* IP address of next server to use in bootstrap */<br>
> - *curr = xmalloc(sizeof("siaddr=255.255.255.255"));<br>
> - sprint_nip(*curr, "siaddr=", (uint8_t *) &packet->siaddr_nip);<br>
> - putenv(*curr++);<br>
> + new_opt = xmalloc(sizeof("siaddr=255.255.255.255"));<br>
> + sprint_nip(new_opt, "siaddr=", (uint8_t *) &packet->siaddr_nip);<br>
> + putenvp(&envp, new_opt);<br>
> }<br>
> if (packet->gateway_nip) {<br>
> /* IP address of DHCP relay agent */<br>
> - *curr = xmalloc(sizeof("giaddr=255.255.255.255"));<br>
> - sprint_nip(*curr, "giaddr=", (uint8_t *) &packet->gateway_nip);<br>
> - putenv(*curr++);<br>
> + new_opt = xmalloc(sizeof("giaddr=255.255.255.255"));<br>
> + sprint_nip(new_opt, "giaddr=", (uint8_t *) &packet->gateway_nip);<br>
> + putenvp(&envp, new_opt);<br>
> }<br>
> - if (!(overload & FILE_FIELD) && packet->file[0]) {<br>
> + if (!(scan_state.overload & FILE_FIELD) && packet->file[0]) {<br>
> /* watch out for invalid packets */<br>
> - *curr = xasprintf("boot_file=%."DHCP_PKT_FILE_LEN_STR"s", packet->file);<br>
> - putenv(*curr++);<br>
> + new_opt = xasprintf("boot_file=%."DHCP_PKT_FILE_LEN_STR"s", packet->file);<br>
> + putenvp(&envp, new_opt);<br>
> }<br>
> - if (!(overload & SNAME_FIELD) && packet->sname[0]) {<br>
> + if (!(scan_state.overload & SNAME_FIELD) && packet->sname[0]) {<br>
> /* watch out for invalid packets */<br>
> - *curr = xasprintf("sname=%."DHCP_PKT_SNAME_LEN_STR"s", packet->sname);<br>
> - putenv(*curr++);<br>
> - }<br>
> -<br>
> - /* Export known DHCP options */<br>
> - opt_name = dhcp_option_strings;<br>
> - i = 0;<br>
> - while (*opt_name) {<br>
> - uint8_t code = dhcp_optflags[i].code;<br>
> - BITMAP *found_ptr = &FOUND_OPTS(code);<br>
> - BITMAP found_mask = BMASK(code);<br>
> - if (!(*found_ptr & found_mask))<br>
> - goto next;<br>
> - *found_ptr &= ~found_mask; /* leave only unknown options */<br>
> - temp = udhcp_get_option(packet, code);<br>
> - *curr = xmalloc_optname_optval(temp, &dhcp_optflags[i], opt_name);<br>
> - putenv(*curr++);<br>
> - if (code == DHCP_SUBNET && temp[-OPT_DATA + OPT_LEN] == 4) {<br>
> - /* Subnet option: make things like "$ip/$mask" possible */<br>
> - uint32_t subnet;<br>
> - move_from_unaligned32(subnet, temp);<br>
> - *curr = xasprintf("mask=%u", mton(subnet));<br>
> - putenv(*curr++);<br>
> - }<br>
> - next:<br>
> - opt_name += strlen(opt_name) + 1;<br>
> - i++;<br>
> - }<br>
> - /* Export unknown options */<br>
> - for (i = 0; i < 256;) {<br>
> - BITMAP bitmap = FOUND_OPTS(i);<br>
> - if (!bitmap) {<br>
> - i += BBITS;<br>
> - continue;<br>
> - }<br>
> - if (bitmap & BMASK(i)) {<br>
> - unsigned len, ofs;<br>
> -<br>
> - temp = udhcp_get_option(packet, i);<br>
> - /* udhcp_get_option returns ptr to data portion,<br>
> - * need to go back to get len<br>
> - */<br>
> - len = temp[-OPT_DATA + OPT_LEN];<br>
> - *curr = xmalloc(sizeof("optNNN=") + 1 + len*2);<br>
> - ofs = sprintf(*curr, "opt%u=", i);<br>
> - *bin2hex(*curr + ofs, (void*) temp, len) = '\0';<br>
> - putenv(*curr++);<br>
> - }<br>
> - i++;<br>
> + new_opt = xasprintf("sname=%."DHCP_PKT_SNAME_LEN_STR"s", packet->sname);<br>
> + putenvp(&envp, new_opt);<br>
> }<br>
><br>
> return envp;<br>
> @@ -531,7 +507,7 @@ static char **fill_envp(struct dhcp_packet *packet)<br>
> /* Call a script with a par file and env vars */<br>
> static void udhcp_run_script(struct dhcp_packet *packet, const char *name)<br>
> {<br>
> - char **envp, **curr;<br>
> + llist_t *envp;<br>
> char *argv[3];<br>
><br>
> envp = fill_envp(packet);<br>
> @@ -543,11 +519,8 @@ static void udhcp_run_script(struct dhcp_packet *packet, const char *name)<br>
> argv[2] = NULL;<br>
> spawn_and_wait(argv);<br>
><br>
> - for (curr = envp; *curr; curr++) {<br>
> - log2(" %s", *curr);<br>
> - bb_unsetenv_and_free(*curr);<br>
> - }<br>
> - free(envp);<br>
> + /* Free all allocated environment variables */<br>
> + llist_free(envp, (void (*)(void *))bb_unsetenv_and_free);<br>
> }<br>
><br>
><br>
> --<br>
> 2.11.0<br>
><br>
> _______________________________________________<br>
> busybox mailing list<br>
> <a href="mailto:busybox@busybox.net" target="_blank">busybox@busybox.net</a><br>
> <a href="http://lists.busybox.net/mailman/listinfo/busybox" rel="noreferrer" target="_blank">http://lists.busybox.net/mailman/listinfo/busybox</a><br>
</blockquote></div>