On Wed, May 27, 2026 at 9:46 AM David Gibson wrote: > On Tue, May 26, 2026 at 06:01:12PM +0530, Anshu Kumari wrote: > > A user can enter lots of options in command-line which may not fit in > > existing buffer, So when the options field is full, overflow remaining > > DHCP options into the file and sname fields per RFC 2132 option 52. > > > > Also, when the file field is not used for overload, copy the boot > > file URL there directly for legacy PXE clients. > > > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > > Signed-off-by: Anshu Kumari > > --- > > v2: > > - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME > constants > > - Added comment documenting space reservation: /* Reserve 3 bytes for > option 52 */ > > - Fixed DNS search length: sizeof(m->o) only, not combined with > file+sname > > - Removed dhcp_boot references — reply.file copy now reads from > opts[67] > > - Used DHCP_OVERLOAD_FILE constant in reply.file guard > > --- > > dhcp.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 75 insertions(+), 9 deletions(-) > > > > diff --git a/dhcp.c b/dhcp.c > > index e126063..a49a05a 100644 > > --- a/dhcp.c > > +++ b/dhcp.c > > @@ -306,14 +306,59 @@ static bool fill_one(uint8_t *buf, size_t size, > int o, int *offset) > > return false; > > } > > > > +#define DHCP_OVERLOAD_FILE 1 > > +#define DHCP_OVERLOAD_SNAME 2 > > IIUC, these values are from the RFC - might be worth referencing the > relevant section in a comment to make it clear they can't be changed > arbitrarily. > > > + > > +/** > > + * fill_overflow() - Fill remaining options into file and sname fields > > + * @m: Message whose file/sname fields may be used for > overflow > > + * > > + * Return: option 52 overload value: 0 if no overflow, > > + * DHCP_OVERLOAD_FILE for file, DHCP_OVERLOAD_SNAME for sname, > > + * or both OR'd together > > + */ > > +static int fill_overflow(struct msg *m) > > +{ > > + int file_off = 0, sname_off = 0, overload = 0; > > + int o; > > + > > + for (o = 0; o < 255; o++) { > > You could use ARRAY_size(opts) instead of raw 255, yes? > > > + if (opts[o].slen == -1 || opts[o].sent) > > + continue; > > + fill_one(m->file, sizeof(m->file) - 1, o, &file_off); > > + } > > + > > + for (o = 0; o < 255; o++) { > > + if (opts[o].slen == -1 || opts[o].sent) > > + continue; > > + if (fill_one(m->sname, sizeof(m->sname) - 1, o, > &sname_off)) > > + debug("DHCP: skipping option %i (overload full)", > o); > > + } > > + > > + if (file_off) { > > + m->file[file_off] = 255; > > + overload |= DHCP_OVERLOAD_FILE; > > + } > > + > > + if (sname_off) { > > + m->sname[sname_off] = 255; > > + overload |= DHCP_OVERLOAD_SNAME; > > + } > > + > > + return overload; > > +} > > + > > /** > > - * fill() - Fill options in message > > + * fill() - Fill options in message, with overload into file/sname if > needed > > * @m: Message to fill > > + * @overload: Set to option 52 value (0 if none, 1/2/3 per RFC > 2132) > > * > > * Return: current size of options field > > */ > > -static int fill(struct msg *m) > > +static int fill(struct msg *m, int *overload) > > { > > + /* Reserve 3 bytes for option 52 (overload) if needed */ > > + size_t size = OPT_MAX - 3; > > int i, o, offset = 0; > > > > for (o = 0; o < 255; o++) > > @@ -324,20 +369,25 @@ static int fill(struct msg *m) > > * Put it there explicitly, unless requested via option 55. > > */ > > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > > - if (fill_one(m->o, OPT_MAX, 53, &offset)) > > - debug("DHCP: skipping option 53"); > > + fill_one(m->o, size, 53, &offset); > > > > for (i = 0; i < opts[55].clen; i++) { > > o = opts[55].c[i]; > > if (opts[o].slen != -1) > > - if (fill_one(m->o, OPT_MAX, o, &offset)) > > - debug("DHCP: skipping option %i", o); > > + fill_one(m->o, size, o, &offset); > > } > > > > for (o = 0; o < 255; o++) { > > if (opts[o].slen != -1 && !opts[o].sent) > > - if (fill_one(m->o, OPT_MAX, o, &offset)) > > - debug("DHCP: skipping option %i", o); > > + fill_one(m->o, size, o, &offset); > > + } > > + > > + *overload = fill_overflow(m); > > + > > + if (*overload) { > > + m->o[offset++] = 52; > > + m->o[offset++] = 1; > > + m->o[offset++] = *overload; > > } > > > > m->o[offset++] = 255; > > @@ -462,6 +512,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > > struct msg const *m; > > struct msg reply; > > unsigned int i; > > + int overload; > > > > eh = IOV_REMOVE_HEADER(data, eh_storage); > > iph = IOV_PEEK_HEADER(data, iph_storage); > > @@ -613,7 +664,22 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > > if (!c->no_dhcp_dns_search) > > opt_set_dns_search(c, sizeof(m->o)); > > > > - dlen = offsetof(struct msg, o) + fill(&reply); > > + for (i = 0; i < (unsigned int)c->custom_opts_count; i++) { > > + uint8_t code = c->custom_opts[i].code; > > + > > + opts[code].slen = c->custom_opts[i].len; > > + memcpy(opts[code].s, c->custom_opts[i].val, > > + c->custom_opts[i].len); > > + } > > + > > + dlen = offsetof(struct msg, o) + fill(&reply, &overload); > > + > > + /* Copy boot file name into the file field for legacy PXE clients, > > + * unless the file field is already used for option overload. > > Given we do this, would it make more sense to use the slen field in > preference to the file field for overloads? The logic above appears > to do it the other way around, > are you referring to the sname field here? I am not sure how to use the slen field instead of the file field. > > > + */ > > + if (!(overload & DHCP_OVERLOAD_FILE) && > > + opts[67].slen > 0 && (size_t)opts[67].slen < > sizeof(reply.file)) > > + memcpy(&reply.file, opts[67].s, opts[67].slen + 1); > > > > if (m->flags & FLAG_BROADCAST) > > dst = in4addr_broadcast; > > -- > > 2.54.0 > > > > -- > David Gibson (he or they) | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you, not the other way > | around. > http://www.ozlabs.org/~dgibson >