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=TbBEtZ/2; 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 8EBE25A0269 for ; Mon, 27 Jul 2026 08:19:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1785133143; 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=OhimzTpr6J65DRJkB3L4AMAiT+1URb9fCPZu1UNmlo8=; b=TbBEtZ/2qK7zo3MzqUiM6Elkg7+giXoAyDJfLQl8wupfjr4ICWWyASdX80GQNsXgQ5emnO tNzMWSG1E0gIfshpkHpNBfnFWQ/0aRoec8Jfsvqqo6rMJip7v72dCZnMtHB1wKhm/1NNTT drVUVrMFv+YMHNbOQndzgAU4Rt7YyI8= 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-642-NDjyD6syOYSD1egUZdQOSg-1; Mon, 27 Jul 2026 02:19:01 -0400 X-MC-Unique: NDjyD6syOYSD1egUZdQOSg-1 X-Mimecast-MFC-AGG-ID: NDjyD6syOYSD1egUZdQOSg_1785133140 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-4954c2d4081so17826975e9.2 for ; Sun, 26 Jul 2026 23:19:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785133140; x=1785737940; 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=OhimzTpr6J65DRJkB3L4AMAiT+1URb9fCPZu1UNmlo8=; b=FHfLU8VZGpNsgW6gDUN3SaNia7YO/BPYnFy/JQcy0yaB3XlpYJnnXUPqYjgJmPf+ei 9daFwY8WMPY1czDUYHuMVCTY09G9C7WWHY1O/WGV2Slohaye2VAmDLibNL2cANeVPpu9 5Twy5WczBNt6ONKjx+BuaOCVvbGIFeIMH6ulhRpiCi+DYaTDavbDYISSupdkIigIO4y/ Bshvun8JtXNS2+in7rn83kh2TJtIichl5e748weor6CH7MEveLgEPmPeeRQvtkWHjoBr NIs2Mr1TgolCOwyvarePKjB8eZPTelk4pDYJGYfzz01kh5M6PaY2ld9dNDLWtSm8xzAL kumA== X-Gm-Message-State: AOJu0Yzqayg8pF3yh9KmchB9TJLcfYqYVsZpFepFYveuZVeUBTrxAMyP U9y2NkFkwkG2eCDzclXUdsNM8xAN+AU19W3qdzD4fEOEnf8Y+C8woObtLldweE8cuR0+hQuhHoS 54+23N4N8VIc6LwQkdjqx3PepIh49RSEAvcalrtZSaLsHo7kViXVweg== X-Gm-Gg: AR+sD11NhOTbZZxoP50PKkqh0OGyprhlYhmD1HDse/0JalUT/H4Y3aQK2A1YKEEhf1R CKUdIEi7kezii2WKd55iUFLlSRoDh3aLxpaKEYyYvHhzH+DbNJ6Z8fOLnd+hwEYa/F0a4XV+7nX WAFYlheGgleMoxjbgiHrqAmmiiafGxu8vFFfUhfjW7zb8k14LlXgMp0ZtwU8eR/9qyeAIk8s4k2 HgCF6IntUtQqbLNbSXPU/GQfXoR2NVB6zPBfQa9efuJLJdBIEvpC/loDYZIPzRQSJaTdTVZhvrq 1DFsuheDKdzZCxDKFaaRPnJrZVXDNqGKINyjvK71YesyE43VX+Ap6dtl5CdoRnWuPm/BoC1/rw3 TEMjaS6rnpu8UEkxvCDkZND+1Dwyh X-Received: by 2002:a05:600c:4595:b0:495:3a21:4e5d with SMTP id 5b1f17b1804b1-496b56f461dmr87763565e9.0.1785133139674; Sun, 26 Jul 2026 23:18:59 -0700 (PDT) X-Received: by 2002:a05:600c:4595:b0:495:3a21:4e5d with SMTP id 5b1f17b1804b1-496b56f461dmr87763175e9.0.1785133139046; Sun, 26 Jul 2026 23:18:59 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4957d41ed1bsm187757175e9.2.2026.07.26.23.18.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 26 Jul 2026 23:18:57 -0700 (PDT) From: Stefano Brivio To: Anshu Kumari Subject: Re: [PATCH v5 2/7] dhcp: Add option state management with enum opt_state Message-ID: <20260727081855.18d8c234@elisabeth> In-Reply-To: <20260717175648.879152-3-anskuma@redhat.com> References: <20260717175648.879152-1-anskuma@redhat.com> <20260717175648.879152-3-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:18:56 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: L8vxhoc5sltFkfgrHh7_PUZ7jB0G158dLZdGx0GfCY0_1785133140 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: XH4Q4ASK77BETUG4F6W3XSHDOC7CL4KA X-Message-ID-Hash: XH4Q4ASK77BETUG4F6W3XSHDOC7CL4KA 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:39 +0530 Anshu Kumari wrote: > Introduce enum opt_state to track each DHCP option instead > of overloading slen = -1 for "not set". > > OPT_UNSET means the option is not configured. > OPT_DEFAULT means the option was set from host configuration. > > This replaces all slen = -1 / slen != -1 checks with > state = OPT_UNSET / state != OPT_UNSET, and sets state = OPT_DEFAULT > for options initialised in dhcp_init() and at reply time in dhcp(). > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari > --- > v5: > - New patch: introduce enum opt_state { OPT_UNSET, OPT_DEFAULT } to replace slen = -1 for tracking option state > - Replace all slen = -1 / slen != -1 checks with state = OPT_UNSET / state != OPT_UNSET > - Set OPT_DEFAULT for options initialised in dhcp_init() and at reply time > > --- > dhcp.c | 57 ++++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 40 insertions(+), 17 deletions(-) > > diff --git a/dhcp.c b/dhcp.c > index bb72b72..e5d89fc 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -33,13 +33,24 @@ > #include "log.h" > #include "dhcp.h" > > +/** > + * enum opt_state - DHCP option state > + * @OPT_UNSET: Option not configured > + * @OPT_DEFAULT: Option set from host config Nit: "config" isn't an actual word, it's a common abbreviation, but here we don't need it and we could use the original noun. I would actually say "derived from host configuration", to make it clear that we're not recycling any kind of DHCP server configuration find on the host. > + */ > +enum opt_state { > + OPT_UNSET, > + OPT_DEFAULT, > +}; > + > /** > * struct opt - DHCP option > * @sent: Convenience flag, set while filling replies > - * @slen: Length of option defined for server, -1 if not going to be sent > + * @slen: Length of option defined for server > * @s: Option payload from server > * @clen: Length of option received from client, -1 if not received > * @c: Option payload from client > + * @state: Option state (unset or default) > */ > struct opt { > int sent; > @@ -47,6 +58,7 @@ struct opt { > uint8_t s[255]; > int clen; > uint8_t c[255]; > + enum opt_state state; > }; > > static struct opt opts[256]; > @@ -76,16 +88,19 @@ void dhcp_init(void) > int i; > > for (i = 0; i < ARRAY_SIZE(opts); i++) > - opts[i].slen = -1; > - > - opts[1] = (struct opt) { 0, 4, { 0 }, 0, { 0 }, }; /* Mask */ > - opts[3] = (struct opt) { 0, 4, { 0 }, 0, { 0 }, }; /* Router */ > - opts[51] = (struct opt) { 0, 4, { 0xff, > - 0xff, > - 0xff, > - 0xff }, 0, { 0 }, }; /* Lease time */ > - opts[53] = (struct opt) { 0, 1, { 0 }, 0, { 0 }, }; /* Type */ > - opts[54] = (struct opt) { 0, 4, { 0 }, 0, { 0 }, }; /* Server ID */ > + opts[i].state = OPT_UNSET; This was needed because we were setting opts[i].state to -1. But now OPT_UNSET is 0, so you could force the assignment in the enum just to make that clear, and drop this loop. > + > + /* Mask */ > + opts[1] = (struct opt) { 0, 4, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > + /* Router */ > + opts[3] = (struct opt) { 0, 4, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > + /* Lease time */ > + opts[51] = (struct opt) { 0, 4, { 0xff, 0xff, 0xff, 0xff }, > + 0, { 0 }, OPT_DEFAULT, }; Nit: if you write it like this: opts[51] = (struct opt) { 0, 4, { 0xff, 0xff, 0xff, 0xff }, 0, { 0 }, OPT_DEFAULT, }; at least the last three fields are aligned with the other options and slightly more readable, I think. > + /* Type */ > + opts[53] = (struct opt) { 0, 1, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > + /* Server ID */ > + opts[54] = (struct opt) { 0, 4, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > } -- Stefano