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=jUncfMzZ; 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 CE89B5A0272 for ; Thu, 10 Sep 2026 09:04:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1789023854; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KQkC1CsYLqYs1BMW0jb9hkCRlvg6NAG7o9Ov0kFaxlk=; b=jUncfMzZj2iDZo/92M67Mv/E9URFtTXI62ZbOnGzLoV1hWHUtlqXmxYjqXT9c25qCGX+Xb 7+FfHfRxNyCtej6SNuc9/NU1pQj3IetxY7Rzh5zg23yI0izxzcNUNiNoFEAT6Wk9EmGN6W eez6jvx6DVd8MPcZhJ7FTWldJJiVcYU= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-153-3jvCfhQKNRSyjBFF7ARJqg-1; Thu, 10 Sep 2026 03:04:13 -0400 X-MC-Unique: 3jvCfhQKNRSyjBFF7ARJqg-1 X-Mimecast-MFC-AGG-ID: 3jvCfhQKNRSyjBFF7ARJqg_1789023852 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-4839f9edb08so4139635f8f.0 for ; Thu, 10 Sep 2026 00:04:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1789023852; x=1789628652; h=date:content-transfer-encoding:content-type:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=6G1MZnICZsjKdeZQPP5MV/Da8YFPS3H4XZWrfuXtSkU=; b=HNBOugjcrV7bpod0neKcBwLEdlbdZHKI+a+lWi8DOc4BJcPKGcnoc9jPkbZHX3FIJo mwugpwytSr3cG/KhzMuynbFsuT2UYTiDcl9b+mj7NShW3xEJ606PzfQlOIHJDOtqONDZ 9L+VUGdEy4tqUEFIUhPAPk8nCX+mhMjpDTkzEEukKVgprdsnS3JQYY/RaVglArj+lO+0 abHl43FN9NLTQD5fmyU08rCQkcppRRfN3zHwqkXYBkh0Mx3xYgEFJU7AzazeJDu1sqvl 7/Cd+kIWnl6kNStlCMLjpuFr31PQDMqEOe1Cdpk16QbgRHuYMBiQqwb6VPM54l/xXqDx 1rsA== X-Gm-Message-State: AFuF++kSX4AdnS8V5gWWeF2YudWPyaleWkwv/PekV6S5lE5nRcWseJg1 EmT8KD8/XlIWO3eCeI5g4O86VI3ZqABtYWAVbHigITAFZdHTFl1w4+kGAims5Q62FxAW0JOrO6K SpBGeiUjQZh6/zzrgd/s6l4DFIkYsRNXMVfrQrtAqg0OPq2xBCouJJQ== X-Gm-Gg: AYBFou0LYX7a3oP83qK8sEY66w8rbHuE8EGLzgzEkLdIkRXF5DA6KCOXLZA26mXv+0D uXQh2uF84NXlSxUgaE4v53JguSB1oZ/hslyuweZb6pOhtHPAiWKw6gyYY4j2sQDREM9JIkFoQcj v59GEGd37AtYNu/Il3g4+9HchygnmbYgJj4pwjg4C5EcqnAi92Sb95jG/cPSBWtuBlKiwaIRSxC 74eqv94zfodbJFBB+AlNLDzsc6a4xjJ/sKJLnkuuZPQ0WOn2pnXxqs20Lqh2NhxXim0+xlyqYKL 3WvD6/hbsiPeZLIA31PjVOYaeIzHnadTqP6mQzL6SBvZBLQt0taQ85LInMjppjifn2nQ+oKOSXC EA9HbrDM2Qpw= X-Received: by 2002:a05:6000:1843:b0:485:8c17:975f with SMTP id ffacd0b85a97d-4858c1798damr39721044f8f.33.1789023851825; Thu, 10 Sep 2026 00:04:11 -0700 (PDT) X-Received: by 2002:a05:6000:1843:b0:485:8c17:975f with SMTP id ffacd0b85a97d-4858c1798damr39720991f8f.33.1789023851312; Thu, 10 Sep 2026 00:04:11 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-485883927a3sm38097817f8f.11.2026.09.10.00.04.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Sep 2026 00:04:10 -0700 (PDT) From: Stefano Brivio To: Anshu Kumari Subject: Re: [PATCH v6 3/6] dhcp: Add option overload Message-ID: <20260910090408.3a8d55ce@elisabeth> In-Reply-To: <20260824134436.282300-4-anskuma@redhat.com> References: <20260824134436.282300-1-anskuma@redhat.com> <20260824134436.282300-4-anskuma@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Thu, 10 Sep 2026 09:04:10 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: gqlty-jIdpXQ8yEbTwQLLFKG53PbrwSEuE8i5cEFoJg_1789023852 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 5D3RDDRGUGDZSK5CXCFL5ZNF6KV2WWTM X-Message-ID-Hash: 5D3RDDRGUGDZSK5CXCFL5ZNF6KV2WWTM X-MailFrom: sbrivio@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: passt-dev@passt.top, david@gibson.dropbear.id.au, jmaloy@redhat.com, lvivier@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: On Mon, 24 Aug 2026 19:14:31 +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 > --- > v6: > - Changed fill_one() return type from bool to void. > - Replaced per-call-site debug messages with a single > reporting loop at end of fill(). > - Added explicit numeric values to enum dhcp_overload. > - Removed temporary buffer for boot file copy, > copy directly into reply.file. > - Fixed mixed code and declarations. > - Fixed indentation and whitespace issues. > - Added curly brackets for multi-line if clauses >=20 > 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 > --- > dhcp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 81 insertions(+), 17 deletions(-) >=20 > diff --git a/dhcp.c b/dhcp.c > index 321968a..02c7744 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -146,16 +146,14 @@ struct msg { > * @size:=09Usable size of @buf (excluding end marker) > * @o:=09=09Option number > * @offset:=09Current offset within @buf, updated on insertion > - * > - * Return: false if @buf has space to write the option, true otherwise > */ > -static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) > +static void fill_one(uint8_t *buf, size_t size, int o, int *offset) > { > =09size_t slen =3D opts[o].slen; > =20 > =09/* If we don't have space to write the option, then just skip */ > =09if (*offset + 2 /* code and length of option */ + slen > size) > -=09=09return true; > +=09=09return; > =20 > =09buf[*offset] =3D o; > =09buf[*offset + 1] =3D slen; > @@ -167,19 +165,37 @@ static bool fill_one(uint8_t *buf, size_t size, int= o, int *offset) > =20 > =09opts[o].sent =3D 1; > =09*offset +=3D slen; > -=09return false; > } > =20 > /** > - * fill() - Fill options in message > - * @m:=09=09Message to fill > + * enum dhcp_overload - DHCP option overload values (RFC 2132, Section 9= .3) > + * @DHCP_OVERLOAD_NONE:=09=09No overload > + * @DHCP_OVERLOAD_FILE:=09=09file field carries options > + * @DHCP_OVERLOAD_SNAME:=09sname field carries options > + */ > +enum dhcp_overload { > +=09DHCP_OVERLOAD_NONE =3D 0, > +=09DHCP_OVERLOAD_FILE =3D 1, > +=09DHCP_OVERLOAD_SNAME =3D 2, > +}; > + > +/** > + * fill() - Fill options in message, with overload into file/sname if ne= eded > + * @m:=09=09=09Message to fill > + * @overload:=09=09Set to option 52 value (0 if none, 1/2/3 per RFC 2132= ) > + * @has_bootfile:=09Reserve 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) > { > +=09int sname_off =3D 0, file_off =3D 0; > +=09/* Reserve 3 bytes for option 52 (overload) if needed */ > +=09size_t size =3D OPT_MAX - 3; > =09int i, o, offset =3D 0; > =20 > +=09*overload =3D DHCP_OVERLOAD_NONE; > + > =09for (o =3D 0; o < 255; o++) > =09=09opts[o].sent =3D 0; > =20 > @@ -188,20 +204,54 @@ static int fill(struct msg *m) > =09 * Put it there explicitly, unless requested via option 55. > =09 */ > =09if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > -=09=09if (fill_one(m->o, OPT_MAX, 53, &offset)) > -=09=09=09debug("DHCP: skipping option 53"); > +=09=09fill_one(m->o, size, 53, &offset); > =20 > =09for (i =3D 0; i < opts[55].clen; i++) { > =09=09o =3D opts[55].c[i]; > =09=09if (opts[o].state !=3D OPT_UNSET) > -=09=09=09if (fill_one(m->o, OPT_MAX, o, &offset)) > -=09=09=09=09debug("DHCP: skipping option %i", o); > +=09=09=09fill_one(m->o, size, o, &offset); > =09} > =20 > =09for (o =3D 0; o < 255; o++) { > =09=09if (opts[o].state !=3D OPT_UNSET && !opts[o].sent) > -=09=09=09if (fill_one(m->o, OPT_MAX, o, &offset)) > -=09=09=09=09debug("DHCP: skipping option %i", o); > +=09=09=09fill_one(m->o, size, o, &offset); > +=09} > + > +=09/* Overflow unsent options into sname, then file */ > +=09for (o =3D 0; (size_t)o < ARRAY_SIZE(opts); o++) { > +=09=09if (opts[o].state =3D=3D OPT_UNSET || opts[o].sent) > +=09=09=09continue; > +=09=09fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); > +=09} > + > +=09if (!has_bootfile) { > +=09=09for (o =3D 0; (size_t)o < ARRAY_SIZE(opts); o++) { > +=09=09=09if (opts[o].state =3D=3D OPT_UNSET || opts[o].sent) > +=09=09=09=09continue; > +=09=09=09fill_one(m->file, sizeof(m->file) - 1, o, &file_off); > +=09=09} > +=09} > + > +=09/* Report any options that could not be sent */ > +=09for (o =3D 0; (size_t)o < ARRAY_SIZE(opts); o++) { > +=09=09if (opts[o].state !=3D OPT_UNSET && !opts[o].sent) Maybe at this point you could introduce some iterator macros to avoid open coding these two lines all over the place, based on foreach() (see util.h), something like: - foreach_opt() - foreach_set_opt() (I'm not sure if it's needed as a separate one, see existing foreach_* macros across the codebase for ideas how to include a conditional in a macro) - foreach_opt_to_send() (to finally have this loop with this condition, could be built on foreach_set_opt() or directly on foreach_opt()) > +=09=09=09debug("DHCP: skipping option %i", o); > +=09} > + > +=09if (sname_off) { > +=09=09m->sname[sname_off] =3D 255; > +=09=09*overload |=3D DHCP_OVERLOAD_SNAME; > +=09} > + > +=09if (file_off) { > +=09=09m->file[file_off] =3D 255; > +=09=09*overload |=3D DHCP_OVERLOAD_FILE; > +=09} > + > +=09if (*overload) { > +=09=09m->o[offset++] =3D 52; > +=09=09m->o[offset++] =3D 1; > +=09=09m->o[offset++] =3D *overload; > =09} > =20 > =09m->o[offset++] =3D 255; > @@ -315,6 +365,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) > { > =09char macstr[ETH_ADDRSTRLEN]; > +=09enum dhcp_overload overload; > =09size_t mlen, dlen, opt_len; > =09struct in_addr mask, dst; > =09struct ethhdr eh_storage; > @@ -325,6 +376,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > =09const struct udphdr *uh; > =09struct msg m_storage; > =09struct msg const *m; > +=09bool has_bootfile; > =09struct msg reply; > =09unsigned int i; > =20 > @@ -486,10 +538,22 @@ int dhcp(const struct ctx *c, struct iov_tail *data= ) > =09=09} > =09} > =20 > -=09if (!c->no_dhcp_dns_search) > -=09=09opt_set_dns_search(c, sizeof(m->o)); > +=09if (!c->no_dhcp_dns_search) { > +=09=09/* 3 bytes reserved for option 52 (code, length, value) */ > +=09=09opt_set_dns_search(c, OPT_MAX - 3); > +=09} > + > +=09/* RFC 2132, Section 9.5: put boot file name in the 'file' header > +=09 * field. Suppress option 67 from the options area and reserve > +=09 * the file field from overload. > +=09 */ > +=09has_bootfile =3D opts[67].slen > 0 && > +=09=09 (size_t)opts[67].slen < sizeof(reply.file); > + > +=09dlen =3D offsetof(struct msg, o) + fill(&reply, &overload, has_bootfi= le); > =20 > -=09dlen =3D offsetof(struct msg, o) + fill(&reply); > +=09if (has_bootfile) > +=09=09memcpy(reply.file, opts[67].s, opts[67].slen); > =20 > =09if (m->flags & FLAG_BROADCAST) > =09=09dst =3D in4addr_broadcast; --=20 Stefano