Re: [PATCH v4 2/4] dhcp: Add option overload

[Stefano Brivio <sbrivio@redhat.com>]

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