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=aLEYPQJ6; 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 ESMTPS id 648FC5A0272 for ; Tue, 07 Jan 2025 10:36:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1736242563; 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=JI82u2ynWzvof3ZR9sNwZ6gMrPM19lU1Zzj8QXAv3mI=; b=aLEYPQJ6vVnBqkp89eNeaT4rx9hLIoyG1u1a7XEOZX78lUwbhFLrDUzq9Q4idbDW1dQQ/k FrlxlcKwANzuhYVBUmwEi4vA7v47/O4pOaHugFqXPsePCyRosO6G4rI/1jIMZBeyhRVWpP UC8+/FiZ8pJ//vV5Xl6Wp+fX+kwnpPs= Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-433-UaineBqYNaC0a_LCuLlVlA-1; Tue, 07 Jan 2025 04:36:01 -0500 X-MC-Unique: UaineBqYNaC0a_LCuLlVlA-1 X-Mimecast-MFC-AGG-ID: UaineBqYNaC0a_LCuLlVlA Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-30033ad0158so64902811fa.0 for ; Tue, 07 Jan 2025 01:36:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736242559; x=1736847359; 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=JI82u2ynWzvof3ZR9sNwZ6gMrPM19lU1Zzj8QXAv3mI=; b=TZfiqGe7jVLBjuL03SynVtFQGwQAs79346eisnKs5ySAr1XImw4Isb/dbKz9p0fh5d J1efDYQnRMX4t7WHVCARPxZMYlYr5RVvSPFsrhM9s+QqeB2Z5rEiQk4NsZLNO6465KWd xynbWxEDRjOULogPG4jh8TxfAms4ryZTVsbths2ZvcVnFORTEg/gkXKOHOZ0ZNtQCEFt JDjoA74+8HHISmNqdoTPJJKglvWGt6jlO3lauYVKB292m8U71fj52l8EODbc3B0mLrIT fNo9ZpMMaOqlr/6UcPE64kHobztUK54rYT82uk7iV9/FuKtVyvWU+wjmKH/zDgyOlgLv 784Q== X-Gm-Message-State: AOJu0Yxzz2XvRNpnRcx/bDKdajwkVUQDXzcL2zv44KbpWsR46Z2pOo6y ZYHLsUpPMp3zGr4x3k7zjyciKxwGpiV0LDlkqumg9DYNyN96WF1f1KB+prDu6LP/B4Bk2YHpcZG 80wOLlEZ2q2pY2A6ZVFL1Rtr4UmUcpH3fFEwlBqqjaX0ZyfSIDxFSjJ1aE4hTrNonttTy8X7Zz2 G6C8AljGH93Uya7Jzn0YLG1UJ1 X-Gm-Gg: ASbGncsaRANn4QKiNomAAaBaPYbF+a7szQxX9GePxQOpyZyv+NcUGB8Fip2UNMPaKPv uTE9EAl6WO5yYtKnPbTV41VKU/Y0YhrS2O/X4qRKn/fgaJMXchdvHzQkiJDObQDw9BuEN6BkS X-Received: by 2002:a2e:bc05:0:b0:302:29a5:6ded with SMTP id 38308e7fff4ca-3046858eae9mr204832171fa.17.1736242559162; Tue, 07 Jan 2025 01:35:59 -0800 (PST) X-Google-Smtp-Source: AGHT+IEtcXZUQ7PKRrMahs1IzLlkACgSfLMldgs5eWsrMNRuxEH5O/UP+ououShz8UnTVGprLmOwQqpfTO48I6tTH7s= X-Received: by 2002:a2e:bc05:0:b0:302:29a5:6ded with SMTP id 38308e7fff4ca-3046858eae9mr204832031fa.17.1736242558235; Tue, 07 Jan 2025 01:35:58 -0800 (PST) MIME-Version: 1.0 References: <20241219115557.54978-1-ellorent@redhat.com> <20250101225428.0f10934d@elisabeth> <20250102230021.68e9f4b1@elisabeth> In-Reply-To: <20250102230021.68e9f4b1@elisabeth> From: Enrique Llorente Pastora Date: Tue, 7 Jan 2025 10:35:46 +0100 Message-ID: Subject: Re: [PATCH v5] dhcp, dhcpv6: Add hostname and client fqdn ops To: Stefano Brivio X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: L3eN6jy1OdY_ncmoZG29_VguZVOJQbcFzxmAy8_ERec_1736242560 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 2ONIGBPYNVAOM7H6E4E2MINKSKQVW4DT X-Message-ID-Hash: 2ONIGBPYNVAOM7H6E4E2MINKSKQVW4DT 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, Jan 2, 2025 at 11:00=E2=80=AFPM Stefano Brivio = wrote: > > On Thu, 2 Jan 2025 17:09:48 +0100 > Enrique Llorente Pastora wrote: > > > On Wed, Jan 1, 2025 at 10:54=E2=80=AFPM Stefano Brivio wrote: > > > > > > On Thu, 19 Dec 2024 12:55:57 +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 | 20 +++++++++++-- > > > > dhcp.c | 50 ++++++++++++++++++++++++++++---- > > > > dhcpv6.c | 75 +++++++++++++++++++++++++++++++++++++++-----= ---- > > > > passt.1 | 11 +++++++ > > > > passt.h | 5 ++++ > > > > pasta.c | 18 ++++++++---- > > > > test/lib/setup | 10 +++---- > > > > test/passt.mbuto | 6 ++-- > > > > test/passt/dhcp | 15 +++++++++- > > > > util.c | 23 +++++++++++++++ > > > > util.h | 6 ++++ > > > > 11 files changed, 204 insertions(+), 35 deletions(-) > > > > > > > > diff --git a/conf.c b/conf.c > > > > index df2b016..5f21193 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,11 @@ 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)) > > > > > > Coding style: arguments are preferably aligned with parameter list th= ey > > > belong to: > > > > > > if (snprintf_check(c->fqdn, PASST_MAXDNAME, > > > "%s", optarg)) > > > > > > for another example, look just 20 lines below this (case 's'). > > > > > > > Done > > > > > > + die("Invalid FQDN: %s", optarg); > > > > + break; > > > > case 'd': > > > > c->debug =3D 1; > > > > c->quiet =3D 0; > > > > @@ -1727,6 +1736,11 @@ 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; > > > > diff --git a/dhcp.c b/dhcp.c > > > > index d8515aa..b224bf8 100644 > > > > --- a/dhcp.c > > > > +++ b/dhcp.c > > > > @@ -63,6 +63,12 @@ static struct opt opts[255]; > > > > > > > > #define OPT_MIN 60 /* RFC 951 */ > > > > > > > > +/* 576 (RFC 2131), minus offset > > > > + * of options (268), minus end > > > > + * option and its length (2) > > > > + */ > > > > > > I formatted this multi-comment in my reply to v4 so that the first > > > line would fit with the define: > > > > > > #define OPT_MAX 306 /* 576 (RFC 2131), minus offset ... > > > > > > but if you don't want to have it on the same line (no preference from= my > > > side), then there's no need for all that line wrapping (but then you > > > should mention what it refers to), say: > > > > > > /* Total option size (excluding end option) is 576 (RFC 2131), minus > > > * offset of options (268), minus end option and its length (2). > > > */ > > > #define OPT_MAX 306 > > > > > > > Will go with your multiline comment since it's quite large and it reads= better. > > > > > > +#define OPT_MAX 306 > > > > + > > > > /** > > > > * dhcp_init() - Initialise DHCP options > > > > */ > > > > @@ -122,7 +128,7 @@ struct msg { > > > > uint8_t sname[64]; > > > > uint8_t file[128]; > > > > uint32_t magic; > > > > - uint8_t o[308]; > > > > + uint8_t o[OPT_MAX + 2]; /* End option and length */ > > > > > > ..."plus end option and length", or move the comment so that it refer= s > > > to the "2": > > > > > > uint8_t o[OPT_MAX + 2 /* End option and its length */ ]; > > > > > > Otherwise it looks like we use 310 bytes for the end option and its > > > length. > > > > > > > Going with inlining it next to "2" > > > > > > } __attribute__((__packed__)); > > > > > > > > /** > > > > @@ -130,15 +136,31 @@ struct msg { > > > > * @m: Message to fill > > > > * @o: Option number > > > > * @offset: Current offset within options field, updated on inser= tion > > > > + * > > > > + * Return: offset for the next option field > > > > */ > > > > -static void fill_one(struct msg *m, int o, int *offset) > > > > +static int fill_one(struct msg *m, int o, int *offset) > > > > { > > > > + size_t idx, slen =3D 0; > > > > + > > > > + /* If it cannot write even enum + len + one byte, then just s= kip */ > > > > > > ..."it" who? If we can't write, then skip. > > > > > > It's not clear what "enum" refers to, I guess that should be "number" > > > or "code". > > > > > > > Done > > > > > By the way, I just realised that this is not entirely correct because > > > there's at least one option (option 80, "Rapid Commit") that has leng= th > > > zero (it's some sort of flag, with the presence of the option > > > indicating that the feature is enabled). > > > > > > Maybe there's a more elegant solution to this, but a convenient > > > alternative might be to check *offset + 1 /* length */ + !!slen inste= ad. > > > > Going with the alternative > > > > > > > > That's with slen assigned earlier, there's no point in initialising i= t > > > to zero and then unconditionally reassigning it later. > > > > Done > > > > > > > > > + if (*offset + 2 > OPT_MAX) > > > > + return OPT_MAX; > > > > + > > > > 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; > > > > > > I found it a bit difficult to understand what 'idx' is here (offset o= f > > > option value). Perhaps you could just recycle *offset to keep track o= f > > > the actual offset as we move in the buffer. > > > > > > That is, we just set two bytes starting from *offset, and now we can = do: > > > > > > *offset +=3D 2; > > > > > > > It Makes sense since we just use "idx" everywhere after that so we can > > recycle it without problem. > > > > > > + slen =3D opts[o].slen; > > > > + > > > > + /* Truncate if it goes beyond OPT_MAX */ > > > > > > Shouldn't we also report something via debug() in this case? > > > > > > > Done. > > > > > > + if (idx + slen > OPT_MAX) > > > > + slen =3D OPT_MAX - idx; > > > > > > I didn't check what happens with a zero-length option, but it looks o= kay > > > to me. The (pending) change at: > > > > > > https://github.com/AsahiLinux/muvm/pull/111 > > > > > > uses option 80 ("Rapid Commit"). I can also give it a quick try later= . > > > > Thanks for checking, it would be awesome to have some kind of matrix > > test for all these cases though, like starting passt per test with > > specific arguments. > > This is not so easy to check: muvm is not widely packaged, not > necessarily easy to build, and without it, we would probably need a > small stand-alone DHCP client. > > Eventually we should add muvm tests, just like we run the ones specific > to pasta from Podman's test suite, but it's a bit too early (see > https://github.com/AsahiLinux/muvm/pull/117#issuecomment-2567922524 as > an example). > > > > > + > > > > + memcpy(&m->o[*offset + 2], opts[o].s, slen); > > > > > > > > opts[o].sent =3D 1; > > > > *offset +=3D 2 + opts[o].slen; > > > > + return *offset; > > > > } > > > > > > > > /** > > > > @@ -172,7 +194,10 @@ 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) { > > > > + debug("DHCP: truncating after option = %i", o); > > > > > > I think there's an off-by-one here: if the options exactly fit the > > > maximum size, we'll warn, but no truncation will actually happen. > > > > It will truncate since no more options will be included, this is not > > the same as inter option truncate. > > I understand, but if you don't have further options (other than the end > option), then it's absolutely fine that no further options are included. > Done > > > Perhaps we should print messages directly in fill_one(), as we have a > > > better detail of what's going on, there? > > > > I prefer to log directly at the place where the action is happening > > (skipping the rest of the options. > > ...I guess you can maintain that by refactoring the loop a bit, then. > Done > > > > + break; > > > > + } > > > > } > > > > > > > > m->o[offset++] =3D 255; > > > > @@ -285,7 +310,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 =3D 0, fqdn_len =3D 0; > > > > > > You're assigning those unconditionally before using them, so there's = no > > > need to initialise them here (otherwise one might mistake them as a > > > "counter" kind of thing). > > > > > > > Done. > > > > > Please wrap to 80 columns when feasible: > > > > > > size_t mlen, dlen, offset =3D 0, opt_len, opt_off =3D 0; > > > size_t hostname_len, fqdn_len; > > > > > > and by the way, you can probably recycle opt_len for those. > > > > > > > Sure, recycling. > > > > > > char macstr[ETH_ADDRSTRLEN]; > > > > struct in_addr mask, dst; > > > > const struct ethhdr *eh; > > > > @@ -398,6 +423,21 @@ int dhcp(const struct ctx *c, const struct poo= l *p) > > > > if (!opts[6].slen) > > > > opts[6].slen =3D -1; > > > > > > > > + hostname_len =3D strlen(c->hostname); > > > > + if (hostname_len > 0) { > > > > + opts[12].slen =3D hostname_len; > > > > + memcpy(opts[12].s, &c->hostname, hostname_len); > > > > + } > > > > + > > > > + fqdn_len =3D strlen(c->fqdn); > > > > + if (fqdn_len > 0) { > > > > + size_t encoded_len =3D 0; > > > > > > Sorry, I missed a couple of observations on v4: encoded_len is > > > uselessly initialised here, and RFC 4702, 2.2, says, about the RCODE > > > fields: > > > > > > A server SHOULD set these to 255 when sending the option > > > and MUST ignore them on receipt. > > > > > > but you're leaving them as zeroes here. > > > > Done. > > > > > > > > > + opts[81].s[0] =3D 0x4; /* flags (E) */ > > > > + encoded_len =3D encode_domain_name(c->fqdn, fqdn_len, > > > > + (char *) opts= [81].s + 3); > > > > > > Arguments should be visually aligned: > > > > > > encoded_len =3D encode_domain_name(c->fqdn, fqdn_len, > > > (char *)opts[81].s += 3); > > > > > > By the way, it's more common to have the destination buffer as first > > > argument (see memcpy(), snprintf()). > > > > > > This usage would be more idiomatic and obvious (matching memcpy()) wi= th > > > something like: > > > > > > encode_domain_name(opts[81].s + 3, c->fqdn, fqdn_len); > > > > > > > Done. > > > > > > + opts[81].slen =3D encoded_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..ce3a1bd 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; > > > > @@ -58,6 +59,9 @@ struct opt_hdr { > > > > sizeof(struct opt_hdr)) > > > > #define OPT_VSIZE(x) (sizeof(struct opt_##x) - = \ > > > > sizeof(struct opt_hdr)) > > > > +#define OPT_MAX_SIZE IPV6_MIN_MTU - (sizeof(struct ipv6hdr= ) + \ > > > > + sizeof(struct udphdr)= + \ > > > > + sizeof(struct msg_hdr= )) > > > > > > You can just use tabs, no need to mix them with spaces here. > > > > > > > Done > > > > > > > > > > /** > > > > * struct opt_client_id - DHCPv6 Client Identifier option > > > > @@ -163,6 +167,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 described by RFC 4704 (always zero for = us) > > > > + * @domain_name: Client FQDN > > > > + */ > > > > +struct opt_client_fqdn{ > > > > > > Missing space between identifier and {. > > > > > > > Done > > > > > > + 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 +209,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 +220,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 +246,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 +368,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) > > > > @@ -373,6 +394,7 @@ search: > > > > return offset; > > > > > > > > for (i =3D 0; *c->dns_search[i].n; i++) { > > > > + size_t encoded_name_len =3D 0; > > > > size_t name_len =3D strlen(c->dns_search[i].n); > > > > > > > > /* We already append separators, don't duplicate if p= resent */ > > > > @@ -388,29 +410,53 @@ 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; > > > > + > > > > + encoded_name_len =3D encode_domain_name(c->dns_search= [i].n, > > > > + name_len, srch->list); > > > > > > Please align these arguments: > > > > > > encoded_name_len =3D encode_domain_name(c->dns_search= [i].n, > > > name_len, srch-= >list); > > > > > > > Done. > > > > > > + srch->hdr.l +=3D encoded_name_len; > > > > + offset +=3D encoded_name_len; > > > > } > > > > > > > > if (srch) { > > > > > > There's no need for those curly brackets anymore. > > > > > > > Done. > > > > > > - 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); > > > > } > > > > > > > > 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) > > > > > > Wrap to 80 columns: > > > > > > static size_t dhcpv6_client_fqdn_fill(const struct ctx *c, char *buf, > > > int offset) > > > > > > > Done. > > > > > > +{ > > > > + size_t fqdn_len, opt_hdr_len, opt_len, encoded_fqdn_len; > > > > + struct opt_client_fqdn *o; > > > > + > > > > + opt_hdr_len =3D sizeof(struct opt_hdr); > > > > > > Does this really need its own variable? > > > > > > > Not really, removing. > > > > > > > > + > > > > + fqdn_len =3D MIN(strlen(c->fqdn), OPT_MAX_SIZE - (offset + op= t_hdr_len + 1)); > > > > > > Wrap to 80 columns. This isn't really clear: here you seem to be usin= g > > > it as maximum length for the option, but then encode_domain_name() > > > doesn't use it like that (it's the "Domain name length" according to > > > the comment there). > > > > > > > I am going to re-use "opt_len" for all of this, no need for a new > > variable, if this is the issue. > > > > > > + > > > > > > Stray tab. > > > > > > > Done. > > > > > > + if (fqdn_len =3D=3D 0) > > > > > > What if it's negative? I guess you should use ssize_t for fqdn_len, a= nd > > > adjust the check here. > > > > > > > Done > > > > > > + return offset; > > > > + > > > > + o =3D (struct opt_client_fqdn *)(buf + offset); > > > > + encoded_fqdn_len =3D encode_domain_name(c->fqdn, fqdn_len, > > > > + o->domain_nam= e); > > > > > > Argument alignment: > > > > > > encoded_fqdn_len =3D encode_domain_name(c->fqdn, fqdn_len, > > > o->domain_name); > > > > > > I don't understand: if the option is too long, where is it truncated? > > > > The len after truncate it is already calculated so passing it to > > "encode_domain_name" will truncate the c->fqdn value. > > Yes, but the second argument of encode_domain_name() is an "input" > length, and fqdn_len is the output bound, so it doesn't quite seem to > match. > I will remove the len argument and calculate inside the function, also for your last comment I am going to tally skip the option if there is no size for it instead of truncating the string. > > > > + opt_len =3D encoded_fqdn_len + 1; > > > > + > > > > + o->hdr.t =3D OPT_CLIENT_FQDN; > > > > + o->hdr.l =3D htons(opt_len); > > > > + o->flags =3D 0x00; > > > > > > I guess I mentioned this already: given that we'll set the "S" bit to > > > zero (which is correct), if the client's value for the "S" bit is not > > > zero, we'll need to set the "O" bit. > > > > > > > Totally forgotten, done. > > > > > > + > > > > + return offset + opt_hdr_len + opt_len; > > > > +} > > > > + > > > > /** > > > > * dhcpv6() - Check if this is a DHCPv6 message, reply as needed > > > > * @c: Execution context > > > > @@ -544,6 +590,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); > > > > > > > > resp.hdr.xid =3D mh->xid; > > > > > > > > diff --git a/passt.1 b/passt.1 > > > > index d9cd33e..8f6b194 100644 > > > > --- a/passt.1 > > > > +++ b/passt.1 > > > > @@ -401,6 +401,17 @@ Enable IPv6-only operation. IPv4 traffic will = be ignored. > > > > By default, IPv4 operation is enabled as long as at least an IPv4 = route and an > > > > interface address are configured on a given host interface. > > > > > > > > +.TP > > > > +.BR \-H ", " \-\-hostname " " \fIname > > > > +Hostname to configure client with. > > > > > > For consistency: "the client". > > > > > > > Done. > > > > > > +Send \fIname as DHCP option 12 (hostname). > > > > > > I think you want to have "name" in italic, not the whole rest of line > > > (\fIname\fR). Have a look at the result with 'man ./passt.1'. > > > > > > > Done. > > > > > > + > > > > +.TP > > > > +.BR \-\-fqdn " " \fIname > > > > +FQDN to configure client with. > > > > +Send \fIname as dhcp client fqdn option, for DHCP option 81 and fo= r > > > > > > Same as above. > > > > > > > Done > > > > > It's "DHCP" and "FQDN". > > > > > > > What do you mean ? > > That those are acronyms and, in case of DHCP, the standard name of a > protocol, so they are written uppercase. It's DHCP, not "dhcp", and > FQDN, not "fqdn". > > > > > > +DHCPv6 option 39. > > > > + > > > > .SS \fBpasst\fR-only options > > > > > > > > .TP > > > > diff --git a/passt.h b/passt.h > > > > index 0dd4efa..9909a10 100644 > > > > --- a/passt.h > > > > +++ b/passt.h > > > > @@ -209,6 +209,8 @@ struct ip6_ctx { > > > > * @ifi4: Template interface for IPv4, -1: none, 0: IPv= 4 disabled > > > > * @ip: IPv4 configuration > > > > * @dns_search: DNS search list > > > > + * @hostname: Guest hostname > > > > + * @fqdn: Guest FQDN > > > > * @ifi6: Template interface for IPv6, -1: none, 0: IPv= 6 disabled > > > > * @ip6: IPv6 configuration > > > > * @pasta_ifn: Name of namespace interface for pasta > > > > @@ -268,6 +270,9 @@ struct ctx { > > > > struct ip4_ctx ip4; > > > > > > > > struct fqdn dns_search[MAXDNSRCH]; > > > > + > > > > + char hostname[PASST_MAXDNAME]; > > > > + char fqdn[PASST_MAXDNAME]; > > > > > > Indent with one tab, not spaces. > > > > > > > Done. > > > > > > > > > > int ifi6; > > > > struct ip6_ctx ip6; > > > > diff --git a/pasta.c b/pasta.c > > > > index ff41c95..00678f3 100644 > > > > --- a/pasta.c > > > > +++ b/pasta.c > > > > @@ -173,6 +173,7 @@ void pasta_open_ns(struct ctx *c, const char *n= etns) > > > > struct pasta_spawn_cmd_arg { > > > > const char *exe; > > > > char *const *argv; > > > > + struct ctx *c; > > > > > > The struct comment should be updated as a result. > > > > > > > Done. > > > > > > }; > > > > > > > > /** > > > > @@ -186,6 +187,7 @@ static int pasta_spawn_cmd(void *arg) > > > > { > > > > char hostname[HOST_NAME_MAX + 1] =3D HOSTNAME_PREFIX; > > > > const struct pasta_spawn_cmd_arg *a; > > > > + size_t conf_hostname_len; > > > > sigset_t set; > > > > > > > > /* We run in a detached PID and mount namespace: mount /proc = over */ > > > > @@ -194,10 +196,16 @@ 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 migh= t fail"); > > > > - > > > > - if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1, > > > > - HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFIX))= || > > > > - errno =3D=3D ENAMETOOLONG) { > > > > + > > > > > > Stray tab. > > > > > > > + 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= , > > > > > > Missing space between } and else. > > > > > > > Done. > > > > > > + HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFIX)) || > > > > > > This is an argument to gethostname(), so it should be aligned > > > accordingly. > > > > > > > Done > > > > > > + errno =3D=3D ENAMETOOLONG) { > > > > > > This is now aligned under "if" for some reason, instead of the origin= al > > > (correct) alignment after "(". > > > > > > > Done. > > > > > > hostname[HOST_NAME_MAX] =3D '\0'; > > > > if (sethostname(hostname, strlen(hostname))) > > > > warn("Unable to set pasta-prefixed hostname")= ; > > > > @@ -208,7 +216,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 +237,7 @@ void pasta_start_ns(struct ctx *c, uid_t uid, g= id_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= * 1024)) --trace-children=3Dyes --vgdb=3Dno --error-exitcode=3D1 --suppres= sions=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= * 1024)) --trace-children=3Dyes --vgdb=3Dno --error-exitcode=3D1 --suppres= sions=3Dtest/valgrind.supp ./passt ${__opts} -s ${STATESETUP}/passt.socket = -f -t 10001 -u 10001 -H hostname1 --fqdn fqdn1.passt.test -P ${STATESETUP}/= passt.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 -= -suppressions=3Dtest/valgrind.supp ./passt -f ${__opts} -s ${STATESETUP}/pa= sst.socket -t 10001,10011,10021,10031 -u 10001,10011,10021,10031 -P ${STATE= SETUP}/passt.pid --map-host-loopback ${__map_ns4} --map-host-loopback ${__m= ap_ns6}" > > > > + context_run_bg passt "valgrind --max-stackframe=3D$((= 4 * 1024 * 1024)) --trace-children=3Dyes --vgdb=3Dno --error-exitcode=3D1 -= -suppressions=3Dtest/valgrind.supp ./passt -f ${__opts} -s ${STATESETUP}/pa= sst.socket -H hostname1 --fqdn fqdn1.passt.test -t 10001,10011,10021,10031 = -u 10001,10011,10021,10031 -P ${STATESETUP}/passt.pid --map-host-loopback $= {__map_ns4} --map-host-loopback ${__map_ns6}" > > > > else > > > > context_run passt "make clean" > > > > context_run passt "make" > > > > - context_run_bg passt "./passt -f ${__opts} -s ${STATE= SETUP}/passt.socket -t 10001,10011,10021,10031 -u 10001,10011,10021,10031 -= P ${STATESETUP}/passt.pid --map-host-loopback ${__map_ns4} --map-host-loopb= ack ${__map_ns6}" > > > > + context_run_bg passt "./passt -f ${__opts} -s ${STATE= SETUP}/passt.socket -H hostname1 --fqdn fqdn1.passt.test -t 10001,10011,100= 21,10031 -u 10001,10011,10021,10031 -P ${STATESETUP}/passt.pid --map-host-l= oopback ${__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.sock= et -P ${STATESETUP}/passt_1.pid -f ${__opts} -t 10001 -u 10001" > > > > + context_run_bg passt_1 "./passt -s ${STATESETUP}/passt_1.sock= et -P ${STATESETUP}/passt_1.pid -f ${__opts} --fqdn fqdn1.passt.test -H hos= tname1 -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.sock= et -P ${STATESETUP}/passt_2.pid -f ${__opts} -t 10004 -u 10004" > > > > + context_run_bg passt_2 "./passt -s ${STATESETUP}/passt_2.sock= et -P ${STATESETUP}/passt_2.pid -f ${__opts} --hostname hostname2 --fqdn fq= dn2 -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 ch= mod lsmod > > > > modprobe find grep mknod mv rm umount jq iperf3 dhclient ho= stname > > > > sed tr chown sipcalc cut socat dd strace ping tail killall = sleep sysctl > > > > - nproc tcp_rr tcp_crr udp_rr which tee seq bc sshd ssh-keyge= n cmp}" > > > > + nproc tcp_rr tcp_crr udp_rr which tee seq bc sshd ssh-keyge= n cmp env}" > > > > > > > > # OpenSSH 9.8 introduced split binaries, with sshd being the daemo= n, and > > > > # sshd-session the per-session program. We need the latter as well= , and 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 \${interfa= ce} mtu \${new_interface_mtu} > > > > @@ -54,7 +55,8 @@ set >> \$LOG > > > > [ -n "\${new_ip6_address}" ] && ip addr add \${new_ip6_add= ress}/\${new_ip6_prefixlen} dev \${interface} > > > > [ -n "\${new_dhcp6_name_servers}" ] && for d in \${new_dhcp6_name= _servers}; do echo "nameserver \${d}%\${interface}" >> /etc/resolv.conf; do= ne > > > > [ -n "\${new_dhcp6_domain_search}" ] && (printf "search"; for d in= \${new_dhcp6_domain_search}; do printf " %s" "\${d}"; done; printf "\n") >= > /etc/resolv.conf > > > > -[ -n "\${new_host_name}" ] && hostname "\${new_host_name= }" > > > > +[ -n "\${new_host_name}" ] && echo "\${new_host_name}" >= /tmp/new_host_name > > > > +[ -n "\${new_fqdn_fqdn}" ] && echo "\${new_fqdn_fqdn}" >= /tmp/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.co= nf | sed 's/\.$//g' | sed -n 's/^searc > > > > hout HOST_SEARCH sed 's/\. / /g' /etc/resolv.conf | sed 's/\.$//g'= | sed -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' | s= ed -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..7aeb5b4 100644 > > > > --- a/util.c > > > > +++ b/util.c > > > > @@ -837,3 +837,26 @@ 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= , section 3.1 > > > > + * @domain_name: Input domain name to encode > > > > + * @len: Domain name length > > > > > > > > > > > > > + * @buf: Buffer to fill in with encoded domain name > > > > > > The descriptions should be aligned in the same way. > > > > Done. > > > > > > > > Shouldn't the maximum buffer length to write be also specified? > > > > > > > What do you mean ? > > I mean that the current version of encode_domain_name() just takes > "len", which is the length of the *unencoded* domain (I suppose, the > comment doesn't really say it), but it doesn't take the length of the > encoded domain (output). > > This isn't really safe, because usually formatting functions take the > size of the *output* buffer, and by writing two characters more, you > might trick users of this function to pass a buffer with 'len' size. > > Tricked users include yourself as of this patch, I guess: > dhcpv6_dns_fill() passes the size of the *input*, while > dhcpv6_client_fqdn_fill() passes the size of the *output* (because > it's calculated from the available buffer size in the response), and > encode_domain_name() uses it as *input*. > > Actually, you don't need the input length at all, because the input > string always has a terminator. But to truncate the domain names > properly, you need to know if you're exceeding the size of the > destination buffer. > > If you're about to exceed that, by the way, you can't just stop > there and pretend that the domain is valid. Let's say you have 9 > bytes left and you need to encode passt.top: ApasstBtopC, where A > is the decimal byte value 5, B is 3, and C is 0. > > If you just use 8 bytes, that would be ApasstBt, which comes with > two issues: > > - B doesn't represent the size of the next label anymore > > - the terminator, C, is missing altogether > > ...now, the idea behind truncating these options was to keep this > patch simple for the moment, and somewhat easily allow extending > the feature for DHCP (with option overload) at a later time. But > perhaps it's adding more complexity than we thought... and perhaps > we risk crashing clients, too. > > Should we just omit options if they don't fit? It's probably a > couple of lines of difference, compared to what you have now. > Agree, since we are not using the proper mechanism to extend the option let's just truncate at options level. > > > > + * > > > > + * Return: encoded domain name length > > > > + */ > > > > +size_t encode_domain_name(const char *domain_name, size_t len, cha= r *buf) > > > > +{ > > > > + char *p; > > > > + size_t i; > > > > > > Please order these from longest to shortest. > > > > > > > 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; > > > > > > This is +2 if we append a terminator byte at the end, which we should > > > do as it's part of the encoding, but I don't see it being done. > > > > > > > Added it. > > > > > > +} > > > > diff --git a/util.h b/util.h > > > > index 3fa1d12..c55ef29 100644 > > > > --- a/util.h > > > > +++ b/util.h > > > > @@ -40,6 +40,9 @@ > > > > #ifndef IP_MAX_MTU > > > > #define IP_MAX_MTU USHRT_MAX > > > > #endif > > > > +#ifndef IPV6_MIN_MTU > > > > +#define IPV6_MIN_MTU 1280 > > > > +#endif > > > > > > > > #ifndef MIN > > > > #define MIN(x, y) (((x) < (y)) ? (x) : (y)) > > > > @@ -346,4 +349,7 @@ static inline int wrap_accept4(int sockfd, stru= ct sockaddr *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, cha= r* buf); > > > > > > For consistency: char *domain_name, char *buf > > > > > > > + > > > > #endif /* UTIL_H */ > > -- > Stefano > -- Quique Llorente CNV networking Senior Software Engineer Red Hat EMEA ellorent@redhat.com @RedHat Red Hat Red Hat