* [PATCH v4 0/4] Add --dhcp-boot and --dhcp-opt options
@ 2026-06-17 13:22 Anshu Kumari
2026-06-17 13:22 ` [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Anshu Kumari @ 2026-06-17 13:22 UTC (permalink / raw)
To: passt-dev, anskuma, sbrivio; +Cc: jmaloy, david, lvivier
This series adds support for custom DHCP options in passt, enabling
network boot (PXE/UEFI HTTP Boot) and arbitrary DHCP option injection.
Two new command-line flags are introduced:
--dhcp-boot URL Sets the boot file URL (DHCP option 67 and the
legacy boot file field)
--dhcp-opt CODE,VALUE
Sets any DHCP option by numeric code, with
type-aware parsing per RFC 2132
The DHCP reply path is extended with option overload support (RFC 2132
option 52), allowing options to overflow into the file and sname fields
when the standard options area is full.
v4:
Restructured series from 6 patches to 4:
- 1/4 refactor fill_one()
- 2/4 option overload
- 3/4 --dhcp-opt with parser and man page
- 4/4 --dhcp-boot with man page
Anshu Kumari (4):
dhcp: Refactor fill_one() to operate on a generic buffer
dhcp: Add option overload
dhcp: Add --dhcp-opt with option table and value parser
conf: Add --dhcp-boot command-line option
conf.c | 50 ++++++++-
dhcp.c | 321 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
dhcp.h | 2 +
passt.1 | 49 +++++++++
passt.h | 6 ++
5 files changed, 407 insertions(+), 21 deletions(-)
--
2.54.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer 2026-06-17 13:22 [PATCH v4 0/4] Add --dhcp-boot and --dhcp-opt options Anshu Kumari @ 2026-06-17 13:22 ` Anshu Kumari 2026-06-19 3:26 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio 2026-06-17 13:22 ` [PATCH v4 2/4] dhcp: Add option overload Anshu Kumari ` (2 subsequent siblings) 3 siblings, 2 replies; 18+ messages in thread From: Anshu Kumari @ 2026-06-17 13:22 UTC (permalink / raw) To: passt-dev, anskuma, sbrivio; +Cc: jmaloy, david, lvivier Change fill_one() to accept a buffer pointer and capacity instead of a struct msg pointer. This is a pure refactor with no behavior change, preparing for option overload support where fill_one() will also write into the file and sname fields. Link: https://bugs.passt.top/show_bug.cgi?id=192 Signed-off-by: Anshu Kumari <anskuma@redhat.com> --- v4: - Unchanged from v3. v3: - Restored removed comments: "If we don't have space to write the option, then just skip" and "Move to option". v2: - Renamed parameter cap → size. --- dhcp.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/dhcp.c b/dhcp.c index 1ff8cba..8e3ffd6 100644 --- a/dhcp.c +++ b/dhcp.c @@ -131,28 +131,29 @@ struct msg { } __attribute__((__packed__)); /** - * fill_one() - Fill a single option in message - * @m: Message to fill + * fill_one() - Fill a single option into a buffer + * @buf: Buffer to write option + * @size: Usable size of @buf (excluding end marker) * @o: Option number - * @offset: Current offset within options field, updated on insertion + * @offset: Current offset within @buf, updated on insertion * - * Return: false if m has space to write the option, true otherwise + * Return: false if @buf has space to write the option, true otherwise */ -static bool fill_one(struct msg *m, int o, int *offset) +static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) { size_t slen = opts[o].slen; /* If we don't have space to write the option, then just skip */ - if (*offset + 2 /* code and length of option */ + slen > OPT_MAX) + if (*offset + 2 + slen > size) return true; - m->o[*offset] = o; - m->o[*offset + 1] = slen; + buf[*offset] = o; + buf[*offset + 1] = slen; /* Move to option */ *offset += 2; - memcpy(&m->o[*offset], opts[o].s, slen); + memcpy(&buf[*offset], opts[o].s, slen); opts[o].sent = 1; *offset += slen; @@ -177,20 +178,17 @@ static int fill(struct msg *m) * Put it there explicitly, unless requested via option 55. */ if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) - if (fill_one(m, 53, &offset)) - debug("DHCP: skipping option 53"); + fill_one(m->o, OPT_MAX, 53, &offset); for (i = 0; i < opts[55].clen; i++) { o = opts[55].c[i]; if (opts[o].slen != -1) - if (fill_one(m, o, &offset)) - debug("DHCP: skipping option %i", o); + fill_one(m->o, OPT_MAX, o, &offset); } for (o = 0; o < 255; o++) { if (opts[o].slen != -1 && !opts[o].sent) - if (fill_one(m, o, &offset)) - debug("DHCP: skipping option %i", o); + fill_one(m->o, OPT_MAX, o, &offset); } m->o[offset++] = 255; -- 2.54.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer 2026-06-17 13:22 ` [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari @ 2026-06-19 3:26 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio 1 sibling, 0 replies; 18+ messages in thread From: David Gibson @ 2026-06-19 3:26 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 3443 bytes --] On Wed, Jun 17, 2026 at 06:52:35PM +0530, Anshu Kumari wrote: > Change fill_one() to accept a buffer pointer and capacity instead of > a struct msg pointer. This is a pure refactor with no behavior change, > preparing for option overload support where fill_one() will also write > into the file and sname fields. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > --- > v4: > - Unchanged from v3. Uh.. this doesn't appear to be true... > > v3: > - Restored removed comments: "If we don't have space to write the > option, then just skip" and "Move to option". > > v2: > - Renamed parameter cap → size. > --- > dhcp.c | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > > diff --git a/dhcp.c b/dhcp.c > index 1ff8cba..8e3ffd6 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -131,28 +131,29 @@ struct msg { > } __attribute__((__packed__)); > > /** > - * fill_one() - Fill a single option in message > - * @m: Message to fill > + * fill_one() - Fill a single option into a buffer > + * @buf: Buffer to write option > + * @size: Usable size of @buf (excluding end marker) > * @o: Option number > - * @offset: Current offset within options field, updated on insertion > + * @offset: Current offset within @buf, updated on insertion > * > - * Return: false if m has space to write the option, true otherwise > + * Return: false if @buf has space to write the option, true otherwise > */ > -static bool fill_one(struct msg *m, int o, int *offset) > +static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) > { > size_t slen = opts[o].slen; > > /* If we don't have space to write the option, then just skip */ > - if (*offset + 2 /* code and length of option */ + slen > OPT_MAX) > + if (*offset + 2 + slen > size) > return true; > > - m->o[*offset] = o; > - m->o[*offset + 1] = slen; > + buf[*offset] = o; > + buf[*offset + 1] = slen; > > /* Move to option */ > *offset += 2; > > - memcpy(&m->o[*offset], opts[o].s, slen); > + memcpy(&buf[*offset], opts[o].s, slen); ..LGTM so far, but.. > > opts[o].sent = 1; > *offset += slen; > @@ -177,20 +178,17 @@ static int fill(struct msg *m) > * Put it there explicitly, unless requested via option 55. > */ > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > - if (fill_one(m, 53, &offset)) > - debug("DHCP: skipping option 53"); > + fill_one(m->o, OPT_MAX, 53, &offset); ..you've removed all the debug messages on failed inserts. Maybe there's a reason for that, but it's not mentioned in the commit message, and is new in v4. > > for (i = 0; i < opts[55].clen; i++) { > o = opts[55].c[i]; > if (opts[o].slen != -1) > - if (fill_one(m, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, OPT_MAX, o, &offset); > } > > for (o = 0; o < 255; o++) { > if (opts[o].slen != -1 && !opts[o].sent) > - if (fill_one(m, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, OPT_MAX, o, &offset); > } > > m->o[offset++] = 255; > -- > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer 2026-06-17 13:22 ` [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari 2026-06-19 3:26 ` David Gibson @ 2026-07-01 7:51 ` Stefano Brivio 1 sibling, 0 replies; 18+ messages in thread From: Stefano Brivio @ 2026-07-01 7:51 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, jmaloy, david, lvivier On Wed, 17 Jun 2026 18:52:35 +0530 Anshu Kumari <anskuma@redhat.com> wrote: > Change fill_one() to accept a buffer pointer and capacity instead of > a struct msg pointer. This is a pure refactor with no behavior change, > preparing for option overload support where fill_one() will also write > into the file and sname fields. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > --- > v4: > - Unchanged from v3. > > v3: > - Restored removed comments: "If we don't have space to write the > option, then just skip" and "Move to option". > > v2: > - Renamed parameter cap → size. > --- > dhcp.c | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > > diff --git a/dhcp.c b/dhcp.c > index 1ff8cba..8e3ffd6 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -131,28 +131,29 @@ struct msg { > } __attribute__((__packed__)); > > /** > - * fill_one() - Fill a single option in message > - * @m: Message to fill > + * fill_one() - Fill a single option into a buffer > + * @buf: Buffer to write option > + * @size: Usable size of @buf (excluding end marker) > * @o: Option number > - * @offset: Current offset within options field, updated on insertion > + * @offset: Current offset within @buf, updated on insertion > * > - * Return: false if m has space to write the option, true otherwise > + * Return: false if @buf has space to write the option, true otherwise > */ > -static bool fill_one(struct msg *m, int o, int *offset) > +static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) > { > size_t slen = opts[o].slen; > > /* If we don't have space to write the option, then just skip */ > - if (*offset + 2 /* code and length of option */ + slen > OPT_MAX) > + if (*offset + 2 + slen > size) This looks like an automatic linter dropping comments, but I think the original comment is quite useful. See also Laurent's review of v2: https://archives.passt.top/passt-dev/3b36426b-798c-489c-aad2-abcde13d2e93@redhat.com/ > return true; > > - m->o[*offset] = o; > - m->o[*offset + 1] = slen; > + buf[*offset] = o; > + buf[*offset + 1] = slen; > > /* Move to option */ > *offset += 2; > > - memcpy(&m->o[*offset], opts[o].s, slen); > + memcpy(&buf[*offset], opts[o].s, slen); I think slightly more idiomatic: buf + *offset. Otherwise you have a pointer, dereference it, and then take its address. It looks a bit convoluted. Conversely, using buf[*offset] above makes sense because you need to dereference buf anyway. > > opts[o].sent = 1; > *offset += slen; > @@ -177,20 +178,17 @@ static int fill(struct msg *m) > * Put it there explicitly, unless requested via option 55. > */ > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > - if (fill_one(m, 53, &offset)) > - debug("DHCP: skipping option 53"); > + fill_one(m->o, OPT_MAX, 53, &offset); > > for (i = 0; i < opts[55].clen; i++) { > o = opts[55].c[i]; > if (opts[o].slen != -1) > - if (fill_one(m, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, OPT_MAX, o, &offset); > } > > for (o = 0; o < 255; o++) { > if (opts[o].slen != -1 && !opts[o].sent) > - if (fill_one(m, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, OPT_MAX, o, &offset); > } > > m->o[offset++] = 255; I also had the same comment as David's about dropping debug() messages... until I reached 2/4, which is a good indication that they should go away later. -- Stefano ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 2/4] dhcp: Add option overload 2026-06-17 13:22 [PATCH v4 0/4] Add --dhcp-boot and --dhcp-opt options Anshu Kumari 2026-06-17 13:22 ` [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari @ 2026-06-17 13:22 ` Anshu Kumari 2026-06-19 3:39 ` David Gibson ` (2 more replies) 2026-06-17 13:22 ` [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser Anshu Kumari 2026-06-17 13:22 ` [PATCH v4 4/4] conf: Add --dhcp-boot command-line option Anshu Kumari 3 siblings, 3 replies; 18+ messages in thread From: Anshu Kumari @ 2026-06-17 13:22 UTC (permalink / raw) To: passt-dev, anskuma, sbrivio; +Cc: jmaloy, david, lvivier When the options field is full, overflow remaining DHCP options into the sname and file fields per RFC 2132 option 52. Per RFC 2132, Section 9.5, the boot file name is always placed in the 'file' header field. When a boot file is set, the file field is reserved from overload and overflow uses only the sname field. Link: https://bugs.passt.top/show_bug.cgi?id=192 Signed-off-by: Anshu Kumari <anskuma@redhat.com> --- v4: - Converted overload #defines to enum dhcp_overload. - Fixed missing whitespace in comment before */. - Boot file name always placed in 'file' header field per RFC 2132, Section 9.5; file field reserved from overload when bootfile is set; option 67 suppressed from options area. v3: - Added RFC 2132 Section 9.3 reference comment on overload constants. - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow(). - Swapped overflow order: try sname (64 bytes) first, then file (128 bytes) — better packing and keeps file field available for boot file name. - Removed '&' from &reply.file. - Removed '+1' from memcpy — reply.file already zeroed. - opt_set_dns_search() max_len: OPT_MAX - 3 instead of sizeof(m->o). v2: - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME constants - Added comment documenting space reservation: /* Reserve 3 bytes for option 52 */ - Fixed DNS search length: sizeof(m->o) only, not combined with file+sname - Removed dhcp_boot references — reply.file copy now reads from opts[67] - Used DHCP_OVERLOAD_FILE constant in reply.file guard --- dhcp.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 8 deletions(-) diff --git a/dhcp.c b/dhcp.c index 8e3ffd6..78790d8 100644 --- a/dhcp.c +++ b/dhcp.c @@ -160,14 +160,79 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) return false; } +/* RFC 2132, Section 9.3 - Option Overload */ +enum dhcp_overload { + DHCP_OVERLOAD_NONE = 0, + DHCP_OVERLOAD_FILE = 1, + DHCP_OVERLOAD_SNAME = 2, +}; + /** - * fill() - Fill options in message - * @m: Message to fill + * fill_overflow() - Fill remaining options into sname and file fields + * @m: Msg whose sname/file fields may be used for overflow + * @has_bootfile: If true, reserve the file field for the boot file name + * + * Try the smaller sname field first: small options go there, leaving + * the larger file field available for big options. When @has_bootfile + * is set, the file field is reserved for the boot file name per + * RFC 2132, Section 9.5 and is not used for option overflow. + * + * Return: option 52 overload value + */ +static enum dhcp_overload fill_overflow(struct msg *m, bool has_bootfile) +{ + enum dhcp_overload overload = DHCP_OVERLOAD_NONE; + int sname_off = 0, file_off = 0; + int o; + + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { + if (opts[o].slen == -1 || opts[o].sent) + continue; + fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); + } + + if (!has_bootfile) { + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { + if (opts[o].slen == -1 || opts[o].sent) + continue; + if (fill_one(m->file, sizeof(m->file) - 1, o, + &file_off)) + debug("DHCP: skipping option %i" + " (overload full)", o); + } + } else { + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { + if (opts[o].slen == -1 || opts[o].sent) + continue; + debug("DHCP: skipping option %i (overload full)", o); + } + } + + if (sname_off) { + m->sname[sname_off] = 255; + overload |= DHCP_OVERLOAD_SNAME; + } + + if (file_off) { + m->file[file_off] = 255; + overload |= DHCP_OVERLOAD_FILE; + } + + return overload; +} + +/** + * fill() - Fill options in message, with overload into file/sname if needed + * @m: Message to fill + * @overload: Set to option 52 value (0 if none, 1/2/3 per RFC 2132) + * @has_bootfile: Reserve file field for boot file name * * Return: current size of options field */ -static int fill(struct msg *m) +static int fill(struct msg *m, enum dhcp_overload *overload, bool has_bootfile) { + /* Reserve 3 bytes for option 52 (overload) if needed */ + size_t size = OPT_MAX - 3; int i, o, offset = 0; for (o = 0; o < 255; o++) @@ -178,17 +243,25 @@ static int fill(struct msg *m) * Put it there explicitly, unless requested via option 55. */ if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) - fill_one(m->o, OPT_MAX, 53, &offset); + fill_one(m->o, size, 53, &offset); for (i = 0; i < opts[55].clen; i++) { o = opts[55].c[i]; if (opts[o].slen != -1) - fill_one(m->o, OPT_MAX, o, &offset); + fill_one(m->o, size, o, &offset); } for (o = 0; o < 255; o++) { if (opts[o].slen != -1 && !opts[o].sent) - fill_one(m->o, OPT_MAX, o, &offset); + fill_one(m->o, size, o, &offset); + } + + *overload = fill_overflow(m, has_bootfile); + + if (*overload) { + m->o[offset++] = 52; + m->o[offset++] = 1; + m->o[offset++] = *overload; } m->o[offset++] = 255; @@ -301,6 +374,7 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) int dhcp(const struct ctx *c, struct iov_tail *data) { char macstr[ETH_ADDRSTRLEN]; + enum dhcp_overload overload; size_t mlen, dlen, opt_len; struct in_addr mask, dst; struct ethhdr eh_storage; @@ -309,9 +383,12 @@ int dhcp(const struct ctx *c, struct iov_tail *data) const struct ethhdr *eh; const struct iphdr *iph; const struct udphdr *uh; + uint8_t bootfile[128]; struct msg m_storage; struct msg const *m; + bool has_bootfile; struct msg reply; + int bootfile_len; unsigned int i; eh = IOV_REMOVE_HEADER(data, eh_storage); @@ -462,9 +539,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data) } if (!c->no_dhcp_dns_search) - opt_set_dns_search(c, sizeof(m->o)); + opt_set_dns_search(c, OPT_MAX - 3); + + /* RFC 2132, Section 9.5: put boot file name in the 'file' header + * field. Suppress option 67 from the options area and reserve + * the file field from overload. + */ + has_bootfile = opts[67].slen > 0 && + (size_t)opts[67].slen < sizeof(reply.file); + if (has_bootfile) { + memcpy(bootfile, opts[67].s, opts[67].slen); + bootfile_len = opts[67].slen; + opts[67].slen = -1; + } + + dlen = offsetof(struct msg, o) + fill(&reply, &overload, has_bootfile); - dlen = offsetof(struct msg, o) + fill(&reply); + if (has_bootfile) + memcpy(reply.file, bootfile, bootfile_len); if (m->flags & FLAG_BROADCAST) dst = in4addr_broadcast; -- 2.54.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 2/4] dhcp: Add option overload 2026-06-17 13:22 ` [PATCH v4 2/4] dhcp: Add option overload Anshu Kumari @ 2026-06-19 3:39 ` David Gibson 2026-06-19 7:55 ` Anshu Kumari 2026-07-01 7:51 ` Stefano Brivio 2026-07-01 17:00 ` Stefano Brivio 2 siblings, 1 reply; 18+ messages in thread From: David Gibson @ 2026-06-19 3:39 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 8126 bytes --] On Wed, Jun 17, 2026 at 06:52:36PM +0530, Anshu Kumari wrote: > When the options field is full, overflow remaining DHCP options into > the sname and file fields per RFC 2132 option 52. > > Per RFC 2132, Section 9.5, the boot file name is always placed in the > 'file' header field. When a boot file is set, the file field is > reserved from overload and overflow uses only the sname field. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > --- > v4: > - Converted overload #defines to enum dhcp_overload. > - Fixed missing whitespace in comment before */. > - Boot file name always placed in 'file' header field per RFC 2132, > Section 9.5; file field reserved from overload when bootfile is > set; option 67 suppressed from options area. > > v3: > - Added RFC 2132 Section 9.3 reference comment on overload > constants. > - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow(). > - Swapped overflow order: try sname (64 bytes) first, then file > (128 bytes) — better packing and keeps file field available for > boot file name. > - Removed '&' from &reply.file. > - Removed '+1' from memcpy — reply.file already zeroed. > - opt_set_dns_search() max_len: OPT_MAX - 3 instead of > sizeof(m->o). > > v2: > - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME constants > - Added comment documenting space reservation: /* Reserve 3 bytes for option 52 */ > - Fixed DNS search length: sizeof(m->o) only, not combined with file+sname > - Removed dhcp_boot references — reply.file copy now reads from opts[67] > - Used DHCP_OVERLOAD_FILE constant in reply.file guard > --- > dhcp.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 100 insertions(+), 8 deletions(-) > > diff --git a/dhcp.c b/dhcp.c > index 8e3ffd6..78790d8 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -160,14 +160,79 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) > return false; > } > > +/* RFC 2132, Section 9.3 - Option Overload */ > +enum dhcp_overload { > + DHCP_OVERLOAD_NONE = 0, > + DHCP_OVERLOAD_FILE = 1, > + DHCP_OVERLOAD_SNAME = 2, > +}; > + > /** > - * fill() - Fill options in message > - * @m: Message to fill > + * fill_overflow() - Fill remaining options into sname and file fields > + * @m: Msg whose sname/file fields may be used for overflow > + * @has_bootfile: If true, reserve the file field for the boot file name > + * > + * Try the smaller sname field first: small options go there, leaving > + * the larger file field available for big options. When @has_bootfile > + * is set, the file field is reserved for the boot file name per > + * RFC 2132, Section 9.5 and is not used for option overflow. > + * > + * Return: option 52 overload value > + */ > +static enum dhcp_overload fill_overflow(struct msg *m, bool has_bootfile) > +{ > + enum dhcp_overload overload = DHCP_OVERLOAD_NONE; > + int sname_off = 0, file_off = 0; > + int o; > + > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); Ah, I think I see now. The earlier debug() messages were removed in favour of the ones later, because we don't want spurious messages if the option doesn't fit in the main area but does fit in the overload. That's a reasonable change, but needs to be better explained. It also all belongs in this patch, so the bits that leaked into the previous patch should be here instead. > + } > + > + if (!has_bootfile) { > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + if (fill_one(m->file, sizeof(m->file) - 1, o, > + &file_off)) > + debug("DHCP: skipping option %i" > + " (overload full)", o); > + } > + } else { Rather that checking for fill_one() failures only on the last pass, then having this fallback case, you can have a single unconditional pass at the end looking for skipped options. Then fill_one()s signature can be changed to void to match. > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + debug("DHCP: skipping option %i (overload full)", o); > + } > + } > + > + if (sname_off) { > + m->sname[sname_off] = 255; > + overload |= DHCP_OVERLOAD_SNAME; > + } > + > + if (file_off) { > + m->file[file_off] = 255; > + overload |= DHCP_OVERLOAD_FILE; > + } > + > + return overload; > +} > + > +/** > + * fill() - Fill options in message, with overload into file/sname if needed > + * @m: Message to fill > + * @overload: Set to option 52 value (0 if none, 1/2/3 per RFC 2132) > + * @has_bootfile: Reserve file field for boot file name > * > * Return: current size of options field > */ > -static int fill(struct msg *m) > +static int fill(struct msg *m, enum dhcp_overload *overload, bool has_bootfile) > { > + /* Reserve 3 bytes for option 52 (overload) if needed */ > + size_t size = OPT_MAX - 3; > int i, o, offset = 0; > > for (o = 0; o < 255; o++) > @@ -178,17 +243,25 @@ static int fill(struct msg *m) > * Put it there explicitly, unless requested via option 55. > */ > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > - fill_one(m->o, OPT_MAX, 53, &offset); > + fill_one(m->o, size, 53, &offset); > > for (i = 0; i < opts[55].clen; i++) { > o = opts[55].c[i]; > if (opts[o].slen != -1) > - fill_one(m->o, OPT_MAX, o, &offset); > + fill_one(m->o, size, o, &offset); > } > > for (o = 0; o < 255; o++) { > if (opts[o].slen != -1 && !opts[o].sent) > - fill_one(m->o, OPT_MAX, o, &offset); > + fill_one(m->o, size, o, &offset); > + } > + > + *overload = fill_overflow(m, has_bootfile); Is there a particular reason to put fill_overflow() in its own function, rather than just inline here? > + > + if (*overload) { > + m->o[offset++] = 52; > + m->o[offset++] = 1; > + m->o[offset++] = *overload; > } > > m->o[offset++] = 255; > @@ -301,6 +374,7 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) > int dhcp(const struct ctx *c, struct iov_tail *data) > { > char macstr[ETH_ADDRSTRLEN]; > + enum dhcp_overload overload; > size_t mlen, dlen, opt_len; > struct in_addr mask, dst; > struct ethhdr eh_storage; > @@ -309,9 +383,12 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > const struct ethhdr *eh; > const struct iphdr *iph; > const struct udphdr *uh; > + uint8_t bootfile[128]; > struct msg m_storage; > struct msg const *m; > + bool has_bootfile; > struct msg reply; > + int bootfile_len; > unsigned int i; > > eh = IOV_REMOVE_HEADER(data, eh_storage); > @@ -462,9 +539,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > } > > if (!c->no_dhcp_dns_search) > - opt_set_dns_search(c, sizeof(m->o)); > + opt_set_dns_search(c, OPT_MAX - 3); > + > + /* RFC 2132, Section 9.5: put boot file name in the 'file' header > + * field. Suppress option 67 from the options area and reserve > + * the file field from overload. > + */ > + has_bootfile = opts[67].slen > 0 && > + (size_t)opts[67].slen < sizeof(reply.file); > + if (has_bootfile) { > + memcpy(bootfile, opts[67].s, opts[67].slen); > + bootfile_len = opts[67].slen; > + opts[67].slen = -1; > + } > + > + dlen = offsetof(struct msg, o) + fill(&reply, &overload, has_bootfile); > > - dlen = offsetof(struct msg, o) + fill(&reply); > + if (has_bootfile) > + memcpy(reply.file, bootfile, bootfile_len); > > if (m->flags & FLAG_BROADCAST) > dst = in4addr_broadcast; > -- > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 2/4] dhcp: Add option overload 2026-06-19 3:39 ` David Gibson @ 2026-06-19 7:55 ` Anshu Kumari 2026-06-22 2:43 ` David Gibson 0 siblings, 1 reply; 18+ messages in thread From: Anshu Kumari @ 2026-06-19 7:55 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 9764 bytes --] On Fri, Jun 19, 2026 at 9:24 AM David Gibson <david@gibson.dropbear.id.au> wrote: > On Wed, Jun 17, 2026 at 06:52:36PM +0530, Anshu Kumari wrote: > > When the options field is full, overflow remaining DHCP options into > > the sname and file fields per RFC 2132 option 52. > > > > Per RFC 2132, Section 9.5, the boot file name is always placed in the > > 'file' header field. When a boot file is set, the file field is > > reserved from overload and overflow uses only the sname field. > > > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > > --- > > v4: > > - Converted overload #defines to enum dhcp_overload. > > - Fixed missing whitespace in comment before */. > > - Boot file name always placed in 'file' header field per RFC 2132, > > Section 9.5; file field reserved from overload when bootfile is > > set; option 67 suppressed from options area. > > > > v3: > > - Added RFC 2132 Section 9.3 reference comment on overload > > constants. > > - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow(). > > - Swapped overflow order: try sname (64 bytes) first, then file > > (128 bytes) — better packing and keeps file field available for > > boot file name. > > - Removed '&' from &reply.file. > > - Removed '+1' from memcpy — reply.file already zeroed. > > - opt_set_dns_search() max_len: OPT_MAX - 3 instead of > > sizeof(m->o). > > > > v2: > > - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME > constants > > - Added comment documenting space reservation: /* Reserve 3 bytes for > option 52 */ > > - Fixed DNS search length: sizeof(m->o) only, not combined with > file+sname > > - Removed dhcp_boot references — reply.file copy now reads from > opts[67] > > - Used DHCP_OVERLOAD_FILE constant in reply.file guard > > --- > > dhcp.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 100 insertions(+), 8 deletions(-) > > > > diff --git a/dhcp.c b/dhcp.c > > index 8e3ffd6..78790d8 100644 > > --- a/dhcp.c > > +++ b/dhcp.c > > @@ -160,14 +160,79 @@ static bool fill_one(uint8_t *buf, size_t size, > int o, int *offset) > > return false; > > } > > > > +/* RFC 2132, Section 9.3 - Option Overload */ > > +enum dhcp_overload { > > + DHCP_OVERLOAD_NONE = 0, > > + DHCP_OVERLOAD_FILE = 1, > > + DHCP_OVERLOAD_SNAME = 2, > > +}; > > + > > /** > > - * fill() - Fill options in message > > - * @m: Message to fill > > + * fill_overflow() - Fill remaining options into sname and file fields > > + * @m: Msg whose sname/file fields may be used > for overflow > > + * @has_bootfile: If true, reserve the file field for the boot file > name > > + * > > + * Try the smaller sname field first: small options go there, leaving > > + * the larger file field available for big options. When @has_bootfile > > + * is set, the file field is reserved for the boot file name per > > + * RFC 2132, Section 9.5 and is not used for option overflow. > > + * > > + * Return: option 52 overload value > > + */ > > +static enum dhcp_overload fill_overflow(struct msg *m, bool > has_bootfile) > > +{ > > + enum dhcp_overload overload = DHCP_OVERLOAD_NONE; > > + int sname_off = 0, file_off = 0; > > + int o; > > + > > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > > + if (opts[o].slen == -1 || opts[o].sent) > > + continue; > > + fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); > > Ah, I think I see now. The earlier debug() messages were removed in > favour of the ones later, because we don't want spurious messages if > the option doesn't fit in the main area but does fit in the overload. > > That's a reasonable change, but needs to be better explained. It also > all belongs in this patch, so the bits that leaked into the previous > patch should be here instead. > > > + } > > + > > + if (!has_bootfile) { > > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > > + if (opts[o].slen == -1 || opts[o].sent) > > + continue; > > + if (fill_one(m->file, sizeof(m->file) - 1, o, > > + &file_off)) > > + debug("DHCP: skipping option %i" > > + " (overload full)", o); > > + } > > + } else { > > Rather that checking for fill_one() failures only on the last pass, > then having this fallback case, you can have a single unconditional > pass at the end looking for skipped options. Then fill_one()s > signature can be changed to void to match. > > > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > > + if (opts[o].slen == -1 || opts[o].sent) > > + continue; > > + debug("DHCP: skipping option %i (overload full)", > o); > > + } > > + } > > + > > + if (sname_off) { > > + m->sname[sname_off] = 255; > > + overload |= DHCP_OVERLOAD_SNAME; > > + } > > + > > + if (file_off) { > > + m->file[file_off] = 255; > > + overload |= DHCP_OVERLOAD_FILE; > > + } > > + > > + return overload; > > +} > > + > > +/** > > + * fill() - Fill options in message, with overload into file/sname if > needed > > + * @m: Message to fill > > + * @overload: Set to option 52 value (0 if none, 1/2/3 > per RFC 2132) > > + * @has_bootfile: Reserve file field for boot file name > > * > > * Return: current size of options field > > */ > > -static int fill(struct msg *m) > > +static int fill(struct msg *m, enum dhcp_overload *overload, bool > has_bootfile) > > { > > + /* Reserve 3 bytes for option 52 (overload) if needed */ > > + size_t size = OPT_MAX - 3; > > int i, o, offset = 0; > > > > for (o = 0; o < 255; o++) > > @@ -178,17 +243,25 @@ static int fill(struct msg *m) > > * Put it there explicitly, unless requested via option 55. > > */ > > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > > - fill_one(m->o, OPT_MAX, 53, &offset); > > + fill_one(m->o, size, 53, &offset); > > > > for (i = 0; i < opts[55].clen; i++) { > > o = opts[55].c[i]; > > if (opts[o].slen != -1) > > - fill_one(m->o, OPT_MAX, o, &offset); > > + fill_one(m->o, size, o, &offset); > > } > > > > for (o = 0; o < 255; o++) { > > if (opts[o].slen != -1 && !opts[o].sent) > > - fill_one(m->o, OPT_MAX, o, &offset); > > + fill_one(m->o, size, o, &offset); > > + } > > + > > + *overload = fill_overflow(m, has_bootfile); > > Is there a particular reason to put fill_overflow() in its own > function, rather than just inline here? > There is no particular reason. I just thought it would be better to have a separate function for option overload (may be for better clarity). > > + > > + if (*overload) { > > + m->o[offset++] = 52; > > + m->o[offset++] = 1; > > + m->o[offset++] = *overload; > > } > > > > m->o[offset++] = 255; > > @@ -301,6 +374,7 @@ static void opt_set_dns_search(const struct ctx *c, > size_t max_len) > > int dhcp(const struct ctx *c, struct iov_tail *data) > > { > > char macstr[ETH_ADDRSTRLEN]; > > + enum dhcp_overload overload; > > size_t mlen, dlen, opt_len; > > struct in_addr mask, dst; > > struct ethhdr eh_storage; > > @@ -309,9 +383,12 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > > const struct ethhdr *eh; > > const struct iphdr *iph; > > const struct udphdr *uh; > > + uint8_t bootfile[128]; > > struct msg m_storage; > > struct msg const *m; > > + bool has_bootfile; > > struct msg reply; > > + int bootfile_len; > > unsigned int i; > > > > eh = IOV_REMOVE_HEADER(data, eh_storage); > > @@ -462,9 +539,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > > } > > > > if (!c->no_dhcp_dns_search) > > - opt_set_dns_search(c, sizeof(m->o)); > > + opt_set_dns_search(c, OPT_MAX - 3); > > + > > + /* RFC 2132, Section 9.5: put boot file name in the 'file' header > > + * field. Suppress option 67 from the options area and reserve > > + * the file field from overload. > > + */ > > + has_bootfile = opts[67].slen > 0 && > > + (size_t)opts[67].slen < sizeof(reply.file); > > + if (has_bootfile) { > > + memcpy(bootfile, opts[67].s, opts[67].slen); > > + bootfile_len = opts[67].slen; > > + opts[67].slen = -1; > > + } > > + > > + dlen = offsetof(struct msg, o) + fill(&reply, &overload, > has_bootfile); > > > > - dlen = offsetof(struct msg, o) + fill(&reply); > > + if (has_bootfile) > > + memcpy(reply.file, bootfile, bootfile_len); > > > > if (m->flags & FLAG_BROADCAST) > > dst = in4addr_broadcast; > > -- > > 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 > [-- Attachment #2: Type: text/html, Size: 12625 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 2/4] dhcp: Add option overload 2026-06-19 7:55 ` Anshu Kumari @ 2026-06-22 2:43 ` David Gibson 0 siblings, 0 replies; 18+ messages in thread From: David Gibson @ 2026-06-22 2:43 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 1819 bytes --] On Fri, Jun 19, 2026 at 01:25:18PM +0530, Anshu Kumari wrote: > On Fri, Jun 19, 2026 at 9:24 AM David Gibson <david@gibson.dropbear.id.au> > wrote: > > > On Wed, Jun 17, 2026 at 06:52:36PM +0530, Anshu Kumari wrote: [snip] > > > @@ -178,17 +243,25 @@ static int fill(struct msg *m) > > > * Put it there explicitly, unless requested via option 55. > > > */ > > > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > > > - fill_one(m->o, OPT_MAX, 53, &offset); > > > + fill_one(m->o, size, 53, &offset); > > > > > > for (i = 0; i < opts[55].clen; i++) { > > > o = opts[55].c[i]; > > > if (opts[o].slen != -1) > > > - fill_one(m->o, OPT_MAX, o, &offset); > > > + fill_one(m->o, size, o, &offset); > > > } > > > > > > for (o = 0; o < 255; o++) { > > > if (opts[o].slen != -1 && !opts[o].sent) > > > - fill_one(m->o, OPT_MAX, o, &offset); > > > + fill_one(m->o, size, o, &offset); > > > + } > > > + > > > + *overload = fill_overflow(m, has_bootfile); > > > > Is there a particular reason to put fill_overflow() in its own > > function, rather than just inline here? > > > > There is no particular reason. I just thought it would be better to > have a separate function for option overload (may be for better > clarity). Ok. I think the overall flow - try to fill this field, then this field, then this field - would be a bit clearer if it were inline. Not a big deal though. -- 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 --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 2/4] dhcp: Add option overload 2026-06-17 13:22 ` [PATCH v4 2/4] dhcp: Add option overload Anshu Kumari 2026-06-19 3:39 ` David Gibson @ 2026-07-01 7:51 ` Stefano Brivio 2026-07-01 17:00 ` Stefano Brivio 2 siblings, 0 replies; 18+ messages in thread From: Stefano Brivio @ 2026-07-01 7:51 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, jmaloy, david, lvivier On Wed, 17 Jun 2026 18:52:36 +0530 Anshu Kumari <anskuma@redhat.com> wrote: > When the options field is full, overflow remaining DHCP options into > the sname and file fields per RFC 2132 option 52. > > Per RFC 2132, Section 9.5, the boot file name is always placed in the > 'file' header field. When a boot file is set, the file field is > reserved from overload and overflow uses only the sname field. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > --- > v4: > - Converted overload #defines to enum dhcp_overload. > - Fixed missing whitespace in comment before */. > - Boot file name always placed in 'file' header field per RFC 2132, > Section 9.5; file field reserved from overload when bootfile is > set; option 67 suppressed from options area. > > v3: > - Added RFC 2132 Section 9.3 reference comment on overload > constants. > - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow(). > - Swapped overflow order: try sname (64 bytes) first, then file > (128 bytes) — better packing and keeps file field available for > boot file name. > - Removed '&' from &reply.file. > - Removed '+1' from memcpy — reply.file already zeroed. > - opt_set_dns_search() max_len: OPT_MAX - 3 instead of > sizeof(m->o). > > v2: > - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME constants > - Added comment documenting space reservation: /* Reserve 3 bytes for option 52 */ > - Fixed DNS search length: sizeof(m->o) only, not combined with file+sname > - Removed dhcp_boot references — reply.file copy now reads from opts[67] > - Used DHCP_OVERLOAD_FILE constant in reply.file guard > --- > dhcp.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 100 insertions(+), 8 deletions(-) > > diff --git a/dhcp.c b/dhcp.c > index 8e3ffd6..78790d8 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -160,14 +160,79 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) > return false; > } > > +/* RFC 2132, Section 9.3 - Option Overload */ See your own enum dhcp_opt_type in 3/4 of this series for an example of how to document enum in kerneldoc-style, or my review of 3/6 from your first revision: https://archives.passt.top/passt-dev/20260520023932.6a7e3537@elisabeth/ > +enum dhcp_overload { > + DHCP_OVERLOAD_NONE = 0, > + DHCP_OVERLOAD_FILE = 1, > + DHCP_OVERLOAD_SNAME = 2, > +}; > + > /** > - * fill() - Fill options in message > - * @m: Message to fill > + * fill_overflow() - Fill remaining options into sname and file fields > + * @m: Msg whose sname/file fields may be used for overflow > + * @has_bootfile: If true, reserve the file field for the boot file name > + * > + * Try the smaller sname field first: small options go there, leaving > + * the larger file field available for big options. When @has_bootfile > + * is set, the file field is reserved for the boot file name per > + * RFC 2132, Section 9.5 and is not used for option overflow. > + * > + * Return: option 52 overload value > + */ > +static enum dhcp_overload fill_overflow(struct msg *m, bool has_bootfile) > +{ > + enum dhcp_overload overload = DHCP_OVERLOAD_NONE; > + int sname_off = 0, file_off = 0; > + int o; > + > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); > + } > + > + if (!has_bootfile) { > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + if (fill_one(m->file, sizeof(m->file) - 1, o, > + &file_off)) > + debug("DHCP: skipping option %i" > + " (overload full)", o); > + } > + } else { > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + debug("DHCP: skipping option %i (overload full)", o); > + } > + } You could merge these two: for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { if (opts[o].slen == -1 || opts[o].sent) continue; if (!has_bootfile || fill_one(m->file, sizeof(m->file) - 1, o, &file_off)) debug("DHCP: skipping option %i (overload full)", o); } or, maybe a bit silly but probably more readable: size_t bootfile_size = has_bootfile ? sizeof(m->file) - 1 : 0; [...] for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { if (fill_one(m->file, bootfile_size, o, &file_off)) debug("DHCP: skipping option %i (overload full)", o); } > + > + if (sname_off) { > + m->sname[sname_off] = 255; > + overload |= DHCP_OVERLOAD_SNAME; > + } > + > + if (file_off) { > + m->file[file_off] = 255; > + overload |= DHCP_OVERLOAD_FILE; > + } > + > + return overload; > +} > + > +/** > + * fill() - Fill options in message, with overload into file/sname if needed > + * @m: Message to fill > + * @overload: Set to option 52 value (0 if none, 1/2/3 per RFC 2132) > + * @has_bootfile: Reserve file field for boot file name > * > * Return: current size of options field > */ > -static int fill(struct msg *m) > +static int fill(struct msg *m, enum dhcp_overload *overload, bool has_bootfile) > { > + /* Reserve 3 bytes for option 52 (overload) if needed */ > + size_t size = OPT_MAX - 3; > int i, o, offset = 0; > > for (o = 0; o < 255; o++) > @@ -178,17 +243,25 @@ static int fill(struct msg *m) > * Put it there explicitly, unless requested via option 55. > */ > if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > - fill_one(m->o, OPT_MAX, 53, &offset); > + fill_one(m->o, size, 53, &offset); > > for (i = 0; i < opts[55].clen; i++) { > o = opts[55].c[i]; > if (opts[o].slen != -1) > - fill_one(m->o, OPT_MAX, o, &offset); > + fill_one(m->o, size, o, &offset); > } > > for (o = 0; o < 255; o++) { > if (opts[o].slen != -1 && !opts[o].sent) > - fill_one(m->o, OPT_MAX, o, &offset); > + fill_one(m->o, size, o, &offset); > + } > + > + *overload = fill_overflow(m, has_bootfile); > + > + if (*overload) { > + m->o[offset++] = 52; > + m->o[offset++] = 1; > + m->o[offset++] = *overload; > } > > m->o[offset++] = 255; > @@ -301,6 +374,7 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) > int dhcp(const struct ctx *c, struct iov_tail *data) > { > char macstr[ETH_ADDRSTRLEN]; > + enum dhcp_overload overload; > size_t mlen, dlen, opt_len; > struct in_addr mask, dst; > struct ethhdr eh_storage; > @@ -309,9 +383,12 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > const struct ethhdr *eh; > const struct iphdr *iph; > const struct udphdr *uh; > + uint8_t bootfile[128]; > struct msg m_storage; > struct msg const *m; > + bool has_bootfile; > struct msg reply; > + int bootfile_len; > unsigned int i; > > eh = IOV_REMOVE_HEADER(data, eh_storage); > @@ -462,9 +539,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > } > > if (!c->no_dhcp_dns_search) > - opt_set_dns_search(c, sizeof(m->o)); > + opt_set_dns_search(c, OPT_MAX - 3); Why - 3? A comment would be nice to have. > + > + /* RFC 2132, Section 9.5: put boot file name in the 'file' header > + * field. Suppress option 67 from the options area and reserve > + * the file field from overload. > + */ > + has_bootfile = opts[67].slen > 0 && > + (size_t)opts[67].slen < sizeof(reply.file); > + if (has_bootfile) { > + memcpy(bootfile, opts[67].s, opts[67].slen); > + bootfile_len = opts[67].slen; > + opts[67].slen = -1; > + } > + > + dlen = offsetof(struct msg, o) + fill(&reply, &overload, has_bootfile); > > - dlen = offsetof(struct msg, o) + fill(&reply); > + if (has_bootfile) > + memcpy(reply.file, bootfile, bootfile_len); > > if (m->flags & FLAG_BROADCAST) > dst = in4addr_broadcast; Except for pending comments from David on 3/4 and 4/4, the rest looks good to me. -- Stefano ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 2/4] dhcp: Add option overload 2026-06-17 13:22 ` [PATCH v4 2/4] dhcp: Add option overload Anshu Kumari 2026-06-19 3:39 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio @ 2026-07-01 17:00 ` Stefano Brivio 2 siblings, 0 replies; 18+ messages in thread From: Stefano Brivio @ 2026-07-01 17:00 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, jmaloy, david, lvivier On Wed, 17 Jun 2026 18:52:36 +0530 Anshu Kumari <anskuma@redhat.com> wrote: > [...] > > +static enum dhcp_overload fill_overflow(struct msg *m, bool has_bootfile) > +{ > + enum dhcp_overload overload = DHCP_OVERLOAD_NONE; > + int sname_off = 0, file_off = 0; > + int o; > + > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off); > + } > + > + if (!has_bootfile) { > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + if (fill_one(m->file, sizeof(m->file) - 1, o, > + &file_off)) > + debug("DHCP: skipping option %i" > + " (overload full)", o); > + } > + } else { > + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { > + if (opts[o].slen == -1 || opts[o].sent) > + continue; > + debug("DHCP: skipping option %i (overload full)", o); > + } > + } Sorry for not mentioning this earlier: reviewing the DHCPv6 series reminded me of RFC 3396. Skip right away to Section 8 for an example: https://datatracker.ietf.org/doc/html/rfc3396#section-8 ...but note that the applicability is rather limited, see Section 4: A DHCP protocol agent SHOULD NOT split an option as described in this document unless it has no choice, or it knows that its peer can properly handle split options. A peer is assumed to properly handle split options if it has provided or requested at least one concatenation-requiring option. [...] What are concatenation-requiring options? I'm not aware of any list, but Internet Draft draft-tojens-dhcp-option-concat-considerations-01 (expired without becoming an RFC) has a list of examples, at least: https://www.ietf.org/archive/id/draft-tojens-dhcp-option-concat-considerations-01.html#section-1 and, from there, I actually realised that our current implementation of RFC 4702 (FQDN option) is not complete because Section 2 requires that: [...] DHCP clients and servers supporting this option MUST implement DHCP option concatenation [9]. In the terminology of [9], the Client FQDN option is a concatenation-requiring option. While the draft reports a practical issue with the implementation of RFC 3396 itself (in Section 4), it's just an expired draft at this point, so we can't safely go with the changes proposed in Section 6. However, they look very reasonable. Note that RFC 3396 comes with an additional nuisance: it defines a strict ordering of the buffers, where 'file' comes before 'sname'. That only applies if there are split options, of course, otherwise the order is not relevant. But if we want to keep trying with 'sname' before 'file', which I think makes sense, this complicates the implementation. We have two options, I think: 1. keep the implementation as it is, ignoring RFC 3396. If somebody tries to configure an option that's longer than 255 bytes, or finds a realistic use case where options don't fit, but would fit if split, we'll think what to do 2. fix things for real... maybe it's not _that_ complicated, I'm not sure: a. mark concatenation-requiring options (say, with a type that's handled in the same way as _STR, for example STR_CONCAT) b. define the maximum supported length for those options as 64 + 128 + 312 - 2 * 3 (sname plus file plus options minus three instances of length and code). Resize the buffer in 'opts' to 496 bytes (regardless of the option code, for simplicity). c. keep trying to fill options in the current order, options field first, then sname, then file. Keep counts of how many bytes we use in each section. d. if we reach this point ("we have no choice") with one of those, see if we can fit it by splitting it (this needs a separate loop because we need to follow the options/file/sname order) e. if we can't (sum of available bytes less than length of option), just skip and print the message, but if we can, add as much as we need, starting from the options field, then file, then sname I would quickly look at option 2., not so much because of RFC 3396 (it adds fundamental problems that aren't currently addressed, so we can assume that it's not universally or completely implemented anyway), but rather because of RFC 4702 and similar, which we're not implementing properly at the moment. If it's exceedingly complicated, I'd suggest to fall back to 1. instead. After all, we never had reports from users not being able to fit the options they wanted. -- Stefano ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser 2026-06-17 13:22 [PATCH v4 0/4] Add --dhcp-boot and --dhcp-opt options Anshu Kumari 2026-06-17 13:22 ` [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari 2026-06-17 13:22 ` [PATCH v4 2/4] dhcp: Add option overload Anshu Kumari @ 2026-06-17 13:22 ` Anshu Kumari 2026-06-19 3:52 ` David Gibson 2026-07-01 17:00 ` Stefano Brivio 2026-06-17 13:22 ` [PATCH v4 4/4] conf: Add --dhcp-boot command-line option Anshu Kumari 3 siblings, 2 replies; 18+ messages in thread From: Anshu Kumari @ 2026-06-17 13:22 UTC (permalink / raw) To: passt-dev, anskuma, sbrivio; +Cc: jmaloy, david, lvivier Introduce the --dhcp-opt flag that allows setting arbitrary DHCP options from command-line in the form [--dhcp-opt CODE,VALUE]. Add a type lookup table mapping option codes to RFC 2132 value types (IPv4, IPv4 list, integer, string) and dhcp_opt_parse() to convert CLI strings to binary wire format. Parsed options are stored in struct ctx and injected into DHCP replies. If the same option code is given more than once, the last value wins. Link: https://bugs.passt.top/show_bug.cgi?id=192 Signed-off-by: Anshu Kumari <anskuma@redhat.com> --- v4: - Renamed custom_opts to dhcp_opts, 256 entries indexed by option code, removed MAX_CUSTOM_DHCP_OPTS and count field. - Changed str buffer from 256 to 255 bytes. - Moved function to conf.c as static conf_dhcp_option(), renamed from dhcp_add_option(). - Made dhcp_opt_parse() non-static, declared in dhcp.h - Dropped val/len from ctx struct; conf_dhcp_option() validates with temp buffer, dhcp() parses str directly into opts[] at reply time. - Replaced strtok_r() + 256-byte buffer with strcspn() + INET_ADDRSTRLEN buffer. - Added DHCP_OPT_SINT32 for option 2 (Time Offset), uses strtol() per RFC 2132 Section 8.2. - All errors in dhcp_opt_parse() return -1, removed die() calls; caller handles error message consistently. - Removed redundant !slen check in DHCP_OPT_STR case. - Omitted explicit array size for dhcp_opt_types[], arraydded bounds check before lookup. - Added errno = 0 + errno check for strtoul() in case 34. - Fixed usage text: "Set DHCP option CODE to VAL". - Improved man page: added format description and examples v3: - Replaced DHCP_OPT_INTEGER with separate DHCP_OPT_INT8/INT16/INT32 enums, removed dhcp_opt_int_width[] array. - Shared logic between DHCP_OPT_IPV4 and DHCP_OPT_IPV4_LIST — parse both as list, error if >1 in single case. - Added errno = 0 before strtoul() and check after. - Fixed range check: 1ULL << (width * 8) for all widths including width==4. - strncpy → memcpy for DHCP_OPT_STR. - Moved enum to dhcp.c since not used in other files. - Removed options 55, 61 (client-only), 119 (DNS compression, use --dhcp-search instead), 33 (IP pairs not supported). - DHCP_OPT_PARSE_BUF 1024 → char tmp[256]. - Upgraded dhcp_add_option() to call dhcp_opt_parse() and populate val[]/len. - Aligned array entries for readability. - Added tab after @DHCP_OPT_IPV4_LIST: in kerneldoc. - Reject empty value strings before parsing - Reject leading/trailing/consecutive commas in IP list values. v2: - Replaced struct lookup table + dhcp_opt_type_lookup() function with flat dhcp_opt_types[256] array indexed by code. - Consolidated DHCP_OPT_UINT8/UINT16/UINT32 into single DHCP_OPT_INTEGER with dhcp_opt_int_width[256] table. - Dropped DHCP_OPT_ROUTES / option 121 entirely. - Added kerneldoc for enum dhcp_opt_type values. - Removed curly braces from switch cases, declarations before switch. - Added newlines before return statements. - Changed IP list delimiter from space to comma (--dhcp-opt 6,1.1.1.1,8.8.8.8). - Defined DHCP_OPT_PARSE_BUF constant for bare 1024. - Added len and val[255] fields to struct here (moved from patch 1). - Added kerneldoc for @custom_opts.len and @custom_opts.val. - Wired dhcp_opt_parse() into case 32 (--dhcp-boot) to populate val/len. --- conf.c | 45 ++++++++++++- dhcp.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ dhcp.h | 2 + passt.1 | 42 +++++++++++++ passt.h | 6 ++ 5 files changed, 285 insertions(+), 1 deletion(-) diff --git a/conf.c b/conf.c index cd05adf..836b297 100644 --- a/conf.c +++ b/conf.c @@ -47,6 +47,7 @@ #include "lineread.h" #include "isolation.h" #include "log.h" +#include "dhcp.h" #include "vhost_user.h" #include "epoll_ctl.h" #include "conf.h" @@ -616,7 +617,8 @@ static void usage(const char *name, FILE *f, int status) " -S, --search LIST Space-separated list, search domains\n" " a single, empty option disables the DNS search list\n" " -H, --hostname NAME Hostname to configure client with\n" - " --fqdn NAME FQDN to configure client with\n"); + " --fqdn NAME FQDN to configure client with\n" + " --dhcp-opt CODE,VAL Set DHCP option CODE to VAL\n"); if (strstr(name, "pasta")) FPRINTF(f, " default: don't use any search list\n"); else @@ -844,6 +846,10 @@ static void conf_print(const struct ctx *c) info(" router: %s", inet_ntop(AF_INET, &c->ip4.guest_gw, buf, sizeof(buf))); + for (i = 1; i < 255; i++) + if (*c->dhcp_opts[i].str) + info(" option %u: %s", i, + c->dhcp_opts[i].str); } for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) { @@ -1150,6 +1156,25 @@ static void conf_sock_listen(const struct ctx *c) die_perror("Couldn't add configuration socket to epoll"); } +/** + * conf_dhcp_option() - Set value for a DHCP option in configuration + * @c: Execution context + * @code: DHCP option code + * @val_str: Value string from command line + */ +static void conf_dhcp_option(struct ctx *c, uint8_t code, const char *val_str) +{ + uint8_t tmp[255]; + + if (dhcp_opt_parse(code, val_str, tmp, sizeof(tmp)) < 0) + die("Invalid value for DHCP option %u: %s", code, val_str); + + if (snprintf_check(c->dhcp_opts[code].str, + sizeof(c->dhcp_opts[0].str), + "%s", val_str)) + die("DHCP option value too long: %s", val_str); +} + /** * conf() - Process command-line arguments and set configuration * @c: Execution context @@ -1233,6 +1258,7 @@ void conf(struct ctx *c, int argc, char **argv) {"migrate-no-linger", no_argument, NULL, 30 }, {"stats", required_argument, NULL, 31 }, {"conf-path", required_argument, NULL, 'c' }, + {"dhcp-opt", required_argument, NULL, 34 }, { 0 }, }; const char *optstring = "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; @@ -1248,10 +1274,13 @@ void conf(struct ctx *c, int argc, char **argv) uint8_t prefix_len_from_opt = 0; unsigned int ifi4 = 0, ifi6 = 0; const char *logfile = NULL; + unsigned long optcode; char *runas = NULL; size_t logsize = 0; + const char *comma; long fd_tap_opt; int name, ret; + char *end; uid_t uid; gid_t gid; @@ -1467,6 +1496,20 @@ void conf(struct ctx *c, int argc, char **argv) die("Can't display statistics if not running in foreground"); c->stats = strtol(optarg, NULL, 0); break; + case 34: + comma = strchr(optarg, ','); + if (!comma) + die("--dhcp-opt requires CODE,VALUE format"); + + errno = 0; + optcode = strtoul(optarg, &end, 0); + if (end != comma || errno || + optcode < 1 || optcode > 254) + die("DHCP option code must be 1-254: %s", + optarg); + + conf_dhcp_option(c, optcode, comma + 1); + break; case 'd': c->debug = 1; c->quiet = 0; diff --git a/dhcp.c b/dhcp.c index 78790d8..47bb524 100644 --- a/dhcp.c +++ b/dhcp.c @@ -23,6 +23,7 @@ #include <unistd.h> #include <string.h> #include <limits.h> +#include <errno.h> #include "util.h" #include "ip.h" @@ -130,6 +131,189 @@ struct msg { uint8_t o[OPT_MAX + 1 /* End option */ ]; } __attribute__((__packed__)); +/** + * enum dhcp_opt_type - DHCP option value types per RFC 2132 + * @DHCP_OPT_NONE: Unsupported or unknown option + * @DHCP_OPT_STR: Variable-length string + * @DHCP_OPT_IPV4: Single IPv4 address + * @DHCP_OPT_IPV4_LIST: Multiple IPv4 addresses, comma-separated + * @DHCP_OPT_INT8: Unsigned 8-bit integer + * @DHCP_OPT_INT16: Unsigned 16-bit integer + * @DHCP_OPT_INT32: Unsigned 32-bit integer + * @DHCP_OPT_SINT32: Signed 32-bit integer + */ +enum dhcp_opt_type { + DHCP_OPT_NONE, + DHCP_OPT_STR, + DHCP_OPT_IPV4, + DHCP_OPT_IPV4_LIST, + DHCP_OPT_INT8, + DHCP_OPT_INT16, + DHCP_OPT_INT32, + DHCP_OPT_SINT32, +}; + +/** + * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by code + */ +static const enum dhcp_opt_type dhcp_opt_types[] = { + [1] = DHCP_OPT_IPV4, /* Subnet Mask */ + [2] = DHCP_OPT_SINT32, /* Time Offset */ + [3] = DHCP_OPT_IPV4_LIST, /* Router */ + [4] = DHCP_OPT_IPV4_LIST, /* Time Server */ + [5] = DHCP_OPT_IPV4_LIST, /* Name Server */ + [6] = DHCP_OPT_IPV4_LIST, /* Domain Name Server */ + [7] = DHCP_OPT_IPV4_LIST, /* Log Server */ + [8] = DHCP_OPT_IPV4_LIST, /* Cookie Server */ + [9] = DHCP_OPT_IPV4_LIST, /* LPR Server */ + [10] = DHCP_OPT_IPV4_LIST, /* Impress Server */ + [11] = DHCP_OPT_IPV4_LIST, /* Resource Location Server */ + [12] = DHCP_OPT_STR, /* Host Name */ + [13] = DHCP_OPT_INT16, /* Boot File Size */ + [15] = DHCP_OPT_STR, /* Domain Name */ + [16] = DHCP_OPT_IPV4, /* Swap Server */ + [17] = DHCP_OPT_STR, /* Root Path */ + [19] = DHCP_OPT_INT8, /* IP Forwarding */ + [23] = DHCP_OPT_INT8, /* Default IP TTL */ + [26] = DHCP_OPT_INT16, /* Interface MTU */ + [28] = DHCP_OPT_IPV4, /* Broadcast Address */ + [37] = DHCP_OPT_INT8, /* TCP Default TTL */ + [38] = DHCP_OPT_INT32, /* TCP Keepalive Interval */ + [40] = DHCP_OPT_STR, /* NIS Domain Name */ + [41] = DHCP_OPT_IPV4_LIST, /* NIS Servers */ + [42] = DHCP_OPT_IPV4_LIST, /* NTP Servers */ + [44] = DHCP_OPT_IPV4_LIST, /* NetBIOS Name Server */ + [50] = DHCP_OPT_IPV4, /* Requested IP Address */ + [51] = DHCP_OPT_INT32, /* IP Address Lease Time */ + [53] = DHCP_OPT_INT8, /* DHCP Message Type */ + [54] = DHCP_OPT_IPV4, /* Server Identifier */ + [57] = DHCP_OPT_INT16, /* Max DHCP Message Size */ + [58] = DHCP_OPT_INT32, /* Renewal (T1) Time */ + [59] = DHCP_OPT_INT32, /* Rebinding (T2) Time */ + [60] = DHCP_OPT_STR, /* Vendor Class Identifier */ + [66] = DHCP_OPT_STR, /* TFTP Server Name */ + [67] = DHCP_OPT_STR, /* Bootfile Name */ + [252] = DHCP_OPT_STR, /* WPAD URL */ +}; + +/** + * dhcp_opt_parse() - Parse a DHCP option value + * @code: DHCP 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 + */ +int dhcp_opt_parse(uint8_t code, const char *str, + uint8_t *buf, size_t buf_len) +{ + enum dhcp_opt_type type; + unsigned long val; + unsigned int i; + uint8_t width; + size_t slen; + char *end; + int len; + + if (code >= ARRAY_SIZE(dhcp_opt_types)) + return -1; + + type = dhcp_opt_types[code]; + + if (!*str) + return -1; + + switch (type) { + case DHCP_OPT_NONE: + return -1; + case DHCP_OPT_IPV4: + case DHCP_OPT_IPV4_LIST: + len = 0; + + while (*str) { + char ipbuf[INET_ADDRSTRLEN]; + size_t chunk; + + chunk = strcspn(str, ","); + + if (!chunk || chunk >= sizeof(ipbuf)) + return -1; + + memcpy(ipbuf, str, chunk); + ipbuf[chunk] = '\0'; + + if (len + (int)sizeof(struct in_addr) > (int)buf_len) + return -1; + + if (inet_pton(AF_INET, ipbuf, buf + len) != 1) + return -1; + + len += sizeof(struct in_addr); + + if (type == DHCP_OPT_IPV4) { + if (str[chunk] == ',') + return -1; + break; + } + + str += chunk + (str[chunk] == ','); + } + + if (!len) + return -1; + + return len; + case DHCP_OPT_INT8: + case DHCP_OPT_INT16: + case DHCP_OPT_INT32: + case DHCP_OPT_SINT32: + if (type == DHCP_OPT_INT8) + width = 1; + else if (type == DHCP_OPT_INT16) + width = 2; + else + width = 4; + + if (buf_len < width) + return -1; + + errno = 0; + if (type == DHCP_OPT_SINT32) { + long sval; + + sval = strtol(str, &end, 0); + if (*end || errno || + sval < INT32_MIN || sval > INT32_MAX) + return -1; + val = (uint32_t)sval; + } else { + val = strtoul(str, &end, 0); + if (*end || errno || + val >= (1ULL << (width * 8))) + return -1; + } + + for (i = width; i > 0; i--) { + buf[i - 1] = val & 0xff; + val >>= 8; + } + + return width; + case DHCP_OPT_STR: + slen = strlen(str); + + if (slen >= buf_len) + return -1; + + memcpy(buf, str, slen); + + return slen; + } + + return -1; +} + /** * fill_one() - Fill a single option into a buffer * @buf: Buffer to write option @@ -541,6 +725,13 @@ int dhcp(const struct ctx *c, struct iov_tail *data) if (!c->no_dhcp_dns_search) opt_set_dns_search(c, OPT_MAX - 3); + for (i = 1; i < 255; i++) { + if (!c->dhcp_opts[i].str[0]) + continue; + opts[i].slen = dhcp_opt_parse(i, c->dhcp_opts[i].str, + opts[i].s, sizeof(opts[i].s)); + } + /* RFC 2132, Section 9.5: put boot file name in the 'file' header * field. Suppress option 67 from the options area and reserve * the file field from overload. diff --git a/dhcp.h b/dhcp.h index cd50c99..cc8d5dd 100644 --- a/dhcp.h +++ b/dhcp.h @@ -8,5 +8,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data); void dhcp_init(void); +int dhcp_opt_parse(uint8_t code, const char *str, + uint8_t *buf, size_t buf_len); #endif /* DHCP_H */ diff --git a/passt.1 b/passt.1 index 908fd4a..ccdcbb2 100644 --- a/passt.1 +++ b/passt.1 @@ -430,6 +430,48 @@ Send \fIname\fR as DHCP option 12 (hostname). FQDN to configure the client with. Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39. +.TP +.BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR +Set DHCP option \fICODE\fR (1\-254) to \fIVALUE\fR. The value format depends +on the option type and is determined automatically from the option code. +Multiple IPv4 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. Options set with +\fB\-\-dhcp-opt\fR override built-in values. +.PP +Examples: +.nf + \-\-dhcp-opt 6,8.8.8.8,4.4.4.4 + \-\-dhcp-opt 12,myhostname +.fi +.PP +Only the following option codes are supported (unsupported codes cause an error): +.RS +.TP +.B IPv4 address options +1 (Subnet Mask), 16 (Swap Server), 28 (Broadcast Address), 50 (Requested IP), +54 (Server Identifier) +.TP +.B IPv4 address list options (comma-separated) +3 (Router), 4 (Time Server), 5 (Name Server), 6 (DNS), 7 (Log Server), +8 (Cookie Server), 9 (LPR Server), 10 (Impress Server), +11 (Resource Location Server), 41 (NIS Servers), +42 (NTP Servers), 44 (NetBIOS Name Server) +.TP +.B Integer options +2 (Time Offset, 32-bit), 13 (Boot File Size, 16-bit), 19 (IP Forwarding, 8-bit), +23 (Default IP TTL, 8-bit), 26 (Interface MTU, 16-bit), +37 (TCP Default TTL, 8-bit), 38 (TCP Keepalive Interval, 32-bit), +51 (IP Address Lease Time, 32-bit), +53 (DHCP Message Type, 8-bit), 57 (Max DHCP Message Size, 16-bit), +58 (Renewal Time, 32-bit), 59 (Rebinding Time, 32-bit) +.TP +.B String options +12 (Host Name), 15 (Domain Name), 17 (Root Path), 40 (NIS Domain Name), +60 (Vendor Class Identifier), 66 (TFTP Server Name), +67 (Bootfile Name), 252 (WPAD URL) +.RE + .TP .BR \-t ", " \-\-tcp-ports " " \fIspec Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of: diff --git a/passt.h b/passt.h index 3a07294..15e2d83 100644 --- a/passt.h +++ b/passt.h @@ -182,6 +182,8 @@ struct ip6_ctx { * @dns_search: DNS search list * @hostname: Guest hostname * @fqdn: Guest FQDN + * @dhcp_opts: User-specified DHCP options from --dhcp-opt + * @dhcp_opts.str: String value from command line * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 disabled * @ip6: IPv6 configuration * @pasta_ifn: Name of namespace interface for pasta @@ -264,6 +266,10 @@ struct ctx { char hostname[PASST_MAXDNAME]; char fqdn[PASST_MAXDNAME]; + struct { + char str[255]; + } dhcp_opts[256]; + int ifi6; struct ip6_ctx ip6; -- 2.54.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser 2026-06-17 13:22 ` [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser Anshu Kumari @ 2026-06-19 3:52 ` David Gibson 2026-06-19 7:29 ` Anshu Kumari 2026-07-01 17:00 ` Stefano Brivio 1 sibling, 1 reply; 18+ messages in thread From: David Gibson @ 2026-06-19 3:52 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 17556 bytes --] On Wed, Jun 17, 2026 at 06:52:37PM +0530, Anshu Kumari wrote: > Introduce the --dhcp-opt flag that allows setting arbitrary DHCP > options from command-line in the form [--dhcp-opt CODE,VALUE]. > > Add a type lookup table mapping option codes to RFC 2132 value types > (IPv4, IPv4 list, integer, string) and dhcp_opt_parse() to convert > CLI strings to binary wire format. Parsed options are stored in > struct ctx and injected into DHCP replies. If the same option code > is given more than once, the last value wins. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > --- > v4: > - Renamed custom_opts to dhcp_opts, 256 entries indexed by option > code, removed MAX_CUSTOM_DHCP_OPTS and count field. > - Changed str buffer from 256 to 255 bytes. > - Moved function to conf.c as static conf_dhcp_option(), renamed > from dhcp_add_option(). > - Made dhcp_opt_parse() non-static, declared in dhcp.h > - Dropped val/len from ctx struct; conf_dhcp_option() validates > with temp buffer, dhcp() parses str directly into opts[] at > reply time. Hmm. So each option is parsed twice. What prevents you from parsing directly into the opts[] array at conf() time? > - Replaced strtok_r() + 256-byte buffer with strcspn() + > INET_ADDRSTRLEN buffer. > - Added DHCP_OPT_SINT32 for option 2 (Time Offset), uses strtol() > per RFC 2132 Section 8.2. > - All errors in dhcp_opt_parse() return -1, removed die() calls; > caller handles error message consistently. > - Removed redundant !slen check in DHCP_OPT_STR case. > - Omitted explicit array size for dhcp_opt_types[], arraydded bounds > check before lookup. > - Added errno = 0 + errno check for strtoul() in case 34. > - Fixed usage text: "Set DHCP option CODE to VAL". > - Improved man page: added format description and examples > > v3: > - Replaced DHCP_OPT_INTEGER with separate DHCP_OPT_INT8/INT16/INT32 > enums, removed dhcp_opt_int_width[] array. > - Shared logic between DHCP_OPT_IPV4 and DHCP_OPT_IPV4_LIST — parse > both as list, error if >1 in single case. > - Added errno = 0 before strtoul() and check after. > - Fixed range check: 1ULL << (width * 8) for all widths including > width==4. > - strncpy → memcpy for DHCP_OPT_STR. > - Moved enum to dhcp.c since not used in other files. > - Removed options 55, 61 (client-only), 119 (DNS compression, use > --dhcp-search instead), 33 (IP pairs not supported). > - DHCP_OPT_PARSE_BUF 1024 → char tmp[256]. > - Upgraded dhcp_add_option() to call dhcp_opt_parse() and populate > val[]/len. > - Aligned array entries for readability. > - Added tab after @DHCP_OPT_IPV4_LIST: in kerneldoc. > - Reject empty value strings before parsing > - Reject leading/trailing/consecutive commas in IP list values. > > v2: > - Replaced struct lookup table + dhcp_opt_type_lookup() function with flat dhcp_opt_types[256] array indexed by code. > - Consolidated DHCP_OPT_UINT8/UINT16/UINT32 into single DHCP_OPT_INTEGER with dhcp_opt_int_width[256] table. > - Dropped DHCP_OPT_ROUTES / option 121 entirely. > - Added kerneldoc for enum dhcp_opt_type values. > - Removed curly braces from switch cases, declarations before switch. > - Added newlines before return statements. > - Changed IP list delimiter from space to comma (--dhcp-opt 6,1.1.1.1,8.8.8.8). > - Defined DHCP_OPT_PARSE_BUF constant for bare 1024. > - Added len and val[255] fields to struct here (moved from patch 1). > - Added kerneldoc for @custom_opts.len and @custom_opts.val. > - Wired dhcp_opt_parse() into case 32 (--dhcp-boot) to populate val/len. > --- > conf.c | 45 ++++++++++++- > dhcp.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > dhcp.h | 2 + > passt.1 | 42 +++++++++++++ > passt.h | 6 ++ > 5 files changed, 285 insertions(+), 1 deletion(-) > > diff --git a/conf.c b/conf.c > index cd05adf..836b297 100644 > --- a/conf.c > +++ b/conf.c > @@ -47,6 +47,7 @@ > #include "lineread.h" > #include "isolation.h" > #include "log.h" > +#include "dhcp.h" > #include "vhost_user.h" > #include "epoll_ctl.h" > #include "conf.h" > @@ -616,7 +617,8 @@ static void usage(const char *name, FILE *f, int status) > " -S, --search LIST Space-separated list, search domains\n" > " a single, empty option disables the DNS search list\n" > " -H, --hostname NAME Hostname to configure client with\n" > - " --fqdn NAME FQDN to configure client with\n"); > + " --fqdn NAME FQDN to configure client with\n" > + " --dhcp-opt CODE,VAL Set DHCP option CODE to VAL\n"); > if (strstr(name, "pasta")) > FPRINTF(f, " default: don't use any search list\n"); > else > @@ -844,6 +846,10 @@ static void conf_print(const struct ctx *c) > info(" router: %s", > inet_ntop(AF_INET, &c->ip4.guest_gw, > buf, sizeof(buf))); > + for (i = 1; i < 255; i++) > + if (*c->dhcp_opts[i].str) > + info(" option %u: %s", i, > + c->dhcp_opts[i].str); > } > > for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) { > @@ -1150,6 +1156,25 @@ static void conf_sock_listen(const struct ctx *c) > die_perror("Couldn't add configuration socket to epoll"); > } > > +/** > + * conf_dhcp_option() - Set value for a DHCP option in configuration > + * @c: Execution context > + * @code: DHCP option code > + * @val_str: Value string from command line > + */ > +static void conf_dhcp_option(struct ctx *c, uint8_t code, const char *val_str) > +{ > + uint8_t tmp[255]; > + > + if (dhcp_opt_parse(code, val_str, tmp, sizeof(tmp)) < 0) > + die("Invalid value for DHCP option %u: %s", code, val_str); > + > + if (snprintf_check(c->dhcp_opts[code].str, > + sizeof(c->dhcp_opts[0].str), > + "%s", val_str)) > + die("DHCP option value too long: %s", val_str); > +} > + > /** > * conf() - Process command-line arguments and set configuration > * @c: Execution context > @@ -1233,6 +1258,7 @@ void conf(struct ctx *c, int argc, char **argv) > {"migrate-no-linger", no_argument, NULL, 30 }, > {"stats", required_argument, NULL, 31 }, > {"conf-path", required_argument, NULL, 'c' }, > + {"dhcp-opt", required_argument, NULL, 34 }, > { 0 }, > }; > const char *optstring = "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; > @@ -1248,10 +1274,13 @@ void conf(struct ctx *c, int argc, char **argv) > uint8_t prefix_len_from_opt = 0; > unsigned int ifi4 = 0, ifi6 = 0; > const char *logfile = NULL; > + unsigned long optcode; > char *runas = NULL; > size_t logsize = 0; > + const char *comma; > long fd_tap_opt; > int name, ret; > + char *end; > uid_t uid; > gid_t gid; > > @@ -1467,6 +1496,20 @@ void conf(struct ctx *c, int argc, char **argv) > die("Can't display statistics if not running in foreground"); > c->stats = strtol(optarg, NULL, 0); > break; > + case 34: > + comma = strchr(optarg, ','); > + if (!comma) > + die("--dhcp-opt requires CODE,VALUE format"); > + > + errno = 0; > + optcode = strtoul(optarg, &end, 0); > + if (end != comma || errno || > + optcode < 1 || optcode > 254) > + die("DHCP option code must be 1-254: %s", > + optarg); > + > + conf_dhcp_option(c, optcode, comma + 1); > + break; > case 'd': > c->debug = 1; > c->quiet = 0; > diff --git a/dhcp.c b/dhcp.c > index 78790d8..47bb524 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -23,6 +23,7 @@ > #include <unistd.h> > #include <string.h> > #include <limits.h> > +#include <errno.h> > > #include "util.h" > #include "ip.h" > @@ -130,6 +131,189 @@ struct msg { > uint8_t o[OPT_MAX + 1 /* End option */ ]; > } __attribute__((__packed__)); > > +/** > + * enum dhcp_opt_type - DHCP option value types per RFC 2132 > + * @DHCP_OPT_NONE: Unsupported or unknown option > + * @DHCP_OPT_STR: Variable-length string > + * @DHCP_OPT_IPV4: Single IPv4 address > + * @DHCP_OPT_IPV4_LIST: Multiple IPv4 addresses, comma-separated > + * @DHCP_OPT_INT8: Unsigned 8-bit integer > + * @DHCP_OPT_INT16: Unsigned 16-bit integer > + * @DHCP_OPT_INT32: Unsigned 32-bit integer > + * @DHCP_OPT_SINT32: Signed 32-bit integer For consistency with C conventions, I'd suggset UINT{8,16,32} and just INT32 for the signed case. > + */ > +enum dhcp_opt_type { > + DHCP_OPT_NONE, > + DHCP_OPT_STR, > + DHCP_OPT_IPV4, > + DHCP_OPT_IPV4_LIST, > + DHCP_OPT_INT8, > + DHCP_OPT_INT16, > + DHCP_OPT_INT32, > + DHCP_OPT_SINT32, > +}; > + > +/** > + * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by code > + */ > +static const enum dhcp_opt_type dhcp_opt_types[] = { > + [1] = DHCP_OPT_IPV4, /* Subnet Mask */ > + [2] = DHCP_OPT_SINT32, /* Time Offset */ > + [3] = DHCP_OPT_IPV4_LIST, /* Router */ > + [4] = DHCP_OPT_IPV4_LIST, /* Time Server */ > + [5] = DHCP_OPT_IPV4_LIST, /* Name Server */ > + [6] = DHCP_OPT_IPV4_LIST, /* Domain Name Server */ > + [7] = DHCP_OPT_IPV4_LIST, /* Log Server */ > + [8] = DHCP_OPT_IPV4_LIST, /* Cookie Server */ > + [9] = DHCP_OPT_IPV4_LIST, /* LPR Server */ > + [10] = DHCP_OPT_IPV4_LIST, /* Impress Server */ > + [11] = DHCP_OPT_IPV4_LIST, /* Resource Location Server */ > + [12] = DHCP_OPT_STR, /* Host Name */ > + [13] = DHCP_OPT_INT16, /* Boot File Size */ > + [15] = DHCP_OPT_STR, /* Domain Name */ > + [16] = DHCP_OPT_IPV4, /* Swap Server */ > + [17] = DHCP_OPT_STR, /* Root Path */ > + [19] = DHCP_OPT_INT8, /* IP Forwarding */ > + [23] = DHCP_OPT_INT8, /* Default IP TTL */ > + [26] = DHCP_OPT_INT16, /* Interface MTU */ > + [28] = DHCP_OPT_IPV4, /* Broadcast Address */ > + [37] = DHCP_OPT_INT8, /* TCP Default TTL */ > + [38] = DHCP_OPT_INT32, /* TCP Keepalive Interval */ > + [40] = DHCP_OPT_STR, /* NIS Domain Name */ > + [41] = DHCP_OPT_IPV4_LIST, /* NIS Servers */ > + [42] = DHCP_OPT_IPV4_LIST, /* NTP Servers */ > + [44] = DHCP_OPT_IPV4_LIST, /* NetBIOS Name Server */ > + [50] = DHCP_OPT_IPV4, /* Requested IP Address */ > + [51] = DHCP_OPT_INT32, /* IP Address Lease Time */ > + [53] = DHCP_OPT_INT8, /* DHCP Message Type */ > + [54] = DHCP_OPT_IPV4, /* Server Identifier */ > + [57] = DHCP_OPT_INT16, /* Max DHCP Message Size */ > + [58] = DHCP_OPT_INT32, /* Renewal (T1) Time */ > + [59] = DHCP_OPT_INT32, /* Rebinding (T2) Time */ > + [60] = DHCP_OPT_STR, /* Vendor Class Identifier */ > + [66] = DHCP_OPT_STR, /* TFTP Server Name */ > + [67] = DHCP_OPT_STR, /* Bootfile Name */ > + [252] = DHCP_OPT_STR, /* WPAD URL */ > +}; > + > +/** > + * dhcp_opt_parse() - Parse a DHCP option value > + * @code: DHCP 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 > + */ > +int dhcp_opt_parse(uint8_t code, const char *str, > + uint8_t *buf, size_t buf_len) > +{ > + enum dhcp_opt_type type; > + unsigned long val; > + unsigned int i; > + uint8_t width; > + size_t slen; > + char *end; > + int len; > + > + if (code >= ARRAY_SIZE(dhcp_opt_types)) > + return -1; > + > + type = dhcp_opt_types[code]; > + > + if (!*str) > + return -1; > + > + switch (type) { > + case DHCP_OPT_NONE: > + return -1; > + case DHCP_OPT_IPV4: > + case DHCP_OPT_IPV4_LIST: > + len = 0; > + > + while (*str) { > + char ipbuf[INET_ADDRSTRLEN]; > + size_t chunk; > + > + chunk = strcspn(str, ","); > + > + if (!chunk || chunk >= sizeof(ipbuf)) > + return -1; > + > + memcpy(ipbuf, str, chunk); > + ipbuf[chunk] = '\0'; > + > + if (len + (int)sizeof(struct in_addr) > (int)buf_len) Both sides are necessarily non-negative, so it would make more sense to make len unsigned than to cast the other things to signed. > + return -1; > + > + if (inet_pton(AF_INET, ipbuf, buf + len) != 1) > + return -1; > + > + len += sizeof(struct in_addr); > + > + if (type == DHCP_OPT_IPV4) { > + if (str[chunk] == ',') > + return -1; > + break; > + } > + > + str += chunk + (str[chunk] == ','); > + } > + > + if (!len) > + return -1; > + > + return len; > + case DHCP_OPT_INT8: > + case DHCP_OPT_INT16: > + case DHCP_OPT_INT32: > + case DHCP_OPT_SINT32: > + if (type == DHCP_OPT_INT8) > + width = 1; > + else if (type == DHCP_OPT_INT16) > + width = 2; > + else > + width = 4; > + > + if (buf_len < width) > + return -1; > + > + errno = 0; > + if (type == DHCP_OPT_SINT32) { > + long sval; > + > + sval = strtol(str, &end, 0); > + if (*end || errno || > + sval < INT32_MIN || sval > INT32_MAX) > + return -1; > + val = (uint32_t)sval; > + } else { > + val = strtoul(str, &end, 0); > + if (*end || errno || > + val >= (1ULL << (width * 8))) > + return -1; > + } > + > + for (i = width; i > 0; i--) { > + buf[i - 1] = val & 0xff; > + val >>= 8; > + } > + > + return width; > + case DHCP_OPT_STR: > + slen = strlen(str); > + > + if (slen >= buf_len) > + return -1; > + > + memcpy(buf, str, slen); > + > + return slen; > + } > + > + return -1; > +} > + > /** > * fill_one() - Fill a single option into a buffer > * @buf: Buffer to write option > @@ -541,6 +725,13 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > if (!c->no_dhcp_dns_search) > opt_set_dns_search(c, OPT_MAX - 3); > > + for (i = 1; i < 255; i++) { > + if (!c->dhcp_opts[i].str[0]) > + continue; > + opts[i].slen = dhcp_opt_parse(i, c->dhcp_opts[i].str, > + opts[i].s, sizeof(opts[i].s)); > + } > + > /* RFC 2132, Section 9.5: put boot file name in the 'file' header > * field. Suppress option 67 from the options area and reserve > * the file field from overload. > diff --git a/dhcp.h b/dhcp.h > index cd50c99..cc8d5dd 100644 > --- a/dhcp.h > +++ b/dhcp.h > @@ -8,5 +8,7 @@ > > int dhcp(const struct ctx *c, struct iov_tail *data); > void dhcp_init(void); > +int dhcp_opt_parse(uint8_t code, const char *str, > + uint8_t *buf, size_t buf_len); > > #endif /* DHCP_H */ > diff --git a/passt.1 b/passt.1 > index 908fd4a..ccdcbb2 100644 > --- a/passt.1 > +++ b/passt.1 > @@ -430,6 +430,48 @@ Send \fIname\fR as DHCP option 12 (hostname). > FQDN to configure the client with. > Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39. > > +.TP > +.BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR > +Set DHCP option \fICODE\fR (1\-254) to \fIVALUE\fR. The value format depends > +on the option type and is determined automatically from the option code. > +Multiple IPv4 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. Options set with > +\fB\-\-dhcp-opt\fR override built-in values. > +.PP > +Examples: > +.nf > + \-\-dhcp-opt 6,8.8.8.8,4.4.4.4 > + \-\-dhcp-opt 12,myhostname > +.fi > +.PP > +Only the following option codes are supported (unsupported codes cause an error): > +.RS > +.TP > +.B IPv4 address options > +1 (Subnet Mask), 16 (Swap Server), 28 (Broadcast Address), 50 (Requested IP), > +54 (Server Identifier) > +.TP > +.B IPv4 address list options (comma-separated) > +3 (Router), 4 (Time Server), 5 (Name Server), 6 (DNS), 7 (Log Server), > +8 (Cookie Server), 9 (LPR Server), 10 (Impress Server), > +11 (Resource Location Server), 41 (NIS Servers), > +42 (NTP Servers), 44 (NetBIOS Name Server) > +.TP > +.B Integer options > +2 (Time Offset, 32-bit), 13 (Boot File Size, 16-bit), 19 (IP Forwarding, 8-bit), > +23 (Default IP TTL, 8-bit), 26 (Interface MTU, 16-bit), > +37 (TCP Default TTL, 8-bit), 38 (TCP Keepalive Interval, 32-bit), > +51 (IP Address Lease Time, 32-bit), > +53 (DHCP Message Type, 8-bit), 57 (Max DHCP Message Size, 16-bit), > +58 (Renewal Time, 32-bit), 59 (Rebinding Time, 32-bit) > +.TP > +.B String options > +12 (Host Name), 15 (Domain Name), 17 (Root Path), 40 (NIS Domain Name), > +60 (Vendor Class Identifier), 66 (TFTP Server Name), > +67 (Bootfile Name), 252 (WPAD URL) > +.RE > + > .TP > .BR \-t ", " \-\-tcp-ports " " \fIspec > Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of: > diff --git a/passt.h b/passt.h > index 3a07294..15e2d83 100644 > --- a/passt.h > +++ b/passt.h > @@ -182,6 +182,8 @@ struct ip6_ctx { > * @dns_search: DNS search list > * @hostname: Guest hostname > * @fqdn: Guest FQDN > + * @dhcp_opts: User-specified DHCP options from --dhcp-opt > + * @dhcp_opts.str: String value from command line > * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 disabled > * @ip6: IPv6 configuration > * @pasta_ifn: Name of namespace interface for pasta > @@ -264,6 +266,10 @@ struct ctx { > char hostname[PASST_MAXDNAME]; > char fqdn[PASST_MAXDNAME]; > > + struct { > + char str[255]; > + } dhcp_opts[256]; > + > int ifi6; > struct ip6_ctx ip6; > > -- > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser 2026-06-19 3:52 ` David Gibson @ 2026-06-19 7:29 ` Anshu Kumari 2026-06-22 2:42 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio 0 siblings, 2 replies; 18+ messages in thread From: Anshu Kumari @ 2026-06-19 7:29 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 21835 bytes --] On Fri, Jun 19, 2026 at 9:24 AM David Gibson <david@gibson.dropbear.id.au> wrote: > On Wed, Jun 17, 2026 at 06:52:37PM +0530, Anshu Kumari wrote: > > Introduce the --dhcp-opt flag that allows setting arbitrary DHCP > > options from command-line in the form [--dhcp-opt CODE,VALUE]. > > > > Add a type lookup table mapping option codes to RFC 2132 value types > > (IPv4, IPv4 list, integer, string) and dhcp_opt_parse() to convert > > CLI strings to binary wire format. Parsed options are stored in > > struct ctx and injected into DHCP replies. If the same option code > > is given more than once, the last value wins. > > > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > > --- > > v4: > > - Renamed custom_opts to dhcp_opts, 256 entries indexed by option > > code, removed MAX_CUSTOM_DHCP_OPTS and count field. > > - Changed str buffer from 256 to 255 bytes. > > - Moved function to conf.c as static conf_dhcp_option(), renamed > > from dhcp_add_option(). > > - Made dhcp_opt_parse() non-static, declared in dhcp.h > > - Dropped val/len from ctx struct; conf_dhcp_option() validates > > with temp buffer, dhcp() parses str directly into opts[] at > > reply time. > > Hmm. So each option is parsed twice. What prevents you from parsing > directly into the opts[] array at conf() time? > The first parse acts as a validation step to check that the user has entered the correct value format for the option. Without it, if the user passes something like *--dhcp-opt 3,notanip*, the error would surface only when the first DHCP client connects, not at startup. I think it's better to fail during startup if correct value format is not entered in command-line rather than failing at later stage during reply time. > > > - Replaced strtok_r() + 256-byte buffer with strcspn() + > > INET_ADDRSTRLEN buffer. > > - Added DHCP_OPT_SINT32 for option 2 (Time Offset), uses strtol() > > per RFC 2132 Section 8.2. > > - All errors in dhcp_opt_parse() return -1, removed die() calls; > > caller handles error message consistently. > > - Removed redundant !slen check in DHCP_OPT_STR case. > > - Omitted explicit array size for dhcp_opt_types[], arraydded bounds > > check before lookup. > > - Added errno = 0 + errno check for strtoul() in case 34. > > - Fixed usage text: "Set DHCP option CODE to VAL". > > - Improved man page: added format description and examples > > > > v3: > > - Replaced DHCP_OPT_INTEGER with separate DHCP_OPT_INT8/INT16/INT32 > > enums, removed dhcp_opt_int_width[] array. > > - Shared logic between DHCP_OPT_IPV4 and DHCP_OPT_IPV4_LIST — parse > > both as list, error if >1 in single case. > > - Added errno = 0 before strtoul() and check after. > > - Fixed range check: 1ULL << (width * 8) for all widths including > > width==4. > > - strncpy → memcpy for DHCP_OPT_STR. > > - Moved enum to dhcp.c since not used in other files. > > - Removed options 55, 61 (client-only), 119 (DNS compression, use > > --dhcp-search instead), 33 (IP pairs not supported). > > - DHCP_OPT_PARSE_BUF 1024 → char tmp[256]. > > - Upgraded dhcp_add_option() to call dhcp_opt_parse() and populate > > val[]/len. > > - Aligned array entries for readability. > > - Added tab after @DHCP_OPT_IPV4_LIST: in kerneldoc. > > - Reject empty value strings before parsing > > - Reject leading/trailing/consecutive commas in IP list values. > > > > v2: > > - Replaced struct lookup table + dhcp_opt_type_lookup() function with > flat dhcp_opt_types[256] array indexed by code. > > - Consolidated DHCP_OPT_UINT8/UINT16/UINT32 into single > DHCP_OPT_INTEGER with dhcp_opt_int_width[256] table. > > - Dropped DHCP_OPT_ROUTES / option 121 entirely. > > - Added kerneldoc for enum dhcp_opt_type values. > > - Removed curly braces from switch cases, declarations before switch. > > - Added newlines before return statements. > > - Changed IP list delimiter from space to comma (--dhcp-opt > 6,1.1.1.1,8.8.8.8). > > - Defined DHCP_OPT_PARSE_BUF constant for bare 1024. > > - Added len and val[255] fields to struct here (moved from patch 1). > > - Added kerneldoc for @custom_opts.len and @custom_opts.val. > > - Wired dhcp_opt_parse() into case 32 (--dhcp-boot) to populate > val/len. > > --- > > conf.c | 45 ++++++++++++- > > dhcp.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > dhcp.h | 2 + > > passt.1 | 42 +++++++++++++ > > passt.h | 6 ++ > > 5 files changed, 285 insertions(+), 1 deletion(-) > > > > diff --git a/conf.c b/conf.c > > index cd05adf..836b297 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -47,6 +47,7 @@ > > #include "lineread.h" > > #include "isolation.h" > > #include "log.h" > > +#include "dhcp.h" > > #include "vhost_user.h" > > #include "epoll_ctl.h" > > #include "conf.h" > > @@ -616,7 +617,8 @@ static void usage(const char *name, FILE *f, int > status) > > " -S, --search LIST Space-separated list, search > domains\n" > > " a single, empty option disables the DNS search list\n" > > " -H, --hostname NAME Hostname to configure client > with\n" > > - " --fqdn NAME FQDN to configure client with\n"); > > + " --fqdn NAME FQDN to configure client with\n" > > + " --dhcp-opt CODE,VAL Set DHCP option CODE to VAL\n"); > > if (strstr(name, "pasta")) > > FPRINTF(f, " default: don't use any search list\n"); > > else > > @@ -844,6 +846,10 @@ static void conf_print(const struct ctx *c) > > info(" router: %s", > > inet_ntop(AF_INET, &c->ip4.guest_gw, > > buf, sizeof(buf))); > > + for (i = 1; i < 255; i++) > > + if (*c->dhcp_opts[i].str) > > + info(" option %u: %s", i, > > + c->dhcp_opts[i].str); > > } > > > > for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) { > > @@ -1150,6 +1156,25 @@ static void conf_sock_listen(const struct ctx *c) > > die_perror("Couldn't add configuration socket to epoll"); > > } > > > > +/** > > + * conf_dhcp_option() - Set value for a DHCP option in configuration > > + * @c: Execution context > > + * @code: DHCP option code > > + * @val_str: Value string from command line > > + */ > > +static void conf_dhcp_option(struct ctx *c, uint8_t code, const char > *val_str) > > +{ > > + uint8_t tmp[255]; > > + > > + if (dhcp_opt_parse(code, val_str, tmp, sizeof(tmp)) < 0) > > + die("Invalid value for DHCP option %u: %s", code, val_str); > > + > > + if (snprintf_check(c->dhcp_opts[code].str, > > + sizeof(c->dhcp_opts[0].str), > > + "%s", val_str)) > > + die("DHCP option value too long: %s", val_str); > > +} > > + > > /** > > * conf() - Process command-line arguments and set configuration > > * @c: Execution context > > @@ -1233,6 +1258,7 @@ void conf(struct ctx *c, int argc, char **argv) > > {"migrate-no-linger", no_argument, NULL, 30 > }, > > {"stats", required_argument, NULL, 31 > }, > > {"conf-path", required_argument, NULL, > 'c' }, > > + {"dhcp-opt", required_argument, NULL, 34 > }, > > { 0 }, > > }; > > const char *optstring = > "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; > > @@ -1248,10 +1274,13 @@ void conf(struct ctx *c, int argc, char **argv) > > uint8_t prefix_len_from_opt = 0; > > unsigned int ifi4 = 0, ifi6 = 0; > > const char *logfile = NULL; > > + unsigned long optcode; > > char *runas = NULL; > > size_t logsize = 0; > > + const char *comma; > > long fd_tap_opt; > > int name, ret; > > + char *end; > > uid_t uid; > > gid_t gid; > > > > @@ -1467,6 +1496,20 @@ void conf(struct ctx *c, int argc, char **argv) > > die("Can't display statistics if not > running in foreground"); > > c->stats = strtol(optarg, NULL, 0); > > break; > > + case 34: > > + comma = strchr(optarg, ','); > > + if (!comma) > > + die("--dhcp-opt requires CODE,VALUE > format"); > > + > > + errno = 0; > > + optcode = strtoul(optarg, &end, 0); > > + if (end != comma || errno || > > + optcode < 1 || optcode > 254) > > + die("DHCP option code must be 1-254: %s", > > + optarg); > > + > > + conf_dhcp_option(c, optcode, comma + 1); > > + break; > > case 'd': > > c->debug = 1; > > c->quiet = 0; > > diff --git a/dhcp.c b/dhcp.c > > index 78790d8..47bb524 100644 > > --- a/dhcp.c > > +++ b/dhcp.c > > @@ -23,6 +23,7 @@ > > #include <unistd.h> > > #include <string.h> > > #include <limits.h> > > +#include <errno.h> > > > > #include "util.h" > > #include "ip.h" > > @@ -130,6 +131,189 @@ struct msg { > > uint8_t o[OPT_MAX + 1 /* End option */ ]; > > } __attribute__((__packed__)); > > > > +/** > > + * enum dhcp_opt_type - DHCP option value types per RFC 2132 > > + * @DHCP_OPT_NONE: Unsupported or unknown option > > + * @DHCP_OPT_STR: Variable-length string > > + * @DHCP_OPT_IPV4: Single IPv4 address > > + * @DHCP_OPT_IPV4_LIST: Multiple IPv4 addresses, comma-separated > > + * @DHCP_OPT_INT8: Unsigned 8-bit integer > > + * @DHCP_OPT_INT16: Unsigned 16-bit integer > > + * @DHCP_OPT_INT32: Unsigned 32-bit integer > > + * @DHCP_OPT_SINT32: Signed 32-bit integer > > For consistency with C conventions, I'd suggset UINT{8,16,32} and just > INT32 for the signed case. > > > + */ > > +enum dhcp_opt_type { > > + DHCP_OPT_NONE, > > + DHCP_OPT_STR, > > + DHCP_OPT_IPV4, > > + DHCP_OPT_IPV4_LIST, > > + DHCP_OPT_INT8, > > + DHCP_OPT_INT16, > > + DHCP_OPT_INT32, > > + DHCP_OPT_SINT32, > > +}; > > + > > +/** > > + * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by > code > > + */ > > +static const enum dhcp_opt_type dhcp_opt_types[] = { > > + [1] = DHCP_OPT_IPV4, /* Subnet Mask */ > > + [2] = DHCP_OPT_SINT32, /* Time Offset */ > > + [3] = DHCP_OPT_IPV4_LIST, /* Router */ > > + [4] = DHCP_OPT_IPV4_LIST, /* Time Server */ > > + [5] = DHCP_OPT_IPV4_LIST, /* Name Server */ > > + [6] = DHCP_OPT_IPV4_LIST, /* Domain Name Server */ > > + [7] = DHCP_OPT_IPV4_LIST, /* Log Server */ > > + [8] = DHCP_OPT_IPV4_LIST, /* Cookie Server */ > > + [9] = DHCP_OPT_IPV4_LIST, /* LPR Server */ > > + [10] = DHCP_OPT_IPV4_LIST, /* Impress Server */ > > + [11] = DHCP_OPT_IPV4_LIST, /* Resource Location Server */ > > + [12] = DHCP_OPT_STR, /* Host Name */ > > + [13] = DHCP_OPT_INT16, /* Boot File Size */ > > + [15] = DHCP_OPT_STR, /* Domain Name */ > > + [16] = DHCP_OPT_IPV4, /* Swap Server */ > > + [17] = DHCP_OPT_STR, /* Root Path */ > > + [19] = DHCP_OPT_INT8, /* IP Forwarding */ > > + [23] = DHCP_OPT_INT8, /* Default IP TTL */ > > + [26] = DHCP_OPT_INT16, /* Interface MTU */ > > + [28] = DHCP_OPT_IPV4, /* Broadcast Address */ > > + [37] = DHCP_OPT_INT8, /* TCP Default TTL */ > > + [38] = DHCP_OPT_INT32, /* TCP Keepalive Interval */ > > + [40] = DHCP_OPT_STR, /* NIS Domain Name */ > > + [41] = DHCP_OPT_IPV4_LIST, /* NIS Servers */ > > + [42] = DHCP_OPT_IPV4_LIST, /* NTP Servers */ > > + [44] = DHCP_OPT_IPV4_LIST, /* NetBIOS Name Server */ > > + [50] = DHCP_OPT_IPV4, /* Requested IP Address */ > > + [51] = DHCP_OPT_INT32, /* IP Address Lease Time */ > > + [53] = DHCP_OPT_INT8, /* DHCP Message Type */ > > + [54] = DHCP_OPT_IPV4, /* Server Identifier */ > > + [57] = DHCP_OPT_INT16, /* Max DHCP Message Size */ > > + [58] = DHCP_OPT_INT32, /* Renewal (T1) Time */ > > + [59] = DHCP_OPT_INT32, /* Rebinding (T2) Time */ > > + [60] = DHCP_OPT_STR, /* Vendor Class Identifier */ > > + [66] = DHCP_OPT_STR, /* TFTP Server Name */ > > + [67] = DHCP_OPT_STR, /* Bootfile Name */ > > + [252] = DHCP_OPT_STR, /* WPAD URL */ > > +}; > > + > > +/** > > + * dhcp_opt_parse() - Parse a DHCP option value > > + * @code: DHCP 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 > > + */ > > +int dhcp_opt_parse(uint8_t code, const char *str, > > + uint8_t *buf, size_t buf_len) > > +{ > > + enum dhcp_opt_type type; > > + unsigned long val; > > + unsigned int i; > > + uint8_t width; > > + size_t slen; > > + char *end; > > + int len; > > + > > + if (code >= ARRAY_SIZE(dhcp_opt_types)) > > + return -1; > > + > > + type = dhcp_opt_types[code]; > > + > > + if (!*str) > > + return -1; > > + > > + switch (type) { > > + case DHCP_OPT_NONE: > > + return -1; > > + case DHCP_OPT_IPV4: > > + case DHCP_OPT_IPV4_LIST: > > + len = 0; > > + > > + while (*str) { > > + char ipbuf[INET_ADDRSTRLEN]; > > + size_t chunk; > > + > > + chunk = strcspn(str, ","); > > + > > + if (!chunk || chunk >= sizeof(ipbuf)) > > + return -1; > > + > > + memcpy(ipbuf, str, chunk); > > + ipbuf[chunk] = '\0'; > > + > > + if (len + (int)sizeof(struct in_addr) > > (int)buf_len) > > Both sides are necessarily non-negative, so it would make more sense > to make len unsigned than to cast the other things to signed. > > > + return -1; > > + > > + if (inet_pton(AF_INET, ipbuf, buf + len) != 1) > > + return -1; > > + > > + len += sizeof(struct in_addr); > > + > > + if (type == DHCP_OPT_IPV4) { > > + if (str[chunk] == ',') > > + return -1; > > + break; > > + } > > + > > + str += chunk + (str[chunk] == ','); > > + } > > + > > + if (!len) > > + return -1; > > + > > + return len; > > + case DHCP_OPT_INT8: > > + case DHCP_OPT_INT16: > > + case DHCP_OPT_INT32: > > + case DHCP_OPT_SINT32: > > + if (type == DHCP_OPT_INT8) > > + width = 1; > > + else if (type == DHCP_OPT_INT16) > > + width = 2; > > + else > > + width = 4; > > + > > + if (buf_len < width) > > + return -1; > > + > > + errno = 0; > > + if (type == DHCP_OPT_SINT32) { > > + long sval; > > + > > + sval = strtol(str, &end, 0); > > + if (*end || errno || > > + sval < INT32_MIN || sval > INT32_MAX) > > + return -1; > > + val = (uint32_t)sval; > > + } else { > > + val = strtoul(str, &end, 0); > > + if (*end || errno || > > + val >= (1ULL << (width * 8))) > > + return -1; > > + } > > + > > + for (i = width; i > 0; i--) { > > + buf[i - 1] = val & 0xff; > > + val >>= 8; > > + } > > + > > + return width; > > + case DHCP_OPT_STR: > > + slen = strlen(str); > > + > > + if (slen >= buf_len) > > + return -1; > > + > > + memcpy(buf, str, slen); > > + > > + return slen; > > + } > > + > > + return -1; > > +} > > + > > /** > > * fill_one() - Fill a single option into a buffer > > * @buf: Buffer to write option > > @@ -541,6 +725,13 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > > if (!c->no_dhcp_dns_search) > > opt_set_dns_search(c, OPT_MAX - 3); > > > > + for (i = 1; i < 255; i++) { > > + if (!c->dhcp_opts[i].str[0]) > > + continue; > > + opts[i].slen = dhcp_opt_parse(i, c->dhcp_opts[i].str, > > + opts[i].s, > sizeof(opts[i].s)); > > + } > > + > > /* RFC 2132, Section 9.5: put boot file name in the 'file' header > > * field. Suppress option 67 from the options area and reserve > > * the file field from overload. > > diff --git a/dhcp.h b/dhcp.h > > index cd50c99..cc8d5dd 100644 > > --- a/dhcp.h > > +++ b/dhcp.h > > @@ -8,5 +8,7 @@ > > > > int dhcp(const struct ctx *c, struct iov_tail *data); > > void dhcp_init(void); > > +int dhcp_opt_parse(uint8_t code, const char *str, > > + uint8_t *buf, size_t buf_len); > > > > #endif /* DHCP_H */ > > diff --git a/passt.1 b/passt.1 > > index 908fd4a..ccdcbb2 100644 > > --- a/passt.1 > > +++ b/passt.1 > > @@ -430,6 +430,48 @@ Send \fIname\fR as DHCP option 12 (hostname). > > FQDN to configure the client with. > > Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39. > > > > +.TP > > +.BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR > > +Set DHCP option \fICODE\fR (1\-254) to \fIVALUE\fR. The value format > depends > > +on the option type and is determined automatically from the option code. > > +Multiple IPv4 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. Options set with > > +\fB\-\-dhcp-opt\fR override built-in values. > > +.PP > > +Examples: > > +.nf > > + \-\-dhcp-opt 6,8.8.8.8,4.4.4.4 > > + \-\-dhcp-opt 12,myhostname > > +.fi > > +.PP > > +Only the following option codes are supported (unsupported codes cause > an error): > > +.RS > > +.TP > > +.B IPv4 address options > > +1 (Subnet Mask), 16 (Swap Server), 28 (Broadcast Address), 50 > (Requested IP), > > +54 (Server Identifier) > > +.TP > > +.B IPv4 address list options (comma-separated) > > +3 (Router), 4 (Time Server), 5 (Name Server), 6 (DNS), 7 (Log Server), > > +8 (Cookie Server), 9 (LPR Server), 10 (Impress Server), > > +11 (Resource Location Server), 41 (NIS Servers), > > +42 (NTP Servers), 44 (NetBIOS Name Server) > > +.TP > > +.B Integer options > > +2 (Time Offset, 32-bit), 13 (Boot File Size, 16-bit), 19 (IP > Forwarding, 8-bit), > > +23 (Default IP TTL, 8-bit), 26 (Interface MTU, 16-bit), > > +37 (TCP Default TTL, 8-bit), 38 (TCP Keepalive Interval, 32-bit), > > +51 (IP Address Lease Time, 32-bit), > > +53 (DHCP Message Type, 8-bit), 57 (Max DHCP Message Size, 16-bit), > > +58 (Renewal Time, 32-bit), 59 (Rebinding Time, 32-bit) > > +.TP > > +.B String options > > +12 (Host Name), 15 (Domain Name), 17 (Root Path), 40 (NIS Domain Name), > > +60 (Vendor Class Identifier), 66 (TFTP Server Name), > > +67 (Bootfile Name), 252 (WPAD URL) > > +.RE > > + > > .TP > > .BR \-t ", " \-\-tcp-ports " " \fIspec > > Configure TCP port forwarding to guest or namespace. \fIspec\fR can be > one of: > > diff --git a/passt.h b/passt.h > > index 3a07294..15e2d83 100644 > > --- a/passt.h > > +++ b/passt.h > > @@ -182,6 +182,8 @@ struct ip6_ctx { > > * @dns_search: DNS search list > > * @hostname: Guest hostname > > * @fqdn: Guest FQDN > > + * @dhcp_opts: User-specified DHCP options from --dhcp-opt > > + * @dhcp_opts.str: String value from command line > > * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 > disabled > > * @ip6: IPv6 configuration > > * @pasta_ifn: Name of namespace interface for pasta > > @@ -264,6 +266,10 @@ struct ctx { > > char hostname[PASST_MAXDNAME]; > > char fqdn[PASST_MAXDNAME]; > > > > + struct { > > + char str[255]; > > + } dhcp_opts[256]; > > + > > int ifi6; > > struct ip6_ctx ip6; > > > > -- > > 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 > [-- Attachment #2: Type: text/html, Size: 27754 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser 2026-06-19 7:29 ` Anshu Kumari @ 2026-06-22 2:42 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio 1 sibling, 0 replies; 18+ messages in thread From: David Gibson @ 2026-06-22 2:42 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 2433 bytes --] On Fri, Jun 19, 2026 at 12:59:10PM +0530, Anshu Kumari wrote: > On Fri, Jun 19, 2026 at 9:24 AM David Gibson <david@gibson.dropbear.id.au> > wrote: > > > On Wed, Jun 17, 2026 at 06:52:37PM +0530, Anshu Kumari wrote: > > > Introduce the --dhcp-opt flag that allows setting arbitrary DHCP > > > options from command-line in the form [--dhcp-opt CODE,VALUE]. > > > > > > Add a type lookup table mapping option codes to RFC 2132 value types > > > (IPv4, IPv4 list, integer, string) and dhcp_opt_parse() to convert > > > CLI strings to binary wire format. Parsed options are stored in > > > struct ctx and injected into DHCP replies. If the same option code > > > is given more than once, the last value wins. > > > > > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > > > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > > > --- > > > v4: > > > - Renamed custom_opts to dhcp_opts, 256 entries indexed by option > > > code, removed MAX_CUSTOM_DHCP_OPTS and count field. > > > - Changed str buffer from 256 to 255 bytes. > > > - Moved function to conf.c as static conf_dhcp_option(), renamed > > > from dhcp_add_option(). > > > - Made dhcp_opt_parse() non-static, declared in dhcp.h > > > - Dropped val/len from ctx struct; conf_dhcp_option() validates > > > with temp buffer, dhcp() parses str directly into opts[] at > > > reply time. > > > > Hmm. So each option is parsed twice. What prevents you from parsing > > directly into the opts[] array at conf() time? > > > > The first parse acts as a validation step to check that the user has > entered the correct value format for the option. Without it, if the > user passes something like *--dhcp-opt 3,notanip*, the error would > surface only when the first DHCP client connects, not at startup. > > I think it's better to fail during startup if correct value format > is not entered in command-line rather than failing at later stage > during reply time. Right, I understand that. What I'm suggesting is that you keep the binary value from the initial parse (that also error checks) - like you did in the earlier versions - but that it be kept in the opts[] array instead of in a new data structure. -- 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 --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser 2026-06-19 7:29 ` Anshu Kumari 2026-06-22 2:42 ` David Gibson @ 2026-07-01 7:51 ` Stefano Brivio 1 sibling, 0 replies; 18+ messages in thread From: Stefano Brivio @ 2026-07-01 7:51 UTC (permalink / raw) To: Anshu Kumari; +Cc: David Gibson, passt-dev, jmaloy, lvivier On Fri, 19 Jun 2026 12:59:10 +0530 Anshu Kumari <anskuma@redhat.com> wrote: > On Fri, Jun 19, 2026 at 9:24 AM David Gibson <david@gibson.dropbear.id.au> > wrote: > > > On Wed, Jun 17, 2026 at 06:52:37PM +0530, Anshu Kumari wrote: > > > Introduce the --dhcp-opt flag that allows setting arbitrary DHCP > > > options from command-line in the form [--dhcp-opt CODE,VALUE]. > > > > > > Add a type lookup table mapping option codes to RFC 2132 value types > > > (IPv4, IPv4 list, integer, string) and dhcp_opt_parse() to convert > > > CLI strings to binary wire format. Parsed options are stored in > > > struct ctx and injected into DHCP replies. If the same option code > > > is given more than once, the last value wins. > > > > > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > > > Signed-off-by: Anshu Kumari <anskuma@redhat.com> > > > --- > > > v4: > > > - Renamed custom_opts to dhcp_opts, 256 entries indexed by option > > > code, removed MAX_CUSTOM_DHCP_OPTS and count field. > > > - Changed str buffer from 256 to 255 bytes. > > > - Moved function to conf.c as static conf_dhcp_option(), renamed > > > from dhcp_add_option(). > > > - Made dhcp_opt_parse() non-static, declared in dhcp.h > > > - Dropped val/len from ctx struct; conf_dhcp_option() validates > > > with temp buffer, dhcp() parses str directly into opts[] at > > > reply time. > > > > Hmm. So each option is parsed twice. What prevents you from parsing > > directly into the opts[] array at conf() time? > > The first parse acts as a validation step to check that the user has > entered the > correct value format for the option. Without it, if the user passes > something > like *--dhcp-opt 3,notanip*, the error would surface only when the first > DHCP client connects, not at startup. > > I think it's better to fail during startup if correct value format is not > entered in command-line rather than failing at later > stage during reply time. Sure, I would say we have to. But I think David was suggesting (that would be my suggestion as well) that you actually validate values right away, and store them directly into opts[], instead of having two steps and keeping a c->dhcp_opts[] array in parallel. The downside of a single parsing step, without duplicating values, is that you would need to render values back from opts[] to print them in conf_print(). But it should be relatively trivial. It might actually be less code than it is now. -- Stefan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser 2026-06-17 13:22 ` [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser Anshu Kumari 2026-06-19 3:52 ` David Gibson @ 2026-07-01 17:00 ` Stefano Brivio 1 sibling, 0 replies; 18+ messages in thread From: Stefano Brivio @ 2026-07-01 17:00 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, jmaloy, david, lvivier On Wed, 17 Jun 2026 18:52:37 +0530 Anshu Kumari <anskuma@redhat.com> wrote: > [...] > > @@ -182,6 +182,8 @@ struct ip6_ctx { > * @dns_search: DNS search list > * @hostname: Guest hostname > * @fqdn: Guest FQDN > + * @dhcp_opts: User-specified DHCP options from --dhcp-opt > + * @dhcp_opts.str: String value from command line > * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 disabled > * @ip6: IPv6 configuration > * @pasta_ifn: Name of namespace interface for pasta > @@ -264,6 +266,10 @@ struct ctx { > char hostname[PASST_MAXDNAME]; > char fqdn[PASST_MAXDNAME]; > > + struct { > + char str[255]; > + } dhcp_opts[256]; This just occurred to me after reviewing the DHCPv6 equivalent: as you only support options in the table, you could use the count of items in that table, instead of 256. See my review of v2 1/2 of the DHCPv6 series for one way to do that. -- Stefano ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 4/4] conf: Add --dhcp-boot command-line option 2026-06-17 13:22 [PATCH v4 0/4] Add --dhcp-boot and --dhcp-opt options Anshu Kumari ` (2 preceding siblings ...) 2026-06-17 13:22 ` [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser Anshu Kumari @ 2026-06-17 13:22 ` Anshu Kumari 2026-06-19 3:53 ` David Gibson 3 siblings, 1 reply; 18+ messages in thread From: Anshu Kumari @ 2026-06-17 13:22 UTC (permalink / raw) To: passt-dev, anskuma, sbrivio; +Cc: jmaloy, david, lvivier Add a convenience shorthand --dhcp-boot FILE that sets the boot file name (DHCP option 67) for network boot. This is equivalent to --dhcp-opt 67,FILE. Link: https://bugs.passt.top/show_bug.cgi?id=192 Signed-off-by: Anshu Kumari <anskuma@redhat.com> --- v4: - Changed argument name from URL to FILE in usage and man page. - Fixed UEFI HTTP boot wording in man page v3: - case 32 now calls dhcp_add_option(c, 67, optarg). - Handles duplicate codes: --dhcp-boot and --dhcp-opt 67 coexist correctly, last value wins. v2: - Removed separate dhcp_boot[PATH_MAX] field — --dhcp-boot foo now stores into custom_opts[] as code 67 (same as --dhcp-opt 67,foo) --- conf.c | 5 +++++ passt.1 | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/conf.c b/conf.c index 836b297..f100e0b 100644 --- a/conf.c +++ b/conf.c @@ -618,6 +618,7 @@ static void usage(const char *name, FILE *f, int status) " a single, empty option disables the DNS search list\n" " -H, --hostname NAME Hostname to configure client with\n" " --fqdn NAME FQDN to configure client with\n" + " --dhcp-boot FILE Boot file name for network boot\n" " --dhcp-opt CODE,VAL Set DHCP option CODE to VAL\n"); if (strstr(name, "pasta")) FPRINTF(f, " default: don't use any search list\n"); @@ -1258,6 +1259,7 @@ void conf(struct ctx *c, int argc, char **argv) {"migrate-no-linger", no_argument, NULL, 30 }, {"stats", required_argument, NULL, 31 }, {"conf-path", required_argument, NULL, 'c' }, + {"dhcp-boot", required_argument, NULL, 33 }, {"dhcp-opt", required_argument, NULL, 34 }, { 0 }, }; @@ -1496,6 +1498,9 @@ void conf(struct ctx *c, int argc, char **argv) die("Can't display statistics if not running in foreground"); c->stats = strtol(optarg, NULL, 0); break; + case 33: + conf_dhcp_option(c, 67, optarg); + break; case 34: comma = strchr(optarg, ','); if (!comma) diff --git a/passt.1 b/passt.1 index ccdcbb2..f2341ef 100644 --- a/passt.1 +++ b/passt.1 @@ -430,6 +430,13 @@ Send \fIname\fR as DHCP option 12 (hostname). FQDN to configure the client with. Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39. +.TP +.BR \-\-dhcp-boot " " \fIfile +Convenience shorthand for \fB\-\-dhcp-opt\fR 67,\fIfile\fR. +Sets the boot file name (DHCP option 67) for network boot. +For UEFI HTTP boot, the vendor class identifier also needs to be set using +\fB\-\-dhcp-opt\fR 60,HTTPClient. + .TP .BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR Set DHCP option \fICODE\fR (1\-254) to \fIVALUE\fR. The value format depends -- 2.54.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 4/4] conf: Add --dhcp-boot command-line option 2026-06-17 13:22 ` [PATCH v4 4/4] conf: Add --dhcp-boot command-line option Anshu Kumari @ 2026-06-19 3:53 ` David Gibson 0 siblings, 0 replies; 18+ messages in thread From: David Gibson @ 2026-06-19 3:53 UTC (permalink / raw) To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier [-- Attachment #1: Type: text/plain, Size: 3156 bytes --] On Wed, Jun 17, 2026 at 06:52:38PM +0530, Anshu Kumari wrote: > Add a convenience shorthand --dhcp-boot FILE that sets the boot file > name (DHCP option 67) for network boot. This is equivalent to > --dhcp-opt 67,FILE. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari <anskuma@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > v4: > - Changed argument name from URL to FILE in usage and man page. > - Fixed UEFI HTTP boot wording in man page > > v3: > - case 32 now calls dhcp_add_option(c, 67, optarg). > - Handles duplicate codes: --dhcp-boot and --dhcp-opt 67 coexist > correctly, last value wins. > > v2: > - Removed separate dhcp_boot[PATH_MAX] field — --dhcp-boot foo now stores into custom_opts[] as code 67 (same as --dhcp-opt 67,foo) > > --- > conf.c | 5 +++++ > passt.1 | 7 +++++++ > 2 files changed, 12 insertions(+) > > diff --git a/conf.c b/conf.c > index 836b297..f100e0b 100644 > --- a/conf.c > +++ b/conf.c > @@ -618,6 +618,7 @@ static void usage(const char *name, FILE *f, int status) > " a single, empty option disables the DNS search list\n" > " -H, --hostname NAME Hostname to configure client with\n" > " --fqdn NAME FQDN to configure client with\n" > + " --dhcp-boot FILE Boot file name for network boot\n" > " --dhcp-opt CODE,VAL Set DHCP option CODE to VAL\n"); > if (strstr(name, "pasta")) > FPRINTF(f, " default: don't use any search list\n"); > @@ -1258,6 +1259,7 @@ void conf(struct ctx *c, int argc, char **argv) > {"migrate-no-linger", no_argument, NULL, 30 }, > {"stats", required_argument, NULL, 31 }, > {"conf-path", required_argument, NULL, 'c' }, > + {"dhcp-boot", required_argument, NULL, 33 }, > {"dhcp-opt", required_argument, NULL, 34 }, > { 0 }, > }; > @@ -1496,6 +1498,9 @@ void conf(struct ctx *c, int argc, char **argv) > die("Can't display statistics if not running in foreground"); > c->stats = strtol(optarg, NULL, 0); > break; > + case 33: > + conf_dhcp_option(c, 67, optarg); > + break; > case 34: > comma = strchr(optarg, ','); > if (!comma) > diff --git a/passt.1 b/passt.1 > index ccdcbb2..f2341ef 100644 > --- a/passt.1 > +++ b/passt.1 > @@ -430,6 +430,13 @@ Send \fIname\fR as DHCP option 12 (hostname). > FQDN to configure the client with. > Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39. > > +.TP > +.BR \-\-dhcp-boot " " \fIfile > +Convenience shorthand for \fB\-\-dhcp-opt\fR 67,\fIfile\fR. > +Sets the boot file name (DHCP option 67) for network boot. > +For UEFI HTTP boot, the vendor class identifier also needs to be set using > +\fB\-\-dhcp-opt\fR 60,HTTPClient. > + > .TP > .BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR > Set DHCP option \fICODE\fR (1\-254) to \fIVALUE\fR. The value format depends > -- > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-07-01 17:00 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-06-17 13:22 [PATCH v4 0/4] Add --dhcp-boot and --dhcp-opt options Anshu Kumari 2026-06-17 13:22 ` [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari 2026-06-19 3:26 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio 2026-06-17 13:22 ` [PATCH v4 2/4] dhcp: Add option overload Anshu Kumari 2026-06-19 3:39 ` David Gibson 2026-06-19 7:55 ` Anshu Kumari 2026-06-22 2:43 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio 2026-07-01 17:00 ` Stefano Brivio 2026-06-17 13:22 ` [PATCH v4 3/4] dhcp: Add --dhcp-opt with option table and value parser Anshu Kumari 2026-06-19 3:52 ` David Gibson 2026-06-19 7:29 ` Anshu Kumari 2026-06-22 2:42 ` David Gibson 2026-07-01 7:51 ` Stefano Brivio 2026-07-01 17:00 ` Stefano Brivio 2026-06-17 13:22 ` [PATCH v4 4/4] conf: Add --dhcp-boot command-line option Anshu Kumari 2026-06-19 3:53 ` David Gibson
Code repositories for project(s) associated with this public inbox https://passt.top/passt This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for IMAP folder(s).