On Mon, Jul 20, 2026 at 9:54 AM David Gibson wrote: > On Fri, Jul 17, 2026 at 11:26:43PM +0530, Anshu Kumari wrote: > > Implement option splitting per RFC 3396 for options that may exceed > > 255 bytes. A new DHCP_OPT_STR_CONCAT type marks concatenation- > > requiring options (currently option 81, Client FQDN per RFC 4702). > > > > The opts[].s buffer is resized from 255 to 496 bytes to hold the > > maximum data that can be split across the options field, file field, > > and sname field. > > > > When a concatenation-requiring option does not fit as a single option > > in any field, fill() splits it across fields in RFC 3396 order: > > options field first, then file, then sname. > > > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > > Signed-off-by: Anshu Kumari > > --- > > v5: > > - New patch: implement option splitting per RFC 3396 for options > exceeding 255 bytes > > - Add DHCP_OPT_STR_CONCAT type, is_concat_opt(), fill_split() helpers > > - Resize opts[].s from 255 to OPT_CONCAT_MAX (496) bytes > > - Add /* fallthrough */ between DHCP_OPT_STR and DHCP_OPT_STR_CONCAT > case > > > > --- > > dhcp.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 106 insertions(+), 3 deletions(-) > > > > diff --git a/dhcp.c b/dhcp.c > > index cc910ee..6bebb5f 100644 > > --- a/dhcp.c > > +++ b/dhcp.c > > @@ -34,6 +34,11 @@ > > #include "log.h" > > #include "dhcp.h" > > > > +/* RFC 3396: maximum option data that can be split across options field, > > + * file field, and sname field (minus code+length overhead per portion). > > + */ > > +#define OPT_CONCAT_MAX 496 > > + > > /** > > * enum opt_state - DHCP option state > > * @OPT_UNSET: Option not configured > > @@ -58,7 +63,7 @@ enum opt_state { > > struct opt { > > int sent; > > int slen; > > - uint8_t s[255]; > > + uint8_t s[OPT_CONCAT_MAX]; > > int clen; > > uint8_t c[255]; > > enum opt_state state; > > @@ -159,6 +164,7 @@ struct msg { > > * @DHCP_OPT_UINT16: Unsigned 16-bit integer > > * @DHCP_OPT_UINT32: Unsigned 32-bit integer > > * @DHCP_OPT_INT32: Signed 32-bit integer > > + * @DHCP_OPT_STR_CONCAT:Concatenation-requiring string (RFC 3396) > > It's not entirely clear to me that encoding this in the opt_type enum > makes sense. Generally the dhcp_opt_type is saying how the option is > encoded as a string for the user. This is saying how it's encoded > within the DHCP packet itself, which seems qualitatively different > information. > > Since we don't limit on the string length that user can set via --dhcp-opt, DHCP_OPT_STR rejects anything over 255 bytes but with STR_CONCAT longer input can be parsed. is_concat_opt() checks dhcp_opt_types[] for STR_CONCAT to decide which options need splitting in fill(). I think these two concerns are coupled: an option that accepts long strings from the CLI is exactly the same option that needs splitting on the wire. > > */ > > enum dhcp_opt_type { > > DHCP_OPT_NONE, > > @@ -169,6 +175,7 @@ enum dhcp_opt_type { > > DHCP_OPT_UINT16, > > DHCP_OPT_UINT32, > > DHCP_OPT_INT32, > > + DHCP_OPT_STR_CONCAT, > > }; > > > > /** > > @@ -319,6 +326,10 @@ static int dhcp_opt_parse(uint8_t code, const char > *str, > > > > return width; > > case DHCP_OPT_STR: > > + if (strlen(str) > 255) > > + return -1; > > + /* fallthrough */ > > + case DHCP_OPT_STR_CONCAT: > > slen = strlen(str); > > > > if (slen >= buf_len) > > @@ -465,6 +476,53 @@ enum dhcp_overload { > > DHCP_OVERLOAD_SNAME, > > }; > > > > +/** > > + * is_concat_opt() - Check if option requires RFC 3396 concatenation > support > > + * @o: Option number > > + * > > + * Return: true if option is a concatenation-requiring type > > + */ > > +static bool is_concat_opt(int o) > > +{ > > + if ((size_t)o >= ARRAY_SIZE(dhcp_opt_types)) > > + return false; > > IIUC, passing an out of bounds option code here would already be a bug > in our code, so an assert() might make more sense. > > > + return dhcp_opt_types[o] == DHCP_OPT_STR_CONCAT; > > +} > > + > > +/** > > + * fill_split() - Write a split portion of an option into a buffer > > + * @buf: Buffer to write into > > + * @size: Usable size of @buf > > + * @o: Option number (code) > > + * @offset: Current offset within @buf, updated on write > > + * @data: Pointer to remaining option data to write > > + * @remaining: Bytes of option data still to write > > + * > > + * Return: number of data bytes written (excluding code+length header) > > + */ > > +static size_t fill_split(uint8_t *buf, size_t size, int o, int *offset, > > + const uint8_t *data, size_t remaining) > > +{ > > + size_t avail, chunk; > > + > > + if (*offset + 2 >= (int)size) > > + return 0; > > + > > + avail = size - *offset - 2; > > + chunk = remaining < avail ? remaining : avail; > > You can use the existing MIN() macro here. > > > + if (!chunk) > > + return 0; > > + > > + buf[*offset] = o; > > + buf[*offset + 1] = chunk; > > + *offset += 2; > > + > > + memcpy(buf + *offset, data, chunk); > > + *offset += chunk; > > + > > + return chunk; > > +} > > + > > /** > > * fill() - Fill options in message, with overload into file/sname if > needed > > * @m: Message to fill > > @@ -513,14 +571,59 @@ static int fill(struct msg *m, enum dhcp_overload > *overload, bool has_bootfile) > > for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > > if (opts[o].state == OPT_UNSET || opts[o].sent) > > continue; > > - > > if (!has_bootfile && > > fill_one(m->file, sizeof(m->file) - 1, o, > > - &file_off)) > > + &file_off)) > > + if (!is_concat_opt(o)) > > debug("DHCP: skipping option %i" > > " (overload full)", o); > > } > > > > + /* RFC 3396: split concatenation-requiring options that didn't fit > > + * as a single option. Split order: options, file, sname. > > + */ > > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > > + size_t file_cap, sname_cap, total, written; > > + > > + if (opts[o].state == OPT_UNSET || opts[o].sent || > > + !is_concat_opt(o)) > > + continue; > > + > > + sname_cap = sizeof(m->sname) - 1 > (size_t)sname_off ? > > + sizeof(m->sname) - 1 - sname_off : 0; > > + > > + if (has_bootfile || sizeof(m->file) - 1 <= > (size_t)file_off) > > + file_cap = 0; > > + else > > + file_cap = sizeof(m->file) - 1 - file_off; > > + > > + total = (size > (size_t)offset ? size - offset : 0) > > + + file_cap + sname_cap; > > + > > + if (total < (size_t)opts[o].slen) { > > > > This doesn't account for the extra 2 bytes per chunk of option code > and length, right? > yes, I missed it by chance while calculating the total available bytes. > > + debug("DHCP: skipping option %i (no space to > split)", > > + o); > > + continue; > > + } > > + > > + written = 0; > > + written += fill_split(m->o, size, o, &offset, > > + opts[o].s, opts[o].slen); > > + if (written < (size_t)opts[o].slen && !has_bootfile) > > + written += fill_split(m->file, > > + sizeof(m->file) - 1, o, > > + &file_off, > > + opts[o].s + written, > > + opts[o].slen - written); > > + if (written < (size_t)opts[o].slen) > > + fill_split(m->sname, > > + sizeof(m->sname) - 1, o, > > + &sname_off, > > + opts[o].s + written, > > + opts[o].slen - written); > > + opts[o].sent = 1; > > Since you didn't account for the per-chunk header, I think you need to > check for written < opts[o].slen and not set sent in that case. > > > + } > > + > > if (sname_off) { > > m->sname[sname_off] = 255; > > *overload |= DHCP_OVERLOAD_SNAME; > > -- > > 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 > -- Anshu