From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ZdjR/CZt; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id EB5685A0269 for ; Mon, 20 Jul 2026 11:14:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1784538890; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EXfwZgO4l4OvDZhTM3hseCD+J3rKbwugBonf7Uknjnw=; b=ZdjR/CZtwTBWk+XETRdniN2SnIzt10piEhczZlhNo09vuyKJE3VdaIPO0DjyCN650gv4um DGdvdIFtv8Xmo28CkYnnYuUs3Df/QdEN7hH1gj+fk7Z3Y/8hCn1qc3O79JLCJd+L/eDGbA bSLigicqboEtgub8F+G4lnlMEapnp5o= Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-37-sQElD6XfM0SNXOV2cN6FuQ-1; Mon, 20 Jul 2026 05:14:48 -0400 X-MC-Unique: sQElD6XfM0SNXOV2cN6FuQ-1 X-Mimecast-MFC-AGG-ID: sQElD6XfM0SNXOV2cN6FuQ_1784538887 Received: by mail-lf1-f71.google.com with SMTP id 2adb3069b0e04-5b14d538d06so3804814e87.0 for ; Mon, 20 Jul 2026 02:14:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784538887; x=1785143687; h=content-type:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to:content-type; bh=EXfwZgO4l4OvDZhTM3hseCD+J3rKbwugBonf7Uknjnw=; b=n86bQPX0dbhndSKlwHRVuTS2H//iU26x9k1L4D8at9bRLEH9n6vrzAnROigjjvj5f6 6rdgSC0vL7J69RZVZ+SqPISfnnVuT0EulmNQ1NORyxQA80tgkv+bJR1EQ8rXG/6j7DBO E7EE9DnXb75I7sZokeNpXINaIWXmtnC+033zEDV2/sLiFtJ1P6+dqHPjOHWs9S29tV7p OKK+Ncau+GBihdjkfWBS2MjqvBR4pQJbdZxX1s3afrbioOzLbixayy3g9w6bxfh/1Wp3 ZreELTkiXIp6H6lDmNA4AoVBcA0ozUvzootHEk4T8CaaGAvRPUxXT6wyquqkv5B+MXBt CKZg== X-Forwarded-Encrypted: i=1; AHgh+Rr3VfYZB5v6426mwLgT9KMmKXKiY2nyEqm+WBab7vQ552EvKrHKIDs30gxql8M8NdprmYO6jmDsh0I=@passt.top X-Gm-Message-State: AOJu0YzYOWNqe1DdxcWuiKPo55GKMLiRcP+HjC/AYEZ/ct3K38LeY1jM 2Ata0SEt3a1rfuU/dd6xX7dYB5RDOBBpgrh1BSYKvh71a5NrJLyz27G74aGtJdmcG+GrSZqoa7I OR24orCPdeaFeEJ6LMtCRkDF9wT0VAhNbY+6naZlORrZrJukwCOsMEcW+x9Na0r1j6DUcNaiSqY n6owzaMEI8Gz6fDxQYDXYVwkMRafZu X-Gm-Gg: AfdE7clxKvJKyXRlnuz1WJMQefnp7Fna7Lq8BrUiczkdrxpLiqaB9scQx42U8zkCJ8m WmmG5+S7X9OWGZU9Yo8R5Ab4/ugTo4kSyN66JHIX03MTsoPiFbz/1DK6XTyHPimJvrD1dOn8Jaw UJ3B+41JTFPWPq3Xf0VhTcmUKXqqD+qVStD0e2jMOKGPHOhTQUiO+AzeRrEuOEtxslyqQjHNp1W F76UnfS+wWKuB4LNw== X-Received: by 2002:a05:6512:800c:10b0:5ae:bfd6:173b with SMTP id 2adb3069b0e04-5b28f86653emr1622613e87.20.1784538886755; Mon, 20 Jul 2026 02:14:46 -0700 (PDT) X-Received: by 2002:a05:6512:800c:10b0:5ae:bfd6:173b with SMTP id 2adb3069b0e04-5b28f86653emr1622605e87.20.1784538886171; Mon, 20 Jul 2026 02:14:46 -0700 (PDT) MIME-Version: 1.0 References: <20260717175648.879152-1-anskuma@redhat.com> <20260717175648.879152-7-anskuma@redhat.com> In-Reply-To: From: Anshu Kumari Date: Mon, 20 Jul 2026 14:44:34 +0530 X-Gm-Features: AUfX_myQEkXcGjVE43EPG4B-CfYT2o11aPIup0IFdmraEwYP2-0eUyE4ejSF1l4 Message-ID: Subject: Re: [PATCH v5 6/7] dhcp: Add RFC 3396 option splitting for concatenation-requiring options To: David Gibson X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: FN7PvQ0u6nQoRt_oQgKarsZ-rYWOyxF7Kba8R4BUPlM_1784538887 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="00000000000065ff37065707560e" Message-ID-Hash: DMQFOETRI7HKSKXYQRXFPEFXSG26MUUL X-Message-ID-Hash: DMQFOETRI7HKSKXYQRXFPEFXSG26MUUL X-MailFrom: anskuma@redhat.com 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: --00000000000065ff37065707560e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Jul 20, 2026 at 9:54=E2=80=AFAM 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=3D192 > > 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 fiel= d, > > + * 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 =3D strlen(str); > > > > if (slen >=3D 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 >=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 > needed > > * @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); > > } > > > > + /* RFC 3396: split concatenation-requiring options that didn't fi= t > > + * 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? > yes, I missed it by chance while calculating the total available bytes. > > + 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; > > -- > > 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 wa= y > | around. > http://www.ozlabs.org/~dgibson > --=20 Anshu --00000000000065ff37065707560e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Mon, Jul 20,= 2026 at 9:54=E2=80=AFAM David Gibson <david@gibson.dropbear.id.au> 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.=C2=A0 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<= br> > 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=3D192<= /a>
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v5:
>=C2=A0 =C2=A0- New patch: implement option splitting per RFC 3396 for o= ptions exceeding 255 bytes
>=C2=A0 =C2=A0- Add DHCP_OPT_STR_CONCAT type, is_concat_opt(), fill_spli= t() helpers
>=C2=A0 =C2=A0- Resize opts[].s from 255 to OPT_CONCAT_MAX (496) bytes >=C2=A0 =C2=A0- Add /* fallthrough */ between DHCP_OPT_STR and DHCP_OPT_= STR_CONCAT case
>
> ---
>=C2=A0 dhcp.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++= ++++--
>=C2=A0 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 @@
>=C2=A0 #include "log.h"
>=C2=A0 #include "dhcp.h"
>=C2=A0
> +/* RFC 3396: maximum option data that can be split across options fie= ld,
> + * file field, and sname field (minus code+length overhead per portio= n).
> + */
> +#define OPT_CONCAT_MAX=C2=A0 =C2=A0 =C2=A0 =C2=A0496
> +
>=C2=A0 /**
>=C2=A0 =C2=A0* enum opt_state - DHCP option state
>=C2=A0 =C2=A0* @OPT_UNSET:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0Option not configured
> @@ -58,7 +63,7 @@ enum opt_state {
>=C2=A0 struct opt {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int sent;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int slen;
> -=C2=A0 =C2=A0 =C2=A0uint8_t s[255];
> +=C2=A0 =C2=A0 =C2=A0uint8_t s[OPT_CONCAT_MAX];
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int clen;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0uint8_t c[255];
>=C2=A0 =C2=A0 =C2=A0 =C2=A0enum opt_state state;
> @@ -159,6 +164,7 @@ struct msg {
>=C2=A0 =C2=A0* @DHCP_OPT_UINT16: Unsigned 16-bit integer
>=C2=A0 =C2=A0* @DHCP_OPT_UINT32: Unsigned 32-bit integer
>=C2=A0 =C2=A0* @DHCP_OPT_INT32:=C2=A0 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.=C2=A0 Generally the dhcp_opt_type is saying how the option is<= br> encoded as a string for the user.=C2=A0 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 us= er 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 exa= ctly the same option that needs splitting on the wire.
>=C2=A0 =C2=A0*/
>=C2=A0 enum dhcp_opt_type {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DHCP_OPT_NONE,
> @@ -169,6 +175,7 @@ enum dhcp_opt_type {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DHCP_OPT_UINT16,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DHCP_OPT_UINT32,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DHCP_OPT_INT32,
> +=C2=A0 =C2=A0 =C2=A0DHCP_OPT_STR_CONCAT,
>=C2=A0 };
>=C2=A0
>=C2=A0 /**
> @@ -319,6 +326,10 @@ static int dhcp_opt_parse(uint8_t code, const cha= r *str,
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return width; >=C2=A0 =C2=A0 =C2=A0 =C2=A0case DHCP_OPT_STR:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (strlen(str) > = 255)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0return -1;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* fallthrough */
> +=C2=A0 =C2=A0 =C2=A0case DHCP_OPT_STR_CONCAT:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0slen =3D strlen(= str);
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (slen >=3D= buf_len)
> @@ -465,6 +476,53 @@ enum dhcp_overload {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DHCP_OVERLOAD_SNAME,
>=C2=A0 };
>=C2=A0
> +/**
> + * is_concat_opt() - Check if option requires RFC 3396 concatenation = support
> + * @o:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Option n= umber
> + *
> + * Return: true if option is a concatenation-requiring type
> + */
> +static bool is_concat_opt(int o)
> +{
> +=C2=A0 =C2=A0 =C2=A0if ((size_t)o >=3D ARRAY_SIZE(dhcp_opt_types))=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return 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.

> +=C2=A0 =C2=A0 =C2=A0return dhcp_opt_types[o] =3D=3D DHCP_OPT_STR_CONC= AT;
> +}
> +
> +/**
> + * fill_split() - Write a split portion of an option into a buffer > + * @buf:=C2=A0 =C2=A0 =C2=A0Buffer to write into
> + * @size:=C2=A0 =C2=A0 Usable size of @buf
> + * @o:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Option n= umber (code)
> + * @offset:=C2=A0 Current offset within @buf, updated on write
> + * @data:=C2=A0 =C2=A0 Pointer to remaining option data to write
> + * @remaining:=C2=A0 =C2=A0 =C2=A0 =C2=A0Bytes of option data still t= o 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 *offse= t,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 const uint8_t *data, size_t remaining)
> +{
> +=C2=A0 =C2=A0 =C2=A0size_t avail, chunk;
> +
> +=C2=A0 =C2=A0 =C2=A0if (*offset + 2 >=3D (int)size)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
> +
> +=C2=A0 =C2=A0 =C2=A0avail =3D size - *offset - 2;
> +=C2=A0 =C2=A0 =C2=A0chunk =3D remaining < avail ? remaining : avai= l;

You can use the existing MIN() macro here.

> +=C2=A0 =C2=A0 =C2=A0if (!chunk)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
> +
> +=C2=A0 =C2=A0 =C2=A0buf[*offset] =3D o;
> +=C2=A0 =C2=A0 =C2=A0buf[*offset + 1] =3D chunk;
> +=C2=A0 =C2=A0 =C2=A0*offset +=3D 2;
> +
> +=C2=A0 =C2=A0 =C2=A0memcpy(buf + *offset, data, chunk);
> +=C2=A0 =C2=A0 =C2=A0*offset +=3D chunk;
> +
> +=C2=A0 =C2=A0 =C2=A0return chunk;
> +}
> +
>=C2=A0 /**
>=C2=A0 =C2=A0* fill() - Fill options in message, with overload into fil= e/sname if needed
>=C2=A0 =C2=A0* @m:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Message to fill
> @@ -513,14 +571,59 @@ static int fill(struct msg *m, enum dhcp_overloa= d *overload, bool has_bootfile)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0for (o =3D 0; (size_t)o < ARRAY_SIZE(opts= ); o++) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (opts[o].stat= e =3D=3D OPT_UNSET || opts[o].sent)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0continue;
> -
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!has_bootfil= e &&
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0fill_one(m->file, sizeof(m->file) - 1, o,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &file_off))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &file_off))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0if (!is_concat_opt(o))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0debug("DHCP: skipping option = %i"
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0" (overl= oad full)", o);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0
> +=C2=A0 =C2=A0 =C2=A0/* RFC 3396: split concatenation-requiring option= s that didn't fit
> +=C2=A0 =C2=A0 =C2=A0 * as a single option.=C2=A0 Split order: options= , file, sname.
> +=C2=A0 =C2=A0 =C2=A0 */
> +=C2=A0 =C2=A0 =C2=A0for (o =3D 0; (size_t)o < ARRAY_SIZE(opts); o+= +) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size_t file_cap, snam= e_cap, total, written;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (opts[o].state =3D= =3D OPT_UNSET || opts[o].sent ||
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0!is_con= cat_opt(o))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0continue;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sname_cap =3D sizeof(= m->sname) - 1 > (size_t)sname_off ?
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0sizeof(m->sname) - 1 - sname_off : 0;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (has_bootfile || s= izeof(m->file) - 1 <=3D (size_t)file_off)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0file_cap =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0file_cap =3D sizeof(m->file) - 1 - file_off;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total =3D (size > = (size_t)offset ? size - offset : 0)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0+ file_cap + sname_cap;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (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.

+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0debug("DHCP: skipping option %i (no space to split)", > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0o);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0continue;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0written =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0written +=3D fill_spl= it(m->o, size, o, &offset,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opts[o].s, opts[o].= slen);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (written < (siz= e_t)opts[o].slen && !has_bootfile)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0written +=3D fill_split(m->file,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0sizeof(m->file) - 1, o,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0&file_off,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0opts[o].s + written,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0opts[o].slen - written);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (written < (siz= e_t)opts[o].slen)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0fill_split(m->sname,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeof(m->sname) - 1, o,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &sname_off,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 opts[o].s + written,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 opts[o].slen - written);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opts[o].sent =3D 1;
Since you didn't account for the per-chunk header, I think you need to<= br> check for written < opts[o].slen and not set sent in that case.

> +=C2=A0 =C2=A0 =C2=A0}
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (sname_off) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0m->sname[snam= e_off] =3D 255;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*overload |=3D D= HCP_OVERLOAD_SNAME;
> --
> 2.54.0
>

--
David Gibson (he or they)=C2=A0 =C2=A0 =C2=A0 =C2=A0| I'll have my musi= c baroque, and my code
david AT gibson.dropbear.id.au=C2=A0 | minimalist, thank you, not th= e other way
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | around.
http://www.ozlabs.org/~dgibson


--
Anshu
--00000000000065ff37065707560e--