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=gu66XweL; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id E34ED5A0274 for ; Thu, 19 Dec 2024 13:35:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734611642; 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=GgzPAhNCuzkU6Ne+eIy6zM5XRrG6BY0VW78AiWacRNc=; b=gu66XweLCVuYLkiVEwPjx2dDSbTxM4rCxiDE/oSu3dcufMHbH9KbRSboS2AP/KcGs2qpS9 ixeLU8IA/iZYhIaaV50EMxxLKKk238J9vgr+9yPHPGMQLa/JA9uPxyWUSWvQDowdaFuSGa 7F5PjdDEHL3dHlq79FTy5rQnDjAuiIg= Received: from mail-ej1-f69.google.com (mail-ej1-f69.google.com [209.85.218.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-227-JetFYHPUPTeM-5UdQ7TjbA-1; Thu, 19 Dec 2024 06:51:41 -0500 X-MC-Unique: JetFYHPUPTeM-5UdQ7TjbA-1 X-Mimecast-MFC-AGG-ID: JetFYHPUPTeM-5UdQ7TjbA Received: by mail-ej1-f69.google.com with SMTP id a640c23a62f3a-aa67fcbb549so70021466b.0 for ; Thu, 19 Dec 2024 03:51:40 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734609100; x=1735213900; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=GgzPAhNCuzkU6Ne+eIy6zM5XRrG6BY0VW78AiWacRNc=; b=cXk/Q48fVXWYWfpfK8PdKxtefXg+Ym4QEVPZrppYIcVMZXWKwfJ5gIGb6sBgQXX6/+ BlAwVnIL9tJXSFowc3ZLSxzScGIb3hZnHYPXFzWWNmqrLMMvwsi0m3avC5Pm9a+EXQOS VfSayTNKz3gcEAOeYS8PlHN5PcZjOuVWECzDtBH+SyjuHJrHSaFaepaTBYPc44z/q0by 7tJOYTfJmRFm2G0cPuONeijww3iKfyoHd44Scixljeau62EVurBedaMLX/1QmqbLroPT WLn+K8EbNzBioFZpv4jUZC2hb7oZkldWwIW6IBT9qieUcVlxq2MkRI9asbVxCESzcd6v jr0w== X-Gm-Message-State: AOJu0YwF3X4uFGbMfsJjR8ZbPxdrssVukwXBB5z0M+HtaBKKXAuD7re4 Xm/YOFkF56webj9zSCg7bFEtgbK5pwHS+j4TsGoVmwcPA6A3J8PoGhTLhKNPLTDBVn4hKabsHMV 7k/JZ2uYHY1R8g3SWxL6HOS2AELmVo+edqWeuIqoV5Re0hVgzed/wpE4si5wi9rmQQJ09qfKp6B Y/DdtNJ2B8oqJzwgKSUeUhD3zG X-Gm-Gg: ASbGnctGarBDulfr6vDTVIRSNoeM9HS8TzvAHq/xz9m/roTQtbkOLWo260McNblvM0t 0PrjO0soley/Pw2NUTqeQlmoozhfrSFEYDy4TLhOPmy8wjd2mymtG6i2Is4VFEjdfjvf2D7uz X-Received: by 2002:a17:907:742:b0:aa6:a966:da12 with SMTP id a640c23a62f3a-aabf47a0c3dmr626452466b.29.1734609099711; Thu, 19 Dec 2024 03:51:39 -0800 (PST) X-Google-Smtp-Source: AGHT+IGFrT21y2QHewENnbUxHN2fUHqJSojwLKVBSgWu5WEg+z8zno1FA89nyCw9fDOXV95nNP8GOLXm/m74Cuu3SGU= X-Received: by 2002:a17:907:742:b0:aa6:a966:da12 with SMTP id a640c23a62f3a-aabf47a0c3dmr626448766b.29.1734609099036; Thu, 19 Dec 2024 03:51:39 -0800 (PST) MIME-Version: 1.0 References: <20241217065028.23718-1-ellorent@redhat.com> <20241218003454.4a4cbc8b@elisabeth> <20241219011933.071cd876@elisabeth> In-Reply-To: <20241219011933.071cd876@elisabeth> From: Enrique Llorente Pastora Date: Thu, 19 Dec 2024 12:51:27 +0100 Message-ID: Subject: Re: [PATCH v4] dhcp, dhcpv6: Add hostname and client fqdn ops To: Stefano Brivio X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: RMRbC1uqTex-uqzRbSJozE83v3_qktOc0gKGBHhJnHU_1734609100 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 5SSJWK3UPC2FJVYPYWQU5ASBZFUELKIO X-Message-ID-Hash: 5SSJWK3UPC2FJVYPYWQU5ASBZFUELKIO X-MailFrom: ellorent@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 Thu, Dec 19, 2024 at 1:19=E2=80=AFAM Stefano Brivio = wrote: > > 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 comment= s > > > below: > > > > > > On Tue, 17 Dec 2024 07:50:28 +0100 > > > Enrique Llorente wrote: > > > > > > > Both DHCPv4 and DHCPv6 has the capability to pass the hostname to > > > > clients, the DHCPv4 uses option 12 (hostname) while the DHCPv6 uses= option 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) = and > > > > 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, in= t status) > > > > FPRINTF(f, " default: use addresses from /etc/reso= lv.conf\n"); > > > > FPRINTF(f, > > > > " -S, --search LIST Space-separated list, search = domains\n" > > > > - " a single, empty option disables the DNS search l= ist\n"); > > > > + " a single, empty option disables the DNS search l= ist\n" > > > > + " -H, --hostname NAME Hostname to configure client = with\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 **arg= v) > > > > {"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 **arg= v) > > > > /* 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 **arg= v) > > > > 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:46= t: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:46= 1t: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 **ar= gv) > > > > case 26: > > > > vu_print_capabilities(); > > > > break; > > > > + case 27: > > > > + if (snprintf_check(c->fqdn, PASST_MAXDNAME, > > > > + "%s", optarg)) { > > > > > > Nit: arguments should be visually aligned to the function you're > > > calling, that is, here: > > > > > > if (snprintf_check(c->fqdn, PASST_MAXDNAME, > > > "%s", optarg)) { > > > > Done. > > > > > > > > > > > > + die("Invalid fqdn: %s", optarg); > > > > > > FQDN > > > > Done. > > > > > > > > > + } > > > > + break; > > > > case 'd': > > > > c->debug =3D 1; > > > > c->quiet =3D 0; > > > > @@ -1727,6 +1737,12 @@ void conf(struct ctx *c, int argc, char **ar= gv) > > > > > > > > die("Cannot use DNS search domain %s", optarg= ); > > > > break; > > > > + case 'H': > > > > + if (snprintf_check(c->hostname, PASST_MAXDNAM= E, > > > > + "%s", optarg)) { > > > > > > Same as above. > > > > > > > Done. > > > > > > + 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 **arg= v) > > > > > > > > c->one_off =3D true; > > > > break; > > > > + > > > > > > Spurious change. > > > > Done. > > > > > > > > > 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 header= s */ > > > > > > 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. > > > > 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: > > > > 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: > > /* 576 (RFC 2131), minus offset > * of options (268), minus end > * option and its length (2) > */ Sure, will add the comment. > > > > By the way, this reminds me that, as I mentioned previously, we shoul= d > > > 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 clie= nt > > > 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. > > > > 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: > > struct 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. I will do a new patch after this one to cover that. > > > > > /** > > > > * 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]; > > > > > > Spaces around operator for consistency (OPT_MAX + 2). Should it reall= y > > > be + 2, though? > > > > Yes, "fill(..)" function is adding a pair of extra ones at the end so > > we should limit the option to make space for them. > > > > m->o[offset++] =3D 255; > > m->o[offset++] =3D 0; > > Then we should make it clear: OPT_MAX + 2 /* End option and length */ > Make sense adding that too. > > > > } __attribute__((__packed__)); > > > > > > > > /** > > > > @@ -131,14 +132,28 @@ struct msg { > > > > * @o: Option number > > > > * @offset: Current offset within options field, updated on inser= tion > > > > */ > > > > -static void fill_one(struct msg *m, int o, int *offset) > > > > +static int fill_one(struct msg *m, int o, int *offset) > > > > > > You should update the function comment to say what it returns, now th= at > > > it returns something (try: 'grep Return: *.c'). > > > > > > > Done. > > > > > > { > > > > + size_t idx, slen =3D 0; > > > > + > > > > + // We cannot write even enum + len + one byte then just skip > > > > > > For consistency, no "C++ style" comments. > > > > > > I would make it clear it's a conditional: "If we can't ..., then ..." > > > > > > > Done > > > > > > + if (*offset + 2 > OPT_MAX) { > > > > + return OPT_MAX; > > > > + } > > > > > > No need for curly brackets here. > > > > Done. > > > > > > > > > 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; > > > > > > Spaces around assignment operator (idx =3D *offset ...). > > > > Done. > > > > > > > > > + > > > > + // Truncate if it goes beyond OPT_MAX > > > > > > No C++ style comments. > > > > Done. > > > > > > > > > + if (idx + slen > OPT_MAX) { > > > > + slen =3D OPT_MAX - idx; > > > > + } > > > > > > No need for curly brackets. > > > > Done. > > > > > > > > > + 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; > > > > + } > > > > > > 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 D= HCP > > > client exploiting this to make log files blow up). Something like: > > > > > > debug("DHCP: truncating after option = %i", o); > > > > > > > Done. > > > > > > } > > > > > > > > m->o[offset++] =3D 255; > > > > @@ -398,6 +415,20 @@ int dhcp(const struct ctx *c, const struct poo= l *p) > > > > if (!opts[6].slen) > > > > opts[6].slen =3D -1; > > > > > > > > + size_t hostname_len =3D strlen(c->hostname); > > > > > > No mixed declarations and code (move the hostname_len declaration at > > > the beginning of the function). > > > > Done. > > > > > > > > > + 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); > > > > > > Same as above. > > > > Done. > > > > > > > > > + if (fqdn_len > 0) { > > > > + opts[81].s[0] =3D 0x4; // flags (E) > > > > > > No C++ style comments. > > > > Done. > > > > > > > > > + size_t encoded_fqdn_len =3D encode_domain_name(c->fqd= n, fqdn_len, > > > > + (char *) opts= [81].s + 3); > > > > > > Don't mix declarations and code. I think that 'encoded_len' would be > > > clear enough here. > > > > Done. > > > > > > > > > + 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 l= ink." > > > > > > > > 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) > > > > > > "described by RFC 4704" (see also comments to v3). > > > > > > > Done. > > > > > > + * @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 stor= age size > > > > * @dns_search: Domain Search List, here just for sto= rage 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= _search[i].n, name_len, srch->list); > > > > > > No mixed declarations and code. Wrap at 80 columns if possible. > > > > Done. > > > > > > > > > + 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); > > > > > > Hah, thanks, that looks much better now. > > > > > > > } > > > > > > > > return offset; > > > > } > > > > > > > > +/** > > > > + * dhcpv6_client_fqdn_fill() - Fill in client FQDN option > > > > + * @c: Execution context > > > > + * @buf: Response message buffer where options will be appende= d > > > > + * @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 *b= uf, 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) { > > > > > > I'm a bit lost: why 508? Would it be possible to have this as a defin= e > > > using sizeof() on the relevant structs (struct udphdr, struct ipv6hdr= )? > > > > 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: I am going to create a new OPT_MAX_SIZE with the computation and also a new IPV6_MIN_MTU to be part of it and use that later on with MIN. > > > > > + fqdn_len =3D 508 - (offset + opt_hdr_len + 1); > > fqdn_len =3D MAX(fqdn_len, offset + ...); > > -- > Stefano > --=20 Quique Llorente CNV networking Senior Software Engineer Red Hat EMEA ellorent@redhat.com @RedHat Red Hat Red Hat