Re: [PATCH v5 6/7] dhcp: Add RFC 3396 option splitting for concatenation-requiring options

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.

*/ 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? + 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