Re: [PATCH v3 3/6] dhcp: Add option type table and value parser

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2578 bytes --]

On Fri, Jun 12, 2026 at 01:04:37AM +0200, Stefano Brivio wrote:
> On Tue, 2 Jun 2026 12:23:47 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Jun 01, 2026 at 01:07:53PM +0530, Anshu Kumari wrote:
> >
> > > [...]
> > >
> > > +/**
> > > + * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by code
> > > + */
> > > +static const enum dhcp_opt_type dhcp_opt_types[256] = {
> > > +	[1]   = DHCP_OPT_IPV4,		/* Subnet Mask */
> > > +	[2]   = DHCP_OPT_INT32,		/* Time Offset */
> > > +	[3]   = DHCP_OPT_IPV4_LIST,	/* Router */  
> > 
> > I'm still a bit unsure if we want to allow user modification of the
> > options, like this one, which we already manage ourselves.
> 
> I think we don't lose any functionality if we allow users to overwrite
> them, and we might gain some: for example one might want to directly
> set a specific default route in the target network namespace, in pasta,
> but change it as a DHCP client runs.
> 
> Not that I see any point in doing that, but it's harmless, so why not.
> Maybe even just for testing, go figure.

Fair point, I concur.

> > > [...]
> > >
> > > @@ -61,7 +227,15 @@ int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
> > >  		c->custom_opts_count++;
> > >  	}
> > >  
> > > +	ret = dhcp_opt_parse(code, val_str,
> > > +			     c->custom_opts[idx].val,
> > > +			     sizeof(c->custom_opts[0].val));  
> > 
> > Now that this parsing and adding code is all in dhcp.c, could we parse
> > the options directly into the existing opts[] global, rather than
> > requiring both the string and parsed forms in c->custom_opts?
> 
> I see one downside with that: the day we add the possibility for
> pesto(1) to set and read out options, we would need to add an
> implementation to decode them back, which could be rather complicated
> if you think of stuff such as "compressed" (yes, that madness) domain
> names.

Sorry, let me clarify.  I'm not suggesting we remove the string
version of the value, for exactly that reason (along with making
debugging messages easier).

However, at the moment I think we end up with *two* copies of the
parsed binary value: one in c->custom_opts[].val, and another in the
opts[].s global within dhcp.c.  I'm hoping we can remove one of them;
the one in custom_opts seeming like the easier one.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]