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=J/69FOIi; 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 2F1E95A0269 for ; Mon, 27 Jul 2026 08:19:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1785133177; 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=HQFQqsLHlmYzXFyTUS7amwRc/ASHVPz8OSA8YUDAZlo=; b=J/69FOIibAlXppIVtBhJNrU4QeqFc2FWHRiKEmPfOqDrLLIAZK2OpBLzG5AoBU1LLccmRf +sC7oKaAbCnLxyLc8qUrOhd8AWHI0KrDqAED2a2sXeKjlq3R8XRLoHFhV84hPPO5uxrEHo z3SaFV1kR3e9DfYL6n/2XrYqO5l72qY= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-502-j-6xzO7vMTG92GZief2EeA-1; Mon, 27 Jul 2026 02:19:35 -0400 X-MC-Unique: j-6xzO7vMTG92GZief2EeA-1 X-Mimecast-MFC-AGG-ID: j-6xzO7vMTG92GZief2EeA_1785133174 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-4954dcd6131so25478215e9.3 for ; Sun, 26 Jul 2026 23:19:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785133174; x=1785737974; 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=HQFQqsLHlmYzXFyTUS7amwRc/ASHVPz8OSA8YUDAZlo=; b=aRl6hEpqpxZR9/n/MVc2ILCB2aegEc9n/Wrys4lS2yzO0L3AVJJeDEzB4exg+rtRQd i8SdTQkvR0qqdQCe7q0OmSAXMI3DKMy4ptzDhG1iTFOz9+fxwfeS/wSftSf1i01tpiX/ 7Ddt9nr6KQGiIb9tkEyviXQNX9GZJ2CmvXcumgg5q/yNETq7zNccL452SeuNFs8Uqxvf Ty+TbjekvB+8hkEynqMWOdmkBnD7pToagtA9aT9TmPNT196qkNRxPuAau3ZgnX5SiEp1 UVVGgVnUYYK7DEE25uHy+zA3bfZpb9N5SKAnGlht6aDrtE6SlHiAtL6zPB8HmAYdHoK3 WzEA== X-Gm-Message-State: AOJu0YwgAk1UKnaC3pYzc9TBIJ4EOqNrwIfkUpZeM+HAzZZU3SQLRSLK NIBrGI3P2rUPqbzpS76X/LFDFJ1ulNtKMSlso6Zj6gJ6fUMnDPQF4S86Tm834Soa1G9C29+F6rJ WQ7R6aV+zLLelt0NjSLICmJuwMI3kf1gHV4E8+FaazYEJGjCwLNblLw== X-Gm-Gg: AR+sD13WChBON/OopFRR5YtiJudNJUpWJCjS8sBHQk/y35GBLNpX3i1n3SMcE4R0oMg 9FrZIR8kMOEmzWslWcLzETAoV0D8RmbprpcvmhWPRvvXD77YEHxYuFssA/1qy1lV/TTsiyprrYb jKVxuRKnEL5cDo2Sy5ph46goFG4GpDsCYpapZZD0fBRRD6p+7Q/uztfRBL3rWRN5xnbRP5PvYfr fyYc5FiCAwiunYU3jtoCg5V1NzseZBtoiYjjOUJYfDJm0ygympmuVH5NHRt4TIkOPTOgkAMiq4d K+KRaXt6+1JwU1SsjG6jiQAnTh8+F7NGIitMzh7Z9xR8rkceQDrpSqFW0UItRcfAhPIElIpiyY3 HgfUBMihSAfE= X-Received: by 2002:a05:600c:3113:b0:496:b39f:1a03 with SMTP id 5b1f17b1804b1-496b56e6dc9mr85172125e9.5.1785133173984; Sun, 26 Jul 2026 23:19:33 -0700 (PDT) X-Received: by 2002:a05:600c:3113:b0:496:b39f:1a03 with SMTP id 5b1f17b1804b1-496b56e6dc9mr85171945e9.5.1785133173494; Sun, 26 Jul 2026 23:19:33 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-496b4f302f8sm191111565e9.12.2026.07.26.23.19.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 26 Jul 2026 23:19:32 -0700 (PDT) From: Stefano Brivio To: Anshu Kumari Subject: Re: [PATCH v5 6/7] dhcp: Add RFC 3396 option splitting for concatenation-requiring options Message-ID: <20260727081931.64afba39@elisabeth> In-Reply-To: <20260717175648.879152-7-anskuma@redhat.com> References: <20260717175648.879152-1-anskuma@redhat.com> <20260717175648.879152-7-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: Mon, 27 Jul 2026 08:19:32 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: uhifcVIArXC0aEwaJG9d2DQUwL1AGqqVzO6qtaKSjaI_1785133174 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: FIMSLWX6NDZLVZNEF3ILIUNEQCTKOIQK X-Message-ID-Hash: FIMSLWX6NDZLVZNEF3ILIUNEQCTKOIQK 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, lvivier@redhat.com, david@gibson.dropbear.id.au, 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: On Fri, 17 Jul 2026 23:26:43 +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=192 > 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 field, > + * file field, and sname field (minus code+length overhead per portion). > + */ > +#define OPT_CONCAT_MAX 496 I think this definition should be built using OPT_MAX (see below), adding 64 + 128 to it (and keeping it written instead of just calculating the sum). I'm not sure why it's 496 by the way: I thought it would be: OPT_MIN + 64 /* sname */ - 1 /* end */ + 128 /* file */ - 1 /* end */ ...that is, 497 instead of 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) As David pointed out: I also think this should be a separate flag because it doesn't represent the data type. Right now the only concatenation-requiring option we support is option 81, and it's a string, but, using the convenient list of examples of: https://www.ietf.org/archive/id/draft-tojens-dhcp-option-concat-considerations-01.html#section-1 I just realised that RFC 7291 (DHCP Options for the Port Control Protocol) introduces a list of IPv4 addresses as a concatenation-requiring option. If you do something like: static bool concat_req[255] = { [81] = true, }; then, later: > */ > 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 = strlen(str); > > if (slen >= 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 >= ARRAY_SIZE(dhcp_opt_types)) > + return false; > + return dhcp_opt_types[o] == DHCP_OPT_STR_CONCAT; > +} ...you could skip this whole function (as well as patch 7/7), and just do: > + > +/** > + * 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 >= (int)size) > + return 0; > + > + avail = size - *offset - 2; > + chunk = remaining < avail ? remaining : avail; > + if (!chunk) > + return 0; > + > + buf[*offset] = o; > + buf[*offset + 1] = chunk; > + *offset += 2; > + > + memcpy(buf + *offset, data, chunk); > + *offset += 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 = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > if (opts[o].state == 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)) if (concat_req[o]) ... > debug("DHCP: skipping option %i" > " (overload full)", o); > } > > + /* RFC 3396: split concatenation-requiring options that didn't fit > + * as a single option. Split order: options, file, sname. > + */ > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + size_t file_cap, sname_cap, total, written; > + > + if (opts[o].state == OPT_UNSET || opts[o].sent || > + !is_concat_opt(o)) > + continue; > + > + sname_cap = sizeof(m->sname) - 1 > (size_t)sname_off ? > + sizeof(m->sname) - 1 - sname_off : 0; > + > + if (has_bootfile || sizeof(m->file) - 1 <= (size_t)file_off) > + file_cap = 0; > + else > + file_cap = sizeof(m->file) - 1 - file_off; > + > + total = (size > (size_t)offset ? size - offset : 0) > + + file_cap + sname_cap; > + > + if (total < (size_t)opts[o].slen) { > + debug("DHCP: skipping option %i (no space to split)", > + o); > + continue; > + } > + > + written = 0; > + written += fill_split(m->o, size, o, &offset, > + opts[o].s, opts[o].slen); > + if (written < (size_t)opts[o].slen && !has_bootfile) > + written += 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 = 1; > + } > + > if (sname_off) { > m->sname[sname_off] = 255; > *overload |= DHCP_OVERLOAD_SNAME; -- Stefano