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=202602 header.b=PtoHmfDX; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id B39AB5A0265 for ; Fri, 05 Jun 2026 03:08:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1780621736; bh=FdOnhgx5Dt8x0FoY1Y6cEPen5OeMt/33gM0dyyA8dag=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PtoHmfDXhsZJCI5fsxLrkM1oZcEsuo+gjy7/oMoweAKC32FSJcuLfwrnaIkrG1rJl OUFXYaVeQsC5CuwEwXJ/DYs4ZJCqWrH/jlD+eyAGUH67yj+kTIdRf7N6m2VJowU29n Svsz+puwm5KG2aiq0HmXuR7tFmE1f2tsp6tg9Xrnc8l7lxSGF72m/P+VGZWbdT/Xuj PPWgigeutb+cXizhujv9dAiZwbEYVuVw5H68fcTit7sgI9zsZBQ2KoYKYGgxsNYtGG ClPEo+wmKh09FdGh/xgPloyDD57y2mI+uRfCVuzSTxFlkAd353DJevGCRv/9PeaW0F xGfKMBsUqEJAQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gWjxh4zktz58dt; Fri, 05 Jun 2026 11:08:56 +1000 (AEST) Date: Fri, 5 Jun 2026 11:08:52 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH 2/3] dhcpv6: Add option type table and value parser Message-ID: References: <20260604105150.1977905-1-anskuma@redhat.com> <20260604105150.1977905-3-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="OWJykQgs6hCwmzy4" Content-Disposition: inline In-Reply-To: <20260604105150.1977905-3-anskuma@redhat.com> Message-ID-Hash: X3THNBHA7EOYF4WVUKNXIHDRPYX3BPNO X-Message-ID-Hash: X3THNBHA7EOYF4WVUKNXIHDRPYX3BPNO 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: passt-dev@passt.top, sbrivio@redhat.com, jmaloy@redhat.com, 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: --OWJykQgs6hCwmzy4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 04, 2026 at 04:21:49PM +0530, Anshu Kumari wrote: > Add a type table mapping DHCPv6 option codes to value types, > and a parser that encodes CLI strings into the correct binary wire > format for each type. >=20 > Supported types: plain string, IPv6 address, IPv6 address list, > 8/16/32-bit integers, vendor class (enterprise number + length-prefixed > data, per RFC 8415 Section 21.16), status code (uint16 + message), > and length-prefixed string lists. >=20 > Modify dhcpv6_add_option() to call the parser for validation and binary > encoding at configuration time, storing both the raw string and the > encoded value. >=20 > Signed-off-by: Anshu Kumari Many comments below, but all of them minor. > --- > dhcpv6.c | 228 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > passt.1 | 28 ++++++- > passt.h | 4 + > 3 files changed, 255 insertions(+), 5 deletions(-) >=20 > diff --git a/dhcpv6.c b/dhcpv6.c > index a0fb77c..85e2926 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > =20 > #include "packet.h" > #include "util.h" > @@ -32,20 +33,233 @@ > #include "tap.h" > #include "log.h" > =20 > +/** > + * enum dhcpv6_opt_type - DHCPv6 option value types > + * @DHCPV6_OPT_NONE: Unsupported or unknown option > + * @DHCPV6_OPT_STR: Variable-length string > + * @DHCPV6_OPT_IPV6: Single IPv6 address > + * @DHCPV6_OPT_IPV6_LIST: Multiple IPv6 addresses, comma-separated > + * @DHCPV6_OPT_INT8: Unsigned 8-bit integer > + * @DHCPV6_OPT_INT16: Unsigned 16-bit integer > + * @DHCPV6_OPT_INT32: Unsigned 32-bit integer > + * @DHCPV6_OPT_VENDOR_CLASS: Enterprise number + length-prefixed data > + * @DHCPV6_OPT_LEN_STR_LIST: Length-prefixed string list > + */ > +enum dhcpv6_opt_type { > + DHCPV6_OPT_NONE, > + DHCPV6_OPT_STR, > + DHCPV6_OPT_IPV6, > + DHCPV6_OPT_IPV6_LIST, > + DHCPV6_OPT_INT8, > + DHCPV6_OPT_INT16, > + DHCPV6_OPT_INT32, > + DHCPV6_OPT_VENDOR_CLASS, > + DHCPV6_OPT_LEN_STR_LIST, > +}; > + > +/** > + * dhcpv6_opt_types - Maps DHCPv6 option code to value type, indexed by = code > + * RFC 8415 Options: 7, 15, 16, 17, 32, 82, 83 > + * RFC 5970 Options: 59, 60 > + * RFC 4075 Options: 31 > + */ > +static const enum dhcpv6_opt_type dhcpv6_opt_types[256] =3D { Nit: AFAICT when you look up a value here, you already check against the array size. So, I think you could not specify the array size explicitly here, and the compiler should infer it from initializer, omitting all the empty space past option 83. > + [7] =3D DHCPV6_OPT_INT8, /* Preference */ > + [15] =3D DHCPV6_OPT_LEN_STR_LIST, /* User Class */ > + [16] =3D DHCPV6_OPT_VENDOR_CLASS, /* Vendor Class */ > + [17] =3D DHCPV6_OPT_VENDOR_CLASS, /* Vendor Opts */ > + [31] =3D DHCPV6_OPT_IPV6_LIST, /* SNTP Servers */ > + [32] =3D DHCPV6_OPT_INT32, /* Information Refresh Time */ > + [59] =3D DHCPV6_OPT_STR, /* Boot File URL */ > + [60] =3D DHCPV6_OPT_LEN_STR_LIST, /* Boot File Params */ > + [82] =3D DHCPV6_OPT_INT32, /* SOL_MAX_RT */ > + [83] =3D DHCPV6_OPT_INT32, /* INF_MAX_RT */ > +}; > + > +/** > + * dhcpv6_opt_parse() - Parse a DHCPv6 option value string into binary > + * @code: DHCPv6 option code > + * @str: Value string from command line > + * @buf: Output buffer for binary value > + * @buf_len: Size of output buffer > + * > + * Return: number of bytes written to @buf, or -1 on error > + */ > +static int dhcpv6_opt_parse(uint16_t code, const char *str, > + uint8_t *buf, size_t buf_len) > +{ > + char chunk[INET6_ADDRSTRLEN]; > + enum dhcpv6_opt_type type; > + unsigned long val; > + const char *colon; > + unsigned int i; > + uint8_t width; > + size_t slen; > + char *end; > + int len; > + > + if (!*str) > + die("Empty value for DHCPv6 option %u", code); Nit: I can imagine an option for which an empty string is a valid and meaningful value, though I don't know if there exist any in practice. > + > + if (code >=3D ARRAY_SIZE(dhcpv6_opt_types)) > + die("Unsupported DHCPv6 option: %u," > + " see passt(1) for supported codes", code); > + > + type =3D dhcpv6_opt_types[code]; > + > + switch (type) { > + case DHCPV6_OPT_NONE: > + die("Unsupported DHCPv6 option: %u," > + " see passt(1) for supported codes", code); > + case DHCPV6_OPT_IPV6: > + case DHCPV6_OPT_IPV6_LIST: > + len =3D 0; > + > + while (*str) { Nit: 'chunk' can be moved into this while block. > + slen =3D strcspn(str, ","); > + if (!slen || slen >=3D sizeof(chunk)) Nit: I don't think you need the explicit !slen check - inet_pton() should fail on an empty string anyway. > + return -1; > + > + if (len + (int)sizeof(struct in6_addr) > (int)buf_len) > + return -1; > + > + memcpy(chunk, str, slen); > + chunk[slen] =3D '\0'; > + > + if (inet_pton(AF_INET6, chunk, > + buf + len) !=3D 1) > + return -1; > + > + len +=3D sizeof(struct in6_addr); > + > + if (type =3D=3D DHCPV6_OPT_IPV6) { > + if (str[slen] =3D=3D ',') > + return -1; > + break; > + } > + > + str +=3D slen; > + if (*str =3D=3D ',') > + str++; > + } > + > + if (!len) > + return -1; > + > + return len; > + case DHCPV6_OPT_INT8: > + case DHCPV6_OPT_INT16: > + case DHCPV6_OPT_INT32: > + if (type =3D=3D DHCPV6_OPT_INT8) > + width =3D 1; > + else if (type =3D=3D DHCPV6_OPT_INT16) > + width =3D 2; > + else > + width =3D 4; > + > + errno =3D 0; > + val =3D strtoul(str, &end, 0); > + > + if (*end || errno) > + return -1; > + > + if (buf_len < width) > + return -1; > + > + if (val >=3D (1ULL << (width * 8))) > + return -1; > + > + for (i =3D width; i > 0; i--) { > + buf[i - 1] =3D val & 0xff; > + val >>=3D 8; > + } > + > + return width; > + case DHCPV6_OPT_STR: > + slen =3D strlen(str); > + > + if (!slen || slen >=3D buf_len) Nit: Again, I'm not sure an empty string option is necessarily impossible, and in any case you already checked for that case. > + return -1; > + > + memcpy(buf, str, slen); > + > + return slen; > + case DHCPV6_OPT_VENDOR_CLASS: > + colon =3D strchr(str, ':'); > + if (!colon) > + return -1; > + > + errno =3D 0; > + val =3D strtoul(str, &end, 0); > + if (end !=3D colon || errno || val > UINT32_MAX) > + return -1; > + > + slen =3D strlen(colon + 1); > + if (!slen) > + return -1; > + > + len =3D sizeof(uint32_t) + sizeof(uint16_t) + slen; > + if ((size_t)len > buf_len) > + return -1; > + > + uint32_t ent =3D htonl(val); Nit: Although they are permitted in C11, by convention we avoid inline declarations in passt. > + > + memcpy(buf, &ent, sizeof(ent)); > + > + buf[4] =3D slen >> 8; > + buf[5] =3D slen & 0xff; > + > + memcpy(buf + sizeof(uint32_t) + sizeof(uint16_t), > + colon + 1, slen); For this type of structured option value, it might be nicer to declare a structure and cast the buf pointer into that. This approach is also fine, though. > + > + return len; > + case DHCPV6_OPT_LEN_STR_LIST: > + len =3D 0; > + > + while (*str) { > + slen =3D strcspn(str, ","); > + if (!slen) > + return -1; > + > + if (len + (int)(sizeof(uint16_t) + slen) > (int)buf_len) > + return -1; > + > + buf[len] =3D slen >> 8; > + buf[len + 1] =3D slen & 0xff; > + len +=3D sizeof(uint16_t); > + > + memcpy(buf + len, str, slen); > + len +=3D slen; > + > + str +=3D slen; > + if (*str =3D=3D ',') > + str++; > + } > + > + if (!len) > + return -1; > + > + return len; > + } > + > + return -1; > +} > + > /** > * dhcpv6_add_option() - Add or update a custom DHCPv6 option > * @c: Execution context > * @code: DHCPv6 option code > * @val_str: Value string from command line > * > - * Stores the option code and raw string value. Binary encoding and > - * injection into DHCPv6 replies are handled by later patches. > + * Parses @val_str according to the type registered for @code in > + * dhcpv6_opt_types[]. If @code was already added, the previous value > + * is overwritten. > * > * Return: 0 on success > */ > int dhcpv6_add_option(struct ctx *c, uint16_t code, const char *val_str) > { > - int idx; > + int idx, ret; > =20 > for (idx =3D 0; idx < c->custom_v6opts_count; idx++) { > if (c->custom_v6opts[idx].code =3D=3D code) > @@ -59,7 +273,15 @@ int dhcpv6_add_option(struct ctx *c, uint16_t code, c= onst char *val_str) > c->custom_v6opts_count++; > } > =20 > + ret =3D dhcpv6_opt_parse(code, val_str, > + c->custom_v6opts[idx].val, > + sizeof(c->custom_v6opts[0].val)); Unlike IPV4 DHCP, we don't appear to have an existing array of DHCP option values in the DHCPv6 code we could reuse. Nonetheless, it seems like it would be nicer to have that table of values in something local to the DHCPv6 code, rather than in the global context structure. > + if (ret < 0) > + die("Invalid value for DHCPv6 option %u: %s", > + code, val_str); > + > c->custom_v6opts[idx].code =3D code; > + c->custom_v6opts[idx].len =3D ret; > =20 > if (snprintf_check(c->custom_v6opts[idx].str, > sizeof(c->custom_v6opts[0].str), > diff --git a/passt.1 b/passt.1 > index 9c25214..3c4f3ac 100644 > --- a/passt.1 > +++ b/passt.1 > @@ -433,8 +433,32 @@ Send \fIname\fR as Client FQDN: DHCP option 81 and D= HCPv6 option 39. > .TP > .BR \-\-dhcpv6-opt " " \fICODE\fR,\fIVALUE\fR > Set a DHCPv6 option by numeric code. The value format is determined > -automatically from the option code. This option can be specified multiple > -times. If the same option code is given more than once, the last value w= ins. > +automatically from the option code. Multiple IPv6 addresses are > +comma-separated. This option can be specified multiple times. If the same > +option code is given more than once, the last value wins. > +.RS > +.TP > +.B String options > +59 (Boot File URL, RFC 5970) > +.TP > +.B Length-prefixed string list options (comma-separated entries) > +15 (User Class, RFC 8415), 60 (Boot File Params, RFC 5970). > +Each comma-separated entry is encoded with a 2-byte length prefix. > +Example: \fB\-\-dhcpv6-opt 15,class1,class2\fR. > +.TP > +.B Vendor class options (ENTERPRISE:DATA format) > +16 (Vendor Class, RFC 8415), 17 (Vendor-specific Info, RFC 8415). > +VALUE is \fIENTERPRISE\fR:\fIDATA\fR where \fIENTERPRISE\fR is the IANA > +Private Enterprise Number and \fIDATA\fR is the vendor class string. > +Example: \fB\-\-dhcpv6-opt 16,0:HTTPClient\fR for UEFI HTTP Boot. > +.TP > +.B IPv6 address list options (comma-separated) > +31 (SNTP Servers) > +.TP > +.B Integer options > +7 (Preference, 8-bit), 32 (Information Refresh Time, 32-bit), > +82 (SOL_MAX_RT, 32-bit), 83 (INF_MAX_RT, 32-bit) > +.RE > =20 > .TP > .BR \-t ", " \-\-tcp-ports " " \fIspec > diff --git a/passt.h b/passt.h > index 91509df..21f43d8 100644 > --- a/passt.h > +++ b/passt.h > @@ -184,6 +184,8 @@ struct ip6_ctx { > * @fqdn: Guest FQDN > * @custom_v6opts: User-specified DHCPv6 options from --dhcpv6-opt > * @custom_v6opts.code: DHCPv6 option code > + * @custom_v6opts.len: Length of binary value in @val > + * @custom_v6opts.val: Binary-encoded option value > * @custom_v6opts.str: Original string value from command line > * @custom_v6opts_count:Number of entries in @custom_v6opts > * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 disabled > @@ -271,6 +273,8 @@ struct ctx { > =20 > struct { > uint16_t code; > + uint16_t len; > + uint8_t val[255]; > char str[256]; > } custom_v6opts[MAX_CUSTOM_DHCPV6_OPTS]; > int custom_v6opts_count; > --=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 --OWJykQgs6hCwmzy4 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoiIaMACgkQzQJF27ox 2GfKow//YwZiVbfmbPxlWXxIOywqPmCkH882VSrYWN4YSxVigE89ryLLPdEjC3Tf jLcljj3QegB7oRyKBf6ASZX/o3CO2xrakUpMohHoMZChoZjxbZJCU5Gy2GypakLS 2vkY/E8Qcf6iSbGRFX7ceaaeqZYVXO4r0QfaPssxRsvAyNq1+6mu4FiTtUzO1t59 2ciooOCFumlQe4eh1rNSdCdYSMotw9pHn/7ovujIdNmqftWA1NTk7Z1gFMPHyoIx ULLBXelio7RYR7bAIuZcOrOSBLlqlMaClsSZ5pyN90Q9JW1v6EHHVfezcpdY6i8S RzL/5mnTw3//ClHQerIROVnOCSfGsfXli81Liup4EOcYD2x10D1wZ1pIvx4jR3YE N3JHFZq5RNbRsK7iTUUFht20wkTpow42Pz4/D0edZGHQ93h4FlEpWhVIqs65FV/M opr0fH2gKY5Zqp2zu3/yn224LsObDp+rEflJL6F0JOc89EH2SOOt/sGXZkE+Lmk5 WmlL3EFdfgHivl88h3tnyyOXceTTZecoRbvlxAVGdRYVD8zIvsbFu+poxMxW67gp gIAfyW6V0gus7mGmn1x/9EGujFv1zsNQs/14npPk30nQSAWTY+tDbEsTFazEWBuM q22CbopulwkXpegtUc0P8g+5EZo/zHZ4X3/CZTcRdg1SkUQ/qIE= =WDND -----END PGP SIGNATURE----- --OWJykQgs6hCwmzy4--