[PATCH] udhcpc: add support for long options
Martin Lewis
martin.lewis.x84 at gmail.com
Sat Aug 15 15:35:17 UTC 2020
Tested both regular options and long options against ISC's dhcpd, looks
good.
Martin
On Thu, 13 Aug 2020 at 10:00, Denys Vlasenko <vda.linux at googlemail.com>
wrote:
> On Tue, Aug 4, 2020 at 5:25 PM Martin Lewis <martin.lewis.x84 at gmail.com>
> wrote:
> > Duplicate options are currently overridden (only the last option is
> kept).
> > This leads to unexpected behavior when using long options.
> >
> > The patch adds support for long options in compliance with RFC 3396.
> >
> > Fixes #13136.
> >
> > function old new delta
> > udhcp_run_script 704 765 +61
> > optitem_unset_env_and_free - 31 +31
> > static.xmalloc_optname_optval 837 833 -4
> > putenvp 60 52 -8
> >
> ------------------------------------------------------------------------------
> > (add/remove: 1/0 grow/shrink: 1/2 up/down: 92/-12) Total: 80
> bytes
> > text data bss dec hex filename
> > 994091 16939 1872 1012902 f74a6 busybox_old
> > 994171 16939 1872 1012982 f74f6 busybox_unstripped
> >
> > Signed-off-by: Martin Lewis <martin.lewis.x84 at gmail.com>
> > ---
> > networking/udhcp/dhcpc.c | 125
> ++++++++++++++++++++++++++++++++++-------------
> > networking/udhcp/dhcpc.h | 1 +
> > 2 files changed, 93 insertions(+), 33 deletions(-)
> >
> > diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
> > index 50dfead63..d4a7a825e 100644
> > --- a/networking/udhcp/dhcpc.c
> > +++ b/networking/udhcp/dhcpc.c
> > @@ -115,6 +115,13 @@ enum {
> >
> >
> > /*** Script execution code ***/
> > +struct dhcp_optitem
> > +{
> > + unsigned len;
> > + uint8_t *data;
> > + char *env;
> > + uint8_t code;
> > +};
> >
> > /* get a rough idea of how long an option will be (rounding up...) */
> > static const uint8_t len_of_option_as_string[] ALIGN1 = {
> > @@ -186,15 +193,15 @@ static int good_hostname(const char *name)
> > #endif
> >
> > /* Create "opt_name=opt_value" string */
> > -static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const
> struct dhcp_optflag *optflag, const char *opt_name)
> > +static NOINLINE char *xmalloc_optname_optval(const struct dhcp_optitem
> *opt_item, const struct dhcp_optflag *optflag, const char *opt_name)
> > {
> > unsigned upper_length;
> > int len, type, optlen;
> > char *dest, *ret;
> > + uint8_t *option;
> >
> > - /* option points to OPT_DATA, need to go back to get OPT_LEN */
> > - len = option[-OPT_DATA + OPT_LEN];
> > -
> > + option = opt_item->data;
> > + len = opt_item->len;
> > type = optflag->flags & OPTION_TYPE_MASK;
> > optlen = dhcp_option_lengths[type];
> > upper_length = len_of_option_as_string[type]
> > @@ -386,11 +393,50 @@ static NOINLINE char
> *xmalloc_optname_optval(uint8_t *option, const struct dhcp_
> > return ret;
> > }
> >
> > -static void putenvp(llist_t **envp, char *new_opt)
> > +/* Used by static options (interface, siaddr, etc) */
> > +static void putenvp(char *new_opt)
> > {
> > + struct dhcp_optitem *opt_item;
> > + opt_item = xzalloc(sizeof(struct dhcp_optitem));
> > + /* Set opt_item->code = 0, so it won't appear in concat_option's
> lookup. */
> > + opt_item->env = new_opt;
> > putenv(new_opt);
> > - log2(" %s", new_opt);
> > - llist_add_to(envp, new_opt);
> > + llist_add_to(&client_data.envp, opt_item);
> > +}
>
> This removed logging of environment
>
>
> > +/* Support RFC3396 Long Encoded Options */
> > +static struct dhcp_optitem *concat_option(uint8_t *data, uint8_t len,
> uint8_t code)
> > +{
> > + llist_t *item;
> > + struct dhcp_optitem *opt_item;
> > + unsigned new_len = len;
> > +
> > + /* Check if an option with the code already exists.
> > + * A possible optimization is to create a bitmap of all existing
> options in the packet,
> > + * and iterate over the option list only if they exist.
> > + * This will result in bigger code, and because dhcp packets
> don't have too many options it
> > + * shouldn't have a big impact on performance.
> > + */
> > + for (item = client_data.envp; item != NULL; item = item->link) {
> > + opt_item = (struct dhcp_optitem *)item->data;
> > + if (opt_item->code == code)
> > + break;
> > + }
> > +
> > + if (item) {
> > + /* This is a duplicate, concatenate data according to
> RFC 3396 */
> > + new_len += opt_item->len;
> > + } else {
> > + /* This is a new option, add a new dhcp_optitem to the
> list */
> > + opt_item = xzalloc(sizeof(struct dhcp_optitem));
> > + opt_item->code = code;
> > + llist_add_to(&client_data.envp, opt_item);
> > + }
> > +
> > + opt_item->data = xrealloc(opt_item->data, new_len); /* xrealloc
> on the first occurrence (NULL) will call malloc */
> > + memcpy(opt_item->data + opt_item->len, data, len);
> > + opt_item->len = new_len;
> > + return opt_item;
> > }
>
> This will create a copy for every option, duplicated or not.
> Most options are not dups, I propose to track this status and only
> make copies as needed:
> most of the time, we won't make any:
>
> +static struct dhcp_optitem *concat_option(uint8_t *data, uint8_t len,
> uint8_t code)
> +{
> + llist_t *item;
> + struct dhcp_optitem *opt_item;
> +
> + /* Check if an option with the code already exists.
> + * A possible optimization is to create a bitmap of all
> existing options in the packet,
> + * and iterate over the option list only if they exist.
> + * This will result in bigger code, and because dhcp packets
> don't have too many options it
> + * shouldn't have a big impact on performance.
> + */
> + for (item = client_data.envp; item != NULL; item = item->link) {
> + opt_item = (struct dhcp_optitem *)item->data;
> + if (opt_item->code == code) {
> + /* This option was seen already, concatenate */
> + uint8_t *new_data;
> +
> + new_data = xmalloc(len + opt_item->len);
> + memcpy(
> + mempcpy(new_data, opt_item->data,
> opt_item->len),
> + data, len
> + );
> + opt_item->len += len;
> + if (opt_item->malloced)
> + free(opt_item->data);
> + opt_item->malloced = 1;
> + opt_item->data = new_data;
> + return opt_item;
> + }
> + }
> +
> + /* This is a new option, add a new dhcp_optitem to the list */
> + opt_item = xzalloc(sizeof(*opt_item));
> + opt_item->code = code;
> + /* opt_item->malloced = 0 */
> + opt_item->data = data;
> + opt_item->len = len;
> + llist_add_to(&client_data.envp, opt_item);
> + return opt_item;
> }
>
> Applying with this change. Please test current git.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20200815/5ad03622/attachment.html>
More information about the busybox
mailing list