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

On Fri, 17 Jul 2026 23:26:43 +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

I think this definition should be built using OPT_MAX (see below), adding 64 + 128 to it (and keeping it written instead of just calculating the sum). I'm not sure why it's 496 by the way: I thought it would be: OPT_MIN + 64 /* sname */ - 1 /* end */ + 128 /* file */ - 1 /* end */ ...that is, 497 instead of 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)

As David pointed out: I also think this should be a separate flag because it doesn't represent the data type. Right now the only concatenation-requiring option we support is option 81, and it's a string, but, using the convenient list of examples of: https://www.ietf.org/archive/id/draft-tojens-dhcp-option-concat-consideratio... I just realised that RFC 7291 (DHCP Options for the Port Control Protocol) introduces a list of IPv4 addresses as a concatenation-requiring option. If you do something like: static bool concat_req[255] = { [81] = true, }; then, later:

*/ 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; + return dhcp_opt_types[o] == DHCP_OPT_STR_CONCAT; +}

...you could skip this whole function (as well as patch 7/7), and just do:

+ +/** + * 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; + 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))

if (concat_req[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) { + 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; + } + if (sname_off) { m->sname[sname_off] = 255; *overload |= DHCP_OVERLOAD_SNAME;

-- Stefano