From: Enrique Llorente Pastora <ellorent@redhat.com>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v4] dhcp, dhcpv6: Add hostname and client fqdn ops
Date: Wed, 18 Dec 2024 12:57:24 +0100 [thread overview]
Message-ID: <CAHVoYmLChQDEA0XBEQCPShwsH+tmUTu8C2cuH73xd-CdQrqOmQ@mail.gmail.com> (raw)
In-Reply-To: <20241218003454.4a4cbc8b@elisabeth>
On Wed, Dec 18, 2024 at 12:35 AM Stefano Brivio <sbrivio@redhat.com> 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 <ellorent@redhat.com> 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 <ellorent@redhat.com>
> > ---
> > conf.c | 23 ++++++++++++++--
> > dhcp.c | 39 +++++++++++++++++++++++---
> > dhcpv6.c | 72 +++++++++++++++++++++++++++++++++++++++---------
> > passt.1 | 11 ++++++++
> > passt.h | 6 ++++
> > pasta.c | 15 ++++++++--
> > test/lib/setup | 10 +++----
> > test/passt.mbuto | 6 ++--
> > test/passt/dhcp | 15 +++++++++-
> > util.c | 22 +++++++++++++++
> > util.h | 3 ++
> > 11 files changed, 191 insertions(+), 31 deletions(-)
> >
> > diff --git a/conf.c b/conf.c
> > index df2b016..29e4bed 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -854,7 +854,9 @@ static void usage(const char *name, FILE *f, int status)
> > FPRINTF(f, " default: use addresses from /etc/resolv.conf\n");
> > FPRINTF(f,
> > " -S, --search LIST Space-separated list, search domains\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, 26 },
> > {"socket-path", required_argument, NULL, 's' },
> > + {"fqdn", required_argument, NULL, 27 },
> > { 0 },
> > };
> > const char *logname = (c->mode == MODE_PASTA) ? "pasta" : "passt";
> > @@ -1379,9 +1383,9 @@ void conf(struct ctx *c, int argc, char **argv)
> > if (c->mode == MODE_PASTA) {
> > c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
> > fwd_default = FWD_AUTO;
> > - optstring = "+dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:46t:u:T:U:";
> > + optstring = "+dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:H:46t:u:T:U:";
> > } else {
> > - optstring = "+dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:461t:u:";
> > + optstring = "+dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:";
> > }
> >
> > c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 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 = 1;
> > c->quiet = 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 = true;
> > v6_only = false;
> > @@ -1741,6 +1757,7 @@ void conf(struct ctx *c, int argc, char **argv)
> >
> > c->one_off = 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++] = 255;
m->o[offset++] = 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++] = 255;
m->o[offset++] = 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 = 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] = o;
> > m->o[*offset + 1] = opts[o].slen;
> > - memcpy(&m->o[*offset + 2], opts[o].s, opts[o].slen);
> > + idx=*offset + 2;
> > + slen=opts[o].slen;
>
> Spaces around assignment operator (idx = *offset ...).
Done.
>
> > +
> > + // Truncate if it goes beyond OPT_MAX
>
> No C++ style comments.
Done.
>
> > + if (idx + slen > OPT_MAX) {
> > + slen = OPT_MAX - idx;
> > + }
>
> No need for curly brackets.
Done.
>
> > + memcpy(&m->o[*offset + 2], opts[o].s, slen);
> >
> > opts[o].sent = 1;
> > *offset += 2 + opts[o].slen;
> > + return *offset;
> > }
> >
> > /**
> > @@ -172,7 +187,9 @@ static int fill(struct msg *m)
> >
> > for (o = 0; o < 255; o++) {
> > if (opts[o].slen != -1 && !opts[o].sent)
> > - fill_one(m, o, &offset);
> > + if (fill_one(m, o, &offset) == 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++] = 255;
> > @@ -398,6 +415,20 @@ int dhcp(const struct ctx *c, const struct pool *p)
> > if (!opts[6].slen)
> > opts[6].slen = -1;
> >
> > + size_t hostname_len = 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 = hostname_len;
> > + memcpy(opts[12].s, &c->hostname, hostname_len);
> > + }
> > +
> > + size_t fqdn_len = strlen(c->fqdn);
>
> Same as above.
Done.
>
> > + if (fqdn_len > 0) {
> > + opts[81].s[0] = 0x4; // flags (E)
>
> No C++ style comments.
Done.
>
> > + size_t encoded_fqdn_len = encode_domain_name(c->fqdn, 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 = 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 = {
> > { 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 = {
> > @@ -346,7 +365,6 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
> > {
> > struct opt_dns_servers *srv = NULL;
> > struct opt_dns_search *srch = NULL;
> > - char *p = NULL;
> > int i;
> >
> > if (c->no_dhcp_dns)
> > @@ -388,29 +406,56 @@ search:
> > offset += sizeof(struct opt_hdr);
> > srch->hdr.t = OPT_DNS_SEARCH;
> > srch->hdr.l = 0;
> > - p = srch->list;
> > }
> >
> > - *p = '.';
> > - p = stpncpy(p + 1, c->dns_search[i].n, name_len);
> > - p++;
> > - srch->hdr.l += name_len + 2;
> > - offset += name_len + 2;
> > + size_t encoded_name_len = 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 += encoded_name_len;
> > + offset += encoded_name_len;
> > }
> >
> > if (srch) {
> > - for (i = 0; i < srch->hdr.l; i++) {
> > - if (srch->list[i] == '.') {
> > - srch->list[i] = strcspn(srch->list + i + 1,
> > - ".");
> > - }
> > - }
> > srch->hdr.l = 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 = 0;
> > +
> > + fqdn_len = strlen(c->fqdn);
> > + opt_hdr_len = 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 = 576 - (sizeof(struct ipv6hdr) +
sizeof(udphdr)), don't know if it's worth it.
" * @dlen: UDP payload length (not including UDP header)"
> > + fqdn_len = 508 - (offset + opt_hdr_len + 1);
> > + }
> > +
> > + if (fqdn_len == 0) {
> > + return offset;
> > + }
>
> No need for curly brackets.
Done.
> > +
> > +
> > + struct opt_client_fqdn *o = (struct opt_client_fqdn *)(buf + offset);
> > + size_t encoded_fqdn_len = encode_domain_name(c->fqdn, fqdn_len,
> > + o->domain_name);
> > + size_t opt_len = encoded_fqdn_len + 1;
>
> No mixed declarations and code.
Done.
>
> > +
> > + o->hdr.t = OPT_CLIENT_FQDN;
> > + o->hdr.l = htons(opt_len);
> > + o->flags = 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 = offsetof(struct resp_t, client_id) +
> > sizeof(struct opt_hdr) + ntohs(client_id->l);
> > n = dhcpv6_dns_fill(c, (char *)&resp, n);
> > + n = dhcpv6_client_fqdn_fill(c, (char*)&resp, n);
>
> (char *) (for consistency, see line just above)
>
Done.
> >
> > resp.hdr.xid = 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 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 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 DHCPv4 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 <net/if.h>
> > #include <linux/un.h>
> >
> > @@ -209,6 +210,8 @@ struct ip6_ctx {
> > * @ifi4: Template interface for IPv4, -1: none, 0: IPv4 disabled
> > * @ip: IPv4 configuration
> > * @dns_search: DNS search list
> > + * @hostname: Guest hostname
> > + * @fqdn: Guest FQDN
> > * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 disabled
> > * @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] = HOSTNAME_PREFIX;
> > const struct pasta_spawn_cmd_arg *a;
> > sigset_t set;
> > + size_t conf_hostname_len = 0;
>
> Align from longest to shortest:
>
> const struct pasta_spawn_cmd_arg *a;
> size_t conf_hostname_len = 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 fail");
> > -
> > - if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1,
> > +
> > + a = (const struct pasta_spawn_cmd_arg *)arg;
> > +
> > + conf_hostname_len = 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 == ENAMETOOLONG) {
>
> These should move to be aligned with the arguments of gethostname() and with
> the if clause:
>
> } else if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1,
> HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFIX)) ||
> errno == ENAMETOOLONG) {
>
>
Done.
> > hostname[HOST_NAME_MAX] = '\0';
> > @@ -208,7 +217,6 @@ static int pasta_spawn_cmd(void *arg)
> > sigaddset(&set, SIGUSR1);
> > sigwaitinfo(&set, NULL);
> >
> > - a = (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 = {
> > .exe = argv[0],
> > .argv = argv,
> > + .c = c,
> > };
> > char uidmap[BUFSIZ], gidmap[BUFSIZ];
> > char *sh_argv[] = { 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=$((4 * 1024 * 1024)) --trace-children=yes --vgdb=no --error-exitcode=1 --suppressions=test/valgrind.supp ./passt ${__opts} -s ${STATESETUP}/passt.socket -f -t 10001 -u 10001 -P ${STATESETUP}/passt.pid"
> > + context_run_bg passt "valgrind --max-stackframe=$((4 * 1024 * 1024)) --trace-children=yes --vgdb=no --error-exitcode=1 --suppressions=test/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=$((4 * 1024 * 1024)) --trace-children=yes --vgdb=no --error-exitcode=1 --suppressions=test/valgrind.supp ./passt -f ${__opts} -s ${STATESETUP}/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 "valgrind --max-stackframe=$((4 * 1024 * 1024)) --trace-children=yes --vgdb=no --error-exitcode=1 --suppressions=test/valgrind.supp ./passt -f ${__opts} -s ${STATESETUP}/passt.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 ${STATESETUP}/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 ${STATESETUP}/passt.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}"
> > fi
> > wait_for [ -f "${STATESETUP}/passt.pid" ]
> >
> > @@ -243,7 +243,7 @@ setup_two_guests() {
> > [ ${TRACE} -eq 1 ] && __opts="${__opts} --trace"
> > [ ${VHOST_USER} -eq 1 ] && __opts="${__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 hostname1 -t 10001 -u 10001"
> > wait_for [ -f "${STATESETUP}/passt_1.pid" ]
> >
> > __opts=
> > @@ -252,7 +252,7 @@ setup_two_guests() {
> > [ ${TRACE} -eq 1 ] && __opts="${__opts} --trace"
> > [ ${VHOST_USER} -eq 1 ] && __opts="${__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="$((${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="${PROGS:-ash,dash,bash ip mount ls insmod mkdir ln cat chmod lsmod
> > modprobe find grep mknod mv rm umount jq iperf3 dhclient hostname
> > 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-keygen cmp}"
> > + nproc tcp_rr tcp_crr udp_rr which tee seq bc sshd ssh-keygen cmp env}"
> >
> > # OpenSSH 9.8 introduced split binaries, with sshd being the daemon, and
> > # sshd-session the per-session program. We need the latter as well, and the path
> > @@ -41,6 +41,7 @@ FIXUP="${FIXUP}"'
> > #!/bin/sh
> > LOG=/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_servers}; 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") >> /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 <sbrivio@redhat.com>
> >
> > -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' | sed -n 's/^search \(.*\)/\1/p' | tr ' \n' ',' | sed 's/,$//;s/$/\n/'
> > check [ "__SEARCH__" = "__HOST_SEARCH__" ]
> >
> > +test DHCP: Hostname
> > +gout NEW_HOST_NAME cat /tmp/new_host_name
> > +check [ "__NEW_HOST_NAME__" = "hostname1" ]
> > +
> > +test DHCP: Client FQDN
> > +gout NEW_FQDN_FQDN cat /tmp/new_fqdn_fqdn
> > +check [ "__NEW_FQDN_FQDN__" = "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__" = "__HOST_SEARCH6__" ]
> > +
> > +test DHCPv6: Hostname
> > +gout NEW_FQDN_FQDN cat /tmp/new_fqdn_fqdn
> > +check [ "__NEW_FQDN_FQDN__" = "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, section 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 *buf){
>
> Opening curly bracket on a newline, for consistency.
>
Done.
> > + char *p = NULL;
>
> No need to initialise this, it's assigned unconditionally below.
>
Done.
> > + size_t i = 0;
>
> No need to initialise this, you only use it in the loop below.
>
Done.
> > +
> > + buf[0] = strcspn(domain_name, ".");
> > + p = buf + 1;
> > + for (i = 0; i < len; i++) {
> > + if (domain_name[i] == '.')
> > + p[i] = strcspn(domain_name + i + 1, ".");
> > + else
> > + p[i] = 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 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, char* buf);
> > +
> > #endif /* UTIL_H */
>
> --
> Stefano
>
--
Quique Llorente
CNV networking Senior Software Engineer
Red Hat EMEA
ellorent@redhat.com
@RedHat Red Hat Red Hat
next prev parent reply other threads:[~2024-12-18 11:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-17 6:50 [PATCH v4] dhcp, dhcpv6: Add hostname and client fqdn ops Enrique Llorente
2024-12-17 23:34 ` Stefano Brivio
2024-12-18 11:57 ` Enrique Llorente Pastora [this message]
2024-12-19 0:19 ` Stefano Brivio
2024-12-19 11:51 ` Enrique Llorente Pastora
2024-12-19 12:06 ` Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHVoYmLChQDEA0XBEQCPShwsH+tmUTu8C2cuH73xd-CdQrqOmQ@mail.gmail.com \
--to=ellorent@redhat.com \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).