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