Thank you for applying the commits! I started implementing support for RFC 3396, hopefully it will be ready soon.
Martin On Mon, 29 Jun 2020 at 08:40, Denys Vlasenko <[email protected]> wrote: > 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
