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=QbYcNaXX; 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 7F0CD5A0269 for ; Thu, 08 Oct 2026 00:06:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1791410798; 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=zri0rhfuBfLblxopgzZR5yi3Aq3KWxu2P4Idvs80WHc=; b=QbYcNaXXIN+5LlEI4uEH/cwUE2CrHzpNpd1+7tB2TA2kw4zen3LSoN1nhYYREybQXE87mN QkCKJIegxSw5IVc7ZxI+d+0YOPfQpVi19bjGfVbPaGzt0Fd3cwQMXNREwu1bv4C/UMtjyU vZtPP2A/wLTRfCfS1A6xORkkBFury2s= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-440-fOZZNFfxOPS2kpa8mfXxZQ-1; Wed, 07 Oct 2026 18:06:37 -0400 X-MC-Unique: fOZZNFfxOPS2kpa8mfXxZQ-1 X-Mimecast-MFC-AGG-ID: fOZZNFfxOPS2kpa8mfXxZQ_1791410796 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-495689bfcc8so36354675e9.1 for ; Wed, 07 Oct 2026 15:06:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1791410796; x=1792015596; 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=zri0rhfuBfLblxopgzZR5yi3Aq3KWxu2P4Idvs80WHc=; b=cZN0jDv0nv9PSDcm0seFO9zC8t8Z8p7v+YOvc9oblMVn81ubLrQ2LRTeIjhjJB+QDS fCFOeXF9onwo9pOAL8i/8ISy6caryIdyo60hmrVpcp0sAHqa8ZNbGirSCuSA1HTo0hL/ K4hWxKFj9TqYEh9n78e4ZSqXlpBjzeyAiOszPZkhrDUlmagSKbKb5a5kqvexvVPSCmcD 6YezS2Ze/mYZvdThLtvUCWjzNYnNVEJERajyJjmNBKX30RUQghe/CmhdciqOolpEYG48 hoTnM5yA3PD4mz0yNOFlgsuHbe0WKrPbpqDhlCKqDeHZRncMLexpjkp7RIv8UHo6BUP0 2XHw== X-Gm-Message-State: AFuF++mVdqOfrXSrS9VotyID/HxZ5S4SxX8pyV7FTzjwIjGYGWouCT6W Mc3HW3b87uDnvvFMHgn0IxvaITCXyNLpBsLw04YU3UCuvPgfhA3nd2QxUWa/7I/nJJ0zTwhIG5B t4SKJxwEnsF9FUhha5PA/rcik6JrlhI3YDnUCRPRB5cPP9NReGnq21+CpNaNcQRtoxgCGjkogH+ G3ZHVjBR8s6d6rCyBW7i1tVBm6FPZ6tv6h6Avh X-Gm-Gg: AYBFou3Bg891SjlFgKQnNYKEKMSsbH0VCxIOo/veWG+JoIiTNgs2On/EESja/piQG7w vfSSxCnFBe7AEgH/7NWz6AnmhfSswoleknxeIYcGuQ4MqQbE52OsAElF9k1KMX74KZ3maIjsfGO Bu5MNU4BEgpJYAtZS8kT77GxuBKR713rbJOn/Dbq6VUAf92Zm9ln812gY0Sh10cDEDfwNkvDRfu JCdz8M62Xkx2yAQmo80WJ42WAJLls+MjGVIAAOkzHGIxCKkuKMrwOLN4bV5qReoLFyh/MaAKc4q DMSvRMqE69mi7D3IMQkw2GOXhdkAB9zCWDk8HFoaqxZNMXQXt/wfQWUsYlHMO+HdnVTg4dQ/jAS dvkdEg0aF9g== X-Received: by 2002:a05:600c:c16f:b0:4a0:2484:2800 with SMTP id 5b1f17b1804b1-4a1800ca1c3mr54164745e9.3.1791410795848; Wed, 07 Oct 2026 15:06:35 -0700 (PDT) X-Received: by 2002:a05:600c:c16f:b0:4a0:2484:2800 with SMTP id 5b1f17b1804b1-4a1800ca1c3mr54164425e9.3.1791410795316; Wed, 07 Oct 2026 15:06:35 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4a1843dbc6bsm23491015e9.11.2026.10.07.15.06.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Oct 2026 15:06:34 -0700 (PDT) From: Stefano Brivio To: Anshu Kumari Subject: Re: [PATCH v7 6/7] dhcp: Add option overload Message-ID: <20261008000633.6300886e@elisabeth> In-Reply-To: <20261001131602.653553-7-anskuma@redhat.com> References: <20261001131602.653553-1-anskuma@redhat.com> <20261001131602.653553-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: Thu, 08 Oct 2026 00:06:34 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: HPOIekVnIGx1QLSwNbrPirkKZ0JF1eWRoTDdDzckqC4_1791410796 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: JQAPEOQGT4KZNG6PKUJHB3V6WO4CSPCT X-Message-ID-Hash: JQAPEOQGT4KZNG6PKUJHB3V6WO4CSPCT 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 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 Thu, 1 Oct 2026 18:45:59 +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. > > 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. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari > --- > > v7: > - Removed overload function parameter > - Added foreach_opt() macro to iterate option table > > Till v6 both "patch 5/7" and "patch 6/7" were one. > --- > dhcp.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 68 insertions(+), 10 deletions(-) > > diff --git a/dhcp.c b/dhcp.c > index 6f69fd62..63ac8e3d 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -67,6 +67,8 @@ struct opt { > > static struct opt opts[256]; > > +#define foreach_opt(o) for ((o) = 0; (size_t)(o) < ARRAY_SIZE(opts); (o)++) > + > #define DHCPDISCOVER 1 > #define DHCPOFFER 2 > #define DHCPREQUEST 3 > @@ -440,13 +442,30 @@ static void fill_one(uint8_t *buf, size_t size, int o, int *offset) > } > > /** > - * fill() - Fill options in message > - * @m: Message to fill > + * enum dhcp_overload - DHCP option overload values (RFC 2132, Section 9.3) > + * @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 = 0, > + DHCP_OVERLOAD_FILE = 1, > + DHCP_OVERLOAD_SNAME = 2, > +}; > + > +/** > + * fill() - Fill options in message, with overload into file/sname if needed > + * @m: Message to fill > + * @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, bool has_bootfile) > { > + enum dhcp_overload overload = DHCP_OVERLOAD_NONE; > + int sname_off = 0, file_off = 0; > + /* Reserve 3 bytes for option 52 (overload) if needed */ > + size_t size = OPT_MAX - 3; > int i, o, offset = 0; > > for (o = 0; o < 255; o++) > @@ -457,17 +476,54 @@ static int fill(struct msg *m) > * Put it there explicitly, unless requested via option 55. > */ > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > - fill_one(m->o, OPT_MAX, 53, &offset); > + fill_one(m->o, size, 53, &offset); > > for (i = 0; i < opts[55].clen; i++) { > o = opts[55].c[i]; > if (opts[o].conf != OPT_UNSET) > - fill_one(m->o, OPT_MAX, o, &offset); > + fill_one(m->o, size, o, &offset); > } > > for (o = 0; o < 255; o++) { > if (opts[o].conf != OPT_UNSET && !opts[o].sent) > - fill_one(m->o, OPT_MAX, o, &offset); > + fill_one(m->o, size, o, &offset); > + } > + > + /* Overflow unsent options into sname, then file */ > + foreach_opt(o) { This comes from my suggestion on v6 but those were really subsequent steps (building a more specialised iterator on top of foreach_opt()), not alternatives. Look at flow_foreach() and flow_foreach_of_type() as examples. That is, here you could use: foreach_unsent_opt(o) fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); with: #define foreach_unset_opt(o) \ foreach_opt((o)) \ /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */ \ if (opts[(o)].conf != OPT_UNSET && !opts[(o)].sent) ...the option with the reverse condition and "continue; else" could also work, I'm not sure what's the most practical here. The direct option looks more... direct, to me. The two loops below could use this iterator, and perhaps the one above as well (I haven't tried). Note that in 7/7 you could probably switch to this other iterator as well, without the explicit need for the base foreach_opt(o) iterator, but I would suggest to keep them as two different macros anyway, it's clearer and more reusable. > + if (opts[o].conf == OPT_UNSET || opts[o].sent) > + continue; > + fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); > + } > + > + if (!has_bootfile) { > + foreach_opt(o) { > + if (opts[o].conf == OPT_UNSET || opts[o].sent) > + continue; > + fill_one(m->file, sizeof(m->file) - 1, o, &file_off); > + } > + } > + > + /* Report any options that could not be sent */ > + foreach_opt(o) { > + if (opts[o].conf != OPT_UNSET && !opts[o].sent) > + debug("DHCP: skipping option %i", o); > + } > + > + if (sname_off) { > + m->sname[sname_off] = 255; > + overload |= DHCP_OVERLOAD_SNAME; > + } > + > + if (file_off) { > + m->file[file_off] = 255; > + overload |= DHCP_OVERLOAD_FILE; > + } > + > + if (overload) { > + m->o[offset++] = 52; > + m->o[offset++] = 1; > + m->o[offset++] = overload; > } > > m->o[offset++] = 255; > @@ -761,16 +817,18 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > } > } > > - if (!c->no_dhcp_dns_search) > - opt_set_dns_search(c, sizeof(m->o)); > + if (!c->no_dhcp_dns_search) { > + /* 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. > + * field. Reserve the file field from overload. > */ > has_bootfile = opts[67].slen > 0 && > (size_t)opts[67].slen < sizeof(reply.file); > > - dlen = offsetof(struct msg, o) + fill(&reply); > + dlen = offsetof(struct msg, o) + fill(&reply, has_bootfile); > > if (has_bootfile) > memcpy(reply.file, opts[67].s, opts[67].slen); -- Stefano