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=E13B9GMs; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 7D9DD5A026D for ; Thu, 28 May 2026 21:01:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1779994918; 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=//JZUTQWZ+6o2/2fFPBxwNady7N7nDSkqS+ixRh7bXE=; b=E13B9GMsBIf4GUcHOmWn+hljTxOpEWCSjn+BONUZ0b5xq0FZQ2apCuT2K8HWaSvcoXnF80 bCzQcQt9i3m0gCR03zCr4bvvnJX+D89Tzjj80UzFRuxoWQk2GX5xrcVnSwXBTAoGYlynT0 IkrRJQh95orKMMJdfigPRKt6bXoOdmI= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-654-BbARN_EQPEaoi_DT9aJHqA-1; Thu, 28 May 2026 15:01:57 -0400 X-MC-Unique: BbARN_EQPEaoi_DT9aJHqA-1 X-Mimecast-MFC-AGG-ID: BbARN_EQPEaoi_DT9aJHqA_1779994916 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-49049100a40so43562465e9.2 for ; Thu, 28 May 2026 12:01:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779994916; x=1780599716; h=date:content-transfer-encoding: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; bh=K24qYFCY2yjxljud8Jwy2CcR1Qzyxps/AR9G6IxfX8U=; b=cla9pEVYbBbAX8to5t91fQiAQf7VmlDUgPazprMOVJIa4o97jtLiy5aiMxdj2+hDVW +MknxxvIhWSqw0VtzHvGt8/0CGOMo/lbcXo/Aw1ch5kZ0987JZ49iSHXWpMPUioZRQU+ O5I5hGXPW9zQFB9pFcPW70Z6pHqNuErR9AnoHYg0sRRjck/BuHAuP9Tv16AZrTC4HMY2 cnBrop0vIi3Z1lIywv93q3i8fq5Xu7hv9/qcWwJT8KbYPic45iZJ1r2pMpn6gJIdW7Mm dSGQm2Z7llu817x8sbvDAaiH5dv6b57Ie8O92ImYTX9jHm6vuRnIHDfoLb/Zn6YKjk6J NHPA== X-Forwarded-Encrypted: i=1; AFNElJ/VyJkK9gcwwNHvf8DkNh6OqlHZmMop24E1fEjIRLPu2K4tBgiWDugT/o9ZIou3V3u93X4pwr5Bxws=@passt.top X-Gm-Message-State: AOJu0YwOfti+AyvZfCPpXpd4nv8hZ4Nq8wNS1bEwiQTht6NSWELk30XT Ki435bQJ4tgH9rWIfCmeOTvbPOddFeH8CuDPOBrxNFtM809dUXBqZljhcmZfEvebHDQt5H1ZpZ5 TDfuXIQC7LJfgfZHrGfbyRTwUuPLiIeoaebElPltN75BxWxeoEjwQcHeMHUuu6g== X-Gm-Gg: Acq92OGXul6srR2NDJFQnsGGQwnX3LePWReSp71utZjGqlKUnqJQHvLmorbV/VZJWcZ nvW0TlhYhiCsxNOnlbP8k4hmrKA+Urn8wfR8atpBb4dvm7AX9BRJal0BhzgAI2T4wvjBKhbV42Z Mjs68wyGtc5bHwbBEqyS/NkqVmKnsHly/RA1Wyj/k0w04LWXNOzDy2N+5AHWQZ6AN85XDRXeXsA rkcoD9qTBcs9Qqdraa6KYebGUUFTFh40yac1y2ydARu4jdqUIEDXUY7eafmAy3926mY/H2qw1Rn C71op00coIn4NvDn3mSJBbXJeYWPc5OZ+9izh2cWEElkU3yPWAYFLxHk1YmX56O8s7XaEaIA8C7 Rrf3VqaAdHA21YSqfLOz4LYsXgGjot5O+82OENMP9xDI= X-Received: by 2002:a05:600c:1381:b0:490:51e2:bc86 with SMTP id 5b1f17b1804b1-49051e2be87mr381323905e9.23.1779994915472; Thu, 28 May 2026 12:01:55 -0700 (PDT) X-Received: by 2002:a05:600c:1381:b0:490:51e2:bc86 with SMTP id 5b1f17b1804b1-49051e2be87mr381323135e9.23.1779994914891; Thu, 28 May 2026 12:01:54 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45edb5a281dsm14767788f8f.24.2026.05.28.12.01.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 May 2026 12:01:54 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 5/6] dhcp: Add option overload Message-ID: <20260528210152.738a54df@elisabeth> In-Reply-To: References: <20260526123115.1226166-1-anskuma@redhat.com> <20260526123115.1226166-6-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, 28 May 2026 21:01:53 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: C5w4p8WI6l2umjbQjH_KPiywQ_Ww9AGYjha8hHMaAu0_1779994916 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: FWYSWQ7CVMZBAXXRJLQGLQ3FUWASGRS6 X-Message-ID-Hash: FWYSWQ7CVMZBAXXRJLQGLQ3FUWASGRS6 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: Anshu Kumari , passt-dev@passt.top, 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 Wed, 27 May 2026 14:03:44 +1000 David Gibson wrote: > On Tue, May 26, 2026 at 06:01:12PM +0530, Anshu Kumari wrote: > > A user can enter lots of options in command-line which may not fit in > > existing buffer, So when the options field is full, overflow remaining > > DHCP options into the file and sname fields per RFC 2132 option 52. > >=20 > > Also, when the file field is not used for overload, copy the boot > > file URL there directly for legacy PXE clients. > >=20 > > Link: https://bugs.passt.top/show_bug.cgi?id=3D192 > > Signed-off-by: Anshu Kumari > > --- > > v2: > > - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME co= nstants > > - Added comment documenting space reservation: /* Reserve 3 bytes for= option 52 */ > > - Fixed DNS search length: sizeof(m->o) only, not combined with file+= sname > > - Removed dhcp_boot references =E2=80=94 reply.file copy now reads fr= om opts[67] > > - Used DHCP_OVERLOAD_FILE constant in reply.file guard > > --- > > dhcp.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 75 insertions(+), 9 deletions(-) > >=20 > > diff --git a/dhcp.c b/dhcp.c > > index e126063..a49a05a 100644 > > --- a/dhcp.c > > +++ b/dhcp.c > > @@ -306,14 +306,59 @@ static bool fill_one(uint8_t *buf, size_t size, i= nt o, int *offset) > > =09return false; > > } > > =20 > > +#define DHCP_OVERLOAD_FILE=091 > > +#define DHCP_OVERLOAD_SNAME=092 =20 >=20 > IIUC, these values are from the RFC - might be worth referencing the > relevant section in a comment to make it clear they can't be changed > arbitrarily. >=20 > > + > > +/** > > + * fill_overflow() - Fill remaining options into file and sname fields > > + * @m:=09=09Message whose file/sname fields may be used for overflow > > + * > > + * Return: option 52 overload value: 0 if no overflow, > > + * DHCP_OVERLOAD_FILE for file, DHCP_OVERLOAD_SNAME for sname, > > + * or both OR'd together > > + */ > > +static int fill_overflow(struct msg *m) > > +{ > > +=09int file_off =3D 0, sname_off =3D 0, overload =3D 0; > > +=09int o; > > + > > +=09for (o =3D 0; o < 255; o++) { =20 >=20 > You could use ARRAY_size(opts) instead of raw 255, yes? >=20 > > +=09=09if (opts[o].slen =3D=3D -1 || opts[o].sent) > > +=09=09=09continue; > > +=09=09fill_one(m->file, sizeof(m->file) - 1, o, &file_off); > > +=09} > > + > > +=09for (o =3D 0; o < 255; o++) { > > +=09=09if (opts[o].slen =3D=3D -1 || opts[o].sent) > > +=09=09=09continue; > > +=09=09if (fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off)) > > +=09=09=09debug("DHCP: skipping option %i (overload full)", o); > > +=09} > > + > > +=09if (file_off) { > > +=09=09m->file[file_off] =3D 255; > > +=09=09overload |=3D DHCP_OVERLOAD_FILE; > > +=09} > > + > > +=09if (sname_off) { > > +=09=09m->sname[sname_off] =3D 255; > > +=09=09overload |=3D DHCP_OVERLOAD_SNAME; > > +=09} > > + > > +=09return overload; > > +} > > + > > /** > > - * fill() - Fill options in message > > + * fill() - Fill options in message, with overload into file/sname if = needed > > * @m:=09=09Message to fill > > + * @overload:=09Set to option 52 value (0 if none, 1/2/3 per RFC 2132) > > * > > * Return: current size of options field > > */ > > -static int fill(struct msg *m) > > +static int fill(struct msg *m, int *overload) > > { > > +=09/* Reserve 3 bytes for option 52 (overload) if needed */ > > +=09size_t size =3D OPT_MAX - 3; > > =09int i, o, offset =3D 0; > > =20 > > =09for (o =3D 0; o < 255; o++) > > @@ -324,20 +369,25 @@ 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=09 debug("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].slen !=3D -1) > > -=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].slen !=3D -1 && !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*overload =3D fill_overflow(m); > > + > > +=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; > > @@ -462,6 +512,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data= ) > > =09struct msg const *m; > > =09struct msg reply; > > =09unsigned int i; > > +=09int overload; > > =20 > > =09eh =3D IOV_REMOVE_HEADER(data, eh_storage); > > =09iph =3D IOV_PEEK_HEADER(data, iph_storage); > > @@ -613,7 +664,22 @@ int dhcp(const struct ctx *c, struct iov_tail *dat= a) > > =09if (!c->no_dhcp_dns_search) > > =09=09opt_set_dns_search(c, sizeof(m->o)); > > =20 > > -=09dlen =3D offsetof(struct msg, o) + fill(&reply); > > +=09for (i =3D 0; i < (unsigned int)c->custom_opts_count; i++) { > > +=09=09uint8_t code =3D c->custom_opts[i].code; > > + > > +=09=09opts[code].slen =3D c->custom_opts[i].len; > > +=09=09memcpy(opts[code].s, c->custom_opts[i].val, > > +=09=09 c->custom_opts[i].len); > > +=09} > > + > > +=09dlen =3D offsetof(struct msg, o) + fill(&reply, &overload); > > + > > +=09/* Copy boot file name into the file field for legacy PXE clients, > > +=09 * unless the file field is already used for option overload. =20 >=20 > Given we do this, would it make more sense to use the slen field in > preference to the file field for overloads? The logic above appears > to do it the other way around, Assuming s/slen/sname/: I didn't really think this through, but there might be a more fundamental advantage in doing this, rather than just the boot file consideration: each single option we add needs to fit entirely in one of the two areas. If we try sname (smaller) first, we *should* have strictly better chances than the other way around. Example: we need to fit (hypothetical) options 20 (20 bytes), option 30 (30 bytes) and option 120 (120 bytes). If we try 'file' first: - option 20 fits, option 30 fits, nothing else, move to 'sname' - nothing else fits: two options encoded If we try 'sname' first: - option 20 fits, option 30 fits, nothing else, move to 'file' - option 120 fits: three options encoded There must be some relatively simple proof or theorem for this but right now I can't think of any name / reference. > > +=09 */ > > +=09if (!(overload & DHCP_OVERLOAD_FILE) && > > +=09 opts[67].slen > 0 && (size_t)opts[67].slen < sizeof(reply.file)= ) > > +=09=09memcpy(&reply.file, opts[67].s, opts[67].slen + 1); > > =20 > > =09if (m->flags & FLAG_BROADCAST) > > =09=09dst =3D in4addr_broadcast; > > --=20 > > 2.54.0 --=20 Stefano