From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=f/8FYDH6; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 2BFD65A0262 for ; Mon, 20 Jul 2026 04:07:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1784513214; bh=mf2Jsluhyh4pQUae7kDkDE9/1ox++J2sMEOflk3slHI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=f/8FYDH674lLMEOIPyVOdUsH5zPwD1THM9RcX0AuGqQSzY+Dy5TlUnU4FbYco6L83 7nCfu4w9oASdEpTgaCyqUBYNoDgI2VKb3oDHh+4mq5wcUe2sIyRblZEfUuXyXIBd9L RrPxwTobgVuTq5v6jWguHiyqCEy0sdV8pOa3hinSNJ0zfFdfazGfxL0O/iSlXhcgMl Wp+XimgZ7eWKpe0GYxC0XazMw5MOkPttYtigv9kSYrbRFTKeJGepKGDBPuxP5b+T+c JhCaPU+xwN0YRTOsCeOc++7IM6CddNlahfXCO52nSRWo0yngG1L6Cxg6/JDhtLiF58 pxx4nky0GbAAg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4h3P5p49n7z4w1f; Mon, 20 Jul 2026 12:06:54 +1000 (AEST) Date: Mon, 20 Jul 2026 11:36:19 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH v5 2/7] dhcp: Add option state management with enum opt_state Message-ID: References: <20260717175648.879152-1-anskuma@redhat.com> <20260717175648.879152-3-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="0730cIISyomVtVi1" Content-Disposition: inline In-Reply-To: <20260717175648.879152-3-anskuma@redhat.com> Message-ID-Hash: 54ZOAFELACMRQYATUJZJOKK3YMM4LJGR X-Message-ID-Hash: 54ZOAFELACMRQYATUJZJOKK3YMM4LJGR X-MailFrom: dgibson@gandalf.ozlabs.org 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: sbrivio@redhat.com, passt-dev@passt.top, lvivier@redhat.com, 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: --0730cIISyomVtVi1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 17, 2026 at 11:26:39PM +0530, Anshu Kumari wrote: > Introduce enum opt_state to track each DHCP option instead > of overloading slen =3D -1 for "not set". >=20 > OPT_UNSET means the option is not configured. > OPT_DEFAULT means the option was set from host configuration. >=20 > This replaces all slen =3D -1 / slen !=3D -1 checks with > state =3D OPT_UNSET / state !=3D OPT_UNSET, and sets state =3D OPT_DEFAULT > for options initialised in dhcp_init() and at reply time in dhcp(). >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D192 > Signed-off-by: Anshu Kumari This patch looks like it correctly does what it says here. However, the commit message doesn't really explain *why* that's a desirable thing to do. > --- > v5: > - New patch: introduce enum opt_state { OPT_UNSET, OPT_DEFAULT } to rep= lace slen =3D -1 for tracking option state > - Replace all slen =3D -1 / slen !=3D -1 checks with state =3D OPT_UNSE= T / state !=3D OPT_UNSET > - Set OPT_DEFAULT for options initialised in dhcp_init() and at reply t= ime >=20 > --- > dhcp.c | 57 ++++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 40 insertions(+), 17 deletions(-) >=20 > 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" > =20 > +/** > + * enum opt_state - DHCP option state > + * @OPT_UNSET: Option not configured > + * @OPT_DEFAULT: Option set from host config > + */ > +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; > }; > =20 > static struct opt opts[256]; > @@ -76,16 +88,19 @@ void dhcp_init(void) > int i; > =20 > for (i =3D 0; i < ARRAY_SIZE(opts); i++) > - opts[i].slen =3D -1; > - > - opts[1] =3D (struct opt) { 0, 4, { 0 }, 0, { 0 }, }; /* Mask */ > - opts[3] =3D (struct opt) { 0, 4, { 0 }, 0, { 0 }, }; /* Router */ > - opts[51] =3D (struct opt) { 0, 4, { 0xff, > - 0xff, > - 0xff, > - 0xff }, 0, { 0 }, }; /* Lease time */ > - opts[53] =3D (struct opt) { 0, 1, { 0 }, 0, { 0 }, }; /* Type */ > - opts[54] =3D (struct opt) { 0, 4, { 0 }, 0, { 0 }, }; /* Server ID = */ > + opts[i].state =3D OPT_UNSET; > + > + /* Mask */ > + opts[1] =3D (struct opt) { 0, 4, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > + /* Router */ > + opts[3] =3D (struct opt) { 0, 4, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > + /* Lease time */ > + opts[51] =3D (struct opt) { 0, 4, { 0xff, 0xff, 0xff, 0xff }, > + 0, { 0 }, OPT_DEFAULT, }; > + /* Type */ > + opts[53] =3D (struct opt) { 0, 1, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > + /* Server ID */ > + opts[54] =3D (struct opt) { 0, 4, { 0 }, 0, { 0 }, OPT_DEFAULT, }; > } > =20 > /** > @@ -183,13 +198,13 @@ static int fill(struct msg *m) > =20 > for (i =3D 0; i < opts[55].clen; i++) { > o =3D opts[55].c[i]; > - if (opts[o].slen !=3D -1) > + if (opts[o].state !=3D OPT_UNSET) > if (fill_one(m->o, OPT_MAX, o, &offset)) > debug("DHCP: skipping option %i", o); > } > =20 > for (o =3D 0; o < 255; o++) { > - if (opts[o].slen !=3D -1 && !opts[o].sent) > + if (opts[o].state !=3D OPT_UNSET && !opts[o].sent) > if (fill_one(m->o, OPT_MAX, o, &offset)) > debug("DHCP: skipping option %i", o); > } > @@ -243,6 +258,7 @@ static void opt_set_dns_search(const struct ctx *c, s= ize_t max_len) > int i; > =20 > opts[119].slen =3D 0; > + opts[119].state =3D OPT_DEFAULT; > =20 > for (i =3D 0; i < 255; i++) > max_len -=3D opts[i].slen; > @@ -291,7 +307,7 @@ static void opt_set_dns_search(const struct ctx *c, s= ize_t max_len) > } > =20 > if (!opts[119].slen) > - opts[119].slen =3D -1; > + opts[119].state =3D OPT_UNSET; > } > =20 > /** > @@ -389,7 +405,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > iov_drop_header(data, *olen); > } > =20 > - opts[80].slen =3D -1; > + opts[80].state =3D OPT_UNSET; > if (opts[53].clen > 0 && opts[53].c[0] =3D=3D DHCPDISCOVER) { > if (opts[80].clen =3D=3D -1) { > info("DHCP: offer to discover"); > @@ -398,6 +414,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > info("DHCP: ack to discover (Rapid Commit)"); > opts[53].s[0] =3D DHCPACK; > opts[80].slen =3D 0; > + opts[80].state =3D OPT_DEFAULT; > } > } else if (opts[53].clen <=3D 0 || opts[53].c[0] =3D=3D DHCPREQUEST) { > info("%s: ack to request", /* DHCP needs a valid message type */ > @@ -421,6 +438,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > !=3D (c->ip4.guest_gw.s_addr & mask.s_addr)) { > /* a.b.c.d/32:0.0.0.0, 0:a.b.c.d */ > opts[121].slen =3D 14; > + opts[121].state =3D OPT_DEFAULT; > opts[121].s[0] =3D 32; > memcpy(opts[121].s + 1, > &c->ip4.guest_gw, sizeof(c->ip4.guest_gw)); > @@ -430,6 +448,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > =20 > if (c->mtu) { > opts[26].slen =3D 2; > + opts[26].state =3D OPT_DEFAULT; > opts[26].s[0] =3D c->mtu / 256; > opts[26].s[1] =3D c->mtu % 256; > } > @@ -441,12 +460,15 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > ((struct in_addr *)opts[6].s)[i] =3D c->ip4.dns[i]; > opts[6].slen +=3D sizeof(uint32_t); > } > - if (!opts[6].slen) > - opts[6].slen =3D -1; > + if (opts[6].slen) > + opts[6].state =3D OPT_DEFAULT; > + else > + opts[6].state =3D OPT_UNSET; > =20 > opt_len =3D strlen(c->hostname); > if (opt_len > 0) { > opts[12].slen =3D opt_len; > + opts[12].state =3D OPT_DEFAULT; > memcpy(opts[12].s, &c->hostname, opt_len); > } > =20 > @@ -463,6 +485,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > encode_domain_name((char *)opts[81].s + 3, c->fqdn); > =20 > opts[81].slen =3D opt_len; > + opts[81].state =3D OPT_DEFAULT; > } else { > debug("DHCP: client FQDN option doesn't fit, skipping"); > } > --=20 > 2.54.0 >=20 --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --0730cIISyomVtVi1 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpde5IACgkQzQJF27ox 2GeUSBAAhT21unwEcIAEfiHgn3eg4kiWt8ZrcWGkN1kfYmuTjfP1w/q4fTt1n7dr I5MaPoCfE8O6j+Ri1gB3B+6RD2ZwYg265txT1kfpmkaOgbQLGD54AgHrvVs4IsEL CvV9qPv6NH/TdEqYQNbDTsUriV0ggEQl+SArtPfs1Kf/tV77oYxVeA4m6aKTuqKH uiXw9kwKeuXhuuUw5ViuK6B/TMzv8Lix2CaegPZcxee/7Qr6I2H10J+hfz/Fg03x /55qtQgoiB7e8PDnoOWwrFr8qvdVLlWoH+LtnWmKTDwk6Hmx14e/4TjSziYYevws ANYqXTuj6BXTbu3NDCkmCQ1XeEVKyr7cJDRy9KmLViYJtRQCvk840eqpJjrq+icv k9y0mYwPp9v/EEu73vhz1n3ddr46hBZwo5m4c+9RPBNpn8GBqq9VcXPSxO9HgHTn HF9zOrFvmvLDVjGH4Lf3myxKPHz4TFeMqyNnoWWL9cwKa8iMHHy0zPatI6hRSY1h 8SBnoBnKYnJHv+j+J+wXYcNVjftIabbKbqsmcF6e8U7pkJTSOX8WQXUNlXA/ewM2 X2rC3V4ys7y64vfOz+7bLK6kQr5WiPhCfEIcPfd8S379Leifi/7hg23ebpv7gqui w4w8ba0Vk0uDFgNq+Y7xcJ1XaLZcUU8Kq5wTDRlUcanZMUysQmY= =JB7I -----END PGP SIGNATURE----- --0730cIISyomVtVi1--