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=QjJgImbR; 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 7FCBC5A004E for ; Mon, 18 Nov 2024 13:20:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1731932418; 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=1gh0qQuR3fkQ6ROapMFWu51AYviGahKOAJ4jRDk9abw=; b=QjJgImbRlkR8mw1LofoV5a1R4XDsveYe/YJkyCBNMMT3is+EzaWEYzWgfHPDy5M74mC73D IlH090IRBLbgjkJlxkfB839ptQxmOcdGCvzfEII2feai52pUwTKr+KaasedpzwjJ8BLrih 5JI4TvlFg0HvudviE8bk2CtVuOo3ESg= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-653-o_xrX33iNVCUgAhChH2ffw-1; Mon, 18 Nov 2024 07:20:17 -0500 X-MC-Unique: o_xrX33iNVCUgAhChH2ffw-1 X-Mimecast-MFC-AGG-ID: o_xrX33iNVCUgAhChH2ffw Received: by mail-ej1-f71.google.com with SMTP id a640c23a62f3a-a9adb271de7so378045866b.0 for ; Mon, 18 Nov 2024 04:20:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731932416; x=1732537216; 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=1gh0qQuR3fkQ6ROapMFWu51AYviGahKOAJ4jRDk9abw=; b=rkhvbUU5e+qXHAZ3UmxVU1ymbojGFUnU1opnFoIwOJeOTAQ8poTrL+TX3PkoSsyP/1 TkpQbDKVFs2kwj+QmkPxSEqhC2WVT6xE3FsLFanpxY8Rw9TlMe2rGCb2iysVAyu49wJ4 CYrHFu4gNWO8sUC/YZbAl/CPLC5qhsXAAiD91RFynh9lpUU4Waw2Y7RU0VACX6S1wluv t1XXJt+Mh5EFPUHxpTWwZuTGOlRev5fJqnxZhWE+SmerzZT8bmPKxzocfBdql8ot5QFr ItzjE179CsL4P3WPuEigxcY4FG1LVpeSLRqYIeDaK0dYrP2ix+7erjh4CJU/1AVJmtYk dVgQ== X-Gm-Message-State: AOJu0YwEAWydZVTXC803YDut8LEWxNiX4Q3XG8uXvvNaS/ylyplrizzv yN28af6HSFFJdEv/Hct21MLQREa4JKWVoVwXBxpW9Wxa3vcoXyNrEDtVBRq9NMnnUWg5ChxyCoA HZeAy29Msd5t/cRWch92RnzuBJfGl7IcM8LuZqwICv2sshMLMqoxPJ7wNHMuwY7YGi7Ud+kHl/f 9esF3kserrNj9hBpaPmukwpP8h X-Received: by 2002:a17:907:7f1e:b0:a99:4615:f58c with SMTP id a640c23a62f3a-aa4833f6697mr1142795866b.2.1731932415641; Mon, 18 Nov 2024 04:20:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IF0SlTifVUJfsRCtOA0rcGBXmDuOjuQP4xAoGIXTNRgN8z+IWk/VcLI/tocQqktHewYTTqeO5riv39KdAZPRuw= X-Received: by 2002:a17:907:7f1e:b0:a99:4615:f58c with SMTP id a640c23a62f3a-aa4833f6697mr1142793366b.2.1731932415069; Mon, 18 Nov 2024 04:20:15 -0800 (PST) MIME-Version: 1.0 References: <20241114084727.37263-1-ellorent@redhat.com> <20241114181323.14fd3d36@elisabeth> <20241115122857.7bf2b8e2@elisabeth> In-Reply-To: <20241115122857.7bf2b8e2@elisabeth> From: Enrique Llorente Pastora Date: Mon, 18 Nov 2024 13:20:03 +0100 Message-ID: Subject: Re: [PATCH v2] dhcp, dhcpv6: Add hostname and client fqdn ops To: Stefano Brivio X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: hZsZT8o8fnJS81OpaqOMfAs0UK9knJ5KXoYvc18jMkg_1731932416 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: JDAO72UBR6TQNR62LRS2YASLG7JHGLCN X-Message-ID-Hash: JDAO72UBR6TQNR62LRS2YASLG7JHGLCN 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 Fri, Nov 15, 2024 at 12:29=E2=80=AFPM Stefano Brivio wrote: > > On Fri, 15 Nov 2024 10:25:33 +0100 > Enrique Llorente Pastora wrote: > > > On Thu, Nov 14, 2024 at 6:13=E2=80=AFPM Stefano Brivio wrote: > > > > > > On Thu, 14 Nov 2024 09:47:27 +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 -H --hostname to configure the DHCPv4 and DHCPv= 6 > > > > options to will send hostname to clients. > > > > > > > > Signed-off-by: Enrique Llorente > > > > --- > > > > conf.c | 13 ++++++++++--- > > > > dhcp.c | 8 +++++++- > > > > dhcpv6.c | 44 +++++++++++++++++++++++++++++++++++++++++++- > > > > passt.h | 2 ++ > > > > test/lib/setup | 10 +++++----- > > > > test/passt.mbuto | 1 + > > > > test/passt/dhcp | 11 ++++++++++- > > > > 7 files changed, 78 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/conf.c b/conf.c > > > > index 14411b4..ddb585c 100644 > > > > --- a/conf.c > > > > +++ b/conf.c > > > > @@ -847,7 +847,8 @@ static void usage(const char *name, FILE *f, in= t status) > > > > " --freebind Bind to any address for forwa= rding\n" > > > > " --no-map-gw Don't map gateway address to = host\n" > > > > " -4, --ipv4-only Enable IPv4 operation only\n" > > > > - " -6, --ipv6-only Enable IPv6 operation only\n"= ); > > > > + " -6, --ipv6-only Enable IPv6 operation only\n" > > > > + " -H, --hostname NAME Hostname to configure client w= ith\n"); > > > > > > Use one tab instead of one space between "NAME" and "Hostname" so tha= t > > > the output (and the code itself) is aligned. > > > > > > > > > > > if (strstr(name, "pasta")) > > > > goto pasta_opts; > > > > @@ -1303,6 +1304,7 @@ void conf(struct ctx *c, int argc, char **arg= v) > > > > {"map-guest-addr", required_argument, NULL, = 22 }, > > > > {"host-lo-to-ns-lo", no_argument, NULL, = 23 }, > > > > {"dns-host", required_argument, NULL, = 24 }, > > > > + {"hostname", required_argument, NULL, = 'H' }, > > > > > > Maybe this could go a bit more up, just after the "dns" and "search" > > > options. Two reasons: we have all the options without a short version > > > at the end, and this logically belongs together with other name-relat= ed > > > stuff we send to the guest. > > > > > > > { 0 }, > > > > }; > > > > const char *logname =3D (c->mode =3D=3D MODE_PASTA) ? "pasta"= : "passt"; > > > > @@ -1325,9 +1327,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:46t:= u:T:U:H:"; > > > > > > Same here: maybe after S: it makes more sense. > > > > > > > } 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:461t= :u:H:"; > > > > } > > > > > > > > c->tcp.fwd_in.mode =3D c->tcp.fwd_out.mode =3D FWD_UNSET; > > > > @@ -1680,6 +1682,11 @@ void conf(struct ctx *c, int argc, char **ar= gv) > > > > > > > > c->one_off =3D true; > > > > break; > > > > + case 'H': > > > > + ret =3D snprintf(c->hostname.n, sizeof(c->hos= tname.n), "%s", optarg); > > > > > > We (generally) wrap lines at 80 columns where doable: > > > > > > ret =3D snprintf(c->hostname.n, sizeof(c->hos= tname.n), > > > "%s", optarg); > > > > > > > + if (ret <=3D 0 || ret >=3D (int)sizeof(c->hos= tname.n)) > > > > + die("Invalid hostname: %s", optarg); > > > > + break; > > > > case 't': > > > > case 'u': > > > > case 'T': > > > > diff --git a/dhcp.c b/dhcp.c > > > > index a06f143..ec7e78a 100644 > > > > --- a/dhcp.c > > > > +++ b/dhcp.c > > > > @@ -275,7 +275,7 @@ static void opt_set_dns_search(const struct ctx= *c, size_t max_len) > > > > */ > > > > int dhcp(const struct ctx *c, const struct pool *p) > > > > { > > > > - size_t mlen, dlen, offset =3D 0, opt_len, opt_off =3D 0; > > > > + size_t mlen, dlen, offset =3D 0, opt_len, opt_off =3D 0, host= name_len; > > > > char macstr[ETH_ADDRSTRLEN]; > > > > const struct ethhdr *eh; > > > > const struct iphdr *iph; > > > > @@ -375,6 +375,12 @@ int dhcp(const struct ctx *c, const struct poo= l *p) > > > > opts[6].slen +=3D sizeof(uint32_t); > > > > } > > > > > > > > + hostname_len =3D strlen(c->hostname.n); > > > > + if ( hostname_len > 0 ) { > > > > > > No space around parentheses in expressions: > > > > > > if (hostname_len > 0) { > > > > > > > + opts[12].slen =3D hostname_len; > > > > + memcpy(opts[12].s, &c->hostname.n, hostname_len); > > > > + } > > > > + > > > > if (!c->no_dhcp_dns_search) > > > > opt_set_dns_search(c, sizeof(m->o)); > > > > > > > > diff --git a/dhcpv6.c b/dhcpv6.c > > > > index 14a5c7e..190dc0e 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) > > > > > > One tab instead of spaces. > > > > > > > #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 > > > > > > s/stated at/described by/ > > > > > > Maybe mention that we don't really use those ("always zero for us"). > > > > > > > + * @hostname: Client fqdn > > > > > > A hostname is not necessarily a fully-qualified domain name. The RFC > > > calls this "domain name", so we could at least call this 'domain' in > > > the struct to make it clear we're referring to that. > > > > > > As to what it should contain: RFC 4702 defines option 81 (FQDN) for > > > DHCP, which is equivalent to DHCPv6's option 39 (RFC 4704). Should we > > > just call this whole thing "FQDN" and switch to option 81 for DHCP > > > instead? Would this work for KubeVirt? > > > > > > Or introduce two options: -H / --hostname (option 12, DHCP only), and > > > --fqdn (setting both DHCP option 81 and DHCPv6 option 39)? > > > > Let me experiment with DHCPv4 option 81 with kubevirt machines, if it's= enough > > we use 81 instead of 12 and call it --fqdn. > > > > If not we go with --fqdn and --hostname, but in this case it's not > > clear to me what kubevirt/libvirt > > has to pass though, also is this going to be expose at the domain xml > > or libvirt will directly pass always > > the vm name as one of them ? (for dual stack or single stack ipv4 > > hostname is enough, problem is single stack ipv6) > > Note that they are (of course) different things, and neither RFC 4702 > nor RFC 4703 make any mention whatsoever of hostname and option 12. > > That is, you could legitimately pass x as hostname and y.tld as FQDN > (which gives 'y' as hostname... but RFCs don't specify which one wins). > > I think for libvirt we should propose to add one or two separate > attributes ("hostname" / "fqdn"), and not imply them from the name of > the machine or "domain" (the guest "domain", nothing to do with network > domain), because that's not necessarily what users want. > > About option 81 itself: it's not just a matter of changing the number, > unfortunately. You would need to set the 'E' flag. We have an > implementation of name compression according to RFC 1035, 4.1.4 in > opt_set_dns_search(), but RFC 4702 says we shouldn't use it. > > No other flags set, as those are for clients. > >From some experiments I have do with networkmanager at a fedora 41, the DHCPv4 client fqdn is only used for clients to state the fqdn but never for client to configure it's own hostname But the DHCPv6 client does update the client's hostname with single stack ipv6, so I think we should do the following: - Have only one argument -H/--hostname - Delegate the config to libvirt - For IPv4 Options(12) with it - For IPv6 Option(39) with flags S and O <- so we can totally ignore the cl= ient - Document this and if someone is not happy, don't use that option. Also for DHCPv4 we have also the option (15) for domain names, so maybe if we detect that -H has a domain name we also add option(15). > But there's another aspect, RFC 4702 section 2: > > Only one Client FQDN option MAY appear in a message, though it may be > instantiated in a message as multiple options [9]. 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. > > ...and we don't support concatenation as described by RFC 3396. The > messy part of it would be option overload (RFC 2131), which we don't do > at all. But RFC 3396 doesn't make it mandatory. > > The question is whether we would practically need it though: can we, > with option 81, reasonably end up with UDP packets that are too large, > so we need to resort to option overload? > > I mean, it's nothing crazy to add, it's just a bit of pointer trickery. > > As a first step, *if* option 81 helps, I would just stick to what's > mandatory (including option concatenation in the "options" section, > that is, n parts of options where options < n have 255 as length). > > Maybe, even before that, we could simply accept FQDNs up to 255 (253, > even) bytes. > > As a second step we could look into option overload. > > -- > Stefano > --=20 Quique Llorente CNV networking Senior Software Engineer Red Hat EMEA ellorent@redhat.com @RedHat Red Hat Red Hat