From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=TRQltmjd; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id E88935A0269 for ; Mon, 20 Jul 2026 06:24:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1784521446; bh=yvifgqXoP8CV3/TLwg+5UlPyvNJiIJWruXcok7aEF/w=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=TRQltmjdRik9GBiemTPSc2CE6/5TPMVYm+KoNev3BQq8C265pT7oo2pAvoin4zkDN WInzIhayQSSmqj4E49Nfit4iyFuhcUaSPNjYLiQykH+LPSB3Mh4CO8xCw1djqypA2i hfJTpqAqHyNr1rHdnrP0YArE2DSNNYTOrkqwFeXoBoNbkPe3MT6j/riUcjmMh/CLA1 MGsaAWCxJjk9M3KoZOISfbMILrvc5eBhO0vEpYsZ4HfaJpheGgYBw9N2CFsnMWuIAe v/ndNidAJR/lsCOQpV9MoQeq+brXu2hzvaDFJ9G6WJRVwPHOj4cLDUDlKOR5Kmi1Wv R8I11rolkr2gw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4h3S8620Gjz4w1c; Mon, 20 Jul 2026 14:24:06 +1000 (AEST) Date: Mon, 20 Jul 2026 14:21:23 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH v5 6/7] dhcp: Add RFC 3396 option splitting for concatenation-requiring options Message-ID: References: <20260717175648.879152-1-anskuma@redhat.com> <20260717175648.879152-7-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="wPCcAF2XSsgvr0kE" Content-Disposition: inline In-Reply-To: <20260717175648.879152-7-anskuma@redhat.com> Message-ID-Hash: FQF2LBDW4BQL2XBS6G5TWYATFCD637GC X-Message-ID-Hash: FQF2LBDW4BQL2XBS6G5TWYATFCD637GC X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: sbrivio@redhat.com, passt-dev@passt.top, lvivier@redhat.com, jmaloy@redhat.com X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --wPCcAF2XSsgvr0kE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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). >=20 > 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. >=20 > 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. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D192 > Signed-off-by: Anshu Kumari > --- > v5: > - New patch: implement option splitting per RFC 3396 for options exceed= ing 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 ca= se >=20 > --- > dhcp.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 106 insertions(+), 3 deletions(-) >=20 > 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" > =20 > +/* 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, > }; > =20 > /** > @@ -319,6 +326,10 @@ static int dhcp_opt_parse(uint8_t code, const char *= str, > =20 > return width; > case DHCP_OPT_STR: > + if (strlen(str) > 255) > + return -1; > + /* fallthrough */ > + case DHCP_OPT_STR_CONCAT: > slen =3D strlen(str); > =20 > if (slen >=3D buf_len) > @@ -465,6 +476,53 @@ enum dhcp_overload { > DHCP_OVERLOAD_SNAME, > }; > =20 > +/** > + * is_concat_opt() - Check if option requires RFC 3396 concatenation sup= port > + * @o: Option number > + * > + * Return: true if option is a concatenation-requiring type > + */ > +static bool is_concat_opt(int o) > +{ > + if ((size_t)o >=3D 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] =3D=3D 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 >=3D (int)size) > + return 0; > + > + avail =3D size - *offset - 2; > + chunk =3D remaining < avail ? remaining : avail; You can use the existing MIN() macro here. > + if (!chunk) > + return 0; > + > + buf[*offset] =3D o; > + buf[*offset + 1] =3D chunk; > + *offset +=3D 2; > + > + memcpy(buf + *offset, data, chunk); > + *offset +=3D chunk; > + > + return chunk; > +} > + > /** > * fill() - Fill options in message, with overload into file/sname if ne= eded > * @m: Message to fill > @@ -513,14 +571,59 @@ static int fill(struct msg *m, enum dhcp_overload *= overload, bool has_bootfile) > for (o =3D 0; (size_t)o < ARRAY_SIZE(opts); o++) { > if (opts[o].state =3D=3D 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); > } > =20 > + /* RFC 3396: split concatenation-requiring options that didn't fit > + * as a single option. Split order: options, file, sname. > + */ > + for (o =3D 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + size_t file_cap, sname_cap, total, written; > + > + if (opts[o].state =3D=3D OPT_UNSET || opts[o].sent || > + !is_concat_opt(o)) > + continue; > + > + sname_cap =3D sizeof(m->sname) - 1 > (size_t)sname_off ? > + sizeof(m->sname) - 1 - sname_off : 0; > + > + if (has_bootfile || sizeof(m->file) - 1 <=3D (size_t)file_off) > + file_cap =3D 0; > + else > + file_cap =3D sizeof(m->file) - 1 - file_off; > + > + total =3D (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 =3D 0; > + written +=3D fill_split(m->o, size, o, &offset, > + opts[o].s, opts[o].slen); > + if (written < (size_t)opts[o].slen && !has_bootfile) > + written +=3D 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 =3D 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] =3D 255; > *overload |=3D DHCP_OVERLOAD_SNAME; > --=20 > 2.54.0 >=20 --=20 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 --wPCcAF2XSsgvr0kE Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpdojEACgkQzQJF27ox 2GeOJA/8CIKmO0vMAiJL8uYv2ExT1ljB+eXZOy55JSy+4SNQb7owaNr16E8HQU7x c4YK5xi/4oNptQdB2bF6SSyvsaFkVXgZct8G3BnJAk1m9bF4F8XgcW762Zh3YqRv J13j7Vysk0IyEtQFMNM1TuZbx+gfGbs6hZ/nA/MNuWbl4HPzVh2Qn3sIN3LOtAJV A9pKnUtrB/j7rg8mvjhMSDdG1WvOALWJEceEVNOHZcam49zQqQZAFZYxJ82ODtZJ l5WDI5Tr3xrbz5rtqTINABk81FSurevyar5J6MFZNGJiw30tt2rHYjUfJJm4YWeY m4m1M+FMCIbB/VciKEEkRRiGrU11XQyo8mOXmgS4VPanfqfb9eofeLbVpn2FIguR m76SoK1zeepW8L9wSPuSVFuxpe76SBofayh+eLuLDIZgrunyeShtlKmzaJGB+PD5 A9/3rxY2Z2kVHBAShKW78nirTtpU2O6r9owi3qAU+IrHCbJ2ByTVfJnIWvSB8U/y lixUEiX7AYKO0t8SYEFxjUMM0bE/ZsFoMmOm1ZVTnsQMB/10/GqOnTPvNTv1MEph za5MnPR3jMu5ub6TVy7ndS3sCbpjsIlePzMPSqwS9PLenbS/ngqy6BWsycd5+f69 5esZulj2FW1hD3QNIr51ni62PJJe0zJp70muVMYNwAYGT1f9RWY= =BauD -----END PGP SIGNATURE----- --wPCcAF2XSsgvr0kE--