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=BV4wXVIF; 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 80B775A004E for ; Wed, 18 Dec 2024 12:57:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734523060; 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=ugU5QgZb40Y2yVZuENPEnz9XJRmlKku9cAd2RneG8A4=; b=BV4wXVIF9bSXBUat6WtnSVxuyl8apAapJGPy3hrI1+trvlZApAxcUvh0I4ZW6OZ2JKHkfB Y9EqQhQbkbkThc8pC7orx2ioaiib68Kz1Skcy8ZKmnyLu9vbWHvtJziIjT/s15en8GjIsl NzfGv3NpwOATNApZbDM4qce/qS63YUc= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-235-8pPR4XiYMIiNOVEuolPfIA-1; Wed, 18 Dec 2024 06:57:39 -0500 X-MC-Unique: 8pPR4XiYMIiNOVEuolPfIA-1 X-Mimecast-MFC-AGG-ID: 8pPR4XiYMIiNOVEuolPfIA Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-aa6704ffcaeso320835466b.2 for ; Wed, 18 Dec 2024 03:57:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734523057; x=1735127857; 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=ugU5QgZb40Y2yVZuENPEnz9XJRmlKku9cAd2RneG8A4=; b=KYw9sNcynNv+CjWXUNGgTN3UMJSU+2SeBnljMlNXf084tDorlpDi1ALs9foX297XPC //rfJh0hIn7yNr5U4M9wiH3Miv7ca4f0J4vTXwLa2hV2qALsGs5ljBVQDiMM7Xa18xUu h8LeMgLCzA5UK3sEThdULwfDt6+q6JH4NFYQeMdYjgnQy2MLCaNRlhf5wffLlOKO2ohW uYzJPty/Y8BxvcW1s8bUc6h5oOWn/deSHeVjYxR+xISIHftBY0raqT4c9dw5mEYG7NQt CH+/Pwqrb234ZGAJk42gt6hIvH4D7WNiZu2F0zWP5O3l3t3JO5mkLUdzTgIk+UVSkJwT T/vw== X-Gm-Message-State: AOJu0YwgJ0RfVj0JJWJ9DRaaUntBn7XU47p55gLCYFTNc1LiWiGFVNFK /CFHFLtruJUk1y4RfGg5zlTCAgE1VcB1QasJjmZiXSPD+vPIYd81WQHdrTtjCl+4ApdV6iZ9JlL 810YilY3fsfv5no95kJmcEQ8gUTqd94sNdOg1DfrVCAUCTArnu3N4rpDMePD95J1cmLsGMLGWSR 11Br3siqxk4WgcaCTIesHM2aXvA6jECLovgaS1sg== X-Gm-Gg: ASbGncukCSGWuG5OQWqyjNGwAKLo/fc8TnPR+9p+mQOFh0ucWpbZZRYMprbJlNDidjV 32bfoOenFOl0Xg5IdOUB4ixvYcTYBkqscq40VYU7+fY9jnuxBNNi7A6G6ep8EGbZNUk8vEVI= X-Received: by 2002:a17:907:2da6:b0:aa6:ac4c:8dc with SMTP id a640c23a62f3a-aabf476e7cemr223813066b.18.1734523056450; Wed, 18 Dec 2024 03:57:36 -0800 (PST) X-Google-Smtp-Source: AGHT+IFnMgXFucFI0dfZ8dbXT4eLrn57qkc78GFuI/ex6G4mbP79qmMVda+iaMV+M4JgbLUxVapooPkWK+RRkr5ZJDU= X-Received: by 2002:a17:907:2da6:b0:aa6:ac4c:8dc with SMTP id a640c23a62f3a-aabf476e7cemr223809066b.18.1734523055658; Wed, 18 Dec 2024 03:57:35 -0800 (PST) MIME-Version: 1.0 References: <20241217065028.23718-1-ellorent@redhat.com> <20241218003454.4a4cbc8b@elisabeth> In-Reply-To: <20241218003454.4a4cbc8b@elisabeth> From: Enrique Llorente Pastora Date: Wed, 18 Dec 2024 12:57:24 +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: rRvj3Rtho72MbPg-qeLfzggq5MiBraHLg0ZJBQtY1YA_1734523058 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 3FPT22J7FH3ZGVUZ2HPDW2X4ZAQUCPIW X-Message-ID-Hash: 3FPT22J7FH3ZGVUZ2HPDW2X4ZAQUCPIW 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 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: > > > Both DHCPv4 and DHCPv6 has the capability to pass the hostname to > > clients, the DHCPv4 uses option 12 (hostname) while the DHCPv6 uses opt= ion 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, int st= atus) > > FPRINTF(f, " default: use addresses from /etc/resolv.c= onf\n"); > > FPRINTF(f, > > " -S, --search LIST Space-separated list, search doma= ins\n" > > - " a single, empty option disables the DNS search list\= n"); > > + " a single, empty option disables the DNS search list\= 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 **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, 2= 6 }, > > {"socket-path", required_argument, NULL, '= s' }, > > + {"fqdn", required_argument, NULL, 2= 7 }, > > { 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)) { > > 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 **argv) > > > > die("Cannot use DNS search domain %s", optarg); > > break; > > + case 'H': > > + if (snprintf_check(c->hostname, PASST_MAXDNAME, > > + "%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 **argv) > > > > 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 headers */ > > 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; > 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. > Do we do this at this change or at a follow up one ? > > /** > > * 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 really > 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; > > > } __attribute__((__packed__)); > > > > /** > > @@ -131,14 +132,28 @@ struct msg { > > * @o: Option number > > * @offset: Current offset within options field, updated on insertion > > */ > > -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 that > 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 DHCP > 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 pool *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->fqdn, f= qdn_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 link.= " > > > > 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 storage = size > > * @dns_search: Domain Search List, here just for storage= 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_sea= rch[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 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) { > > 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)? > 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. " * @dlen: UDP payload length (not including UDP header)" > > + fqdn_len =3D 508 - (offset + opt_hdr_len + 1); > > + } > > + > > + if (fqdn_len =3D=3D 0) { > > + return offset; > > + } > > No need for curly brackets. Done. > > + > > + > > + struct opt_client_fqdn *o =3D (struct opt_client_fqdn *)(buf + of= fset); > > + size_t encoded_fqdn_len =3D encode_domain_name(c->fqdn, fqdn_len, > > + o->domain_name); > > + size_t opt_len =3D encoded_fqdn_len + 1; > > No mixed declarations and code. Done. > > > + > > + o->hdr.t =3D OPT_CLIENT_FQDN; > > + o->hdr.l =3D htons(opt_len); > > + o->flags =3D 0x00; > > + > > + return offset + opt_hdr_len + opt_len; > > +} > > + > > /** > > * dhcpv6() - Check if this is a DHCPv6 message, reply as needed > > * @c: Execution context > > @@ -544,6 +589,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, > > n =3D offsetof(struct resp_t, client_id) + > > sizeof(struct opt_hdr) + ntohs(client_id->l); > > n =3D dhcpv6_dns_fill(c, (char *)&resp, n); > > + n =3D dhcpv6_client_fqdn_fill(c, (char*)&resp, n); > > (char *) (for consistency, see line just above) > Done. > > > > resp.hdr.xid =3D mh->xid; > > > > diff --git a/passt.1 b/passt.1 > > index d9cd33e..27e70a7 100644 > > --- a/passt.1 > > +++ b/passt.1 > > @@ -401,6 +401,17 @@ Enable IPv6-only operation. IPv4 traffic will be i= gnored. > > By default, IPv4 operation is enabled as long as at least an IPv4 rout= e and an > > interface address are configured on a given host interface. > > > > +.TP > > +.BR \-H ", " \-\-hostname " " \fIname > > +Hostname to configure client with. > > +For dhcp client aware encode \fIname as DHCPv4 option 12 (hostname). > > DHCP: there's no such thing as DHCPv4. > Done > ...who is aware of... what? I would just say "Send \fIname\fR as > hostname via DHCP (Option 12)". I don't think we reason much about what > the client expects or not, right? > > > + > > +.TP > > +.BR \-\-fqdn " " \fIname > > +FQDN to configure client with. > > +For dhcp client aware encode \fIname as dhcp client fqdn option, for D= HCPv4 option 81 and for > > Same here, I'm not sure who is aware of what: I would just say we send > \fIname\fR as FQDN (option 81 ...). Same as above about DHCP, FQDN, and > DHCPv4. Done. > > > +DHCPv6 option 39. > > + > > .SS \fBpasst\fR-only options > > > > .TP > > diff --git a/passt.h b/passt.h > > index 0dd4efa..577e734 100644 > > --- a/passt.h > > +++ b/passt.h > > @@ -91,6 +91,7 @@ struct fqdn { > > char n[NS_MAXDNAME]; > > }; > > > > + > > Spurious change. > Done. > > #include > > #include > > > > @@ -209,6 +210,8 @@ struct ip6_ctx { > > * @ifi4: Template interface for IPv4, -1: none, 0: IPv4 di= sabled > > * @ip: IPv4 configuration > > * @dns_search: DNS search list > > + * @hostname: Guest hostname > > + * @fqdn: Guest FQDN > > * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 di= sabled > > * @ip6: IPv6 configuration > > * @pasta_ifn: Name of namespace interface for pasta > > @@ -268,6 +271,9 @@ struct ctx { > > struct ip4_ctx ip4; > > > > struct fqdn dns_search[MAXDNSRCH]; > > + > > + char hostname[PASST_MAXDNAME]; > > + char fqdn[PASST_MAXDNAME]; > > > > int ifi6; > > struct ip6_ctx ip6; > > diff --git a/pasta.c b/pasta.c > > index ff41c95..cdc9aa6 100644 > > --- a/pasta.c > > +++ b/pasta.c > > @@ -173,6 +173,7 @@ void pasta_open_ns(struct ctx *c, const char *netns= ) > > struct pasta_spawn_cmd_arg { > > const char *exe; > > char *const *argv; > > + struct ctx *c; > > }; > > > > /** > > @@ -187,6 +188,7 @@ static int pasta_spawn_cmd(void *arg) > > char hostname[HOST_NAME_MAX + 1] =3D HOSTNAME_PREFIX; > > const struct pasta_spawn_cmd_arg *a; > > sigset_t set; > > + size_t conf_hostname_len =3D 0; > > Align from longest to shortest: > > const struct pasta_spawn_cmd_arg *a; > size_t conf_hostname_len =3D 0; > sigset_t set; > > and there's no need to initialise conf_hostname_len (it makes the > reader think that it might be used without the strlen() assignment in > some cases, but it's not true). > Done. > > > > /* We run in a detached PID and mount namespace: mount /proc over= */ > > if (mount("", "/proc", "proc", 0, NULL)) > > @@ -194,8 +196,15 @@ static int pasta_spawn_cmd(void *arg) > > > > if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0")) > > warn("Cannot set ping_group_range, ICMP requests might fa= il"); > > - > > - if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1, > > + > > + a =3D (const struct pasta_spawn_cmd_arg *)arg; > > + > > + conf_hostname_len =3D strlen(a->c->hostname); > > + if (conf_hostname_len > 0) { > > + if (sethostname(a->c->hostname, conf_hostname_len)) > > + warn("Unable to set configured hostname"); > > + } > > + else if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1, > > } else if > > > HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFIX)) || > > errno =3D=3D ENAMETOOLONG) { > > These should move to be aligned with the arguments of gethostname() and w= ith > the if clause: > > } else if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1, > HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFI= X)) || > errno =3D=3D ENAMETOOLONG) { > > Done. > > hostname[HOST_NAME_MAX] =3D '\0'; > > @@ -208,7 +217,6 @@ static int pasta_spawn_cmd(void *arg) > > sigaddset(&set, SIGUSR1); > > sigwaitinfo(&set, NULL); > > > > - a =3D (const struct pasta_spawn_cmd_arg *)arg; > > execvp(a->exe, a->argv); > > > > die_perror("Failed to start command or shell"); > > @@ -230,6 +238,7 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t= gid, > > struct pasta_spawn_cmd_arg arg =3D { > > .exe =3D argv[0], > > .argv =3D argv, > > + .c =3D c, > > }; > > char uidmap[BUFSIZ], gidmap[BUFSIZ]; > > char *sh_argv[] =3D { NULL, NULL }; > > diff --git a/test/lib/setup b/test/lib/setup > > index 580825f..ee67152 100755 > > --- a/test/lib/setup > > +++ b/test/lib/setup > > @@ -49,7 +49,7 @@ setup_passt() { > > > > context_run passt "make clean" > > context_run passt "make valgrind" > > - context_run_bg passt "valgrind --max-stackframe=3D$((4 * 1024 * 1= 024)) --trace-children=3Dyes --vgdb=3Dno --error-exitcode=3D1 --suppression= s=3Dtest/valgrind.supp ./passt ${__opts} -s ${STATESETUP}/passt.socket -f -= t 10001 -u 10001 -P ${STATESETUP}/passt.pid" > > + context_run_bg passt "valgrind --max-stackframe=3D$((4 * 1024 * 1= 024)) --trace-children=3Dyes --vgdb=3Dno --error-exitcode=3D1 --suppression= s=3Dtest/valgrind.supp ./passt ${__opts} -s ${STATESETUP}/passt.socket -f -= t 10001 -u 10001 -H hostname1 --fqdn fqdn1.passt.test -P ${STATESETUP}/pass= t.pid" > > > > # pidfile isn't created until passt is listening > > wait_for [ -f "${STATESETUP}/passt.pid" ] > > @@ -160,11 +160,11 @@ setup_passt_in_ns() { > > if [ ${VALGRIND} -eq 1 ]; then > > context_run passt "make clean" > > context_run passt "make valgrind" > > - context_run_bg passt "valgrind --max-stackframe=3D$((4 * = 1024 * 1024)) --trace-children=3Dyes --vgdb=3Dno --error-exitcode=3D1 --sup= pressions=3Dtest/valgrind.supp ./passt -f ${__opts} -s ${STATESETUP}/passt.= socket -t 10001,10011,10021,10031 -u 10001,10011,10021,10031 -P ${STATESETU= P}/passt.pid --map-host-loopback ${__map_ns4} --map-host-loopback ${__map_n= s6}" > > + context_run_bg passt "valgrind --max-stackframe=3D$((4 * = 1024 * 1024)) --trace-children=3Dyes --vgdb=3Dno --error-exitcode=3D1 --sup= pressions=3Dtest/valgrind.supp ./passt -f ${__opts} -s ${STATESETUP}/passt.= socket -H hostname1 --fqdn fqdn1.passt.test -t 10001,10011,10021,10031 -u 1= 0001,10011,10021,10031 -P ${STATESETUP}/passt.pid --map-host-loopback ${__m= ap_ns4} --map-host-loopback ${__map_ns6}" > > else > > context_run passt "make clean" > > context_run passt "make" > > - context_run_bg passt "./passt -f ${__opts} -s ${STATESETU= P}/passt.socket -t 10001,10011,10021,10031 -u 10001,10011,10021,10031 -P ${= STATESETUP}/passt.pid --map-host-loopback ${__map_ns4} --map-host-loopback = ${__map_ns6}" > > + context_run_bg passt "./passt -f ${__opts} -s ${STATESETU= P}/passt.socket -H hostname1 --fqdn fqdn1.passt.test -t 10001,10011,10021,1= 0031 -u 10001,10011,10021,10031 -P ${STATESETUP}/passt.pid --map-host-loopb= ack ${__map_ns4} --map-host-loopback ${__map_ns6}" > > fi > > wait_for [ -f "${STATESETUP}/passt.pid" ] > > > > @@ -243,7 +243,7 @@ setup_two_guests() { > > [ ${TRACE} -eq 1 ] && __opts=3D"${__opts} --trace" > > [ ${VHOST_USER} -eq 1 ] && __opts=3D"${__opts} --vhost-user" > > > > - context_run_bg passt_1 "./passt -s ${STATESETUP}/passt_1.socket -= P ${STATESETUP}/passt_1.pid -f ${__opts} -t 10001 -u 10001" > > + context_run_bg passt_1 "./passt -s ${STATESETUP}/passt_1.socket -= P ${STATESETUP}/passt_1.pid -f ${__opts} --fqdn fqdn1.passt.test -H hostnam= e1 -t 10001 -u 10001" > > wait_for [ -f "${STATESETUP}/passt_1.pid" ] > > > > __opts=3D > > @@ -252,7 +252,7 @@ setup_two_guests() { > > [ ${TRACE} -eq 1 ] && __opts=3D"${__opts} --trace" > > [ ${VHOST_USER} -eq 1 ] && __opts=3D"${__opts} --vhost-user" > > > > - context_run_bg passt_2 "./passt -s ${STATESETUP}/passt_2.socket -= P ${STATESETUP}/passt_2.pid -f ${__opts} -t 10004 -u 10004" > > + context_run_bg passt_2 "./passt -s ${STATESETUP}/passt_2.socket -= P ${STATESETUP}/passt_2.pid -f ${__opts} --hostname hostname2 --fqdn fqdn2 = -t 10004 -u 10004" > > wait_for [ -f "${STATESETUP}/passt_2.pid" ] > > > > __vmem=3D"$((${MEM_KIB} / 1024 / 4))" > > diff --git a/test/passt.mbuto b/test/passt.mbuto > > index 138d365..1e07693 100755 > > --- a/test/passt.mbuto > > +++ b/test/passt.mbuto > > @@ -13,7 +13,7 @@ > > PROGS=3D"${PROGS:-ash,dash,bash ip mount ls insmod mkdir ln cat chmod = lsmod > > modprobe find grep mknod mv rm umount jq iperf3 dhclient hostna= me > > sed tr chown sipcalc cut socat dd strace ping tail killall slee= p sysctl > > - nproc tcp_rr tcp_crr udp_rr which tee seq bc sshd ssh-keygen cm= p}" > > + nproc tcp_rr tcp_crr udp_rr which tee seq bc sshd ssh-keygen cm= p env}" > > > > # OpenSSH 9.8 introduced split binaries, with sshd being the daemon, a= nd > > # sshd-session the per-session program. We need the latter as well, an= d the path > > @@ -41,6 +41,7 @@ FIXUP=3D"${FIXUP}"' > > #!/bin/sh > > LOG=3D/var/log/dhclient-script.log > > echo \${reason} \${interface} >> \$LOG > > +env >> \$LOG > > set >> \$LOG > > > > [ -n "\${new_interface_mtu}" ] && ip link set dev \${interface} = mtu \${new_interface_mtu} > > @@ -54,7 +55,8 @@ set >> \$LOG > > [ -n "\${new_ip6_address}" ] && ip addr add \${new_ip6_address= }/\${new_ip6_prefixlen} dev \${interface} > > [ -n "\${new_dhcp6_name_servers}" ] && for d in \${new_dhcp6_name_ser= vers}; do echo "nameserver \${d}%\${interface}" >> /etc/resolv.conf; done > > [ -n "\${new_dhcp6_domain_search}" ] && (printf "search"; for d in \${= new_dhcp6_domain_search}; do printf " %s" "\${d}"; done; printf "\n") >> /e= tc/resolv.conf > > -[ -n "\${new_host_name}" ] && hostname "\${new_host_name}" > > +[ -n "\${new_host_name}" ] && echo "\${new_host_name}" > /tm= p/new_host_name > > +[ -n "\${new_fqdn_fqdn}" ] && echo "\${new_fqdn_fqdn}" > /tm= p/new_fqdn_fqdn > > exit 0 > > EOF > > chmod 755 /sbin/dhclient-script > > diff --git a/test/passt/dhcp b/test/passt/dhcp > > index 9925ab9..145f1ba 100644 > > --- a/test/passt/dhcp > > +++ b/test/passt/dhcp > > @@ -11,7 +11,7 @@ > > # Copyright (c) 2021 Red Hat GmbH > > # Author: Stefano Brivio > > > > -gtools ip jq dhclient sed tr > > +gtools ip jq dhclient sed tr hostname > > htools ip jq sed tr head > > > > test Interface name > > @@ -47,7 +47,16 @@ gout SEARCH sed 's/\. / /g' /etc/resolv.conf |= sed 's/\.$//g' | sed -n 's/^searc > > hout HOST_SEARCH sed 's/\. / /g' /etc/resolv.conf | sed 's/\.$//g' | s= ed -n 's/^search \(.*\)/\1/p' | tr ' \n' ',' | sed 's/,$//;s/$/\n/' > > check [ "__SEARCH__" =3D "__HOST_SEARCH__" ] > > > > +test DHCP: Hostname > > +gout NEW_HOST_NAME cat /tmp/new_host_name > > +check [ "__NEW_HOST_NAME__" =3D "hostname1" ] > > + > > +test DHCP: Client FQDN > > +gout NEW_FQDN_FQDN cat /tmp/new_fqdn_fqdn > > +check [ "__NEW_FQDN_FQDN__" =3D "fqdn1.passt.test" ] > > + > > test DHCPv6: address > > +guest rm /tmp/new_fqdn_fqdn > > guest /sbin/dhclient -6 __IFNAME__ > > # Wait for DAD to complete > > guest while ip -j -6 addr show tentative | jq -e '.[].addr_info= '; do sleep 0.1; done > > @@ -70,3 +79,7 @@ test DHCPv6: search list > > gout SEARCH6 sed 's/\. / /g' /etc/resolv.conf | sed 's/\.$//g' | sed -= n 's/^search \(.*\)/\1/p' | tr ' \n' ',' | sed 's/,$//;s/$/\n/' > > hout HOST_SEARCH6 sed 's/\. / /g' /etc/resolv.conf | sed 's/\.$//g' | = sed -n 's/^search \(.*\)/\1/p' | tr ' \n' ',' | sed 's/,$//;s/$/\n/' > > check [ "__SEARCH6__" =3D "__HOST_SEARCH6__" ] > > + > > +test DHCPv6: Hostname > > +gout NEW_FQDN_FQDN cat /tmp/new_fqdn_fqdn > > +check [ "__NEW_FQDN_FQDN__" =3D "fqdn1.passt.test" ] > > diff --git a/util.c b/util.c > > index 11973c4..f3cf7b1 100644 > > --- a/util.c > > +++ b/util.c > > @@ -837,3 +837,25 @@ void raw_random(void *buf, size_t buflen) > > if (random_read < buflen) > > die("Unexpected EOF on random data source"); > > } > > +/** > > + * encode_domain_name() - Encode domain name according to RFC 1035, se= ction 3.1 > > + * @domain_name: Input domain name to encode > > + * @len: Domain name length > > + * @buf: Buffer to fill in with encoded domain name > > + * > > + * Return: encoded domain name length > > + */ > > +size_t encode_domain_name(const char *domain_name, size_t len, char *b= uf){ > > Opening curly bracket on a newline, for consistency. > Done. > > + char *p =3D NULL; > > No need to initialise this, it's assigned unconditionally below. > Done. > > + size_t i =3D 0; > > No need to initialise this, you only use it in the loop below. > Done. > > + > > + buf[0] =3D strcspn(domain_name, "."); > > + p =3D buf + 1; > > + for (i =3D 0; i < len; i++) { > > + if (domain_name[i] =3D=3D '.') > > + p[i] =3D strcspn(domain_name + i + 1, "."); > > + else > > + p[i] =3D domain_name[i]; > > + } > > + return len + 2; > > +} > > diff --git a/util.h b/util.h > > index 3fa1d12..d7751a8 100644 > > --- a/util.h > > +++ b/util.h > > @@ -346,4 +346,7 @@ static inline int wrap_accept4(int sockfd, struct s= ockaddr *addr, > > #define accept4(s, addr, addrlen, flags) \ > > wrap_accept4((s), (addr), (addrlen), (flags)) > > > > +#define PASST_MAXDNAME 253 /* RFC 1035 */ > > +size_t encode_domain_name(const char* domain_name, size_t len, char* b= uf); > > + > > #endif /* UTIL_H */ > > -- > Stefano > -- Quique Llorente CNV networking Senior Software Engineer Red Hat EMEA ellorent@redhat.com @RedHat Red Hat Red Hat