From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202410 header.b=a7t36E4U; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 4C3965A004E for ; Mon, 25 Nov 2024 02:11:47 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202410; t=1732497096; bh=QCgdij7AcGcq1bPJk0YeE2QkwAZqbtTmXTgU1h1yxp4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=a7t36E4U/s4AiFe0/kEq1ghY3hr4kOdBEQMn51XBMnL+r3CL3W4UFLzGY6iQObERL uAo4pjbtTV5tt3CqacKhe0INSZz+9PDQ4Rrec+Z+Rp8XW4/dOnFzan9p7988RoaeLC +ucOT/LqlM6l0qfmCS2rwjBByD2wyLYorSPh4p7mTFVLOI8EtysOvcvuoFi/qo0vbr oIIJsDMTSgjMhgs/7JADeoYKOKzPw7GLVupac9OQgHEbneHcKAfEYN91YQ+SA+AtU5 82je29oAS1sjIXwYy6Ss6PpLNC9RZ5TAIscNfWvivq9LB92+lNUKMuDUog7AT6jtPy qWHCrf1Q48HKA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4XxSMr14nJz4xTx; Mon, 25 Nov 2024 12:11:36 +1100 (AEDT) Date: Mon, 25 Nov 2024 12:08:35 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/3] dhcp: Use -1 as "missing option" length instead of 0 Message-ID: References: <20241125000423.4131458-1-sbrivio@redhat.com> <20241125000423.4131458-2-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="X0hBgGxYUDp81/md" Content-Disposition: inline In-Reply-To: <20241125000423.4131458-2-sbrivio@redhat.com> Message-ID-Hash: 2PROTAZDSPPGBTL2GOQAEKJUCTZ5BYC6 X-Message-ID-Hash: 2PROTAZDSPPGBTL2GOQAEKJUCTZ5BYC6 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --X0hBgGxYUDp81/md Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Nov 25, 2024 at 01:04:21AM +0100, Stefano Brivio wrote: > We want to add support for option 80 (Rapid Commit, RFC 4039), whose > length is 0. >=20 > Signed-off-by: Stefano Brivio > --- > dhcp.c | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) >=20 > diff --git a/dhcp.c b/dhcp.c > index a06f143..2fe4a4d 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -36,9 +36,9 @@ > /** > * struct opt - DHCP option > * @sent: Convenience flag, set while filling replies > - * @slen: Length of option defined for server > + * @slen: Length of option defined for server, -1 if not going to be sent > * @s: Option payload from server > - * @clen: Length of option received from client > + * @clen: Length of option received from client, -1 if not received > * @c: Option payload from client > */ > struct opt { > @@ -154,17 +154,17 @@ static int fill(struct msg *m) > * option 53 at the beginning of the list. > * Put it there explicitly, unless requested via option 55. > */ > - if (!memchr(opts[55].c, 53, opts[55].clen)) > + if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) > fill_one(m, 53, &offset); > =20 > for (i =3D 0; i < opts[55].clen; i++) { > o =3D opts[55].c[i]; > - if (opts[o].slen) > + if (opts[o].slen !=3D -1) > fill_one(m, o, &offset); > } > =20 > for (o =3D 0; o < 255; o++) { > - if (opts[o].slen && !opts[o].sent) > + if (opts[o].slen !=3D -1 && !opts[o].sent) > fill_one(m, o, &offset); > } > =20 > @@ -264,6 +264,9 @@ static void opt_set_dns_search(const struct ctx *c, s= ize_t max_len) > ".\xc0"); > } > } > + > + if (!opts[119].slen) > + opts[119].slen =3D -1; > } > =20 > /** > @@ -313,6 +316,13 @@ int dhcp(const struct ctx *c, const struct pool *p) > =20 > offset +=3D offsetof(struct msg, o); > =20 > + for (i =3D 0; i < ARRAY_SIZE(opts); i++) { > + if (!opts[i].slen) > + opts[i].slen =3D -1; > + > + opts[i].clen =3D -1; > + } Could this move to dhcp_init()? I think there you wouldn't need test and could unconditionally initialize all the lengths to -1 before initializing the options we actually use. > while (opt_off + 2 < opt_len) { > const uint8_t *olen, *val; > uint8_t *type; > @@ -334,8 +344,9 @@ int dhcp(const struct ctx *c, const struct pool *p) > if (opts[53].c[0] =3D=3D DHCPDISCOVER) { > info("DHCP: offer to discover"); > opts[53].s[0] =3D DHCPOFFER; > - } else if (opts[53].c[0] =3D=3D DHCPREQUEST || !opts[53].clen) { > - info("%s: ack to request", opts[53].clen ? "DHCP" : "BOOTP"); > + } else if (opts[53].c[0] =3D=3D DHCPREQUEST || opts[53].clen <=3D 0) { > + info("%s: ack to request", > + (opts[53].clen <=3D 0) ? "DHCP" : "BOOTP"); Should this be <=3D 0, or < 0? i.e. Wouldn't even an empty option 53 indicate we're dealing with DHCP rather than BOOTP? > opts[53].s[0] =3D DHCPACK; > } else { > return -1; > @@ -374,6 +385,8 @@ int dhcp(const struct ctx *c, const struct pool *p) > ((struct in_addr *)opts[6].s)[i] =3D c->ip4.dns[i]; > opts[6].slen +=3D sizeof(uint32_t); > } > + if (!opts[6].slen) > + opts[6].slen =3D -1; > =20 > if (!c->no_dhcp_dns_search) > opt_set_dns_search(c, sizeof(m->o)); --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --X0hBgGxYUDp81/md Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmdDzgMACgkQzQJF27ox 2GeGfw/6A9pnTWt1NJf1epU+VAmn5tmvixR5zNy6v4KlPFzW0eU5IuGCtvFXkcRs s5UGP6kQtrGSR7PPvVspQBSip5BgSSxLDV7wcxiHKwHJcy4p6ir/c9ygst3xJ7fE r8Al7OSEMD+VSTDXuWMp+31zbwbvQv1vrE5EaXD37M0HyOwe9ZVn/ITtE/HIMUt1 CSl/9yFBhr6j79aGDckmhLmhsrwkmnejTHuGBfH4bXsacwvdRrWdYmStGaQLK+YF HTgSYlvHUiEEA1rjWbRCjW46wMgYzmyXgPRNLpMq+FXFTuARJSmsr2z85awtYStU QCQKY6XdD9dUU3GmdzBzINzTSx5H9wYvVmdRYJWymKHYdba0MYo5iGJGVWfkgoxH GL7Pfu7LRklC5mhzU7xWag1cZNUhNWoQRLV7uNlTKRapp7+xfrQCNCUYW3TXdzqT CFUl7bwk6BYc9pA8GUa6ldQqnNByWPBy8r/QSLS5BtYpRmh8CgFpLG7hMcHGFbZy mp7SJZoOZsUe68noJsfzZx7sFi6Skh6f8Fw3c39NZbZRMBs1wGvuAetUg3MpoVkJ BgFilpnEPX9qjEFqqmndR4jTJpB4ay28HH1nRH9SZVIviUdKWwLDYhfv/KG8romH fBO7LX8vi7GsmFvYNw9jop4EWF8K5xI8jrqrHd2OR2WrMQAqH14= =Z/2Y -----END PGP SIGNATURE----- --X0hBgGxYUDp81/md--