From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=LUDBB5wa; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 5F7AA5A004E for ; Thu, 19 Dec 2024 01:19:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734567581; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+tWG1jHlaFc6eQyWw2h8sS9Ut0IOoZgKemiqDLnF/EY=; b=LUDBB5wasa56mfQg5nAEOtfEwn3baESWFwVyVHQQLUD+EIbiP5tLn3iMr5o94W3dDrph9R NeQrgwSZ4JdzWT7JYduGuMFna3lfXDdrBMRnxmmpC+FNVqW1nDbBD5KE3Y0Rxp0hy8WXR1 7oatUjBhmPj2i+br04ZHqUlIxiAh2/4= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-79-7JSP0ebWPxOlzRPVNk1gbA-1; Wed, 18 Dec 2024 19:19:39 -0500 X-MC-Unique: 7JSP0ebWPxOlzRPVNk1gbA-1 X-Mimecast-MFC-AGG-ID: 7JSP0ebWPxOlzRPVNk1gbA Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-43625ceae52so1084295e9.0 for ; Wed, 18 Dec 2024 16:19:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734567577; x=1735172377; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=Qe62iQGVmLAiKS6vNFuxT6muNFivs/kXVQ87REZ36bE=; b=Y5T/noHjf0Oj8FIn4q5RkikIqDT59gPW0CQKguWIfpdNFmcxAAT2Z0obmTpZcDnSA1 tLgYGcqc8prWst9IJmqw7JhSI5b2odbnrm0MOIau++tLV/AL57V7p+XwGpIe7chq7fdv n9pHZDyoeMjKdFCZENn0fk1paryzbdIYpN2tgWtp6goXX3Ni9Wxd87Z5ATZi1Jqhowke Bk5rH63wVzWvd3EqdYdy7KbFMJnDWcaPny4fN95qjfKkG7yXPPMkhBfh7MUnnZm2lnUh ATpIvv3JXU2QHzQam9jCYU4uu+PpFgFbt1bmnLiX4pk0eV4i9apeUFXamJBms52DsCC1 nPQA== X-Gm-Message-State: AOJu0YzeTCxtYOBAOkCsCfIdZVFAD/ZG69ddm57XW8eDdNhSeHmwA7r8 10Vm64QsVo9G3IJgg0v6cTu7o+4VsuEPFHyPdN3ECC9AJ3QMC96vrwwVLgJELJ79NgcuvYlLxtA gA5wta6I38nU5LTrIwVP4CcBdD37xNx/DldQcD+1Bmka0yrR4OFeMYE9/n/02FHt7kbts7+6c2/ cdfMjm/VvLhZPr49gMgiRl4JrPcTTjq4Mf X-Gm-Gg: ASbGncuf5l80mkAtrLfjezymZ5zJLLC2szGkFCKW1AT2z3n5lhyWQq0VMgJ/H5hkCAw qj0xb93PPmnMwXkNlvaL/FOBcMth6RcuEXU/EpoEkAK8gxMfL1nPsQ98KffmFPFJ7PutVf54bUw F+jL+jzPZWM0DF4t/Dh3TX2qC6msBfv67yEyYMp8ieaTzCh0rw5ZPBhEfg0Wr7eGVDo0eLmo+/U odc4n36sZ3ONtGhLbeILsmJxqC+n7h8Iv9M8z/HFHoONo5aIvSprhgFDLviQ0ApPQ4UL2/p9Reo 8MmRqhuPog== X-Received: by 2002:a05:6000:4612:b0:386:3918:16a8 with SMTP id ffacd0b85a97d-388e4d5e0f3mr4198509f8f.13.1734567577034; Wed, 18 Dec 2024 16:19:37 -0800 (PST) X-Google-Smtp-Source: AGHT+IEd6FfASRzJ40fKKoS4+5Ts26wpOQnoOu26Bziw/KaUGFT30T3vF++mMlkZPzQAEGmuZ2RFtg== X-Received: by 2002:a05:6000:4612:b0:386:3918:16a8 with SMTP id ffacd0b85a97d-388e4d5e0f3mr4198488f8f.13.1734567576298; Wed, 18 Dec 2024 16:19:36 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c8acb17sm138441f8f.97.2024.12.18.16.19.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Dec 2024 16:19:35 -0800 (PST) Date: Thu, 19 Dec 2024 01:19:33 +0100 From: Stefano Brivio To: Enrique Llorente Pastora Subject: Re: [PATCH v4] dhcp, dhcpv6: Add hostname and client fqdn ops Message-ID: <20241219011933.071cd876@elisabeth> In-Reply-To: References: <20241217065028.23718-1-ellorent@redhat.com> <20241218003454.4a4cbc8b@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: FcTM16RIgGt4GKZlAdHd60A6yZPK_qGvhWeMX8UecWQ_1734567579 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: VZUZ44HEKUMCUBJSE2NWVON5QLMW3A6Q X-Message-ID-Hash: VZUZ44HEKUMCUBJSE2NWVON5QLMW3A6Q X-MailFrom: sbrivio@redhat.com 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: On Wed, 18 Dec 2024 12:57:24 +0100 Enrique Llorente Pastora wrote: > On Wed, Dec 18, 2024 at 12:35=E2=80=AFAM Stefano Brivio wrote: > > > > The truncation logic looks good to me (as a first step). Some comments > > below: > > > > On Tue, 17 Dec 2024 07:50:28 +0100 > > Enrique Llorente wrote: > > =20 > > > Both DHCPv4 and DHCPv6 has the capability to pass the hostname to > > > clients, the DHCPv4 uses option 12 (hostname) while the DHCPv6 uses o= ption 39 > > > (client fqdn), for some virt deployments like kubevirt is expected to > > > have the VirtualMachine name as the guest hostname. > > > > > > This change add the following arguments: > > > - -H --hostname NAME to configure the hostname DHCPv4 option(12) > > > - --fqdn NAME to configure client fqdn option for both DHCPv4(81) an= d > > > DHCPv6(39) > > > > > > Signed-off-by: Enrique Llorente > > > --- > > > conf.c | 23 ++++++++++++++-- > > > dhcp.c | 39 +++++++++++++++++++++++--- > > > dhcpv6.c | 72 +++++++++++++++++++++++++++++++++++++++-------= -- > > > passt.1 | 11 ++++++++ > > > passt.h | 6 ++++ > > > pasta.c | 15 ++++++++-- > > > test/lib/setup | 10 +++---- > > > test/passt.mbuto | 6 ++-- > > > test/passt/dhcp | 15 +++++++++- > > > util.c | 22 +++++++++++++++ > > > util.h | 3 ++ > > > 11 files changed, 191 insertions(+), 31 deletions(-) > > > > > > diff --git a/conf.c b/conf.c > > > index df2b016..29e4bed 100644 > > > --- a/conf.c > > > +++ b/conf.c > > > @@ -854,7 +854,9 @@ static void usage(const char *name, FILE *f, int = status) > > > FPRINTF(f, " default: use addresses from /etc/resolv= .conf\n"); > > > FPRINTF(f, > > > " -S, --search LIST Space-separated list, search do= mains\n" > > > - " a single, empty option disables the DNS search lis= t\n"); > > > + " a single, empty option disables the DNS search lis= t\n" > > > + " -H, --hostname NAME Hostname to configure client wi= th\n" > > > + " --fqdn NAME FQDN to configure client with\n= "); > > > if (strstr(name, "pasta")) > > > FPRINTF(f, " default: don't use any search list\n"); > > > else > > > @@ -1313,6 +1315,7 @@ void conf(struct ctx *c, int argc, char **argv) > > > {"outbound", required_argument, NULL, = 'o' }, > > > {"dns", required_argument, NULL, = 'D' }, > > > {"search", required_argument, NULL, = 'S' }, > > > + {"hostname", required_argument, NULL, = 'H' }, > > > {"no-tcp", no_argument, &c->no_tcp, = 1 }, > > > {"no-udp", no_argument, &c->no_udp, = 1 }, > > > {"no-icmp", no_argument, &c->no_icmp, = 1 }, > > > @@ -1357,6 +1360,7 @@ void conf(struct ctx *c, int argc, char **argv) > > > /* vhost-user backend program convention */ > > > {"print-capabilities", no_argument, NULL, = 26 }, > > > {"socket-path", required_argument, NULL, = 's' }, > > > + {"fqdn", required_argument, NULL, = 27 }, > > > { 0 }, > > > }; > > > const char *logname =3D (c->mode =3D=3D MODE_PASTA) ? "pasta" := "passt"; > > > @@ -1379,9 +1383,9 @@ void conf(struct ctx *c, int argc, char **argv) > > > if (c->mode =3D=3D MODE_PASTA) { > > > c->no_dhcp_dns =3D c->no_dhcp_dns_search =3D 1; > > > fwd_default =3D FWD_AUTO; > > > - optstring =3D "+dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:46t:u:= T:U:"; > > > + optstring =3D "+dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:H:46t:= u:T:U:"; > > > } else { > > > - optstring =3D "+dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:461t:u= :"; > > > + optstring =3D "+dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:H:461t= :u:"; > > > } > > > > > > c->tcp.fwd_in.mode =3D c->tcp.fwd_out.mode =3D FWD_UNSET; > > > @@ -1558,6 +1562,12 @@ void conf(struct ctx *c, int argc, char **argv= ) > > > case 26: > > > vu_print_capabilities(); > > > break; > > > + case 27: > > > + if (snprintf_check(c->fqdn, PASST_MAXDNAME, > > > + "%s", optarg)) { =20 > > > > Nit: arguments should be visually aligned to the function you're > > calling, that is, here: > > > > if (snprintf_check(c->fqdn, PASST_MAXDNAME, > > "%s", optarg)) { =20 >=20 > Done. >=20 > > > > =20 > > > + die("Invalid fqdn: %s", optarg); =20 > > > > FQDN =20 >=20 > Done. >=20 > > =20 > > > + } > > > + break; > > > case 'd': > > > c->debug =3D 1; > > > c->quiet =3D 0; > > > @@ -1727,6 +1737,12 @@ void conf(struct ctx *c, int argc, char **argv= ) > > > > > > die("Cannot use DNS search domain %s", optarg); > > > break; > > > + case 'H': > > > + if (snprintf_check(c->hostname, PASST_MAXDNAME, > > > + "%s", optarg)) { =20 > > > > Same as above. > > =20 >=20 > Done. >=20 > > > + die("Invalid hostname: %s", optarg); > > > + } > > > + break; > > > case '4': > > > v4_only =3D true; > > > v6_only =3D false; > > > @@ -1741,6 +1757,7 @@ void conf(struct ctx *c, int argc, char **argv) > > > > > > c->one_off =3D true; > > > break; > > > + =20 > > > > Spurious change. =20 >=20 > Done. >=20 > > =20 > > > case 't': > > > case 'u': > > > case 'T': > > > diff --git a/dhcp.c b/dhcp.c > > > index d8515aa..a04462b 100644 > > > --- a/dhcp.c > > > +++ b/dhcp.c > > > @@ -62,6 +62,7 @@ static struct opt opts[255]; > > > #define DHCPFORCERENEW 9 > > > > > > #define OPT_MIN 60 /* RFC 951 */ > > > +#define OPT_MAX 302 /* UDP packet limit - dhcp headers = */ =20 > > > > The original value for o[] below, 308, is 576 (see RFC 2131, page 10) > > minus 268 (offset of options within an *IP* datagram). > > > > How did you calculate 302 (+2)? I can't find out. >=20 > My bad it's 306, I found that we are adding a pair of extra bytes > after the options so > that's why I am using 306 instead of 308 for checkings: >=20 > m->o[offset++] =3D 255; > m->o[offset++] =3D 0; Right, that's option 255 ("End") and its length (0). Can we have a comment here actually explaining that? Say: =09=09=09=09=09/* 576 (RFC 2131), minus offset =09=09=09=09=09 * of options (268), minus end =09=09=09=09=09 * option and its length (2) =09=09=09=09=09 */ > > By the way, this reminds me that, as I mentioned previously, we should > > really stop recycling the memory from the original message and simply > > build a new one instead. It's not so much about security (a DHCP client > > can anyway wreck the networking of its guest at will), but about > > robustness. This becomes more relevant as we fill up replies more and > > more. >=20 > Do we do this at this change or at a follow up one ? It's a logically separated change, so it can/should be a different patch but I think that we really risk breaking things, so I would only push them together. It can be before or after this change. It should be kind of simple: instead of filling up (and sending) 'm', we should declare and initialise a separate struct msg, say: =09struct msg r =3D { 0 }; and use that. We'll need to copy a few fields from the request ('htype', 'hlen', 'xid', 'flags', 'giaddr', 'chaddr', see page 28 of RFC 2131), but that's about it. If you're lost I can also submit a patch on top of this one later, let me know. > > > /** > > > * dhcp_init() - Initialise DHCP options > > > @@ -122,7 +123,7 @@ struct msg { > > > uint8_t sname[64]; > > > uint8_t file[128]; > > > uint32_t magic; > > > - uint8_t o[308]; > > > + uint8_t o[OPT_MAX+2]; =20 > > > > Spaces around operator for consistency (OPT_MAX + 2). Should it really > > be + 2, though? =20 >=20 > Yes, "fill(..)" function is adding a pair of extra ones at the end so > we should limit the option to make space for them. >=20 > m->o[offset++] =3D 255; > m->o[offset++] =3D 0; Then we should make it clear: OPT_MAX + 2 /* End option and length */ > > > } __attribute__((__packed__)); > > > > > > /** > > > @@ -131,14 +132,28 @@ struct msg { > > > * @o: Option number > > > * @offset: Current offset within options field, updated on inserti= on > > > */ > > > -static void fill_one(struct msg *m, int o, int *offset) > > > +static int fill_one(struct msg *m, int o, int *offset) =20 > > > > You should update the function comment to say what it returns, now that > > it returns something (try: 'grep Return: *.c'). > > =20 >=20 > Done. >=20 > > > { > > > + size_t idx, slen =3D 0; > > > + > > > + // We cannot write even enum + len + one byte then just skip = =20 > > > > For consistency, no "C++ style" comments. > > > > I would make it clear it's a conditional: "If we can't ..., then ..." > > =20 >=20 > Done >=20 > > > + if (*offset + 2 > OPT_MAX) { > > > + return OPT_MAX; > > > + } =20 > > > > No need for curly brackets here. =20 >=20 > Done. >=20 > > =20 > > > m->o[*offset] =3D o; > > > m->o[*offset + 1] =3D opts[o].slen; > > > - memcpy(&m->o[*offset + 2], opts[o].s, opts[o].slen); > > > + idx=3D*offset + 2; > > > + slen=3Dopts[o].slen; =20 > > > > Spaces around assignment operator (idx =3D *offset ...). =20 >=20 > Done. >=20 > > =20 > > > + > > > + // Truncate if it goes beyond OPT_MAX =20 > > > > No C++ style comments. =20 >=20 > Done. >=20 > > =20 > > > + if (idx + slen > OPT_MAX) { > > > + slen =3D OPT_MAX - idx; > > > + } =20 > > > > No need for curly brackets. =20 >=20 > Done. >=20 > > =20 > > > + memcpy(&m->o[*offset + 2], opts[o].s, slen); > > > > > > opts[o].sent =3D 1; > > > *offset +=3D 2 + opts[o].slen; > > > + return *offset; > > > } > > > > > > /** > > > @@ -172,7 +187,9 @@ static int fill(struct msg *m) > > > > > > for (o =3D 0; o < 255; o++) { > > > if (opts[o].slen !=3D -1 && !opts[o].sent) > > > - fill_one(m, o, &offset); > > > + if (fill_one(m, o, &offset) =3D=3D OPT_MAX) { > > > + break; > > > + } =20 > > > > No need for curly brackets. We should log a debug message, here (a > > warning would be more appropriate but I'm a bit wary of a malicious DHC= P > > client exploiting this to make log files blow up). Something like: > > > > debug("DHCP: truncating after option %i= ", o); > > =20 >=20 > Done. >=20 > > > } > > > > > > m->o[offset++] =3D 255; > > > @@ -398,6 +415,20 @@ int dhcp(const struct ctx *c, const struct pool = *p) > > > if (!opts[6].slen) > > > opts[6].slen =3D -1; > > > > > > + size_t hostname_len =3D strlen(c->hostname); =20 > > > > No mixed declarations and code (move the hostname_len declaration at > > the beginning of the function). =20 >=20 > Done. >=20 > > =20 > > > + if (hostname_len > 0) { > > > + opts[12].slen =3D hostname_len; > > > + memcpy(opts[12].s, &c->hostname, hostname_len); > > > + } > > > + > > > + size_t fqdn_len =3D strlen(c->fqdn); =20 > > > > Same as above. =20 >=20 > Done. >=20 > > =20 > > > + if (fqdn_len > 0) { > > > + opts[81].s[0] =3D 0x4; // flags (E) =20 > > > > No C++ style comments. =20 >=20 > Done. >=20 > > =20 > > > + size_t encoded_fqdn_len =3D encode_domain_name(c->fqdn,= fqdn_len, > > > + (char *) opts[8= 1].s + 3); =20 > > > > Don't mix declarations and code. I think that 'encoded_len' would be > > clear enough here. =20 >=20 > Done. >=20 > > =20 > > > + opts[81].slen =3D encoded_fqdn_len + 3; > > > + } > > > + > > > if (!c->no_dhcp_dns_search) > > > opt_set_dns_search(c, sizeof(m->o)); > > > > > > diff --git a/dhcpv6.c b/dhcpv6.c > > > index 0523bba..b61a41d 100644 > > > --- a/dhcpv6.c > > > +++ b/dhcpv6.c > > > @@ -48,6 +48,7 @@ struct opt_hdr { > > > # define STATUS_NOTONLINK htons_constant(4) > > > # define OPT_DNS_SERVERS htons_constant(23) > > > # define OPT_DNS_SEARCH htons_constant(24) > > > +# define OPT_CLIENT_FQDN htons_constant(39) > > > #define STR_NOTONLINK "Prefix not appropriate for lin= k." > > > > > > uint16_t l; > > > @@ -163,6 +164,18 @@ struct opt_dns_search { > > > char list[MAXDNSRCH * NS_MAXDNAME]; > > > } __attribute__((packed)); > > > > > > +/** > > > + * struct opt_client_fqdn - Client FQDN option (RFC 4704) > > > + * @hdr: Option header > > > + * @flags: Flags as stated at RFC 4704 (always zero for us= ) =20 > > > > "described by RFC 4704" (see also comments to v3). > > =20 >=20 > Done. >=20 > > > + * @domain_name: Client FQDN > > > + */ > > > +struct opt_client_fqdn{ > > > + struct opt_hdr hdr; > > > + uint8_t flags; > > > + char domain_name[PASST_MAXDNAME]; > > > +} __attribute__((packed)); > > > + > > > /** > > > * struct msg_hdr - DHCPv6 client/server message header > > > * @type: DHCP message type > > > @@ -193,6 +206,7 @@ struct msg_hdr { > > > * @client_id: Client Identifier, variable length > > > * @dns_servers: DNS Recursive Name Server, here just for storag= e size > > > * @dns_search: Domain Search List, here just for stora= ge size > > > + * @client_fqdn: Client FQDN, variable length > > > */ > > > static struct resp_t { > > > struct msg_hdr hdr; > > > @@ -203,6 +217,7 @@ static struct resp_t { > > > struct opt_client_id client_id; > > > struct opt_dns_servers dns_servers; > > > struct opt_dns_search dns_search; > > > + struct opt_client_fqdn client_fqdn; > > > } __attribute__((__packed__)) resp =3D { > > > { 0 }, > > > SERVER_ID, > > > @@ -228,6 +243,10 @@ static struct resp_t { > > > { { OPT_DNS_SEARCH, 0, }, > > > { 0 }, > > > }, > > > + > > > + { { OPT_CLIENT_FQDN, 0, }, > > > + 0, { 0 }, > > > + }, > > > }; > > > > > > static const struct opt_status_code sc_not_on_link =3D { > > > @@ -346,7 +365,6 @@ static size_t dhcpv6_dns_fill(const struct ctx *c= , char *buf, int offset) > > > { > > > struct opt_dns_servers *srv =3D NULL; > > > struct opt_dns_search *srch =3D NULL; > > > - char *p =3D NULL; > > > int i; > > > > > > if (c->no_dhcp_dns) > > > @@ -388,29 +406,56 @@ search: > > > offset +=3D sizeof(struct opt_hdr); > > > srch->hdr.t =3D OPT_DNS_SEARCH; > > > srch->hdr.l =3D 0; > > > - p =3D srch->list; > > > } > > > > > > - *p =3D '.'; > > > - p =3D stpncpy(p + 1, c->dns_search[i].n, name_len); > > > - p++; > > > - srch->hdr.l +=3D name_len + 2; > > > - offset +=3D name_len + 2; > > > + size_t encoded_name_len =3D encode_domain_name(c->dns_s= earch[i].n, name_len, srch->list); =20 > > > > No mixed declarations and code. Wrap at 80 columns if possible. =20 >=20 > Done. >=20 > > =20 > > > + srch->hdr.l +=3D encoded_name_len; > > > + offset +=3D encoded_name_len; > > > } > > > > > > if (srch) { > > > - for (i =3D 0; i < srch->hdr.l; i++) { > > > - if (srch->list[i] =3D=3D '.') { > > > - srch->list[i] =3D strcspn(srch->list + = i + 1, > > > - "."); > > > - } > > > - } > > > srch->hdr.l =3D htons(srch->hdr.l); =20 > > > > Hah, thanks, that looks much better now. > > =20 > > > } > > > > > > return offset; > > > } > > > > > > +/** > > > + * dhcpv6_client_fqdn_fill() - Fill in client FQDN option > > > + * @c: Execution context > > > + * @buf: Response message buffer where options will be appended > > > + * @offset: Offset in message buffer for new options > > > + * > > > + * Return: updated length of response message buffer. > > > + */ > > > +static size_t dhcpv6_client_fqdn_fill(const struct ctx *c, char *buf= , int offset) > > > +{ > > > + size_t fqdn_len, opt_hdr_len =3D 0; > > > + > > > + fqdn_len =3D strlen(c->fqdn); > > > + opt_hdr_len =3D sizeof(struct opt_hdr); > > > + > > > + if (offset + opt_hdr_len + fqdn_len + 1 > 508) { =20 > > > > I'm a bit lost: why 508? Would it be possible to have this as a define > > using sizeof() on the relevant structs (struct udphdr, struct ipv6hdr)? >=20 > That's the max UDP packet payload size, since "offset" is referring to > the payload not the whole UDP > maybe we can do we can do max_len =3D 576 - (sizeof(struct ipv6hdr) + > sizeof(udphdr)), don't know if it's worth it. Well, that would be 528... so I think it's worth it. :) But in any case, this is not IPv4. From RFC 8415: The server must be aware of the recommendations on packet sizes and the use of fragmentation as discussed in Section 5 of [RFC8200]. ...so we should start from 1280 (IPV6_MIN_MTU) bytes here, subtract the size of an IPv6 header (40), of a UDP header (8), and of the message header (sizeof(struct msg_hdr), 4). I think we should really use defined constants and sizeof() otherwise it gets confusing quite fast. > " * @dlen: UDP payload length (not including UDP header)" I guess we could have it as a define (OPT_MAX_SIZE?), no need to have it in any struct. By the way, note that we have MAX() defined in util.h, so you don't need a condition and a separate statement, you can just do: > > > + fqdn_len =3D 508 - (offset + opt_hdr_len + 1); =09fqdn_len =3D MAX(fqdn_len, offset + ...); --=20 Stefano