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 = -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 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 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 > + */ > +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; > + > + /* 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, }; > + /* 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, }; > } > > /** > @@ -183,13 +198,13 @@ static int fill(struct msg *m) > > for (i = 0; i < opts[55].clen; i++) { > o = opts[55].c[i]; > - if (opts[o].slen != -1) > + if (opts[o].state != OPT_UNSET) > if (fill_one(m->o, OPT_MAX, o, &offset)) > debug("DHCP: skipping option %i", o); > } > > for (o = 0; o < 255; o++) { > - if (opts[o].slen != -1 && !opts[o].sent) > + if (opts[o].state != 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, size_t max_len) > int i; > > opts[119].slen = 0; > + opts[119].state = OPT_DEFAULT; > > for (i = 0; i < 255; i++) > max_len -= opts[i].slen; > @@ -291,7 +307,7 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) > } > > if (!opts[119].slen) > - opts[119].slen = -1; > + opts[119].state = OPT_UNSET; > } > > /** > @@ -389,7 +405,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > iov_drop_header(data, *olen); > } > > - opts[80].slen = -1; > + opts[80].state = OPT_UNSET; > if (opts[53].clen > 0 && opts[53].c[0] == DHCPDISCOVER) { > if (opts[80].clen == -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] = DHCPACK; > opts[80].slen = 0; > + opts[80].state = OPT_DEFAULT; > } > } else if (opts[53].clen <= 0 || opts[53].c[0] == 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) > != (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 = 14; > + opts[121].state = OPT_DEFAULT; > opts[121].s[0] = 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) > > if (c->mtu) { > opts[26].slen = 2; > + opts[26].state = OPT_DEFAULT; > opts[26].s[0] = c->mtu / 256; > opts[26].s[1] = c->mtu % 256; > } > @@ -441,12 +460,15 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > ((struct in_addr *)opts[6].s)[i] = c->ip4.dns[i]; > opts[6].slen += sizeof(uint32_t); > } > - if (!opts[6].slen) > - opts[6].slen = -1; > + if (opts[6].slen) > + opts[6].state = OPT_DEFAULT; > + else > + opts[6].state = OPT_UNSET; > > opt_len = strlen(c->hostname); > if (opt_len > 0) { > opts[12].slen = opt_len; > + opts[12].state = OPT_DEFAULT; > memcpy(opts[12].s, &c->hostname, opt_len); > } > > @@ -463,6 +485,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > encode_domain_name((char *)opts[81].s + 3, c->fqdn); > > opts[81].slen = opt_len; > + opts[81].state = OPT_DEFAULT; > } else { > debug("DHCP: client FQDN option doesn't fit, skipping"); > } > -- > 2.54.0 > -- 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