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=c9gRQ5Nx; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id A15445A0265 for ; Mon, 20 Jul 2026 04:06:58 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1784513214; bh=VyRw45f/Ta20vOJQ7ZscLivOnZwGqeWX5ZW9zoWYj78=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=c9gRQ5NxrmxtWqGFXamEQTVFQrN5vZDFdVfBmYMbff+ns4xe2hHIWo/ejhmC8Esfo mibbpVJPNW0Febm69UEk1Mnu7DL9qNsr0UbsbKlF2jyD66zC7YiAmq4uwGYc4G3oK7 Ws/wpXDLCiEPsJwXNCWPuq0s4++Lti8VLoKkWagq/ilorA/YPj6Yfe8JVojfZJwTWL ReDt98Vj3XrK0UTp5iRvAzk1/gFzt7ORcDmpck3IDsoQ0/nChUE7v9LjxiN1LdOKrS 68QmnH6k6ide9oad9zyjRKB+vnzhHblUoEKp5oXbmNNXgWy4d61YWASgVRP97fKbk4 8fVgWfL3vmGzw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4h3P5p4Gflz4w9b; Mon, 20 Jul 2026 12:06:54 +1000 (AEST) Date: Mon, 20 Jul 2026 12:03:55 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH v5 3/7] dhcp: Add option overload Message-ID: References: <20260717175648.879152-1-anskuma@redhat.com> <20260717175648.879152-4-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="QbkK/OA3q0x8oLaR" Content-Disposition: inline In-Reply-To: <20260717175648.879152-4-anskuma@redhat.com> Message-ID-Hash: EHI4AGW5KZDAK5AAUVB4IXQSUNQXMYUW X-Message-ID-Hash: EHI4AGW5KZDAK5AAUVB4IXQSUNQXMYUW 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: --QbkK/OA3q0x8oLaR Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 17, 2026 at 11:26:40PM +0530, Anshu Kumari wrote: > When the options field is full, overflow remaining DHCP options into > the sname and file fields per RFC 2132 option 52. >=20 > Per RFC 2132, Section 9.5, the boot file name is always placed in the > 'file' header field. When a boot file is set, the file field is > reserved from overload and overflow uses only the sname field. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D192 > Signed-off-by: Anshu Kumari > --- > v5: > - enhanced enum dhcp_overload to follow kernel-doc. > - Inline fill_overflow() into fill() > - Use state-based checks instead of slen >=20 > v4: > - Converted overload #defines to enum dhcp_overload. > - Fixed missing whitespace in comment before */. > - Boot file name always placed in 'file' header field per RFC 2132, > Section 9.5; file field reserved from overload when bootfile is > set; option 67 suppressed from options area. >=20 > v3: > - Added RFC 2132 Section 9.3 reference comment on overload > constants. > - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow(). > - Swapped overflow order: try sname (64 bytes) first, then file > (128 bytes) =E2=80=94 better packing and keeps file field available f= or > boot file name. > - Removed '&' from &reply.file. > - Removed '+1' from memcpy =E2=80=94 reply.file already zeroed. > - opt_set_dns_search() max_len: OPT_MAX - 3 instead of > sizeof(m->o). >=20 > v2: > - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME cons= tants > - Added comment documenting space reservation: /* Reserve 3 bytes for o= ption 52 */ > - Fixed DNS search length: sizeof(m->o) only, not combined with file+sn= ame > - Removed dhcp_boot references =E2=80=94 reply.file copy now reads from= opts[67] > - Used DHCP_OVERLOAD_FILE constant in reply.file guard >=20 > --- > dhcp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 79 insertions(+), 9 deletions(-) >=20 > diff --git a/dhcp.c b/dhcp.c > index e5d89fc..39f7952 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -176,13 +176,31 @@ static bool fill_one(uint8_t *buf, size_t size, int= o, int *offset) > } > =20 > /** > - * fill() - Fill options in message > - * @m: Message to fill > +* enum dhcp_overload - DHCP option overload values (RFC 2132, Section 9.= 3) Nit: missing space before the *. > + * @DHCP_OVERLOAD_NONE: No overload > + * @DHCP_OVERLOAD_FILE: file field carries options > + * @DHCP_OVERLOAD_SNAME: sname field carries options > + */ > +enum dhcp_overload { > + DHCP_OVERLOAD_NONE, > + DHCP_OVERLOAD_FILE, > + DHCP_OVERLOAD_SNAME, Nit: I'd suggest putting specific value assignments on these, even though they're technically redundant. It serves to make it clearer that the specific numerical values matters, since these go "over the wire". > +}; > + > +/** > + * fill() - Fill options in message, with overload into file/sname if ne= eded > + * @m: Message to fill > + * @overload: Set to option 52 value (0 if none, 1/2/3 per RFC 2132) > + * @has_bootfile: Reserve file field for boot file name > * > * Return: current size of options field > */ > -static int fill(struct msg *m) > +static int fill(struct msg *m, enum dhcp_overload *overload, bool has_bo= otfile) > { > + int sname_off =3D 0, file_off =3D 0; > + *overload =3D DHCP_OVERLOAD_NONE; > + /* Reserve 3 bytes for option 52 (overload) if needed */ > + size_t size =3D OPT_MAX - 3; > int i, o, offset =3D 0; > =20 > for (o =3D 0; o < 255; o++) > @@ -199,14 +217,47 @@ static int fill(struct msg *m) > for (i =3D 0; i < opts[55].clen; i++) { > o =3D opts[55].c[i]; > if (opts[o].state !=3D OPT_UNSET) > - if (fill_one(m->o, OPT_MAX, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, size, o, &offset); > } > =20 > for (o =3D 0; o < 255; o++) { > if (opts[o].state !=3D OPT_UNSET && !opts[o].sent) > - if (fill_one(m->o, OPT_MAX, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, size, o, &offset); > + } > + > + /* Overflow unsent options into sname, then file */ > + for (o =3D 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].state =3D=3D OPT_UNSET || opts[o].sent) > + continue; > + fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); > + } > + > + 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)) > + debug("DHCP: skipping option %i" > + " (overload full)", o); The logic here will omit all the "skipping option" messages if has_bootfile is true. I think it might be cleaner to change fill_one() to return void, and add a final pass generating the "skipping" messages if !opts[o].sent. > + } > + > + if (sname_off) { > + m->sname[sname_off] =3D 255; > + *overload |=3D DHCP_OVERLOAD_SNAME; > + } > + > + if (file_off) { > + m->file[file_off] =3D 255; > + *overload |=3D DHCP_OVERLOAD_FILE; > + } > + > + > + if (*overload) { > + m->o[offset++] =3D 52; > + m->o[offset++] =3D 1; > + m->o[offset++] =3D *overload; > } > =20 > m->o[offset++] =3D 255; > @@ -320,6 +371,7 @@ static void opt_set_dns_search(const struct ctx *c, s= ize_t max_len) > int dhcp(const struct ctx *c, struct iov_tail *data) > { > char macstr[ETH_ADDRSTRLEN]; > + enum dhcp_overload overload; > size_t mlen, dlen, opt_len; > struct in_addr mask, dst; > struct ethhdr eh_storage; > @@ -328,9 +380,12 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > const struct ethhdr *eh; > const struct iphdr *iph; > const struct udphdr *uh; > + uint8_t bootfile[128]; > struct msg m_storage; > struct msg const *m; > + bool has_bootfile; > struct msg reply; > + int bootfile_len; > unsigned int i; > =20 > eh =3D IOV_REMOVE_HEADER(data, eh_storage); > @@ -492,9 +547,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > } > =20 > if (!c->no_dhcp_dns_search) > - opt_set_dns_search(c, sizeof(m->o)); > + /* 3 bytes reserved for option 52 (code, length, value) */ > + opt_set_dns_search(c, OPT_MAX - 3); > + > + /* RFC 2132, Section 9.5: put boot file name in the 'file' header > + * field. Suppress option 67 from the options area and reserve > + * the file field from overload. > + */ > + has_bootfile =3D opts[67].slen > 0 && > + (size_t)opts[67].slen < sizeof(reply.file); > + if (has_bootfile) { > + memcpy(bootfile, opts[67].s, opts[67].slen); > + bootfile_len =3D opts[67].slen; Why copy into the 'bootfile' temporary.. > + } > + > + dlen =3D offsetof(struct msg, o) + fill(&reply, &overload, has_bootfile= ); > =20 > - dlen =3D offsetof(struct msg, o) + fill(&reply); > + if (has_bootfile) > + memcpy(reply.file, bootfile, bootfile_len); =2E.. then out again, rather than copying directly from opts[67].s into reply.file? > =20 > if (m->flags & FLAG_BROADCAST) > dst =3D in4addr_broadcast; > --=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 --QbkK/OA3q0x8oLaR Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpdggoACgkQzQJF27ox 2GeTaxAAiFoYrseP+tAMuuSjd7l3Jd+1KGDtTkv6Npq9MAQZ/VPn+pBBj0fp4evh VWlai2VAxSuqTPy1khi7lBDvaVLherRXDH5iype6wBSsfMRNBZOuCVNVVlvxRErT utvzbJ79P/KbjJuCldlf5Ca470CEszaivNJpRKaJHxW4R35k1asDZpE6NfCDPTmE qqHgrxGHS87j89WT6tvuLFiqj6dH3qefUG7p2S9PSDt+WPoav8eOdRzyf+27EzMI b+ApZQP/+OwhtWaQ7mmk8znlOrr9MolVlwsr2+Twrq96u7oh68jwPjffMlY5C0HT kT6otY0RZdXg/6Em1QehT+dIKbcKkXKdPSxksc6UoUi37DqEpoMvq4JMOUDDiQ+Q gGXHmNcqGXrvMACtRZgWYqLxQjYtlbVPufEP80AY2TM0T9L4kTps4jfbHAMM524f sPcN7LKNuGZ/zBygVYBkeFJ+kgh4VPjxyAivWYV0G1k3mnbNx4Wgx//j8NAxqxkl MmIwY7gdvsG0K2AZTAubd0rh0DMVZhzBZvC+COqIODczjlLMisa/LntAEf9gdIyW 8/I6xZNPCx1FWikNuv4WdkcVwLzjp3um7gPquj7ea1Ns9f+Emyy7M2OJRzzyslDW 5XE6rB9DtuqJ6igouRjG1vYLPJEXV5UjjaR/u9knkqjPsJk1irs= =RNDT -----END PGP SIGNATURE----- --QbkK/OA3q0x8oLaR--